Don't send key release events with modifiers

[Morilli]

This would close #3327. As I've written in that issue thread, the main problem seems to be the fact that key release events were sent with their respective modifier, aka when holding e.g. Ctrl+C and releasing C, a Release:Ctrl+C event would be generated which also happned to release the Ctrl key, even though that was still pressed.

The comments and code which I've removed seem to imply that this functionality was (more or less) intended and had a use case, although in my testing I couldn't figure out an issue with removing it; @zeromus do you recall the specific reason this code was needed?

[zeromus]

Without even thinking about it or checking the code, it seems obvious to me that inputs and bindings connected to e.g. Ctrl+C need to receive a PRESS and RELEASE both or else they'll malfunction. "Release:Ctrl+C event would be generated which also happened to release the Ctrl key" sounds like a bug to be solved directly.

[Morilli]

Hm, you're probably right about that. I couldn't find the exact place where that input event was handled though.

[CasualPokePlayer]

I believe https://github.com/TASEmulators/BizHawk/blob/5c59e6d9c4d56583f5818fbacc24f01d579f78ac/src/BizHawk.Client.Common/input/InputCoalescerControllers.cs#L30-L34 would be the cause of this issue.

[zeromus]

commit 726f1f3f112e9b8947f1b35ea659717604bfdd31 seems considerably different, where it had this before; unclear why a MOVE CODE commit had to be a CHANGE ALGORITHM commit also at the same time

var releases = Buttons.Where((kvp) => kvp.Key.Contains("+") && kvp.Key.EndsWith(ie.LogicalButton.Button)).ToArray();
foreach (var kvp in releases)
	Buttons[kvp.Key] = false;

[YoshiRulz]

You're comparing different code blocks; the matching block before the move is: https://github.com/TASEmulators/BizHawk/blob/25da1153b69a25ba9bac21ccf394a8f439640536/src/BizHawk.Client.EmuHawk/Input/Input.cs#L43-L51 And after: https://github.com/TASEmulators/BizHawk/blob/5c59e6d9c4d56583f5818fbacc24f01d579f78ac/src/BizHawk.Client.Common/input/InputCoalescerControllers.cs#L32-L33

[CasualPokePlayer]

Another thing I guess to note (logging events enqueued here https://github.com/TASEmulators/BizHawk/blob/aa708992e93b35d936035d8956cc679c18f32392/src/BizHawk.Client.EmuHawk/Input/Input.cs#L292 on c547a20f8e747fcdf70a678f12e733a54b36c3d2):

Doing hold Ctrl hold M spam unpress/press M

Press:Ctrl
Press:Ctrl+M
Release:Ctrl+M
Press:Ctrl+M
Release:Ctrl+M

Doing hold Ctrl hold M spam unpress/press Ctrl

Press:Ctrl
Press:Ctrl+M
Release:Ctrl
Press:Ctrl
Release:Ctrl

Something seems wrong here.

[zeromus]

The first log doesn't cause Ctrl to be released. So the original problem isn't happening. Thats strange...

The second log seems wrong.. ctrl+M should be released... but that's the only problem I see.

[CasualPokePlayer]

The first log would have the "original problem" (or well #3327) since the ProcessSubsets will eventually interpret that as unpressing both Ctrl and M. The second would probably not have any issues per se. But it does seem to indicate the logic this PR touches doesn't seem to function correctly entirely anyways.

[Morilli]

Not sure if directly related, but I wanted to understand the logic further: What should happen in the case that Ctrl is held and M is pressed? I get that a Ctrl+M event is generated, and I believe this also counts as a press for M alone, right? Or is this dependent on whether a hotkey is registered for Ctrl+M or not?

[Morilli]

Okay, how about this current logic? It should behave correctly in all scenarios, even ones which weren't handled correctly (I hope) before.

For example, for Hold Ctrl, Hold E, Release Ctrl, E will still count as pressed, but Ctrl+E won't, whereas it did before. Similarly, for Hold Ctrl, Hold E, Release E, Ctrl will still count as pressed while Ctrl+E will not.

[CasualPokePlayer]

Testing this, Hold Ctrl+Z -> Release Ctrl -> Hold Ctrl results in only Z recognized as pressed.

[Morilli]

Testing this, Hold Ctrl+Z -> Release Ctrl -> Hold Ctrl results in only Z recognized as pressed.

That is intentional; after all, holding Z and pressing Ctrl does currently not consider Ctrl+Z as pressed either.

[CasualPokePlayer]

Ah, testing it further and it seems Ctrl and Z are at least consider pressed, but not Ctrl+Z. I guess practically it doesn't matter so much.

[YoshiRulz]

Holding Shift then holding Z still results in keybinds for Shift alone being released.

[Morilli]

Holding Shift then holding Z still results in keybinds for Shift alone being released.

I can't replicate this in my testing, where are you seeing this?

[YoshiRulz]

~~https://youtu.be/4j6s4_dHiQo~~ NEVERMIND I FORGOR U+D DISALLOWED I expect

keydown Shift
#=> U
keydown Z
#=> UDL

but see

keydown Shift
#=> U
keydown Z
#=> DL