Allow modifiers for keybindings - #173
jakobhellermann wants to merge 14 commits into
Conversation
instead of implicitly rebinding when KeyCode == None
Removes the per-frame text update
This allows ConfigManager to use its keybinding UI instead of a text input.
SpaceMonkeyy86
left a comment
SpaceMonkeyy86
left a comment
There was a problem hiding this comment.
Looks great, just a few notes. Thanks!
| } | ||
| } | ||
|
|
||
| internal static Modifier Held() => |
There was a problem hiding this comment.
This is not actually an extension method. IMO it would be better to put this in Modifier instead, along with the other methods in this class that aren't extension methods. Maybe rename to CurrentlyHeld to make it more clear that it reads Unity input
There was a problem hiding this comment.
I don't think you can do that in c#, put methods on enums.
| private static void AddConfigEntryKeybind(ConfigFile config, string bindName, string displayName, Binding defaultBinding, string description) | ||
| { | ||
| // KeyboardShortcut instead of Binding, so that ConfigManager knows how to handle it | ||
| ConfigEntry<KeyboardShortcut> entry = config.Bind("General", displayName, ToShortcut(defaultBinding), description); |
There was a problem hiding this comment.
ModMenu won't be able to display this since it's a KeyboardShortcut. Just using a KeyCode instead should be fine, this option is really just for unbricking your settings.
There was a problem hiding this comment.
What should the key code be if you bind it to a modifier shortcut? Just the main key?
I'd like to use the same mechanism for the command palette shortcut which will definitely have a modifier by default.
I think I would prefer adding KeyboardShortcut into ModList (useful for other mods as well) and keeping this as is.
There was a problem hiding this comment.
2 participants
Reworks the keybinding system around a
Bindingtype containing modifier flags and the main binding.Deserialization should stay backwards compatible.
I also refactored the rebinding logic in
GUIControllerto not rely on the implicitKeyCode.Nonemeans "is currently rebinding" state, and instead store arebindTargetaction name for that. Let me know what you think about that, I can implement modifiers without making that change if you don't want it.Keybindings in the settings file look like this:
Control+Shift+Alt+Meta+X.Modifiers don't distinguish between the left and the right variant, so
LCtrl+XandRCtrl+Xboth perform aControl + Xkeybind. If you bind a modifier key alone it will be the specific key (as was the case previously), stored asBinding { Modifiers = None, Key = KeyCode.LeftControl }.Also I've made the enum parsing case insensitive, in case someone edits their config to e.g.
control+Aby hand.The meta key handling has to be tested on windows and linux. On mac, pressing the left command key apparently emits both
LeftMetaandLeftWindows, so I addedLeftWindowsandRightWindowsto the ignored key list.part of #170