[RDY] Allow diagonal movement via keybinding modifiers in SDL builds

[ZhilkinSerg]

First approach for #20447.

What was changed:

• allows diagonal movement with cursor keys using CTRL and SHIFT modifiers via DIAG_MOVE_WITH_MODIFIERS interface option (currently enabled only in SDL builds and disabled in CURSES build);
• diagonal movement action keys are taken from keybindings, so you need these to be configured for diagonal movements to work.

Default key bindings for diagonal movement:

• 1 = Move Southwest;
• 3 = Move Southeast;
• 7 = Move Northwest;
• 9 = Move Northeast.

New key bindings:

• Shift + Cursor Left -> 7 = Move Northwest;
• Shift + Cursor Right -> 3 = Move Southeast;
• Shift + Cursor Up -> 9 = Move Northeast;
• Shift + Cursor Down -> 1 = Move Southwest.

and

• Ctrl + Cursor Left -> 1 = Move Southwest;
• Ctrl + Cursor Right -> 9 = Move Northeast;
• Ctrl + Cursor Up -> 7 = Move Northwest;
• Ctrl + Cursor Down -> 3 = Move Southeast.

Illustration:

Screenshots:

[codemime]

Numeric constants in src/input.h would be better than the magic numbers.

[ZhilkinSerg]

I agree. My final aim is to be able to load keys from keybindings.

[ZhilkinSerg]

Movement keys are now taken from input manager.

[codemime]

Formatting (here and there).

[ZhilkinSerg]

Thanks. Astyled my changes.

[codemime]

Any luck with ncurses?

[AMurkin]

Illustration:

[ZhilkinSerg]

Any luck with ncurses?

Didn't look into ncurses yet.

@AMurkin, thanks for the picture.

[BevapDin]

My final aim is to be able to load keys from keybindings.

In that case, you should probably change "input.cpp" and add support for modifier keys. input_event already has an (unused) member modifiers. This should be filled with the status of the shift/control/(maybe)alt keys.

At least for SDL builds, the modifiers can be set in "sdltiles.cpp" (maybe where last_input is set input_event( the_key, CATA_INPUT_KEYBOARD)). You'll have to change functions like popup_getkey to return an input_event instead of the plain character to get that information into the key binding setup in "input.cpp". You'll also have to enhance the loading of keybindings (input_manager::load) to load the modifiers from JSON.

[ZhilkinSerg]

In that case, you should probably change "input.cpp" and add support for modifier keys. input_event already has an (unused) member modifiers. This should be filled with the status of the shift/control/(maybe)alt keys.
At least for SDL builds, the modifiers can be set in "sdltiles.cpp" (maybe where last_input is set input_event( the_key, CATA_INPUT_KEYBOARD)). You'll have to change functions like popup_getkey to return an input_event instead of the plain character to get that information into the key binding setup in "input.cpp". You'll also have to enhance the loading of keybindings (input_manager::load) to load the modifiers from JSON.

Thanks, but it happened to be too hard for me to implement this, so I decided not to save modifiers together with keybindings, but simply read action characters from input_manager.

[ZhilkinSerg]

Should be good to go for sdl builds.

I've started research for ncurses support in separate branch.

[BevapDin]

Why are you explicitly casting this using a C-style cast (there is already an implicit conversion for the assignment)?

Why the local variable, why not return it directly?

Why return a char, when the return value (first_input) is a long and the result of the function is promoted to a long again (see return statement in sdl_keysym_to_curse, which returns a long)?

[ZhilkinSerg]

That is true. I've simplified new function to remove redundant type cast.

[BevapDin]

How can you be sure that the vector returned by get_input_for_action always has at least one entry?

[ZhilkinSerg]

I've added vector size check.

[BevapDin]

Why is there a default value for the second parameter? All calls to this function supply a specific value anyway, the default is never used. And why is it a const value, but the first parameter is a reference to const?

[ZhilkinSerg]

That was copied from get_input_for_action.

[BevapDin]

I can't find any input context "keyboard", did you mean "default"?

[ZhilkinSerg]

Context is unnecessary, so I removed it.

[BevapDin]

No other build specific option mentions that is only available in certain build types. (Note that COPT_CURSES_HIDE already hides it on curses builds.) For the sake of consistency, this should not be mentioned at all, or be mentioned by all options, preferably automated by checking cOpt::hide.

[ZhilkinSerg]

I've removed SDL build mention from option description.

[ZhilkinSerg]

Any luck with ncurses?

I have to admit ncurses is out of my expertise now.

[codemime]

Works as expected (although, it took some time to get used to it). I'd only change two things:

• The menu option shouldn't appear in ncurses builds since it doesn't affect them for now;
• I'd make the option default in SDL builds (at first I thought the feature didn't work).

[ZhilkinSerg]

The menu option shouldn't appear in ncurses builds since it doesn't affect them for now;

There is COPT_CURSES_HIDE modifier, so option won't appear in CURSES builds (similar to USE_TILES option).

    add( "DIAG_MOVE_WITH_MODIFIERS", "interface", translate_marker( "Diagonal movement with cursor keys and modifiers" ),
        translate_marker( "If true, allows diagonal movement with cursor keys using CTRL and SHIFT modifiers.  Diagonal movement action keys are taken from keybindings, so you need these to be configured." ),
        true, COPT_CURSES_HIDE
        );

I'd make the option default in SDL builds (at first I thought the feature didn't work).

I've changed default value of DIAG_MOVE_WITH_MODIFIERS to true.

[Litppunk]

does this also affect menus? My game bugged last time I loaded so the diagonals were default. But on save/exit and reload worked fine, unless maybe caps lock got clicked or something, I'm not sure what might have caused it. Something to look out for.

[kevingranade]

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/dropping-things-on-the-wrong-tile-constantly/18223/8