Opened 3 years ago

Closed 3 years ago

#6123 closed defect (fixed)

Do not execute multiple keyboard hotkey events if another hotkey in released simultaneously.

Reported by: nani Owned by: wraitii
Priority: Should Have Milestone: Alpha 25
Component: Core engine Keywords: hotkeys keyboard events multiple handlers
Cc: Patch:

Description (last modified by nani)

Example case: you have two different hotkeys.

"hotkeyA" : "Ctrl+H"
"hotkeyB" : "H"

both have an onKeyDown handler defined.

1) press Ctrl+H: an event "hotkeyA" type "hotkeydown" is fired, the handler is executed
2) release only Ctrl: an event "hotkeyA" type "hotkeyup" is fired AND at the same time (meaning processed in the same tick or whatever is it) and event "hotkeyB" of type "hotkeydown" is fired, the handler for "hotkeyB" is executed

What changed from a23 to a24 that didn't cause this problem? "hotkeyB" wasn't triggered at the same time "hotkeyA" was released, instead it was (ignored implicitly?) executed when the event fired a little more after (the OS keyboard wait time before a pressed key repeats, aprox 500 ms in my case) and the initial "hotkeydown" for "hotkeyB" was never fired.

Possible solutions:
A) Filter out all "hotkeydown" hotkeys from being processed if on the list of events to be processed there is at least one event of type "hotkeyup"

B) Don't fire a "hotkeydown" when that hotkey was already in a theoretical "hotkeydown" state.

Change History (5)

comment:1 by wraitii, 3 years ago

See also #6064

--

I think your description confuses the hotkeys & also you're missing the 'hotkeypress' logic.

--

I don't think 'hotkeydown' is the problem, just hotkeypress.

Last edited 3 years ago by wraitii (previous) (diff)

comment:2 by nani, 3 years ago

What makes it so that hotkeydown is not a problem?

comment:3 by nani, 3 years ago

Description: modified (diff)

in reply to:  2 comment:4 by wraitii, 3 years ago

"hotkeydown" is triggered repeatedly when the hotkey is active. There is also "press" event, that's sent once, when it is triggered.

As such, I would expect the "Ctrl+H -> H" to trigger hotkeydown for H - it is down. I'm not sure why you find the delay particularly useful here, to be honest?
Note that there is also Engine.HotkeyIsPressed function that you can use with onTick/onUpdate to check for "continuous down pressing". Intuitively, I'd say that's better than "hotkeydown" for anything that's not text, but maybe you have a very specific use case in mind?

However, the unexpected behaviour is "hotkey press". In A23, going "Ctrl+H -> H" would not trigger a "hotkey press" for H. In A24, it does. This is arguably correct, but also arguably not because the user didn't really press H.

Keep in mind the following cases:

  • A then Ctrl+A then A needs to move the camera left, rotate, then move the camera left.
  • Ctrl is attack-move, Ctrl+A rotates & unselects attack-move. Likewise, A then Ctrl does not fire attack-move (the behaviour wasn't symmetrical in A23, it is in A24). In either case, switching to just Ctrl triggers attack-move.
  • We probably don't want "Shift + 4" then "4" to trigger a "hotkey press" for "4", since that's kinda unexpected. See #6064.

Feel free to challenge

comment:5 by wraitii, 3 years ago

Owner: set to wraitii
Resolution: fixed
Status: newclosed

In 25169:

Do not trigger "HotkeyPress" events when releasing a key.

Follows rP24675.
Because only the most specific hotkeys can be active at any time, releasing a key may require re-activating less specific hotkeys.
There were two issues with this behaviour:

  • It was buggy, as it only checked one active key, when any still active key can trigger hotkeys.
  • "HotkeyPress" and "HotkeyDown" events where sent, as if the hotkey was pressed, which was unexpected for most code/users (it is unusual to have a "Press" event on key release).

This fixes these issues by "silently" re-triggering the hotkeys in such a case. It also makes it easier for JS code to use "hotkeyPress" instead of "hotkeyDown" for non-continuous behaviour.

Accepted By: nani

Fixes #6123
Refs #6064 (fixes the problem, but not the code weirdness)

Differential Revision: https://code.wildfiregames.com/D3766

Note: See TracTickets for help on using tickets.