fivem icon indicating copy to clipboard operation
fivem copied to clipboard

feat(gameinput): new sorting algo and new native

Open spacevx opened this issue 1 year ago • 12 comments

Goal of this PR

Actually, @tabarra added # in his key mappings to have the txAdmin key mappings at the top of the FiveM key settings. This PR aims to fix this so Tabarra doesn't have to do this workaround

How is this PR achieving the goal

If the left is monitor and not the right one, we return true to indicate that the left needs to be placed before the right. If the right one is txAdmin and not the left one, we return false so that the right is placed before the left. Thanks @FabianTerhorst for fixing the sorting logic.

This PR applies to the following area(s)

FiveM

Successfully tested on

image Here is a screenshot where i did the test with a resource called ns_carrierheist.

Game builds: 3258

Platforms: Windows,

Checklist

  • [x] Code compiles and has been tested successfully.
  • [x] Code explains itself well and/or is documented.
  • [x] My commit message explains what the changes do and what they are for.
  • [x] No extra compilation warnings are added by these changes.

Fixes issues

spacevx avatar Oct 01 '24 17:10 spacevx

Maybe ordering by a integer would be more flexible then limiting it to txAdmin itself?

FabianTerhorst avatar Oct 01 '24 18:10 FabianTerhorst

Thanks for the PR.
Just as clarification, although I like having it at the top of the list, my only real concern is having all the txAdmin keybinds to show grouped as one block. As I see, ideally the sorting should be based off the resource name, and within the group the sorting can be alphabetical.
But ordering by arbitrary integers would work as well, but in tx I would probably hardcode -9999 or +9999 🤣

And btw, the reason why txAdmin has ui_label 'txAdmin' in the fxmanifest.lua is because it was the plan to have it be used for the renaming of resources in the keybinds ui (monitor -> txAdmin), it would also be useful for some big resources like es_extended -> ESX

tabarra avatar Oct 01 '24 18:10 tabarra

Thanks for the PR. Just as clarification, although I like having it at the top of the list, my only real concern is having all the txAdmin keybinds to show grouped as one block. As I see, ideally the sorting should be based off the resource name, and within the group the sorting can be alphabetical. But ordering by arbitrary integers would work as well, but in tx I would probably hardcode -9999 or +9999 🤣

And btw, the reason why txAdmin has ui_label 'txAdmin' in the fxmanifest.lua is because it was the plan to have it be used for the renaming of resources in the keybinds ui (monitor -> txAdmin), it would also be useful for some big resources like es_extended -> ESX

Good idea. So @FabianTerhorst wants me to do an order system when calling RegisterKeyMapping. You can pass an integer; the larger the number, the higher the key map is at the top in terms of priority. If some resources are using the same order, alphabetic sorting will apply. We're also going to group key maps from the same resource together. Is it good for you @FabianTerhorst?

spacevx avatar Oct 01 '24 19:10 spacevx

And for the ui_label thing i think it's better to hide them with a native i don't understand why names are here

spacevx avatar Oct 01 '24 19:10 spacevx

The resource name is there to provide "persistence" as keybinds per resource exist across different servers.

Originally, the resource name was behind the actual keybind name, which ofc makes more sense, but IIRC this was changed because resource developers were able to hide the resource name with some "invalid" Scaleform characters.

(I think allowing resources to define their keybind priority can get quite messy and lacks any deterministic rule, so not sure if implementing this for any resource is a good idea)

tens0rfl0w avatar Oct 01 '24 19:10 tens0rfl0w

The resource name is there to provide "persistence" as keybinds per resource exist across different servers.

Originally, the resource name was behind the actual keybind name, which ofc makes more sense, but IIRC this was changed because resource developers were able to hide the resource name with some "invalid" Scaleform characters.

(I think allowing resources to define their keybind priority can get quite messy and lacks any deterministic rule, so not sure if implementing this for any resource is a good idea)

Yeah but server owners can easily change the keybinds, i think a function to disable/enable name of resources in the key page would be cool, and yeah for the sorting can be messy

spacevx avatar Oct 02 '24 08:10 spacevx

I feel like maybe this is getting derailed or out of scope.
At least for txAdmin's case, just grouping all txAdmin keybinds together is enough for me to remove the silly # prefix that I added, even if the groups themselves end up being sorted alphabetically without any customization option.

tabarra avatar Oct 02 '24 11:10 tabarra

I think that the grouping of keybinds by resource is something interesting to happen, since the fact that resources with many assignments currently have them spread throughout the list, sometimes makes it difficult to locate which one you want to edit, as each server has its own list of resources making the positions change (I would even say that the order changes every time you connect, but I'm not entirely sure).

From my point of view the best thing would be to sort alphabetically first by resource and then by keybind description and if a custom sorting is allowed, that what can be changed is the position of the whole group and that it is at server configuration level, not by decision of the resource creator.

Bossman02 avatar Oct 03 '24 12:10 Bossman02

Ok, so now the sorting algorithm is based on the resource names and then alphabetically within each resource a native was added SET_KEY_MAPPING_HIDE (if you have a better name don't hesitate) which allows hiding resource names from the FiveM keys page

Here is a render of the key page without resource names image

And another for the sorting image

spacevx avatar Oct 04 '24 19:10 spacevx

I do not think that hiding the names of the resources is a good option, there may be several that have the same descriptions in their actions (a clear example is the different libraries that usually have their “interact” action).

Bossman02 avatar Oct 05 '24 09:10 Bossman02

Ok, so now the sorting algorithm is based on the resource names and then alphabetically within each resource a native was added SET_KEY_MAPPING_HIDE (if you have a better name don't hesitate) which allows hiding resource names from the FiveM keys page

Here is a render of the key page without resource names image

And another for the sorting image

Maybe name it SET_KEY_MAPPING_HIDE_RESOURCE.

FabianTerhorst avatar Oct 05 '24 09:10 FabianTerhorst

I do not think that hiding the names of the resources is a good option, there may be several that have the same descriptions in their actions (a clear example is the different libraries that usually have their “interact” action).

Hiding the names is fine in my opinion. The server can decide and the user don't know what "ns__carrierheist" in front of the key means.

FabianTerhorst avatar Oct 05 '24 09:10 FabianTerhorst

Tested, working. Thank you for the contribution.

martonp96 avatar Oct 21 '24 14:10 martonp96