Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CryoTheRenegade wants to merge 6 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3311ee3
fix(input): Keep modifier combos from sticking when keys are released…
CryoTheRenegade Aug 26, 2026
d4687be
fix(input): Include KeyDefs and add KeyDownInfo::reset for CI
CryoTheRenegade Aug 28, 2026
23e1ebc
fix(input): Drop the extra translator bookkeeping from the modifier fix
CryoTheRenegade Aug 31, 2026
3b71ae4
Fix modifier state tracking for hotkey releases
CryoTheRenegade Aug 31, 2026
167bbc8
Fix modifier state tracking on key release
CryoTheRenegade Sep 10, 2026
66d74be
Keep key release state separate from press modifiers
CryoTheRenegade Sep 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Core/GameEngine/Include/Common/MessageStream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ class GameMessage : public MemoryPoolObject
MSG_RAW_MOUSE_WHEEL, ///< (Real spin, + is away, - is toward user)
MSG_RAW_MOUSE_END,

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

@xezon xezon Sep 13, 2026

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 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.


// Refined Mouse messages
// NOTE: All processing should attempt to use these refined mouse messages, rather than the
Expand Down
4 changes: 3 additions & 1 deletion Core/GameEngine/Include/GameClient/Keyboard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ struct KeyboardIO
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

@xezon xezon Sep 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

UnsignedInt keyDownTimeMsec; // real-time in milliseconds when key went down

};
Expand Down Expand Up @@ -124,7 +125,7 @@ class Keyboard : public SubsystemInterface
WideChar getPrintableKey( KeyDefType key, Int state );
enum { MAX_KEY_STATES = 3};
private:
void refreshAltKeys() const; ///< refresh the state of the alt keys, necessary after alt tab
void emitModifierKeyUps() const; ///< emit key-ups for held CTRL/SHIFT/ALT after focus loss
protected:

/** get the key data for a single key, KEY_NONE should be returned when
Expand All @@ -140,6 +141,7 @@ class Keyboard : public SubsystemInterface
void setKeyStateData( KeyDefType key, UnsignedByte data ); ///< get key state

UnsignedShort m_modifiers;
UnsignedShort m_lastPressedKeyState[KEY_COUNT];
// internal keyboard data members
//Bool m_capsState; // 1 if caps lock is on
//Bool m_shiftState; // 1 if either shift key is pressed
Expand Down
115 changes: 73 additions & 42 deletions Core/GameEngine/Source/GameClient/Input/Keyboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ void Keyboard::createStreamMessages()
{
msg->appendIntegerArgument( key->key );
msg->appendIntegerArgument( key->state );
if( BitIsSet( key->state, KEY_STATE_UP ) )
msg->appendIntegerArgument( key->pressedState );

@xezon xezon Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would m_lastPressedKeyState[key->key] work here?

}

// next key please
Expand All @@ -96,6 +98,23 @@ void Keyboard::createStreamMessages()

}

//-------------------------------------------------------------------------------------------------
static Bool isCtrlShiftAltKey(KeyDefType key)
{
switch (key)
{
case KEY_LCTRL:
case KEY_RCTRL:
case KEY_LSHIFT:
case KEY_RSHIFT:
case KEY_LALT:
case KEY_RALT:
return TRUE;
}

return FALSE;
}

//-------------------------------------------------------------------------------------------------
/** update all our key state data */
//-------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -138,65 +157,64 @@ void Keyboard::updateKeys()
/** @todo -- if we don't have focus, we could destroy all the keys retrieved
here so that we don't process anything */

m_keyStatus[ m_keys[ index ].key ].state = m_keys[ index ].state;
m_keyStatus[ m_keys[ index ].key ].status = m_keys[ index ].status;
const KeyDefType key = (KeyDefType)m_keys[ index ].key;
const Bool isModifier = isCtrlShiftAltKey(key) || key == m_shift2Key;

m_keyStatus[ key ].state = m_keys[ index ].state;
m_keyStatus[ key ].status = m_keys[ index ].status;

// Update key down time for new key presses
if( BitIsSet( m_keys[ index ].state, KEY_STATE_DOWN ) )
{
m_keyStatus[ m_keys[ index ].key ].keyDownTimeMsec = m_keys[ index ].keyDownTimeMsec;
m_keyStatus[ key ].keyDownTimeMsec = m_keys[ index ].keyDownTimeMsec;
}

