Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plans on Key Binding API #61

Open
EnnuiL opened this issue Dec 11, 2021 · 18 comments
Open

Plans on Key Binding API #61

EnnuiL opened this issue Dec 11, 2021 · 18 comments
Labels
enhancement New feature or request library: gui Related to the GUI library. t: new api This adds a new API.

Comments

@EnnuiL
Copy link
Contributor

EnnuiL commented Dec 11, 2021

Fabric's Key Binding API was okay, but it could have been much better with a bigger scope. While it does make key registration available, there's much more to key binds than merely registering them.
In my opinion, the key problems for a modded key bind experience are:

  • There aren't enough keys in a keyboard
  • With not enough keys, conflicting key binds are inevitable, but Vanilla only allows one key bind per key
  • And finally, with too many key binds, the key bind screen gets confusing, and when there's a conflict, it's not easy to look up what are the conflicting key

With those key problems in mind, I have planned out a key binding API which would be composed by two iterations, similarly to how the future of the classic Mac OS was split into Blue, a newer version of the preexisting OS with simpler upgrades, and Pink, which was going to be a completely new OS featuring major upgrades.

Quilt Key Bindings API v1 (Blue)

This is the simpler and more concrete iteration of the planned API, and the aim of PR #59. In fact, the PR's pretty much finished!
The features are:

Dynamic Disabling

This feature adds an active state to modded key binds, which is handled by the registry. This allows to hide a key bind from the game (🎵 options.txt being the exception 🎵), effectively disabling it.
This feature will definitely help with the "too many keys" problems. With the ability of disabling key binds being made very practical (and being able to be done in-game), the hope is to have all mods that add non-essential key binds make them able to be disabled through their configuration.

Improvements to the Key Binds screen

