fix(input): Prevent modified key releases from firing plain hotkeys by CryoTheRenegade · Pull Request #3233 · TheSuperHackers/GeneralsGameCode

CryoTheRenegade · 2026-08-28T15:58:12Z

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

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

Hold Ctrl.
Press F.
Release Ctrl.
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.

… out of order Signed-off-by: Jacob Ledbetter <jledbetter460@gmail.com>

qodo-free-for-open-source-projects · 2026-08-28T15:59:09Z

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

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 · 2026-08-28T16:00:47Z

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 · 2026-08-28T16:01:04Z

Greptile Summary

The PR preserves modifier state at event-processing time and carries each key press’s modifier state into its matching release, preventing modified releases from activating plain hotkeys. It also synthesizes releases for held Ctrl, Shift, and Alt keys during keyboard resets.

Adds press-time modifier state to buffered keyboard events and raw key-up messages.
Updates hotkey translation to consider both release-time and press-time modifiers.
Expands focus-reset handling to release all supported modifier keys.
Defines a shared mask for Ctrl, Shift, and Alt state flags.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameClient/Input/Keyboard.cpp	Captures modifier state per input event, tracks press-time state per key, and emits synthetic modifier releases during resets.
Core/GameEngine/Source/GameClient/MessageStream/HotKey.cpp	Suppresses plain hotkey execution when either the press or release carried a keyboard modifier.
Core/GameEngine/Include/GameClient/Keyboard.h	Extends keyboard event and persistent keyboard state storage with press-time modifier information.
Core/GameEngine/Include/Common/MessageStream.h	Documents the expanded raw keyboard message argument contract.
Generals/Code/GameEngine/Include/GameClient/KeyDefs.h	Adds a combined Ctrl, Shift, and Alt modifier-state mask for Generals.
GeneralsMD/Code/GameEngine/Include/GameClient/KeyDefs.h	Adds the equivalent combined modifier-state mask for Zero Hour.

Sequence Diagram

sequenceDiagram
    participant OS as Keyboard input
    participant K as Keyboard
    participant MS as MessageStream
    participant HK as HotKeyTranslator
    OS->>K: Key down with modifier held
    K->>K: Save press-time modifier state
    OS->>K: Modifier up
    OS->>K: Key up
    K->>MS: MSG_RAW_KEY_UP(key, currentState, pressedState)
    MS->>HK: Translate key-up
    HK->>HK: Check currentState OR pressedState
    alt Press or release was modified
        HK-->>MS: Do not execute plain hotkey
    else No modifier involved
        HK->>HK: Execute matching plain hotkey
    end