// prevent ALT-TAB from causing a TAB event
if( m_keys[ index ].key == KEY_TAB )
if( key == KEY_TAB )
{
if( BitIsSet( m_keyStatus[ KEY_LALT ].state, KEY_STATE_DOWN ) ||
BitIsSet( m_keyStatus[ KEY_RALT ].state, KEY_STATE_DOWN ) )
{
m_keys[index].status = KeyboardIO::STATUS_USED;
}
}
else if( m_keys[ index ].key == KEY_CAPS ||
m_keys[ index ].key == KEY_LCTRL ||
m_keys[ index ].key == KEY_RCTRL ||
m_keys[ index ].key == KEY_LSHIFT ||
m_keys[ index ].key == KEY_RSHIFT ||
m_keys[ index ].key == KEY_LALT ||
m_keys[ index ].key == KEY_RALT )
else if( key == KEY_CAPS || isModifier )

{

//
// this keeps our internal key state accurate event though we don't
// use the returned translation ... kinda weird I think
//
translateKey( m_keys[ index ].key );
translateKey( key );

}

// TheSuperHackers @bugfix CryoTheRenegade 31/08/2026 Preserve the current
// modifier state for each buffered event.
BitSet( m_keys[ index ].state, m_modifiers );
m_keys[ index ].pressedState = KEY_STATE_NONE;
if( !isModifier )
{
if( BitIsSet( m_keys[ index ].state, KEY_STATE_DOWN ) )
{
m_lastPressedKeyState[key] = m_modifiers;
}
else
{
// Keep the press state separate so a modified release cannot fire a plain hotkey.
m_keys[ index ].pressedState = m_lastPressedKeyState[key];
m_lastPressedKeyState[key] = KEY_STATE_NONE;
}
}

index++;

}

// check for key repeats
checkKeyRepeat();

if( m_modifiers )
{
index = 0;
while( m_keys[ index ].key != KEY_NONE )
{

// set in the modifier data into the already existing up/down state
BitSet( m_keys[ index ].state, m_modifiers );

// next key
index++;

}

}

}

