Stop unmapped keybinds having an effect on the game

[Richard-L]

Generally didn't we have a discussion somewhere on the default buttons still having an effect unintentionally, even if the user has chosen different keybinds?

This is a separate issue and needs its own discussion. Can you open a ticket for this?

Originally posted by @rr- in https://github.com/rr-/Tomb1Main/issues/479#issuecomment-1129946114

Keys like ALT, which are the default keybind for jumps, will keep having an effect on the game, even if the user has chosen "User Keys" under "Controls", and has not mapped the ALT key for anything. It would be nice if only actually mapped keys had an effect on the game.

This is especially noticeable when using the ALT+ENTER shortcut to toggle fullscreen mode, where Lara will definitely jump no matter what, often messing with the state you wanted to leave the game at. A workaround for now is to just pause the game prior to toggling fullscreen.

Considerations must be taken so the user doesn't softlock their control scheme which is why default keys always work currently.

[rr-]

My concern here is that if the player chooses to do some silly thing such as rename all the arrows to the same key, they won't be able to revert without closing the game and editing the JSON settings file by hand.

My suggestion - when checking if the player has pressed a button with the given role (e.g. the jump button):

if the layout is default:
    check the default layout key
else if the corresponding user layout key is being used to map only one action (e.g. it doesn't flash in the controls menu):
    check the user layout key
else:
    check the default layout key

That way if the player maps up and down keys to the same button, that mapping will have no effect and they'll have to use arrows.

My other concern is that some players may want to remap Lara's movement to WSAD but still navigate the menus with the arrow keys.

[Richard-L]

I would not allow mapping arrow keys to the same key. It just bricks them, as you say.

I also wouldn't separate inventory from in-game direction keys. OpenLara does this (ie in inventory you always use the standard arrow keys), and it's really quite frustrating imo. If they map to WASD then that's what the game should know as directions. Keeps things simple.

[walkawayy]

I agree with Posix. Just don't let the user map two inputs to the same key and that should prevent issues I think? Also in the worst case scenario, the runtime config file can be deleted to remove the custom keybinds.

[walkawayy]

Potential patches from rr:

diff --git a/src/specific/s_input.c b/src/specific/s_input.c
index 83c9828f..1812fcd7 100644
--- a/src/specific/s_input.c
+++ b/src/specific/s_input.c
@@ -352,9 +352,9 @@ static bool S_Input_KbdKey(INPUT_ROLE role, INPUT_LAYOUT layout)

 static bool S_Input_Key(INPUT_ROLE role)
 {
-    return S_Input_KbdKey(role, INPUT_LAYOUT_USER)
-        || (!Input_IsKeyConflictedWithDefault(role)
-            && S_Input_KbdKey(role, INPUT_LAYOUT_DEFAULT));
+    return Input_IsKeyConflictedWithUser(role) ?
+        S_Input_KbdKey(role, INPUT_LAYOUT_DEFAULT) :
+        S_Input_KbdKey(role, INPUT_LAYOUT_USER);
 }

diff --git a/src/game/input.c b/src/game/input.c
index 43edd37a..db9c909f 100644
--- a/src/game/input.c
+++ b/src/game/input.c
@@ -17,7 +17,6 @@ INPUT_STATE g_InputDB = { 0 };
 INPUT_STATE g_OldInputDB = { 0 };
 
 static bool m_KeyConflictWithUser[INPUT_ROLE_NUMBER_OF] = { false };
-static bool m_KeyConflictWithDefault[INPUT_ROLE_NUMBER_OF] = { false };
 static int32_t m_HoldBack = 0;
 static int32_t m_HoldForward = 0;
 
@@ -32,7 +31,6 @@ static void Input_CheckConflicts(void)
         INPUT_SCANCODE scancode1_user =
             Input_GetAssignedScancode(INPUT_LAYOUT_USER, role1);
         m_KeyConflictWithUser[role1] = false;
-        m_KeyConflictWithDefault[role1] = false;
 
         for (INPUT_ROLE role2 = 0; role2 < INPUT_ROLE_NUMBER_OF; role2++) {
             if (role1 == role2) {
@@ -45,9 +43,6 @@ static void Input_CheckConflicts(void)
             if (scancode1_user == scancode2_user) {
                 m_KeyConflictWithUser[role1] = true;
             }
-            if (scancode1_default == scancode2_user) {
-                m_KeyConflictWithDefault[role1] = true;
-            }
         }
     }
 }
