Feature request: bring back Enter to main menu bindings

[rr-]

Not being able to select stuff in the main menu with Enter is really confusing and disruptive. I also had reports of players relying on having the ability to use arrows AND WSAD bound at the same time.

One way out of this that I see is to maybe turning https://github.com/LostArtefacts/Tomb1Main/issues/564 to be optional? Another would be to allow multiple bindings but that's way more effort.

[walkawayy]

I still vote for implementing #804 personally. It might require adding more keybinds, but I don't think the current way makes sense where we link Lara's movement and menu movement together.

[rr-]

We have several complaints. Here's my take on them. Implementation hints at the bottom.

• The complaint that Enter does not work in the menu like it used to. I'd hardcode the Enter key to always work within menus (keyboard only - controller should be fine as-is).

• Currently, rebinding Lara's movement to WSAD makes it impossible to navigate menu with arrows. I'd hardcode the arrows to always work within menus (keyboard only - controller should be fine as-is).

• The complaint that rebinding arrows to WSAD stops menu from working with arrow keys as soon as the user rebinds the first key, without even waiting for the user to exit the menu or anything. I'd hardcode the arrows to always work within menus (keyboard only - controller should be fine as-is). At the same time I'd make the assignment to happen only after the user confirms their keybindings, not immediately.

• We have more raports on people expecting the WSAD AND arrows to work for Lara due to their play style. Similar with other buttons. People are very divided on this. I'd create a config option to enable or disable fallback to default buttons, the amount of code required to have this isn't that much of a bother. I know, config creep, but I think both these behaviors are too expected from people for us to settle this issue more gracefully.

Example approach to sidestep our super generic control routines and hardcode the key roles

Remember that g_Input.select is for confirming stuff in the menus, g_Input.deselect is for exiting stuff and coincidentally firing the inventory. g_Input.action is for Lara grabbing stuff and shooting weapons. If you consider renaming things, prefer MENU_CONFIRM and MENU_EXIT over select / deselect – I think this name is easier to grok.

