Skip to content

fix(input): Keep modifier combos from sticking when keys are released out of order - #3233

Open
CryoTheRenegade wants to merge 4 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/modifier-key-release-fix
Open

fix(input): Keep modifier combos from sticking when keys are released out of order#3233
CryoTheRenegade wants to merge 4 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:fix/modifier-key-release-fix

Conversation

@CryoTheRenegade

@CryoTheRenegade CryoTheRenegade commented Aug 28, 2026

Copy link
Copy Markdown

In collaboration with DrGoldFish - thank you for reporting the issue and testing my fixes

  • CTRL+F and other modifier combos could leave force-attack, waypoint, or shift-select mode on, or fire a plain command-bar hotkey, depending on which key was released first.
  • Modifier-only holds (CTRL, SHIFT, ALT) are now recorded so their UP mappings still fire, and a key pressed with a modifier is not treated as a GUI hotkey on release.
  • Focus loss now synthesizes key-ups for CTRL, SHIFT, and ALT, not only ALT.

… out of order

Signed-off-by: Jacob Ledbetter <jledbetter460@gmail.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Prevent stuck modifier combos on out-of-order key releases

🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Tracks modified presses so combo UP mappings fire regardless of release order.
• Suppresses plain GUI hotkeys when releasing keys previously used in modifier combos.
• Synthesizes all modifier releases after focus loss and clears stale combo tracking.
Diagram

sequenceDiagram
    actor User
    participant Keyboard
    participant Stream as Message Stream
    participant Meta as Meta Events
    participant Hotkey as Hotkey Translator
    participant Manager as Hotkey Manager
    User->>Keyboard: Press modified key
    Keyboard->>Stream: Raw key down
    Stream->>Meta: Track combo state
    Meta->>Manager: Suppress key up
    User->>Keyboard: Release keys
    Keyboard->>Stream: Raw key up
    Stream->>Hotkey: Check GUI hotkey
    Hotkey->>Manager: Consume suppression
    Stream->>Meta: Resolve combo up
    Keyboard->>Stream: Synthetic modifier ups
    Stream->>Meta: Flush reset state
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Unified chord state machine
  • ➕ Centralizes modifier tracking, UP mapping, and GUI suppression ownership.
  • ➕ Reduces coordination through global translator state.
  • ➖ Requires a broad input-pipeline refactor with substantially higher regression risk.
  • ➖ Touches mature event ordering and message-disposition behavior beyond this bug.

Recommendation: Keep the PR's targeted coordination between existing translators because it preserves raw key-up propagation and limits risk. A unified chord state machine would be cleaner long term, but its larger scope is not justified for this compatibility-sensitive fix; add focused regression coverage if the input harness supports synthetic event sequences.

Files changed (6) +115 / -31

Bug fix (6) +115 / -31
HotKey.hAdd one-shot key-up suppression state +5/-0

Add one-shot key-up suppression state

• Extends HotKeyManager with per-key suppression storage and APIs to set, consume, and clear suppression. This lets modified releases bypass GUI hotkey execution exactly once.

Core/GameEngine/Include/GameClient/HotKey.h

Keyboard.hExpose keyboard reset generations +3/-1

Expose keyboard reset generations

• Adds a reset generation counter and generalizes the focus-recovery helper from ALT-only handling to all modifier keys. Translators can now detect that keyboard state was reset.

Core/GameEngine/Include/GameClient/Keyboard.h

MetaEvent.hTrack reset synchronization in meta events +2/-0

Track reset synchronization in meta events

• Adds the last observed keyboard reset generation and a helper for clearing tracked key-down combinations. These declarations support safe recovery after focus loss.

Core/GameEngine/Include/GameClient/MetaEvent.h

Keyboard.cppSynthesize releases for every held modifier +19/-12

Synthesize releases for every held modifier

• Initializes and increments the keyboard reset generation whenever key state is cleared. Focus recovery now emits raw key-up messages for held CTRL and SHIFT keys as well as ALT.