@@ -161,11 +156,6 @@ bool Input_IsKeyConflictedWithUser(INPUT_ROLE role)
     return m_KeyConflictWithUser[role];
 }
 
-bool Input_IsKeyConflictedWithDefault(INPUT_ROLE role)
-{
-    return m_KeyConflictWithDefault[role];
-}
-
 INPUT_SCANCODE Input_GetAssignedScancode(
     INPUT_LAYOUT layout_num, INPUT_ROLE role)
 {
diff --git a/src/game/input.h b/src/game/input.h
index 95525205..0c2b5664 100644
--- a/src/game/input.h
+++ b/src/game/input.h
@@ -18,10 +18,6 @@ void Input_Update(void);
 // within the custom layout.
 bool Input_IsKeyConflictedWithUser(INPUT_ROLE role);
 
-// Returns whether the key assigned to the given role is conflicting with a
-// default key binding.
-bool Input_IsKeyConflictedWithDefault(INPUT_ROLE role);
-
 // 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/specific/s_input.c b/src/specific/s_input.c
index 83c9828f..1812fcd7 100644

[walkawayy]

This issue is delayed until controller support is fully merged. Also, we will have to think about the consequences of unbinding default keys. Lastly with controller support, we probably want to support customizable controller layouts. So, this might require an entire Controls menu rewrite.

[FaLiar]

any progression? i cant play with default settings.... :(

[walkawayy]

I'm going to attempt to take a look at this. No promises though ha.

For now I have increased the number of custom schemes from 1 to 3. I also added a Hold R to reset to defaults (3 second hold) for the currently selected screen. A visual indication of the holding time would be nice to have for this. I also wasn't sure about what key to use, but I wanted to use a non default key and R goes with reset. It's a hard coded key, so the user can reset a scheme if something goes wrong. I also added keybinds for additional settings like equipping weapons and quick medipacks. Here's a WIP image:

I will probably try to incrementally PR this (and hopefully people see the value in it). Additional custom schemes with a reset key to start, then maybe a second PR to for this issue? In the future, I also would like to be able to add buttom combos (mainly for controllers) since combos like Triangle + Up D Pad to draw Pistols, Triangle + Left D Pad to draw shotgun, etc could be really nice. And maybe like L1+R1 to quick save, and L2+R2 to quick load. But that's all way down the line. And I'm not sure it's feasible in TR engine.

I am trying to follow this very accessible game for inspiration: https://www.youtube.com/watch?v=qbkRptgqQg8

[Richard-L]

Very nice walkaway!

I like your approach to this. If I had the power to produce it, I would:

• leave ESC unassignable and make it always execute "go back".
• hold ESC to unmap the selected input.
• Offer only one global "reset to defaults" button underneath the table. (Is it necessary to allow per-input reset-to-default?)

Further on unmapping inputs: I was hoping I could manually delete individual inputs in the config files but didn't find anything. Thus this might be an unrelated separate would-be feature, but something to do with mapping I would welcome a lot, to help keep your mappings clean as they grow in number and specificity. For instance, camera altering cannot be turned off, but I know I don't ever want to use it. Its default mappings interefere with keys I want to use for things like OBS or other programs running next to TR1M. So it can be a bit of a challenge to find obscure keys that I don't use, so that the camera isn't changed. It then happens quite easily though that you accidentally touch some of these inputs, and don't notice until later your camera is off. It then needs another check in the menu what key you have to reset the camera. All a bit messy.

Your ambitions with combos for pad users is a big one. I believe TR3 was the first in the series to include R1+SELECT as a combo for flares (necessitated by the new crouch and sprint inputs). The menu you linked is beautiful, but the restrictions of TR1's UI make it seem more or less impossible to include all these indicators (?).

[walkawayy]

Custom control layouts are stored in: ...\cfg\Tomb1Main_runtime.json5.

Good point on unmapping individual keybinds. I also need to fit that into the UI.

[Richard-L]

Oh does this correspond to the key mapping?

{
  "layout_sdl" : [
    93,
    90,
    89,
    91,
    20,
    8,
    44,
    7,
    22,
    26,
    6,
    4,
    23,
    18,
    12,
    15,
    43,
    40,
    13,
    16,
    17,
    54,
    14
  ]
}

Where would I find what row represends what keybind?

[walkawayy]

Have to sort through this list as best I can tell. https://docs.rs/sdl2-sys/0.22.0/x86_64-pc-windows-gnu/sdl2_sys/scancode/index.html?search=