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

Author: xezon

Core/Libraries/Include/Lib/BaseType.h

@@ -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) ) == x )
 #define BitSet( x, i ) ( (x) |= (i) )
 #define BitClear( x, i ) ( (x ) &= ~(i) )
 #define BitToggle( x, i ) ( (x) ^= (i) )

GeneralsMD/Code/GameEngine/Include/GameClient/MetaEvent.h

@@ -354,9 +354,86 @@ 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;
+		}
+
+		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
+	};
+
+	Int m_lastModState;	// really a MappableKeyModState
+
+	KeyDownInfo m_keyDownInfos[KEY_COUNT];
 
 	enum { NUM_MOUSE_BUTTONS = 3 };
 	ICoord2D m_mouseDownPosition[NUM_MOUSE_BUTTONS];

GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp

@@ -378,14 +378,11 @@ static const FieldParse TheMetaMapFieldParseTable[] =
 
 //-------------------------------------------------------------------------------------------------
 MetaEventTranslator::MetaEventTranslator() :
-	m_lastKeyDown(MK_NONE),
 	m_lastModState(0)
 {
 	for (Int i = 0; i < NUM_MOUSE_BUTTONS; ++i) {
 		m_nextUpShouldCreateDoubleClick[i] = FALSE;
 	}
-
-
 }
 
 //-------------------------------------------------------------------------------------------------
@@ -441,8 +438,19 @@ 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;
+		Int systemKey = msg->getArgument(0)->integer;
+		switch (systemKey)
+		{
+		case KEY_LCTRL:
+		case KEY_RCTRL:
+		case KEY_LSHIFT:
+		case KEY_RSHIFT:
+		case KEY_LALT:
+		case KEY_RALT:
+			systemKey = KEY_NONE;
+		}
+		const MappableKeyType key = (MappableKeyType)systemKey;
+		const 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.
@@ -463,6 +471,52 @@ GameMessageDisposition MetaEventTranslator::translateGameMessage(const GameMessa
 			newModState |= ALT;
 		}
 
+		const Bool modStateRemoved = newModState < m_lastModState;
+
+		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)
+				{
+					const MappableKeyModState keyDownModState = keyDownInfo.getKeyModState(modStateIndex);
+
+					if (keyDownModState == NONE)
+						continue;
+
+					if (BitsAreSet(keyDownModState, newModState))
+						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)
 		{
@@ -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 &&
@@ -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,
@@ -540,13 +576,10 @@ GameMessageDisposition MetaEventTranslator::translateGameMessage(const GameMessa
 			}
 		}
 
-
+		}
 
 		if (t == GameMessage::MSG_RAW_KEY_DOWN)
     {
-			m_lastKeyDown = key;
-
-
 #ifdef DUMP_ALL_KEYS_TO_LOG
 
 		          WideChar Wkey = TheKeyboard->getPrintableKey(key, 0);
@@ -556,11 +589,13 @@ 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);
+			}
     }
 
-
-
     m_lastModState = newModState;
 	}