H-uru / Plasma

Cyan Worlds's Plasma game engine
http://h-uru.github.io/Plasma/
GNU General Public License v3.0
202 stars 80 forks source link

Key Map dialog improvements. #1573

Closed Hoikas closed 1 month ago

Hoikas commented 3 months ago

Previously, it was possible to get the key map chronicle into an invalid state with the same key bound to multiple controls. You can reproduce that by binding UpArrow to "start chat" then rebinding UpArrow to "move forward". The chronicle will then have two entries for UpArrow in it. The bindings will behave as you expect until you quit and restart Uru, at which point UpArrow will be bound to "start chat" again. The only way to fix this is by resetting the key map to default.

To fix this, I cleaned up the code to make the ownership of the key map more clear. Previously, the code half-handedly updated the vault chronicle value and the key map. Now, we bind any input directly, and completely rewrite the chronicle each time a key is bound. In this way, the UI display, key bindings, and key chronicle all have a single source of truth.

Further, you can now unbind keys by pressing the current key binding. This is useful if, for example, you don't want "start chat" to be bound to anything. Previously, you would have needed to bind the "start chat" key to some other control, then bind that other control to what you actually wanted.

With this fixing the major keymap bugs and gotchas, we could probably close #1067.

dgelessus commented 3 months ago

Previously, you would have needed to bind the "start chat" key to some other control, then bind that other control to what you actually wanted.

I always forget that Americans don't have that extra key next to the left Shift key that Uru interprets as (unmapped) 😄

With this fixing the major keymap bugs and gotchas, we could probably close #1067.

I don't think so. That PR mainly implements something different, namely saving the key map to a local config file (in a way that actually works, unlike ptKeyMap.writeKeyMap) so it's used as the default for new avatars (where there's no KeyMap chronicle yet).

dpogue commented 3 months ago

Out of scope for this PR, but there's an open question around how to handle internationalization of keymaps particularly across different keyboard layouts when targeting non-Windows environments: https://github.com/H-uru/Plasma/discussions/786 (mostly linking this to hopefully get some more thoughts in that discussion)