Implement keyboard rebinding support #1301

Fabxx · 2022-12-01T21:53:24Z

Special thanks to:

toxicshadow, dracc and antangelo for suggestions.
SSUPI for correcting some errors in the SDL_Event handling.

Partially fixes #136 for keyboard rebinding. Controller rebinding will come in another PR

-Allows you to rebind keyboard controls

-Controls are stored in the .toml file configuration for next boots (no need to remap)

-Checks if a key has already been bound to any previous key to avoid conflicts.

-Uses same window for the controller, no extra UI windows.

-A white text under the toggles tells to the user what key for xemu he is binding for each key.

-Button press, rumble, analog movement and trigger animations during remapping are disabled to avoid confusing visual conflicts with already mapped keys before changing them.

-When remapping, a "Abort rebinding" toggle will appear, so the user can manually stop the binding process. This will not alter the already done bindings just in case the user presses the toggle by mistake.

-The auto-bind option has been removed, instead there's a "Reset controls to default" option, so the user can reset his controls while running xemu, and doesn't need to reboot to get the controls updated at runtime. The .toml file is updated on reboot with defaults if a remapping is not done after this option.

-While remapping, the "Reset controls to default" toggle will disappear.

-As soon the user by mistake changes window or closes the menu, the remapping process is aborted and we restore the default keys, since SDL still continues to catch events if outside of that window, meaning that the user will involountary map the keys even if it doesn't want to.

-The ESC button will NOT make the user exit the window while remapping. The button can be mapped as well.

Showcase (updated at 6/12/2022 14:56pm):

8mb.video-DjK-X4K1DlWm.mp4

… c_config.

…nding

… while remapping

antangelo

Left a few comments. Another thing I noticed while using it:

While remapping keys, it's possible for the keyboard inputs to be used to navigate the UI. I think this should be disabled while remapping.
The need to disable auto-bind is rather unintuitive. If there is some bug where the config is overwritten by auto-bind being on, then I think it should be fixed instead of requiring it to be disabled.

ui/xemu-input.c

ui/xui/gl-helpers.cc

ui/xui/main-menu.cc

ui/xemu-input.c

…iption and event handling.

… Reworked a bit code structure to be less redundant.

…ng messages from infos. Inform the user after the remapping is done.

Fabxx · 2022-12-06T15:24:00Z

The modifications are tested and complete, feel free to test it and report if you agree with the choices applied to the input management and code design

ElTioRata · 2022-12-10T04:04:11Z

Does this includes rebinding buttons to mouse?

Fabxx · 2022-12-12T12:29:15Z

Does this includes rebinding buttons to mouse?

No, of course it can be implemented by just catching the SDL event for the mouse press and bind it to that scancode, but the right mouse button is used to access the UI. Besides i think it's more proper to implement the mouse binding only on analog axis movement since that's the most useful thing when playing with both keyboard and mouse. If the devs agree with this thinkering i might implement it in this PR already. UPDATE: If we want to make sure that the user can use the ESC Button and the mouse buttons/movement we should:

-Disable the esc button for the UI permanently, i already have an idea for it.
-Disable the right click mouse button for the UI permanently

This is needed if we have to guarantee the usage of all keys for the user while playing and not having interferences with the UI, not to mention if i add mouse remapping as well that could be very useful.

I can already do the code but i need a response to understand how i have to move. So guarantee freedom with keyboard + mouse but disable button UI interaction, and just let the user move the mouse/use the controller to access the UI, or it stays like so.

UPDATE: I guess i'll leave it as it is to avoid a mess. The code works as intended for the keyboard rebinding, mouse/controller might come in another PR later.

ElTioRata · 2023-01-04T01:46:24Z

Tried the build for a few weeks, works fine. However, I think it would be a lot more convenient if you could remap specific buttons separately instead of going through the whole remap process. It would also be nice if you could add key mapping profiles as well, since not all games are comfortable to play with the same keys.

Fabxx · 2023-01-04T09:57:03Z