• Introduce a few INPUT_ROLE_MENU_* constants, for movement, for confirmation and possibly for exiting.
• Introduce menu_left|right|up|down to the INPUT_STATE struct.
• In m_Layout, assign INPUT_ROLE_MENU_* the arrow and Enter SDL keys. In m_ControllerLayout, copy the definitions from Lara movement and the choice of the Action key. This is the hardcoding part – the idea is that we're not going to make these extra keys assignable in options/*.c, to keep the UI simple and not confuse the users with showing two kinds of left/right/up/down keys to rebind.
• In S_Input_GetCurrentState, change linput.select = S_Input_Key(INPUT_ROLE_ACTION, layout_num) to use INPUT_ROLE_MENU_CONFIRM instead.
• In S_Input_GetCurrentState, add linput.menu_left|right|up|down = S_Input_Key(INPUT_MENU_ROLE_*, layout_num).
• In Input_Update, add g_Input.menu_left |= g_Input.left;.
• Repeat similar steps if you decide to split firing inventory input role from the exit menus key. Exiting the inventory should be special and work with both exit menu and inventory button in this case.
• In option/*.c, refer to g_Input.menu_left instead of g_Input.left. Same for g_InputDB.

Example patch for the delayed assignments

Untested pseudocode, but I think it gets my point across, especially the memcpy parts:

diff --git a/src/config.c b/src/config.c
index a6ca8e93..fe078eba 100644
--- a/src/config.c
+++ b/src/config.c
@@ -267,6 +267,7 @@ bool Config_ReadFromJSON(const char *cfg_data)
     CLAMP(g_Config.ui.bar_scale, MIN_BAR_SCALE, MAX_BAR_SCALE);

     char layout_name[50];
+    Input_EnterAssignMode();
     for (INPUT_LAYOUT layout = INPUT_LAYOUT_CUSTOM_1;
          layout < INPUT_LAYOUT_NUMBER_OF; layout++) {
         sprintf(layout_name, "layout_%d", layout);
@@ -278,6 +279,7 @@ bool Config_ReadFromJSON(const char *cfg_data)
             Input_AssignScancode(layout, role, scancode);
         }
     }
+    Input_ExitAssignMode(true);
     Input_CheckConflicts(CM_KEYBOARD, g_Config.input.layout);

     for (INPUT_LAYOUT layout = INPUT_LAYOUT_CUSTOM_1;
diff --git a/src/game/input.c b/src/game/input.c
index 128508ce..ab40703c 100644
--- a/src/game/input.c
+++ b/src/game/input.c
@@ -205,6 +205,16 @@ int16_t Input_GetAssignedAxisDir(INPUT_LAYOUT layout_num, INPUT_ROLE role)
     return S_Input_GetAssignedAxisDir(layout_num, role);
 }

+void Input_EnterAssignMode(void)
+{
+    S_Input_EnterAssignMode();
+}
+
+void Input_ExitAssignMode(bool confirm)
+{
+    S_Input_ExitAssignMode(confirm);
+}
+
 void Input_AssignScancode(
     INPUT_LAYOUT layout_num, INPUT_ROLE role, INPUT_SCANCODE scancode)
 {
diff --git a/src/game/input.h b/src/game/input.h
index a4210f7a..d7f9283e 100644
--- a/src/game/input.h
+++ b/src/game/input.h
@@ -24,6 +24,14 @@ void Input_CheckControllerConflicts(INPUT_LAYOUT layout_num);
 // within the custom layout.
 bool Input_IsKeyConflicted(CONTROL_MODE mode, INPUT_ROLE role);

+// Start assigning keys and buttons. Do not make any assignments
+// effective until calling Input_ExitAssignMode(true). Calling
+// Input_ExitAssignMode(false) will discard any changes.
+void Input_EnterAssignMode(void);
+
+// Finish assigning keys and buttons.
+void Input_ExitAssignMode(bool confirm);
+
 // Assign a concrete scancode to the given layout and key role.
 void Input_AssignScancode(
     INPUT_LAYOUT layout_num, INPUT_ROLE role, INPUT_SCANCODE scancode);
diff --git a/src/game/option/option_control.c b/src/game/option/option_control.c
index 736b7f22..0dfb95d9 100644
--- a/src/game/option/option_control.c
+++ b/src/game/option/option_control.c
@@ -663,6 +663,8 @@ CONTROL_MODE Option_Control(INVENTORY_ITEM *inv_item, CONTROL_MODE mode)
         ? CtrlTextPlacementCheats
         : CtrlTextPlacementNormal;

+    Input_EnterAssignMode();
+
     switch (m_KeyMode) {
     case KM_BROWSE:
         if (layout_num > INPUT_LAYOUT_DEFAULT) {
@@ -683,6 +685,7 @@ CONTROL_MODE Option_Control(INVENTORY_ITEM *inv_item, CONTROL_MODE mode)
         if (g_InputDB.deselect
             || (g_InputDB.select && m_ControlMenu.cur_role == KC_TITLE)) {
             Option_ControlShutdownText();
+            Input_ExitAssignMode(/*?*/);
             m_KeyMode = KM_INACTIVE;
             g_Input = (INPUT_STATE) { 0 };
             g_InputDB = (INPUT_STATE) { 0 };
@@ -783,5 +786,6 @@ CONTROL_MODE Option_Control(INVENTORY_ITEM *inv_item, CONTROL_MODE mode)

     m_ControlMenu.prev_row_num = m_ControlMenu.row_num;

+    Input_ExitAssignMode(/*?*/);
     return mode;
 }
diff --git a/src/specific/s_input.c b/src/specific/s_input.c
index 6cafdd11..eaa10b2b 100644
--- a/src/specific/s_input.c
+++ b/src/specific/s_input.c
@@ -26,7 +26,7 @@ typedef struct CONTROLLER_MAP {
     int16_t axis_dir;
 } CONTROLLER_MAP;