Core/GameEngine/Source/GameClient/Input/Keyboard.cpp

HotKey.cppSkip GUI hotkeys for modified releases +34/-5

Skip GUI hotkeys for modified releases

• Consumes one-shot suppression before translating a raw key-up into a plain GUI hotkey, while leaving the message available to later translators. Initializes and manages the per-key suppression array.

Core/GameEngine/Source/GameClient/MessageStream/HotKey.cpp

MetaEvent.cppPreserve combo state across release ordering +52/-13

Preserve combo state across release ordering

• Records modifier-only holds, suppresses GUI handling for modified key releases, and retains combo state through same-frame release ordering. It also detects keyboard resets, emits pending UP mappings, and clears stale tracking.

Core/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR changes keyboard event tracking so modifier provenance follows each press through release, preventing modifier combinations from sticking or triggering plain GUI hotkeys.

  • Records whether each key was pressed while CTRL, SHIFT, or ALT was active.
  • Preserves that information across focus changes and applies it to releases and autorepeat events.
  • Synthesizes releases for held CTRL, SHIFT, and ALT keys when keyboard state resets.
  • Filters modifier-associated releases from plain GUI hotkey handling.
  • Adds the modifier-on-down state flag to both game variants.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope.

No blocking failure remains.

Important Files Changed

Filename Overview
Core/GameEngine/Source/GameClient/Input/Keyboard.cpp Tracks modifier provenance per key and synthesizes modifier releases during keyboard resets.
Core/GameEngine/Source/GameClient/MessageStream/HotKey.cpp Prevents key releases associated with modifier combinations from executing plain GUI hotkeys.
Core/GameEngine/Include/GameClient/Keyboard.h Adds per-key modifier provenance storage and declares generalized modifier-release handling.
Generals/Code/GameEngine/Include/GameClient/KeyDefs.h Adds the modifier-on-down event-state flag for the Generals variant.
GeneralsMD/Code/GameEngine/Include/GameClient/KeyDefs.h Adds the matching modifier-on-down event-state flag for the Zero Hour variant.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Down[Key-down event] --> Capture[Capture current modifier state]
  Capture --> Store[Store modifier-on-down by key]
  Store --> Repeat[Apply provenance to autorepeat]
  Store --> Up[Matching key-up event]
  Up --> Carry[Attach stored modifier provenance]
  Carry --> Hotkey{Modifier-associated release?}
  Hotkey -- Yes --> Ignore[Skip plain GUI hotkey]
  Hotkey -- No --> Execute[Evaluate plain GUI hotkey]
  Focus[Focus loss or device reset] --> Synthetic[Synthesize held modifier key-ups]
  Synthetic --> Modes[End modifier-controlled modes]
Loading

Reviews (4): Last reviewed commit: "Fix modifier state tracking for hotkey r..." | Re-trigger Greptile

HotKey.h referenced KeyDefType/KEY_COUNT without the key header, and MetaEvent.cpp called a reset helper that was never declared.

Co-authored-by: Cursor <cursoragent@cursor.com>

@xezon xezon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this was entirely generated by LLM? It looks like slop the way the new logic is laid out. It is incomprehensible and unmaintainable. It needs to be unsloppified.

CryoTheRenegade and others added 2 commits August 31, 2026 09:52
Keep the GUI-hotkey and focus-loss behavior, but store the modifier-down
flag on HotKeyTranslator itself and reuse the existing alt-tab key-up path
for CTRL and SHIFT.

Co-authored-by: Cursor <cursoragent@cursor.com>
@xezon

xezon commented Aug 31, 2026

Copy link
Copy Markdown

How do we know the new generated revision is no slop?

@CryoTheRenegade

Copy link
Copy Markdown
Author

Fair criticism. I used LLM assistance on the first pass, and I should have reviewed and simplified the result before asking you to review it. I own that.

I’ve since rewritten the fix. The hotkey translator is stateless now. Modifier press state lives in Keyboard, where buffered events are processed in order, and each release records whether its matching press used Ctrl, Shift, or Alt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants