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 5 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
5 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
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
3 changes: 2 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 @@ -124,7 +124,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 +140,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
111 changes: 69 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 @@ -96,6 +96,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 +155,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;
const UnsignedShort lastPressedKeyState = m_lastPressedKeyState[key];

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 modifier state for
// each buffered event and reuse an action key's press state on its release.
BitSet( m_keys[ index ].state, m_modifiers );
if( BitIsSet( m_keys[ index ].state, KEY_STATE_DOWN ) )
{
m_lastPressedKeyState[key] = isModifier ? KEY_STATE_NONE : m_modifiers & KEY_STATE_MODIFIERS;

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

Why does this save KEY_STATE_NONE if isModifier is true?

Can we do:

if (!isModifier)
{
  m_lastPressedKeyState[key] = m_modifiers;
}

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

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

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.

}
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 +249,8 @@ 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 ].status = KeyboardIO::STATUS_UNUSED;

// Set End Flag
Expand Down Expand Up @@ -699,6 +716,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 +767,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 +785,31 @@ 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);
}
}

//-------------------------------------------------------------------------------------------------
// 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);

@xezon xezon Sep 10, 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.

Is a Shift 2 missing? Key State has one.

@CryoTheRenegade CryoTheRenegade Sep 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Shift2 uses m_shift2Key. It currently maps to KEY_RALT, which is already released here. I also added support for a separate Shift2 key without sending a duplicate RAlt release.

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
32 changes: 6 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,14 @@ GameMessageDisposition HotKeyTranslator::translateGameMessage(const GameMessage

if ( t == GameMessage::MSG_RAW_KEY_UP)
{

//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;
if( keyState & 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
5 changes: 4 additions & 1 deletion Core/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,10 @@ void MetaEventTranslator::onKeyPressed(GameMessageDisposition &disp, Int systemK
continue;

const Bool isMatchingKeyCombo = map->m_key == keyType && map->m_modState == keyModState;
const Bool isMatchingTransitionUp = map->m_transition == UP && (systemKeyState & KEY_STATE_UP) != 0;
// A modifier release can end this mapping before the action key is released.
// Do not generate the same UP event again when the action key arrives.
const Bool isMatchingTransitionUp = map->m_transition == UP && (systemKeyState & KEY_STATE_UP) != 0
&& (keyModState == NONE || m_keyDownInfos[keyType].hasKeyModState(keyModState));
const Bool isMatchingTransitionDown = map->m_transition == DOWN && (systemKeyState & KEY_STATE_DOWN) != 0;
//const Bool isMatchingTransitionDoubleDown = map->m_transition == DOUBLEDOWN && (systemKeyState & KEY_STATE_DOWN) && m_lastKeyDown == key;

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