Tried the build for a few weeks, works fine. However, I think it would be a lot more convenient if you could remap specific buttons separately instead of going through the whole remap process. It would also be nice if you could add key mapping profiles as well, since not all games are comfortable to play with the same keys.

For the profiles might be doable, for the specific buttons i have to see

Fabxx · 2023-01-12T21:39:11Z

The system works as intended, but in the future it will be made more flexible, but for now it's possible to simply bind all the keys to the combination wanted.

antangelo · 2023-06-06T15:50:37Z

ui/xemu-input.c

+    } else if (is_remapping_active) {
+        xemu_input_keyboard_rebind(event);
+    } else if (restore_controls) {
+        xemu_input_restore_defaults();


Why is this done here instead of being called directly when the restore button is pressed?

antangelo · 2023-06-06T15:52:22Z

ui/xemu-input.c

+
+void xemu_input_keyboard_rebind(const SDL_Event *ev)
+{   
+    //Check if the user aborts the remapping process.


nit: This comment adds no value

antangelo · 2023-06-06T15:53:42Z

ui/xui/gl-helpers.cc

}
+

nit: Extra newline

antangelo · 2023-06-06T15:55:03Z

ui/xui/gl-helpers.cc

                                                              primary_color,
-                (state->buttons & CONTROLLER_BUTTON_RSTICK) ? primary_color :
+                (state->buttons & rstick_press_anim) ? primary_color :


Same as above

antangelo · 2023-06-06T15:55:41Z

ui/xui/gl-helpers.cc

    float w = tex_items[obj_lstick].w;
    float h = tex_items[obj_lstick].h;
    float c_x = frame_x+lstick_ctr.x;
    float c_y = frame_y+lstick_ctr.y;
    float lstick_x = (float)state->axis[CONTROLLER_AXIS_LSTICK_X]/32768.0;
    float lstick_y = (float)state->axis[CONTROLLER_AXIS_LSTICK_Y]/32768.0;
+    int lstick_press_anim = CONTROLLER_BUTTON_LSTICK;


Should this take the remapping into account?

antangelo · 2023-06-06T16:01:28Z

ui/xui/main-menu.cc

@@ -38,6 +38,16 @@
 #include "../xemu-xbe.h"

 MainMenuScene g_main_menu;
+bool is_remapping_active = false;
+bool duplicate_found = false;


This variable is unused

antangelo · 2023-06-06T16:01:51Z

ui/xui/main-menu.cc

+    }
+
+    if (!is_remapping_active) {
+        if (Toggle("Reset controls to default", &restore_controls,


Same comment about being a toggle as below

antangelo · 2023-06-06T16:03:17Z

ui/xui/main-menu.cc

@@ -38,6 +38,16 @@
 #include "../xemu-xbe.h"

 MainMenuScene g_main_menu;
+bool is_remapping_active = false;


Globals should be prefixed with g_

antangelo · 2023-06-06T16:07:37Z

ui/xemu-input.c

+    sdl_kbd_scancode_map[23]           = SDL_SCANCODE_K;
+    sdl_kbd_scancode_map[24]           = SDL_SCANCODE_O;
+
+    g_config.input.keyboard_controller_scancode_map.a            = SDL_SCANCODE_A;


Create a function that takes sdl_kbd_scancode_map as a parameter and sets the g_config values like you do when binding is complete, then reuse that here to clean this up.

antangelo · 2023-06-06T16:15:01Z

ui/xui/main-menu.cc

+bool is_remapping_active = false;
+bool duplicate_found = false;
+bool restore_controls = false;
+bool abort_rebinding = false;


You can detect the abort case if you keep a static variable in_progress in xemu_input_keyboard_rebind that tracks this:

Initialize to false

If is_remapping_active is true, and in_progress is false, set it to true

If is_remapping_active is false, and in_progress is true, this is the abort case. Only change you have to add is to set in_progress to false after handling

If both are false do nothing, same as now.

When finished remapping, set in_progress to false.

You can rename it to something better if you want, just a suggestion. This removes the need for an extra global.

Fabxx · 2023-08-07T08:43:40Z

closing this, please follow #1516 since it's in a better state.

Fabxx added 11 commits November 30, 2022 23:48

b2e81c9

Empty previous keyboard config before remappig, store new config into…

c8a8c5d

… c_config.

update comments

154bf73

remove highlight shader while binding, give names of buttons while bi…

e345c02

…nding

don't clean array of bindings at each iteration. Don't use animations…

0880d67

… while remapping

add checks for duplicated keybindings

d81bc5e

Add notification message for duplicated warning

13490b0

Tell the already mapped button what is used for

5051911

check that auto binding is not enabled before remapping

eb04bf5

323f995

Remove FIXME

eb3df6b

antangelo reviewed Dec 1, 2022

View reviewed changes

Fabxx added 3 commits December 2, 2022 01:16

Adjust indentation, remove unnecessary checks, update auto-bind descr…

a95cdb0

…iption and event handling.

Unbind keyboard focus from input UI while remapping

b828dfd

added more checks for mapping, using toggle style instead of buttons.…

1d3bb78

… Reworked a bit code structure to be less redundant.

Fabxx marked this pull request as draft December 4, 2022 22:02

Fabxx added 13 commits December 4, 2022 23:07

make the user rebind button if input out of SDL range.

4a0c89f

disable rumble anim, add more checks for remapping. Distinguish warni…

0bea80b

…ng messages from infos. Inform the user after the remapping is done.

update analog anim comment

b18a1a5

remove redundant warning message when aborting remapping process

241aea6

update Toggle description

de95d9c

Update UI messages and restore window change check

46a0015

remove window focus depency when remapping.

2668daf

remove boolean leftover

ab396c7

adjust indentation

159e571

a5be7a3

Fix ESC button making exit from menu

d4199b3

disable analog and trigger anims while mapping, remove useless new line

6883c4e

disable stick press animation while remapping

c54a930

Fabxx marked this pull request as ready for review December 6, 2022 13:57

Fabxx marked this pull request as draft January 12, 2023 17:42

Fabxx marked this pull request as ready for review January 12, 2023 21:38

antangelo reviewed Jun 6, 2023

View reviewed changes

antangelo mentioned this pull request Aug 7, 2023

ui: Support changing controller/keyboard mappings #1516

Open

Fabxx closed this Aug 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement keyboard rebinding support #1301

Implement keyboard rebinding support #1301

Fabxx commented Dec 1, 2022 •

edited

Loading

antangelo left a comment

Fabxx commented Dec 6, 2022

ElTioRata commented Dec 10, 2022 •

edited

Loading

Fabxx commented Dec 12, 2022 •

edited

Loading

ElTioRata commented Jan 4, 2023 •

edited

Loading

Fabxx commented Jan 4, 2023

Fabxx commented Jan 12, 2023

antangelo Jun 6, 2023

antangelo Jun 6, 2023

antangelo Jun 6, 2023

antangelo Jun 6, 2023

antangelo Jun 6, 2023

antangelo Jun 6, 2023

antangelo Jun 6, 2023

antangelo Jun 6, 2023

antangelo Jun 6, 2023

antangelo Jun 6, 2023

Fabxx commented Aug 7, 2023

Implement keyboard rebinding support #1301

Implement keyboard rebinding support #1301

Conversation

Fabxx commented Dec 1, 2022 • edited Loading

antangelo left a comment

Choose a reason for hiding this comment

Fabxx commented Dec 6, 2022

ElTioRata commented Dec 10, 2022 • edited Loading

Fabxx commented Dec 12, 2022 • edited Loading

ElTioRata commented Jan 4, 2023 • edited Loading

Fabxx commented Jan 4, 2023

Fabxx commented Jan 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fabxx commented Aug 7, 2023

Fabxx commented Dec 1, 2022 •

edited

Loading

ElTioRata commented Dec 10, 2022 •

edited

Loading

Fabxx commented Dec 12, 2022 •

edited

Loading

ElTioRata commented Jan 4, 2023 •

edited

Loading