_{Reviews (6): Last reviewed commit: "Keep key release state separate from pre..." | 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

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.

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 · 2026-08-31T18:40:45Z

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

CryoTheRenegade · 2026-09-01T19:20:23Z

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 · 2026-09-03T08:04:38Z

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 · 2026-09-04T02:05:45Z

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 · 2026-09-11T09:16:12Z

+			if( !isModifier )
+			{
+				BitClear( m_keys[ index ].state, KEY_STATE_MODIFIERS );
+				BitSet( m_keys[ index ].state, lastPressedKeyState );


Does this overwrites the key modifiers for a Key up event, right?

It looks like the UP event is only triggered when a non-modifier key is released. Is that right?

In MetaEvent we implemented this differently I think: Each modifier release triggers a key up event. See MetaEventTranslator::onKeyModStateRemoved

So what happens is:

CTRL + SHIFT + F Down = triggers event and remembers that this combo was pressed.
Release CTRL = triggers CTRL + SHIFT + F Up event, because any of the keys were released.

The state is tracked with KeyDownInfo m_keyDownInfos[KEY_COUNT]

If these are functionally not the same, I suggest to look into making them behave the same. As far as I am aware the implementation in MetaEventTranslator is fundamentally correct, minus any bugs it may have. (It was not created or extensively reviewed with AI)

I suggest to ask the LLM how Meta Event implements the Key up events with modifiers, then how it fundamentally differs with the current Keyboard implementation, and which implementation direction makes more sense for the Keyboard.

To me it looks suspiciuos that the key UP event gets its state overwritten. It means it is not truthful anymore.

I removed the changes to MetaEvent. It still ends the combo when a required modifier is released.

The key-up event now reports the modifiers held at release. It carries the modifiers from the original press separately. HotKeyTranslator checks both, so releasing F after Ctrl+F does not run the plain F command.

I also changed Keyboard to save m_modifiers only for non-modifier keys, as suggested.

Ok wow the implementation now looks completely different to before 😅

xezon · 2026-09-11T17:04:24Z

 	UnsignedByte	key;										// KeyDefType, key data
 	UnsignedByte	status;									// StatusType, above
 	UnsignedShort	state;									// KEY_STATE_* in KeyDefs.h
+	UnsignedShort	pressedState;						// Modifier flags from the matching press, for key-up events


This field increases the struct size from 8 to 12 bytes. Can it not be outside of it, specific for down key presses?

It is confusing why we need pressedState here and then another with m_lastPressedKeyState. I expected that m_lastPressedKeyState is enough.

xezon · 2026-09-13T07:04:09Z

-		MSG_RAW_KEY_DOWN,														///< (KeyDefType) the given key was pressed (uses Microsoft VK_ codes)
-		MSG_RAW_KEY_UP,															///< (KeyDefType) the given key was released
+		MSG_RAW_KEY_DOWN,														///< (KeyDefType, current KEY_STATE_* flags) the given key was pressed
+		MSG_RAW_KEY_UP,															///< (KeyDefType, current KEY_STATE_* flags, modifier flags from the matching press) the given key was released


I suggest put a typedef UnsignedShort KeyState; below the KEY_STATE_* enum and then use that type to refer to the KeyStates everywhere it is used.

xezon · 2026-09-13T07:08:54Z

-		if(newModState != 0)
+		const KeyDefType key = (KeyDefType)msg->getArgument(0)->integer;
+		const Int keyState = msg->getArgument(1)->integer;
+		const Int pressedKeyState = msg->getArgument(2)->integer;


Maybe cast it to the correct types.

xezon · 2026-09-13T07:19:10Z

 			msg->appendIntegerArgument( key->key );
 			msg->appendIntegerArgument( key->state );
+			if( BitIsSet( key->state, KEY_STATE_UP ) )
+				msg->appendIntegerArgument( key->pressedState );


Would m_lastPressedKeyState[key->key] work here?

xezon · 2026-09-13T07:28:27Z

@@ -74,33 +73,15 @@ GameMessageDisposition HotKeyTranslator::translateGameMessage(const GameMessage

 	if ( t == GameMessage::MSG_RAW_KEY_UP)


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?

fix(input): Keep modifier combos from sticking when keys are released…

3311ee3

… out of order Signed-off-by: Jacob Ledbetter <jledbetter460@gmail.com>

fix(input): Include KeyDefs and add KeyDownInfo::reset for CI

d4687be

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 reviewed Aug 31, 2026

View reviewed changes

CryoTheRenegade and others added 2 commits August 31, 2026 09:52

Fix modifier state tracking for hotkey releases

3b71ae4

CryoTheRenegade changed the title ~~fix(input): Keep modifier combos from sticking when keys are released out of order~~ fix(input): Prevent modified key releases from firing plain hotkeys Sep 4, 2026

xezon added Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Input labels Sep 10, 2026

xezon reviewed Sep 10, 2026

View reviewed changes

Comment thread GeneralsMD/Code/GameEngine/Include/GameClient/KeyDefs.h Outdated

Comment thread Core/GameEngine/Include/GameClient/Keyboard.h Outdated

Comment thread Generals/Code/GameEngine/Include/GameClient/KeyDefs.h Outdated

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

Fix modifier state tracking on key release

167bbc8

xezon requested changes Sep 11, 2026

View reviewed changes

Keep key release state separate from press modifiers

66d74be

xezon reviewed Sep 13, 2026

View reviewed changes

		@@ -74,33 +73,15 @@ GameMessageDisposition HotKeyTranslator::translateGameMessage(const GameMessage

		if ( t == GameMessage::MSG_RAW_KEY_UP)

Uh oh!

Conversation

CryoTheRenegade commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 28, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 28, 2026

Code Review by Qodo

Great, no issues found!

Uh oh!

greptile-apps Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

xezon commented Aug 31, 2026

Uh oh!

CryoTheRenegade commented Sep 1, 2026

Uh oh!

xezon commented Sep 3, 2026

Uh oh!

CryoTheRenegade commented Sep 4, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

CryoTheRenegade commented Aug 28, 2026 •

edited

Loading

greptile-apps Bot commented Aug 28, 2026 •

edited

Loading