fix(input): Prevent modified key releases from firing plain hotkeys

[CryoTheRenegade]

Thanks to DrGoldFish, who reported this issue and tested the fix.

The bug can happen in this order using Legi's keybinds:

1. Hold Ctrl.
2. Press F.
3. Release Ctrl.
4. Release F.

The game can treat the last step as a normal F key press and run the F command. It should remember that F was pressed with Ctrl.

There is also a focus problem. If Ctrl or Shift is released while the game is not focused, the game may miss the release. This can leave force-attack or selection mode active.

The keyboard code reads several events at once. It previously gave every event the modifier state from the end of that group. This could give an event the wrong Ctrl, Shift, or Alt state.

The keyboard code now saves the modifier state when each event is handled. It remembers which modifier was held when a key was pressed. It passes that information to the matching key release.

HotKeyTranslator then knows that the release is part of a modified key press and does not run the normal hotkey.

This state is stored in Keyboard because another message handler may remove the key-down event before HotKeyTranslator receives it.

The reset code now sends key-up events for Ctrl, Shift, and Alt. The existing MetaEvent code is unchanged.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

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

🐞 Bug fix 🕐 20-40 Minutes

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]

Code Review by Qodo

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

Great, no issues found!

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

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 ↗

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Changes keyboard input handling and hotkey processing logic.

The PR appears safe to merge based on the reviewed changes.

Summary

The PR records modifier state per keyboard event, synthesizes modifier releases on reset, and changes hotkey handling to run on unmodified key-down events. It also moves the hotkey translator ahead of the meta-event translator in both game variants.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  K[Keyboard event] --> W[WindowTranslator]
  W --> H[HotKeyTranslator]
  H --> M[MetaEventTranslator]
  M --> R[Remaining translators]

Loading

_{Reviews (9) · Last reviewed commit: "Handle active GUI hotkeys before meta co..."}

[xezon]

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.

[xezon]

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

[CryoTheRenegade]

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.

[xezon]

I tried to understand this change but I was unable to. It is lacking context. What was the issue and how was it reproduced, how was it fixed and why is it fixed the way it was fixed.

[CryoTheRenegade]

I've reworded the PR description with a simple example and an explanation of the cause, the fix, and why the state is stored in Keyboard.

[xezon]

Would MSG_RAW_KEY_DOWN perhaps be an option for hotkeys? All the MetaEvents are key down events. Or is there a good reason why hotkey needs to be posted on down?

[CryoTheRenegade]

Key-down could work. I found no comment explaining why hotkeys use key-up. I kept the current timing for now.

[xezon]

I suggest test MSG_RAW_KEY_DOWN. It would be the simpler option and get rid of the new stuff you added to accomodate this event. Players will also be happier if their key presses register faster.

[xezon]

Bump.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 24a4a16b-8a25-40cd-adb7-f920964f5380

📥 Commits

Reviewing files that changed from the base of the PR and between 1e5365b and 67b80bd.

📒 Files selected for processing (2)

• Generals/Code/GameEngine/Source/GameClient/GameClient.cpp
• GeneralsMD/Code/GameEngine/Source/GameClient/GameClient.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

Walkthrough

The changes add a typed key-state alias and modifier mask. Keyboard updates add modifier bits to key states, including repeat states. Reset handling emits key-up messages for tracked modifier keys. Hotkey translation processes eligible raw key-down messages.

Changes

Keyboard input state and message handling

Layer / File(s)	Summary
Key-state types and declarations Generals/.../GameClient/KeyDefs.h, GeneralsMD/.../GameClient/KeyDefs.h, Core/GameEngine/Include/Common/MessageStream.h, Core/GameEngine/Include/GameClient/Keyboard.h, Core/GameEngine/Include/GameClient/MetaEvent.h	The key definitions add KeyState and KEY_STATE_MODIFIERS. Keyboard and message declarations use KeyState for key-state values.
Modifier tracking and reset messages Core/GameEngine/Source/GameClient/Input/Keyboard.cpp	Keyboard updates apply modifier bits before repeat processing. Reset handling emits raw key-up messages for tracked Ctrl, Shift, Alt, and configured shift-2 keys.
Key message translation Core/GameEngine/Source/GameClient/MessageStream/HotKey.cpp, Core/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp, Generals/.../GameClient/GameClient.cpp, GeneralsMD/.../GameClient/GameClient.cpp	Hotkey translation handles raw key-down messages and filters modifier and autorepeat states. Meta-event handling uses KeyState. Both game variants register HotKeyTranslator before MetaEventTranslator.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: xezon

Merge Risk: ⚪ Minimal · up to 67b80

No actionable regression is established in the reviewed changes; the shift-2 modifier path does not trigger plain hotkeys. The PR is mergeable subject to normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 67b80

The change affects local keyboard commands and their handling after focus loss. No security issue was established, but the new command order could change behavior where key bindings overlap.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is the local client keyboard-to-window-command path in both game variants. The inspected path does not establish a remote attacker route or the effect of every downstream window callback.

Trust Boundaries and Controls

• observed — A successful plain-key hotkey dispatch consumes the raw event before MetaEvent can map it. Modified and repeat key-downs remain available to MetaEvent because HotKeyTranslator does not execute them.

Resilience and Maintainability Implications

• observed — Modifier-release cleanup scans tracked modified-key combinations and emits mapped release events when their required modifier state disappears. Autorepeats do not execute ordinary hotkeys.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: preventing releases of modified keys from triggering plain hotkeys.
Description check	✅ Passed	The description explains the reproduction case, focus-related issue, modifier-state tracking, and hotkey handling changes. It is directly related to the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}