Fix keybinding assignment in uilists (e.g., Map Notes) #55288
Conversation
github-actions
bot
added
json-styled
Maleclypse
added
Controls / Input
|
This has broken keycode mode. |
| for( int ch = 'A'; ch <= 'Z'; ++ch ) { | ||
| queue->codes_keychar.emplace_back( ch ); | ||
|
|
||
| std::string available_hotkeys = ctxt.get_available_single_char_hotkeys(); |
There was a problem hiding this comment.
get_available_single_char_hotkeys only works for character mode. Not all characters returned from this call is a valid key code, and that's why get_available_single_char_hotkeys was specifically not used here.
Also, the removal of conflicting keys is actually handled by input_context::first_unassigned_hotkey, so this isn't a proper fix.
| @@ -289,6 +289,7 @@ void uilist::init() | |||
| max_column_len = 0; // for calculating space for second column | |||
|
|
|||
| input_category = "UILIST"; | |||
| input_context ctxt( input_category, keyboard_mode::keychar ); | |||
There was a problem hiding this comment.
This only creates an input context in the local scope and has not effect on the ctxt member, so the ctxt member actually uses keycode mode when filtering, which makes text input impossible. The filter context and main context also are supposed to use different keyboard modes, and this change breaks that too.
| } | ||
| queue->modifiers_keycode.emplace_back(); | ||
| queue->modifiers_keycode.emplace_back( std::set<keymod_t>( { keymod_t::shift } ) ); |
There was a problem hiding this comment.
This was here for a reason and you shouldn't simply remove it. It makes the queue generate hotkeys modified with the shift key in place of capital letters in character mode.
| @@ -1896,26 +1896,19 @@ const hotkey_queue &hotkey_queue::alphabets() | |||
| return *queue; | |||
| } | |||
|
|
|||
| const hotkey_queue &hotkey_queue::alpha_digits() | |||
| const hotkey_queue &hotkey_queue::create_from_available_hotkeys( input_context &ctxt ) | |||
There was a problem hiding this comment.
This will always return the queue created for the first input context this function is called with, because queue is static.
| @@ -348,7 +349,7 @@ void uilist::filterlist() | |||
|
|
|||
| void uilist::inputfilter() | |||
| { | |||
| input_context ctxt( input_category, keyboard_mode::keychar ); | |||
| ctxt.set_category( input_category ); | |||
There was a problem hiding this comment.
This will make the filter context have actions registered in the main context, which may cause incorrect behavior,
Summary
Bugfixes "Fix keybinding assignment in uilists (e.g., Map Notes)"
Purpose of change
Fixes #54778
Describe the solution
Reuse the input_context in uilists and create queues only from available hotkeys
Describe alternatives you've considered
input_context &ctxaround a whole bunch. This felt more difficult than adding a class variable and the set category functionTesting
Created a bunch of map notes. Opened the map and viewed the
List Notesmenu. Compared the keybindings and tested the key functionalities within the menu.This probably needs testing on other uilists to see how this affects them.
Additional context
Next step would be to evaluate if
hotkey_queue &hotkey_queue::alphabets()is causing similar issues for npctalk and veh_interact. I suspect it is, but have not tested this yet. I can make this into another PR once verified it's an issue and this one is validated as working correctly.Before change:
After change: (Note that keys such as f (find), j (down), k (up), E (Edit), M (Mark dangerous), D (Delete) do not show up
