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

Key Map dialog improvements. #1573

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Key Map dialog improvements. #1573

wants to merge 7 commits into from

Conversation

Hoikas
Copy link
Member

@Hoikas Hoikas commented Mar 9, 2024

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.

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.

This also includes general code cleanups, modernizing the Python. For
example, we can now rely on the key dictionary being stored in the same
order that it's initialized.
This entry in the key conversion LUT is hiding the expected value of
"(unmapped)". I fixed the captialization to match the other
localizations.
This is useful if you want to remove the "start chat" binding.
Scripts/Python/xOptionsMenu.py Outdated Show resolved Hide resolved
Sources/Plasma/NucleusLib/pnInputCore/plInputMap.cpp Outdated Show resolved Hide resolved
Comment on lines -734 to -735
# force writing the keymap
km.writeKeyMap()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new version no longer calls km.writeKeyMap() anywhere. I assume this is intentional?

Perhaps we should just remove ptKeyMap.writeKeyMap and the underlying plInputInterfaceMgr::WriteKeyMap(). AFACIT that code did nothing useful anyway. It tries to write a file init/keyboard.fni, but MOULa installs normally don't have an init folder, so that always fails silently.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I intentionally removed this call because it has no effect. I didn't want to remove the writeKeyMap code in this PR, though.

@dgelessus
Copy link
Contributor

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).

Hoikas and others added 2 commits March 9, 2024 20:10
Co-authored-by: dgelessus <dgelessus@users.noreply.github.com>
@dpogue
Copy link
Member

dpogue commented Mar 10, 2024

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: #786 (mostly linking this to hopefully get some more thoughts in that discussion)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants