Input Bug

[TempleOfHorus]

Bizhawk is skipping input when I hold down "CTRL" key as input. On the video below you can notice that Lara stops shooting for a while and then starts shooting again. She should shoot all the time, because I hold down "CTRL" key all the time.

https://youtu.be/xO-29MNUT_U

[YoshiRulz]

A keyup event for any bound or unbound key will set all buttons bound to a modifier key to unpressed, until the next keydown event for any bound or unbound key, at which point all held buttons are correctly set... wat

screencast recorded on https://github.com/TASEmulators/BizHawk/commit/e2a36c7242cdc7497efa61d18b6afda799586f66

[Morilli]

The commit that broke this was accf0f038. I haven't figured out a proper fix yet, but I may have found the cause.

For example, when logging the input events generated here: https://github.com/TASEmulators/BizHawk/blob/3f9fb0eaef88ca6d89489c55d617fb3f0b9f9a66/src/BizHawk.Client.EmuHawk/Input/Input.cs#L101-L107 for the sequence press and hold Ctrl -> press and hold E -> release E -> release Ctrl, it results in:

Before accf0f038:

input event generated: Press:LeftCtrl
input event generated: Press:Ctrl+E
input event generated: Release:Ctrl+E
input event generated: Release:LeftCtrl

After accf0f038:

input event generated: Press:Ctrl
input event generated: Press:Ctrl+E
input event generated: Release:Ctrl+E
input event generated: Release:Ctrl

I believe that the event Release:Ctrl+E was previously not affecting the LeftCtrl key, which is why that kept on being pressed. Now, the Release event will release the same key that was previously Pressed.

[Morilli]

Here's a diff that would fix this issue, however I'm entirely unsure about its correctness, especially given the comments in this function which I didn't fully understand:

diff --git a/src/BizHawk.Client.EmuHawk/Input/Input.cs b/src/BizHawk.Client.EmuHawk/Input/Input.cs
index 7573586ea..116dcff5c 100644
--- a/src/BizHawk.Client.EmuHawk/Input/Input.cs
+++ b/src/BizHawk.Client.EmuHawk/Input/Input.cs
@@ -118,7 +118,9 @@ namespace BizHawk.Client.EmuHawk

                        // don't generate events for things like Ctrl+LeftControl
                        var mods = _modifiers;
-                       if (currentModifier is not 0U)
+                       if (!newState)
+                               mods = 0;
+                       else if (currentModifier is not 0U)
                                mods &= ~currentModifier;

                        var ie = new InputEvent
@@ -127,37 +129,9 @@ namespace BizHawk.Client.EmuHawk
                                        LogicalButton = new(button1, mods, () => _getConfigCallback().ModifierKeysEffective),
                                        Source = source
                                };
+                       Console.WriteLine($"input event generated: {ie}");
                        _lastState[button1] = newState;

-                       // track the pressed events with modifiers that we send so that we can send corresponding unpresses with modifiers
-                       // this is an interesting idea, which we may need later, but not yet.
-                       // for example, you may see this series of events: press:ctrl+c, release:ctrl, release:c
-                       // but you might would rather have press:ctrl+c, release:ctrl+c
-                       // this code relates the releases to the original presses.
-                       // UPDATE - this is necessary for the frame advance key, which has a special meaning when it gets stuck down
-                       // so, i am adding it as of 11-sep-2011
-                       if (newState)
-                       {
-                               _modifierState[button1] = ie.LogicalButton;
-                       }
-                       else
-                       {
-                               if (_modifierState.TryGetValue(button1, out var buttonModifierState))
-                               {
-                                       if (buttonModifierState != ie.LogicalButton && !_ignoreEventsNextPoll)
-                                       {
-                                               _newEvents.Add(
-                                                       new InputEvent
-                                                       {
-                                                               LogicalButton = buttonModifierState,
-                                                               EventType = InputEventType.Release,
-                                                               Source = source
-                                                       });
-                                       }
-                                       _modifierState.Remove(button1);
-                               }
-                       }
-
                        if (!_ignoreEventsNextPoll)
                        {
                                _newEvents.Add(ie);