Improved keyboard support [RFC]

[petchema]

This adds keyboard support for standard dialog boxen (Y/N prompts, etc.), but also some special dialog boxen (Options menu, Resting, Traveling) and screens (Inventory, Trading). It also replaces hardcoded key bindings of daedra quests Y/N prompts and travel map.
I borrowed the HotkeySequence class from the automap then extended it to support virtual key modifiers (say, Shift that matches either LeftShift or RightShift), then after that changed the logic to AND modifiers together.
All bindings are configured in an external DialogShortcuts.txt file, so they can be localized.

Before going on I submit this patch in its current state for review.
For example its integration into UI code (Panel, Button) is superficial and must be called explicitly, that can probably be improved but I'm not sure how it was planned. It could also be that I'm going in a totally wrong direction, just let me know.

Forums: https://forums.dfworkshop.net/viewtopic.php?f=4&t=1448

[Nystul-the-Magician]

nice!
would be great if we eventually find a mechanism to make hotkey sequences customizable
as far as I understood they are still hardcoded like in automap, is this correct?

[petchema]

would be great if we eventually find a mechanism to make hotkey sequences customizable
as far as I understood they are still hardcoded like in automap, is this correct?

Yes, I haven't externalized automap HotkeySequences; It's certainly doable, but I'm not sure what this fallback mechanism is all about so I left it as-is for the moment

[petchema]

I migrated the automap shortcuts:

The most difficult part was replacing the fallback key feature: when the key used to open the automap is used in an automap shortcut, it creates a conflict because the automap implements toggle open (ie can be closed with the same key used to open it). I reimplemented it like this:

• the key used to open the automap also closes the automap (toggle open);
• shortcuts that used the same key are replaced with shortcuts using the Home key, but with the same modifiers;
• if the key used to open the automap changes, just recreate all shortcuts and tooltips.

Added Escape key to exit the automap for consistency and hardcoded fallback

Added HotkeySequence.IsUp(With) methods used for toggle key

Added HotkeySequence.ToString() method

Removed hardcoded modifiers in tooltips texts

Renamed *Control- modifiers as *Ctrl- to shorter descriptions in automap tooltips

[ajrb]

What's this update needed for?

[petchema]

Right now there's no mechanic in either Button or Panel to check and activate them (or their children) thru hotkeys, so I explicitly call my KeyboardActivation() methods from the different UI windows Update() methods.
I'm not satisfied with this solution, I'm sure it could be better integrated, but I don't know if that was already planned or how...

[Interkarma]

This is nice work, I'm sorry for not looking at this one sooner.

I don't have any other plans to integrate hotkeys for message boxes. This fits the bill and shouldn't cause any conflicts with the new keybind setup for game controls.

[petchema]

Thanks!
As I said before, It would be even better without the explicit calls to KeyboardActivation() methods from Update(), so that keyboard support is just as smoothly integrated as mouse support...

[Interkarma]

As I said before, It would be even better without the explicit calls to KeyboardActivation() methods from Update(), so that keyboard support is just as smoothly integrated as mouse support

One possibility would be to handle the KeyboardActivation() pump in DaggerfallUI.OnGUI(). This is where the other key events for UI are collected. You could query uiManager.TopWindow to get the window at top of stack (if one is open) and call a new window method to trigger a recursive pass down through panel hierarchy to activate hotkeys as necessary. This only needs to be done once per gui update for the top window and shouldn't require many cycles.

Just throwing out ideas to help brainstorm a better solution. I like this feature and believe it can get where you want it to be. :)

[petchema]

I don't know how that part of the code works but what you describe sounds good, that's the kind of mechanism I had in mind.
Sometimes I had to look for keyboard shortcuts from two panels (see for example ModLoaderInterfaceWindow.cs), will traversing all panels down from the top window naturally find them?

[petchema]

Mmmh interesting conflict with #1625
Now that I removed the "toggle alternative" click effect, I no longer have something to attach the hotkey to to get the same behavior :/

[Interkarma]

Yeah, I thought it was best to leave this one for now until we look at other means of driving key activation. I'd be happy to collab with you on this one if that's something you'd find helpful? :)

[petchema]

Made a quick fix; But as I said in the commit message, I feel that having extra transparent panels to handle options toggling would be more clean...

[petchema]

With two subsystems handling keys (DaggerfallUI.ProcessHotkeySequences() and windows' Update()), there's some risk of hotkey sequences to be handled twice;
Most common example is "toggle close" keys: ProcessHotkeySequences() detects that you want to open a window, it opens, then the Update() method of that window recognizes the same hotkey and closes immediately.

They're several fixes possible:

• make sure all hotkeys handling is done by ProcessHotkeySequences. This currently requires all hotkeys to be assigned to some buttons, and buttons can only have one hotkey, but that could be made more flexible;

• expose whether ProcessHotKeySequences() already handled current hotkey. Flexible but not very pretty, that's what this commit does;

• move hotkeys handling from Update() method to a new windows' method, say CustomKeysProcessing(KeyModifiers keyModifiers), that can be called from ProcessHotkeySequences() to override other hotkey processing. Looks more satisfying, but turned out to be hard to get working right. petchema/daggerfall-unity@improved-keyboard-support...petchema:improved-keyboard-support-attempt

[Interkarma]

move hotkeys handling from Update() method to a new windows' method, say CustomKeysProcessing(KeyModifiers keyModifiers), that can be called from ProcessHotkeySequences() to override other hotkey processing

Your method here is sound by returning true if CustomKeysProcessing() is handled by the window. I also like that future UI windows can still do custom key processing if required without needing to modify your hotkey system or be forced to extend DialogShortcuts.txt. This is a plus for bespoke UI from mods that maybe couldn't or shouldn't be making changes to core. I think I like this option most of all.

make sure all hotkeys handling is done by ProcessHotkeySequences. This currently requires all hotkeys to be assigned to some buttons, and buttons can only have one hotkey, but that could be made more flexible;

I'd like to say this is the best option long-term. Prior to your work on this PR, this kind of hotkey handling didn't exist. Toggle open/closed was added in isolation just to accomplish that specific task without any extra framework for additional hotkeys. But I understand this presents new challenges as the toggle keys can themselves be rebound elsewhere. The hotkey system would need to support that in addition to (and perhaps conflicting with) the bindings in DialogShortcuts.txt. I don't know about you, but it seems like one dragon too many.

expose whether ProcessHotKeySequences() already handled current hotkey. Flexible but not very pretty, that's what this commit does;

Perfectly sensible compromise, thank you for that.

Would you be comfortable for this PR to make its way into live for user testing? I'm more than happy to include in next round of builds. Just let me know if you want more time in the oven and I'll let it sit for now.

[petchema]

Yes, I think this PR is ready as-is

[Interkarma]

Really lovely work here. Merging now.