The goal of the improvements is to improve the modded key bind experience, so the developers won't have to think about the possible hassles to do a certain thing; QSL will help the players.
Key bind conflicts were the target of two improvements: a textual indicator in addition to the red color, and a tooltip showing all the other conflicting keys. The former assists colorblind players and the latter assists pretty much everyone (who hasn't had some difficulty with finding the other conflicting keys in a modpack?)
And finally, the ability to middle click the bind button in order to unbind the key has been added. If you don't want a key, then you should actually be able to unbind it, not have a junk key where you deposit everything on it. I am a big dummy who somehow didn't see in the code itself that Vanilla already has a method for it. With Vanilla having a method for unbinding keys, this probably goes beyond the scope, and so, it was removed
All these improvements are mere nice-to-haves on a Vanilla context, but on a Modded one, they are going to be very important.

Getter methods for the Registry

a.k.a. getKeyBinding(translationKey), isEnabled(key), and getAllKeyBindings(includeVanilla)
The goal is to assist other mods with messing with the internals. Of course, there's GameOptions' allKeys field, however, with the extra properties managed by the registry, it might be useful to actually provide those methods through the API.

Questions:

  • Should it be getAllKeyBindings(includeVanilla) (for including the vanilla entries with the modded ones) or getAllKeyBindings(vanilla) (instead of returning modded entries, return vanilla ones)?
    • While the first is the one used by the PR, perhaps the latter would be preferable considering that the returned map is has a random order due to hash maps being hash maps; I don't know.

Quilt Key Bindings API v2 (Pink)

This is the iteration that will bring key bind utopia, the one that will do more significant changes in order to make modded key binds perfect, and hopefully, unlike the namesake, not a bloated monster that will inevitably die. But also, this is the one that's the least concrete, with only an outline being determined so far. This is the part where I'll need y'all's feedback.

config/quilt/keybinds.json

The real path is TBD, but the idea itself is simple: instead of storing the key binds at options.txt, they would be stored at a JSON file provided by Quilt. Why?
Well, messing with options.txt was already messy (the handling of disabled keys on them? not really ideal, and i needed an AW ;-;), but with the other features here, it would be even more messier. With a separate config file handled by this API, it would make the implementation of the following features (and of past ones) simpler.
Another TBD, however, is the overall schema of it, however, I think that it isn't much important to get it right now; It can be shaped later

Key Chording

A very simple concept: instead of having key binds accept only a single key, they would be able to accept combinations of them, so things like Ctrl + Shift + Space would do something in-game.
The implementation, however, will require more shenanigans, effectively replacing some of the internal key binding logic with Quilt's, but once again, considering that a keyboard doesn't have many keys, it will be an essential feature for modded key binds
"But your honor, Amecs ", you say. Well, my answer is " Death." that while Amecs is a really useful mod for key bind, it doesn't have the benefit of being a standard, and so, you can't really define a default that everyone would share. Theoretically, you could have a key sequence if Amecs is installed and have a plain one for the rest, but that would cause confusion. With a dominant standard for key chording, it wouldn't require a mod to be installed, you'll already have it installed in the first place.

Questions:

  • Should there be limits for the length of the key sequence? Of course, only one key would fit in a sequence due to how key binds work (so no A + A + A), but should 27-keys-long sequences be allowed? How would the key bind screen show them? Perhaps a scrolling title?
  • How would the registration of a key chord would even be done? Would a QuiltKeyBinding be needed, or would it be preferable to avoid it somehow?

Key Predicates

This was a suggested idea when I began development on the Blue part. The problem is simple: Vanilla only lets one key bind per key, and on Modded, you'll probably need multiple key binds per key, like having a certain button only work on in-world and another only on a specific screen, it would be fine to have them share the key, but with the game's rules, they would be forced to take different keys, and once again, the keyboard doesn't have many keys.
The proposed solution are key predicates, which would basically allow the mod to determine rules like "this key will only work on these specific situations" and instead of having the "one key bind per key" rule, it would be more like "one key bind per predicate per key", with conflict indicators following that rule, making conflicts something that should definitely be fixed.

An alternative solution to the same problem is also one provided by Amecs: have no rules, let many key binds share the same key, and while it does fix the problem in a way, it trivializes conflicts, making them just a little annoyance in the key binds screen. I see this as a big problem, because a conflict is a conflict, it shouldn't be happening at all! And considering the same scenario which key predicates would better work (one in-world key, another screen-specific key), having no rules and still say "it's a conflict!" makes the message ("it's a conflict!") into something that should be ignored, because "everything's fine". And so, key predicates would be the ideal solution to that problem.

While it's a really neat idea, it's also the least concrete one. What would exactly be the predicates? The suggested implementations were:

  • A collection of identifiers which would have some standarized ones (like in_world) but others were free to have their own. It's simple, but it might not describe some more complex scenarios
  • Literally predicates, as in those used by data packs. It would be able to cover those complex situations, but the system itself might be overly complex

Questions:

  • Everything Actually, yeah, pretty much everything

Is the split really necessary?

That has been something that I've been wondering once I got much of Blue done.
While it's a rhetorical question, the answer is "It was", since with that split, I managed to get the Blue feature set ready and the Pink one is now ready to be discussed in order to set it into stone. That was my way to move fast and yet still progress very far without being hindered by bikeshedding, but now, Blue's ready, but does it necessarily mean "ready for release"?
Perhaps the finishing of the Blue feature sets would be simply a milestone, while the real goal would be finishing Pink, and since QSL is still in active development, it wouldn't make much of a difference anyway.
About that, this is up to y'all to decide.

And yeah, that's it!

There has been some solid progress recently, and now it's time to make the future a bit less cloudy in order to actually get the full potential of the API, so if you wanna help, give your feedback! Share your opinion (constructive, by preference)! I wanna know what do people really need in order to make this the best as possible
So, yeah, that's all! (and sorry for rambling a lot)

@Juuxel
Copy link
Contributor

Juuxel commented Dec 11, 2021

And finally, the ability to middle click the bind button in order to unbind the key has been added. If you don't want a key, then you should actually be able to unbind it, not have a junk key where you deposit everything on it.

You can already press Esc while configuring the key for that, though - it clears the binding completely. (See KeyBindingsScreen or whatever its name was)

@EnnuiL
Copy link
Contributor Author

EnnuiL commented Dec 11, 2021

And finally, the ability to middle click the bind button in order to unbind the key has been added. If you don't want a key, then you should actually be able to unbind it, not have a junk key where you deposit everything on it.

You can already press Esc while configuring the key for that, though - it clears the binding completely. (See KeyBindingsScreen or whatever its name was)

I checked Vanilla, and oh god, you're right, i invented an already-existing feature somehow
Well, an excuse to keep it would be "middle click without having to select a key would be more practical", however, the main reason for having it in the QSL module ("there's no way to unbind a key, you need to unbind keys") is pretty much gone; I'll remove that bit

@LambdAurora
Copy link
Contributor

I checked Vanilla, and oh god, you're right, i invented an already-existing feature somehow Well, an excuse to keep it would be "middle click without having to select a key would be more practical", however, the main reason for having it in the QSL module ("there's no way to unbind a key, you need to unbind keys") is pretty much gone; I'll remove that bit

My main criticism of the Vanilla system is how not intuitive it is, something that says that it's a thing would be appreciated.
If we're willing to, when clicking on a keybind to remap we could send a toast saying "press any key to bind, press escape to unbind".

@EnnuiL
Copy link
Contributor Author

EnnuiL commented Dec 11, 2021

I checked Vanilla, and oh god, you're right, i invented an already-existing feature somehow Well, an excuse to keep it would be "middle click without having to select a key would be more practical", however, the main reason for having it in the QSL module ("there's no way to unbind a key, you need to unbind keys") is pretty much gone; I'll remove that bit

My main criticism of the Vanilla system is how not intuitive it is, something that says that it's a thing would be appreciated. If we're willing to, when clicking on a keybind to remap we could send a toast saying "press any key to bind, press escape to unbind".

Hm, good point; I'd only remove it because I'm worried about going beyond the scope, but if it would be fine, then it would be fine
If "Middle Click to Unbind" is removed and something else was to be added, I don't think a toast is going to be a great way to add information to the screen, it wouldn't really fit in. Perhaps it would be better as a text under the "Key Binds" title

@LambdAurora
Copy link
Contributor

LambdAurora commented Dec 11, 2021

Hm, good point; I'd only remove it because I'm worried about going beyond the scope, but if it would be fine, then it would be fine If "Middle Click to Unbind" is removed and something else was to be added, I don't think a toast is going to be a great way to add information to the screen, it wouldn't really fit in. Perhaps it would be better as a text under the "Key Binds" title

imo, the middle click thing doesn't add much, we shouldn't do it.
But a toast definitely should fit, I don't see how there's not enough space. And I am not talking about tooltip.

@EnnuiL
Copy link
Contributor Author

EnnuiL commented Dec 11, 2021

Hm, good point; I'd only remove it because I'm worried about going beyond the scope, but if it would be fine, then it would be fine If "Middle Click to Unbind" is removed and something else was to be added, I don't think a toast is going to be a great way to add information to the screen, it wouldn't really fit in. Perhaps it would be better as a text under the "Key Binds" title

imo, the middle click thing doesn't add much, we shouldn't do it. But a toast definitely should fit, I don't see how there's not enough space. And I am not talking about tooltip.

Yeah, I know you're not talking about the tooltip; 1.18 has cleared a lot of space by moving the mouse settings to a separate menu, I don't think a little text providing useful information under the title would do much harm; However, perhaps instead of extra text, it could be a tooltip to the header; I just fear that a toast would really appear as "Definitely a Quilt thing" instead of "A Quilt thing, but why didn't Mojang add it?"

@LambdAurora
Copy link
Contributor

LambdAurora commented Dec 11, 2021

Yeah, I know you're not talking about the tooltip; 1.18 has cleared a lot of space by moving the mouse settings to a separate menu, I don't think a little text providing useful information under the title would do much harm; However, perhaps instead of extra text, it could be a tooltip to the header; I just fear that a toast would really appear as "Definitely a Quilt thing" instead of "A Quilt thing, but why didn't Mojang add it?"

Tooltip on header defeats the point of being visible to not induce confusion. Toast has the advantage of not risking conflicts.

@EnnuiL
Copy link
Contributor Author

EnnuiL commented Dec 11, 2021

Yeah, I know you're not talking about the tooltip; 1.18 has cleared a lot of space by moving the mouse settings to a separate menu, I don't think a little text providing useful information under the title would do much harm; However, perhaps instead of extra text, it could be a tooltip to the header; I just fear that a toast would really appear as "Definitely a Quilt thing" instead of "A Quilt thing, but why didn't Mojang add it?"

Tooltip on header defeats the point of being visible to not induce confusion. Toast has the advantage of not risking conflicts.

Fair enough; I'm just worried about the toast eventually becoming annoying later

@LambdAurora
Copy link
Contributor

Good point. This is annoying to find a good way.

@EnnuiL
Copy link
Contributor Author

EnnuiL commented Dec 11, 2021

Hm, I've thought about the toast, and perhaps there's a way to make it be good:

  • Have it appear it once (like the tutorial toasts), with a config property saying if it has appeared or not being saved on Quilt's config file
  • Expose a method through the API so the Yet Another Tutorial Toast Removal mod can use it in order to remove the toast
  • I was gonna mention "Have the toast only appear when binding something", but then i realized that it was the original suggestion

This means that this little toast would be held off to the Pink stage, but yeah, if it appears once, then it wouldn't be much of a problem

@EnnuiL EnnuiL mentioned this issue Dec 11, 2021
3 tasks
@Bubblie01
Copy link

Bubblie01 commented Dec 11, 2021

mouse events should be fixed holy mother of god

@Bubblie01
Copy link

Also being able to easily use the keybind API with items and stuff would be really cool as I had problems with that in the past.

@lenrik1589
Copy link

I can't believe that noone has mentioned Masa's Malilib in a context of keybinding API, because his keybinding API is great, it combines the ability to use mouse key binds together with key binds and provides extensive ability to customize when the bind is actually triggered and already had "conflicting key binds" feature, though it might not be in spirit of this API…

@EnnuiL
Copy link
Contributor Author

EnnuiL commented Dec 26, 2021

I can't believe that noone has mentioned Masa's Malilib in a context of keybinding API, because his keybinding API is great, it combines the ability to use mouse key binds together with key binds and provides extensive ability to customize when the bind is actually triggered and already had "conflicting key binds" feature, though it might not be in spirit of this API…

Ohh, I forgot that the library had a custom key bind system
I took a look in the GH wiki page about key binds, and what I can say is that while the Advanced Keybind Settings would be out-of-scope, this API's key predicate system needs to be able to support it; While I was thinking of a static system, it'll probably be better if it was dynamic
About the overlap system, it seems interesting, however, I'll probably just go with "if it's not conflicting, it's not red (or marked with a (!))" unless it ends being necessary

@LambdAurora LambdAurora added enhancement New feature or request library: gui Related to the GUI library. t: new api This adds a new API. labels Jun 9, 2022
@TheEpicBlock
Copy link

I would just like to add my 2 cents, because the current state of keybinds is horrible.

it would be more like "one key bind per predicate per key", with conflict indicators following that rule, making conflicts something that should definitely be fixed.

I don't think it's doable to determine what keybinds are able to conflict, nor do I think it's actually useful. For an example, say you'd have some mod like Origins and a mod like Haema. They both have keybinds that only function when you're in a specific state, eg: a specific origin or being a vampire. Technically, you can be both at the same time, so all of the Haema keybinds should conflict with all of the Origins keybinds. But imo, the users should only have to worry about this until it actually happens. Users tend to stick with a specific layout, so they should only have to worry about the keybinds they actually use in that layout.

@anonymous123-code
Copy link

Actually, is there a difference between the Key Bind Predicate system required for the above scenario and the current system for enabling and disabling key binds - except that one shows the key bind in the config screen and one doesn't?

Because there are essentially three scenarios here:

  1. A key bind is only active in very specific and generic contexts, and actually could be considered active (as in triggerable) all the time

    • This one would be handled by scopes / the first proposition for a Key Bind Predicate system
  2. A key bind is by default inactive, but should always be configurable using the menu

    • This is the case when it e.g. only becomes relevant for a specific armor, or the origins-Haema conflict noted above
  3. The key bind can switch dynamically, but usually won't do so, e.g. via config

    • In this case the key bind shouldn't be shown and shouldn't be active.

In short there are three cases: active-shown, inactive-shown, inactive-hidden
(active-hidden makes no sense, although maybe allow it for completeness and unconsidered cases?)

Both case 2 and 3 share the problem of what happens when a key bind becomes active and a conflict arises. Maybe a toast might work? But this problem already exists with dynamic enabling/disabling of Key Binds.

In summary, allowing for inactive-shown should fix the problem in the comment above, but will make the problem of conflicts arising when dynamically enabling a key bind more apparent.

@TheEpicBlock
Copy link

The ideal UX here imo would be to show all the context-dependent actions you can currently take in the bottom-right. Ofc that's not really in-scope for qsl to do, but I think that's the only real way to deal with the amount of keybinds mods can add.

I'd like to add that I don't think a conflict has to be a problem. If I'm wearing a backpack with a certain key bound to open it, but then I'm holding an item which uses the same keybind, I wouldn't consider this a conflict but I would expect the item to just take priority. There's already precedent for this in vanilla with the right-click button handling practically everything.

Keybinds should be out of the way until the player actually needs them. I don't have the mental capacity to consider all the keybinds in a modpack, I just want to consider the 1 or 2 I'm actually using.

Regarding your 3rd case, I don't think that situation is very common. Most keybinds fall into case 1 or 2. (Some mods also have keybinds that are always active regardless of context, such as an "open map" keybind, but I don't think there are many improvements to be made here so they can be disregarded)

@EnnuiL
Copy link
Contributor Author

EnnuiL commented Jun 29, 2023

Update: it's likely that Quilt Key Binds API will be downscaled on some way on its original release; this might guarantee a quick development and therefore merging of it in the end; dear god do I need to tackle my own motivation problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request library: gui Related to the GUI library. t: new api This adds a new API.
Projects
None yet
Development

No branches or pull requests

7 participants