-static INPUT_SCANCODE m_Layout[INPUT_LAYOUT_NUMBER_OF][INPUT_ROLE_NUMBER_OF] = {
+static INPUT_SCANCODE m_KeyboardLayout[INPUT_LAYOUT_NUMBER_OF][INPUT_ROLE_NUMBER_OF] = {
     // clang-format off
     // built-in controls
     {
@@ -325,6 +325,9 @@ static CONTROLLER_MAP
         // clang-format on
     };

+static INPUT_SCANCODE m_KeyboardLayoutAux[INPUT_LAYOUT_NUMBER_OF][INPUT_ROLE_NUMBER_OF];
+static CONTROLLER_MAP m_ControllerLayoutAux[INPUT_LAYOUT_NUMBER_OF][INPUT_ROLE_NUMBER_OF];
+
 const Uint8 *m_KeyboardState;
 static SDL_GameController *m_Controller = NULL;
 static const char *m_ControllerName = NULL;
@@ -765,7 +768,7 @@ static const char *S_Input_GetAxisName(

 static bool S_Input_Key(INPUT_ROLE role, INPUT_LAYOUT layout_num)
 {
-    INPUT_SCANCODE scancode = m_Layout[layout_num][role];
+    INPUT_SCANCODE scancode = m_KeyboardLayout[layout_num][role];
     if (KEY_DOWN(scancode)) {
         return true;
     }
@@ -958,7 +961,7 @@ INPUT_STATE S_Input_GetCurrentState(
 INPUT_SCANCODE S_Input_GetAssignedScancode(
     INPUT_LAYOUT layout_num, INPUT_ROLE role)
 {
-    return m_Layout[layout_num][role];
+    return m_KeyboardLayout[layout_num][role];
 }

 int16_t S_Input_GetUniqueBind(INPUT_LAYOUT layout_num, INPUT_ROLE role)
@@ -997,10 +1000,26 @@ int16_t S_Input_GetAssignedAxisDir(INPUT_LAYOUT layout_num, INPUT_ROLE role)
     return m_ControllerLayout[layout_num][role].axis_dir;
 }

+void S_Input_EnterAssignMode(void)
+{
+    m_AssignMode = true;
+    memcpy(m_KeyboardLayoutAux, m_KeyboardLayout, sizeof(INPUT_SCANCODE) * INPUT_LAYOUT_NUMBER_OF * INPUT_ROLE_NUMBER_OF);
+    memcpy(m_ControllerLayoutAux, m_ControllerLayout, sizeof(CONTROLLER_MAP) * INPUT_LAYOUT_NUMBER_OF * INPUT_ROLE_NUMBER_OF);
+}
+
+void S_Input_ExitAssignMode(bool confirm)
+{
+    m_AssignMode = false;
+    if (confirm) {
+        memcpy(m_KeyboardLayout, m_KeyboardLayoutAux, sizeof(INPUT_SCANCODE) * INPUT_LAYOUT_NUMBER_OF * INPUT_ROLE_NUMBER_OF);
+        memcpy(m_ControllerLayout, m_ControllerLayoutAux, sizeof(CONTROLLER_MAP) * INPUT_LAYOUT_NUMBER_OF * INPUT_ROLE_NUMBER_OF);
+    }
+}
+
 void S_Input_AssignScancode(
     INPUT_LAYOUT layout_num, INPUT_ROLE role, INPUT_SCANCODE scancode)
 {
-    m_Layout[layout_num][role] = scancode;
+    m_KeyboardLayoutAux[layout_num][role] = scancode;
 }

 void S_Input_AssignButton(
@@ -1036,7 +1055,7 @@ bool S_Input_ReadAndAssignKey(
         for (INPUT_SCANCODE scancode = 0; scancode < SDL_NUM_SCANCODES;
              scancode++) {
             if (KEY_DOWN(scancode)) {
-                m_Layout[layout_num][role] = scancode;
+                m_KeyboardLayout[layout_num][role] = scancode;
                 return true;
             }
         }
@@ -1064,9 +1083,11 @@ const char *S_Input_GetKeyName(
     CONTROL_MODE mode, INPUT_LAYOUT layout_num, INPUT_ROLE role)
 {
     if (mode == CM_KEYBOARD) {
-        return S_Input_GetScancodeName(m_Layout[layout_num][role]);
+        const blabla layout = m_EditMode ? m_KeyboardLayoutAux : m_KeyboardLayout;
+        return S_Input_GetScancodeName(layout[layout_num][role]);
     } else {
-        CONTROLLER_MAP check = m_ControllerLayout[layout_num][role];
+        const blabla layout = m_EditMode ? m_ControllerLayoutAux : m_ControllerLayout;
+        CONTROLLER_MAP check = layout[layout_num][role];
         if (check.type == BT_BUTTON) {
             return S_Input_GetButtonName(check.bind.button);
         } else {
diff --git a/src/specific/s_input.h b/src/specific/s_input.h
index b3140555..c0cc2f37 100644
--- a/src/specific/s_input.h
+++ b/src/specific/s_input.h
@@ -23,6 +23,10 @@ int16_t S_Input_GetAssignedBind(INPUT_LAYOUT layout_num, INPUT_ROLE role);

 int16_t S_Input_GetAssignedAxisDir(INPUT_LAYOUT layout_num, INPUT_ROLE role);

+void S_Input_EnterAssignMode(void);
+
+void S_Input_ExitAssignMode(bool confirm);
+
 void S_Input_AssignScancode(
     INPUT_LAYOUT layout_num, INPUT_ROLE role, INPUT_SCANCODE scancode);