Fix input/hotkey bugs

[SuuperW]

Fix: Hotkey overrides input would cause an input to continue after a button was released if the input is bound to a combination such as Shift+A and an "internal hotkey" is bound to a single button (such as A). Also input bound to Alt would not have the alt ignored by menus.

Fix: Input overrides hotkeys did not work if both input and hotkey were bound to a combination such as Shift+A.

Fix: Controller inputs bound to a modifier key combination would not be triggered if additional modifier keys were also pressed.

Fix: autofire/autohold hotkeys would sometimes activate buttons that were already being held when the hotkey was pressed.

Check if completed:

• I have run any relevant test suites
• I, the commit author, have read the licensing terms for contributors (last updated 2024-06-22) and am compliant

[Morilli]

It's pretty hard to verify things are correct now because nothing defines "correct". Let me recap a couple things that happen.

If a key combination like Ctrl+Shift+A is pressed, it will now activate controller bindings for Ctrl, Shift, A, Ctrl+Shift, Ctrl+A, Shift+A and Ctrl+Shift+A. Only a hotkey bound to Ctrl+Shift+A will be triggered though.

"Internal" hotkeys are a separate group from normal hotkeys. If any key press activates a normal hotkey then no internal hotkey will be triggered, regardless of priority setting, except for the Frame Inch hotkey binding. The Frame Inch exception also means that binding a key combination to Frame Inch and some controller input will still have it go through in "Hotkeys override Input" mode.

The "Hotkeys override Input" branch is especially difficult for me to understand. Am I correct that the following code is functionally equivalent to what the code is currently doing?

if (ie.EventType is InputEventType.Press)
{
	var handled = triggers.Aggregate(false, (current, trigger) => current | CheckHotkey(trigger));
	if (!handled)
	{
		hotkeyCoalescer.Receive(ie);
		if (!triggers.Exists(IsInternalHotkey))
		{
			finalHostController.Receive(ie);
		}
	}
}
else
{
	hotkeyCoalescer.Receive(ie);
	finalHostController.Receive(ie);
}

[SuuperW]

It's pretty hard to verify things are correct now because nothing defines "correct".

Yeah, we really should have some tests for this.

If a key combination like Ctrl+Shift+A is pressed, it will now activate controller bindings for Ctrl, Shift, A, Ctrl+Shift, Ctrl+A, Shift+A and Ctrl+Shift+A. Only a hotkey bound to Ctrl+Shift+A will be triggered though.

This is correct, with the caveat that you cannot press Ctrl+Shift+A without first pressing Ctrl+Shift and before that either Ctrl or Shift, so hotkeys bound to those may trigger in the process. A must be pressed last or Ctrl+Shift+A won't happen for a hotkey, but a controller binding would be activated when pressing the keys in any order.

"Internal" hotkeys are a separate group from normal hotkeys. If any key press activates a normal hotkey then no internal hotkey will be triggered, regardless of priority setting, except for the Frame Inch hotkey binding. The Frame Inch exception also means that binding a key combination to Frame Inch and some controller input will still have it go through in "Hotkeys override Input" mode.

Interesting. I had not looked that much into the internal hotkeys.
I think that system should be re-worked. One reason for this is that if you set a very low emulation speed, the frame advance hotkey is unreliable. So is pausing. (because you're likely to press and release between loops, so the loop will never see it pressed)

The "Hotkeys override Input" branch is especially difficult for me to understand. Am I correct that the following code is functionally equivalent to what the code is currently doing?

Yeah lots of this code is tough to read. I believe your code is equivalent.

[YoshiRulz]

[...] if you set a very low emulation speed, the frame advance hotkey is unreliable. So is pausing. (because you're likely to press and release between loops, so the loop will never see it pressed)

#2959?

[SuuperW]

OP is unclear, but yeah this is the same issue as the comments there are discussing.

[Morilli]

if you set a very low emulation speed, the frame advance hotkey is unreliable. So is pausing.

I can't reproduce that with the pausing hotkey. However for frame advance you can use frame inch instead as that immediately pauses instead of waiting for the actual frame loop.

[SuuperW]

Oh pause isn't an internal hotkey. Huh.
I must have been using the frame advance button to try to pause when I experienced that.

[Morilli]

Should be fine.

[SuuperW]

Tests and additional fixes.

"Internal" hotkeys are a separate group from normal hotkeys. If any key press activates a normal hotkey then no internal hotkey will be triggered

This is also fixed.

Rebased onto my undo_redo branch (has its own pending PR) in order to make use of test code there.

outdated

[SuuperW]

Rebased onto master.
I'd like to get this branch merged too.

[SuuperW]

I realized that I messed up mouse coordinate transforming here. I've added a new commit 29ed1b3 that fixes that. (It's weird that we translate screen coordinates to client coordinates then to -10k to +10k range and back to client coordinates again but I assume there's a reason for that somewhere.)
I'll give you a chance to review the updates before I merge, @YoshiRulz.

[YoshiRulz]

(It's weird that we translate screen coordinates to client coordinates then to -10k to +10k range and back to client coordinates again but I assume there's a reason for that somewhere.)

Not a good reason. It goes back to before e12b5d8 when axes used float.

diff still LGTM