Skip to content

fix(metaevent): Ignore order in which modifier keys are released to trigger meta events #2577

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
xezon wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(metaevent): Ignore order in which modifier keys are released to trigger meta events #2577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2135333
fix(metaevent): Ignore order in which modifier keys are released to t…
xezon Apr 11, 2026
f2eab9b
Add missing clearKeyModState
xezon Apr 11, 2026
a7312b0
Flip args BitsAreSet
xezon Apr 12, 2026
d6a0112
Fix clearKeyModState and remove m_lastModState to simplify modstatech…
xezon Apr 12, 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
1 change: 1 addition & 0 deletions Core/Libraries/Include/Lib/BaseType.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ inline Real deg2rad(Real rad) { return rad * (PI/180); }
//-----------------------------------------------------------------------------
// TheSuperHackers @build xezon 17/03/2025 Renames BitTest to BitIsSet to prevent conflict with BitTest macro from winnt.h
#define BitIsSet( x, i ) ( ( (x) & (i) ) != 0 )
#define BitsAreSet( x, i ) ( ( (x) & (i) ) == (i) )
#define BitSet( x, i ) ( (x) |= (i) )
#define BitClear( x, i ) ( (x ) &= ~(i) )
#define BitToggle( x, i ) ( (x) ^= (i) )
Expand Down
81 changes: 78 additions & 3 deletions GeneralsMD/Code/GameEngine/Include/GameClient/MetaEvent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,84 @@ EMPTY_DTOR(MetaMapRec)
class MetaEventTranslator : public GameMessageTranslator
{
private:

Int m_lastKeyDown; // really a MappableKeyType
Int m_lastModState; // really a MappableKeyModState
struct KeyDownInfo
{
KeyDownInfo() : m_modStateBits(0) {}

static UnsignedInt getMaxKeyModStateCount()
{
return 7;
Comment thread
Mauller marked this conversation as resolved.
}

static MappableKeyModState toKeyModState(UnsignedInt index)
{
switch (index)
{
case 0: return CTRL;
case 1: return ALT;
case 2: return SHIFT;
case 3: return CTRL_ALT;
case 4: return SHIFT_CTRL;
case 5: return SHIFT_ALT;
case 6: return SHIFT_ALT_CTRL;
}
return NONE;
}

static UnsignedInt toIndex(MappableKeyModState modState)
{
switch (modState)
{
case CTRL: return 0;
case ALT: return 1;
case SHIFT: return 2;
case CTRL_ALT: return 3;
case SHIFT_CTRL: return 4;
case SHIFT_ALT: return 5;
case SHIFT_ALT_CTRL: return 6;
}
return 7;
}

Bool isKeyDown() const
{
return m_modStateBits != 0;
}

MappableKeyModState getKeyModState(UnsignedInt index)
{
if (BitIsSet(m_modStateBits, 1 << index))
{
return toKeyModState(index);
}
return NONE;
}

void clearKeyModState(UnsignedInt index)
{
BitClear(m_modStateBits, 1 << index);
}

Bool hasKeyModState(MappableKeyModState modState) const
{
return BitIsSet(m_modStateBits, 1 << toIndex(modState));
}

void setKeyModState(MappableKeyModState modState)
{
BitSet(m_modStateBits, 1 << toIndex(modState));
}

void clearKeyModState(MappableKeyModState modState)
{
BitClear(m_modStateBits, 1 << toIndex(modState));
}

private:
UnsignedByte m_modStateBits; ///< Fits all combinations of CTRL+ALT+SHIFT, storing 1 bit for each
};

KeyDownInfo m_keyDownInfos[KEY_COUNT];

enum { NUM_MOUSE_BUTTONS = 3 };
ICoord2D m_mouseDownPosition[NUM_MOUSE_BUTTONS];
Expand Down
109 changes: 76 additions & 33 deletions GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,15 +377,11 @@ static const FieldParse TheMetaMapFieldParseTable[] =
// PUBLIC FUNCTIONS ///////////////////////////////////////////////////////////////////////////////

//-------------------------------------------------------------------------------------------------
MetaEventTranslator::MetaEventTranslator() :
m_lastKeyDown(MK_NONE),
m_lastModState(0)
MetaEventTranslator::MetaEventTranslator()
{
for (Int i = 0; i < NUM_MOUSE_BUTTONS; ++i) {
m_nextUpShouldCreateDoubleClick[i] = FALSE;
}


}

//-------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -441,8 +437,20 @@ GameMessageDisposition MetaEventTranslator::translateGameMessage(const GameMessa

if (t == GameMessage::MSG_RAW_KEY_DOWN || t == GameMessage::MSG_RAW_KEY_UP)
{
MappableKeyType key = (MappableKeyType)msg->getArgument(0)->integer;
Int keyState = msg->getArgument(1)->integer;
const Int systemKey = msg->getArgument(0)->integer;
const Int keyState = msg->getArgument(1)->integer;

MappableKeyType key = (MappableKeyType)systemKey;
switch (systemKey)
{
case KEY_LCTRL:
case KEY_RCTRL:
case KEY_LSHIFT:
case KEY_RSHIFT:
case KEY_LALT:
case KEY_RALT:
key = MK_NONE;
}

// for our purposes here, we don't care to distinguish between right and left keys,
// so just fudge a little to simplify things.
Expand All @@ -463,6 +471,52 @@ GameMessageDisposition MetaEventTranslator::translateGameMessage(const GameMessa
newModState |= ALT;
}

const Bool modStateRemoved = (key == MK_NONE) && (t == GameMessage::MSG_RAW_KEY_UP);

if (modStateRemoved)
{
// TheSuperHackers @fix The key handler now ignores the order in which modifier keys are released.
// This avoids frustrating experiences where a wrong button release order would skip an important key event.

for (Int keyDownIndex = 0; keyDownIndex < ARRAY_SIZE(m_keyDownInfos); ++keyDownIndex)
{
const MappableKeyType keyDown = (MappableKeyType)keyDownIndex;
KeyDownInfo &keyDownInfo = m_keyDownInfos[keyDownIndex];

if (!keyDownInfo.isKeyDown())
continue;

for (UnsignedInt modStateIndex = 0; modStateIndex < KeyDownInfo::getMaxKeyModStateCount(); ++modStateIndex)
Copy link
Copy Markdown

@Mauller Mauller Apr 21, 2026

Choose a reason for hiding this comment

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

Do you think we can extract some of the inner loop handling?
The levels of indentation can make it hard to follow.

Copy link
Copy Markdown
Author

@xezon xezon Apr 25, 2026

Choose a reason for hiding this comment

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

Yes I wanted to make a follow up change that refactors this function because it is quite a bit long.

{
const MappableKeyModState keyDownModState = keyDownInfo.getKeyModState(modStateIndex);

if (keyDownModState == NONE)
continue;

if (BitsAreSet(newModState, keyDownModState))
continue;

// Forget that this key and mod state are pressed.
keyDownInfo.clearKeyModState(modStateIndex);

for (const MetaMapRec *map = TheMetaMap->getFirstMetaMapRec(); map; map = map->m_next)
{
if (!isMessageUsable(map->m_usableIn))
continue;

if (!(map->m_key == keyDown && map->m_modState == keyDownModState && map->m_transition == UP))
continue;

TheMessageStream->appendMessage(map->m_meta);
disp = DESTROY_MESSAGE;
}
}
}
}
else
{
// TheSuperHackers @info The regular key handler only triggers events when the mapped key is pressed,
// not when the modifier (CTRL, ALT, SHIFT) is pressed, unless the key is MK_NONE.

for (const MetaMapRec *map = TheMetaMap->getFirstMetaMapRec(); map; map = map->m_next)
{
Expand All @@ -472,23 +526,6 @@ GameMessageDisposition MetaEventTranslator::translateGameMessage(const GameMessa
if (!isMessageUsable(map->m_usableIn))
continue;

// check for the special case of mods-only-changed.
if (
map->m_key == MK_NONE &&
newModState != m_lastModState &&
(
(map->m_transition == UP && map->m_modState == m_lastModState) ||
(map->m_transition == DOWN && map->m_modState == newModState)
)
)
{
//DEBUG_LOG(("Frame %d: MetaEventTranslator::translateGameMessage() Mods-only change: %s", TheGameLogic->getFrame(), findGameMessageNameByType(map->m_meta)));
/*GameMessage *metaMsg =*/ TheMessageStream->appendMessage(map->m_meta);
disp = DESTROY_MESSAGE;
break;
}

// ok, now check for "normal" key transitions.
if (
map->m_key == key &&
map->m_modState == newModState &&
Expand All @@ -499,7 +536,6 @@ GameMessageDisposition MetaEventTranslator::translateGameMessage(const GameMessa
)
)
{

if( keyState & KEY_STATE_AUTOREPEAT )
{
// if it's an autorepeat of a "known" key, don't generate the meta-event,
Expand Down Expand Up @@ -540,13 +576,8 @@ GameMessageDisposition MetaEventTranslator::translateGameMessage(const GameMessa
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
}



if (t == GameMessage::MSG_RAW_KEY_DOWN)
{
m_lastKeyDown = key;


#ifdef DUMP_ALL_KEYS_TO_LOG

WideChar Wkey = TheKeyboard->getPrintableKey(key, 0);
Expand All @@ -556,12 +587,24 @@ GameMessageDisposition MetaEventTranslator::translateGameMessage(const GameMessa
aKey.translate(uKey);
DEBUG_LOG(("^%s ", aKey.str()));
#endif

if (newModState != NONE)
{
// Remember that this key and mod state are pressed.
m_keyDownInfos[key].setKeyModState((MappableKeyModState)newModState);
}
}
else
{
if (newModState != NONE)
{
DEBUG_ASSERTCRASH(key != MK_NONE, ("Key is expected to be not MK_NONE"));

// Forget that this key and mod state are pressed.
m_keyDownInfos[key].clearKeyModState((MappableKeyModState)newModState);
}
}


m_lastModState = newModState;
}
Comment thread
Mauller marked this conversation as resolved.
}


Expand Down
Loading