//-------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -233,7 +251,9 @@ Bool Keyboard::checkKeyRepeat()
{
// Add key to this frame
m_keys[ index ].key = (UnsignedByte)key;
m_keys[ index ].state = KEY_STATE_DOWN | KEY_STATE_AUTOREPEAT; // note: not a bitset; this is an assignment
// This is an assignment, not a bit set.
m_keys[ index ].state = KEY_STATE_DOWN | KEY_STATE_AUTOREPEAT | m_modifiers;
m_keys[ index ].pressedState = KEY_STATE_NONE;
m_keys[ index ].status = KeyboardIO::STATUS_UNUSED;

// Set End Flag
Expand Down Expand Up @@ -699,6 +719,7 @@ Keyboard::Keyboard()

memset( m_keys, 0, sizeof( m_keys ) );
memset( m_keyStatus, 0, sizeof( m_keyStatus ) );
memset( m_lastPressedKeyState, 0, sizeof( m_lastPressedKeyState ) );
m_modifiers = KEY_STATE_NONE;
m_shift2Key = KEY_NONE;

Expand Down Expand Up @@ -749,13 +770,15 @@ void Keyboard::update()
//-------------------------------------------------------------------------------------------------
void Keyboard::resetKeys()
{

// TheSuperHackers @fix Caball009 13/12/2025 Fix bug where game remains in waypoint mode
// because the key up state for the alt key is not detected after alt tab.
refreshAltKeys();
// CTRL and SHIFT have the same stuck-mode problem (force-attack, prefer-selection).
emitModifierKeyUps();

memset( m_keys, 0, sizeof( m_keys ) );
memset( m_keyStatus, 0, sizeof( m_keyStatus ) );
// A held key can still report its release after focus returns. Do not clear
// m_lastPressedKeyState until that release or a new press arrives.
m_modifiers = KEY_STATE_NONE;
if( getCapsState() )
{
Expand All @@ -765,24 +788,32 @@ void Keyboard::resetKeys()
}

//-------------------------------------------------------------------------------------------------
// Refresh the state of the alt keys, necessary after alt tab
//-------------------------------------------------------------------------------------------------
void Keyboard::refreshAltKeys() const
static void emitRawKeyUpIfDown(const KeyboardIO *keyStatus, KeyDefType key)
{
if (BitIsSet(m_keyStatus[KEY_LALT].state, KEY_STATE_DOWN))
{
GameMessage* msg = TheMessageStream->appendMessage(GameMessage::MSG_RAW_KEY_UP);
msg->appendIntegerArgument(KEY_LALT);
msg->appendIntegerArgument(KEY_STATE_UP);
}
if (BitIsSet(m_keyStatus[KEY_RALT].state, KEY_STATE_DOWN))
if (BitIsSet(keyStatus[key].state, KEY_STATE_DOWN))
{
GameMessage* msg = TheMessageStream->appendMessage(GameMessage::MSG_RAW_KEY_UP);
msg->appendIntegerArgument(KEY_RALT);
msg->appendIntegerArgument(key);
msg->appendIntegerArgument(KEY_STATE_UP);
msg->appendIntegerArgument(KEY_STATE_NONE);
}
}

//-------------------------------------------------------------------------------------------------
// Emit RAW_KEY_UP for still-held modifiers so MetaEvent can end force-attack / waypoints / etc.
//-------------------------------------------------------------------------------------------------
void Keyboard::emitModifierKeyUps() const
{
emitRawKeyUpIfDown(m_keyStatus, KEY_LCTRL);
emitRawKeyUpIfDown(m_keyStatus, KEY_RCTRL);
emitRawKeyUpIfDown(m_keyStatus, KEY_LSHIFT);
emitRawKeyUpIfDown(m_keyStatus, KEY_RSHIFT);
emitRawKeyUpIfDown(m_keyStatus, KEY_LALT);
emitRawKeyUpIfDown(m_keyStatus, KEY_RALT);
Comment thread
xezon marked this conversation as resolved.
if (m_shift2Key != KEY_NONE && !isCtrlShiftAltKey(m_shift2Key))
emitRawKeyUpIfDown(m_keyStatus, m_shift2Key);
}

//-------------------------------------------------------------------------------------------------
/** get the first key in our current state of the keyboard */
//-------------------------------------------------------------------------------------------------
Expand Down
33 changes: 7 additions & 26 deletions Core/GameEngine/Source/GameClient/MessageStream/HotKey.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
//-----------------------------------------------------------------------------
#include "GameClient/HotKey.h"
#include "GameClient/KeyDefs.h"
#include "GameClient/MetaEvent.h"
#include "GameClient/GameWindow.h"
#include "GameClient/GameWindowManager.h"
#include "GameClient/Keyboard.h"
Expand All @@ -74,33 +73,15 @@ GameMessageDisposition HotKeyTranslator::translateGameMessage(const GameMessage

if ( t == GameMessage::MSG_RAW_KEY_UP)

@xezon xezon Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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?

{

//char key = msg->getArgument(0)->integer;
Int keyState = msg->getArgument(1)->integer;

// for our purposes here, we don't care to distinguish between right and left keys,
// so just fudge a little to simplify things.
Int newModState = 0;

if( keyState & KEY_STATE_CONTROL )
{
newModState |= CTRL;
}

if( keyState & KEY_STATE_SHIFT )
{
newModState |= SHIFT;
}

if( keyState & KEY_STATE_ALT )
{
newModState |= ALT;
}
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;

@xezon xezon Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe cast it to the correct types.

if( (keyState | pressedKeyState) & KEY_STATE_MODIFIERS )
return disp;
WideChar key = TheKeyboard->getPrintableKey((KeyDefType)msg->getArgument(0)->integer, 0);

WideChar printableKey = TheKeyboard->getPrintableKey(key, 0);
UnicodeString uKey;
uKey.concat(key);
uKey.concat(printableKey);
AsciiString aKey;
aKey.translate(uKey);
if(TheHotKeyManager && TheHotKeyManager->executeHotKey(aKey))
Expand Down
3 changes: 2 additions & 1 deletion Generals/Code/GameEngine/Include/GameClient/KeyDefs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ enum
// modifier combinations when left/right isn't a factor
KEY_STATE_CONTROL = (KEY_STATE_LCONTROL | KEY_STATE_RCONTROL),
KEY_STATE_SHIFT = (KEY_STATE_LSHIFT | KEY_STATE_RSHIFT | KEY_STATE_SHIFT2 ),
KEY_STATE_ALT = (KEY_STATE_LALT | KEY_STATE_RALT)
KEY_STATE_ALT = (KEY_STATE_LALT | KEY_STATE_RALT),
KEY_STATE_MODIFIERS = (KEY_STATE_CONTROL | KEY_STATE_SHIFT | KEY_STATE_ALT)

};

Expand Down
3 changes: 2 additions & 1 deletion GeneralsMD/Code/GameEngine/Include/GameClient/KeyDefs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ enum
// modifier combinations when left/right isn't a factor
KEY_STATE_CONTROL = (KEY_STATE_LCONTROL | KEY_STATE_RCONTROL),
KEY_STATE_SHIFT = (KEY_STATE_LSHIFT | KEY_STATE_RSHIFT | KEY_STATE_SHIFT2 ),
KEY_STATE_ALT = (KEY_STATE_LALT | KEY_STATE_RALT)
KEY_STATE_ALT = (KEY_STATE_LALT | KEY_STATE_RALT),
KEY_STATE_MODIFIERS = (KEY_STATE_CONTROL | KEY_STATE_SHIFT | KEY_STATE_ALT)

};

Expand Down
Loading