tweak(gamemessage): Reduce number of MSG_DESTROY_SELECTED_GROUP messages

[Caball009]

• Similar to PRs tweak(gamemessage): Remove unused argument of MSG_DESTROY_SELECTED_GROUP #2664 and tweak(gamemessage): Remove unused MSG_AREA_SELECTION message from game logic #2667

The client considers primary mouse clicks in the game world as a deselection, and will also update the game logic. This means that GameMessage::MSG_DESTROY_SELECTED_GROUP messages are sent frequently, even if no objects are currently selected. There's actually an old EA comment on this issue. It's also unnecessary to send this message prior to the creation or selection of a new group.

This PR makes the following two changes:

1. a new check to verify that the local player has objects selected before creating a MSG_DESTROY_SELECTED_GROUP message.
2. Remove MSG_DESTROY_SELECTED_GROUP messages before other messages that already destroy group selection implicitly (MSG_CREATE_SELECTED_GROUP and GameMessage::MSG_SELECT_TEAM).

I've tested with two local clients in multiplayer (VS22 debug builds); one had this feature and one didn't, and everything worked fine.

See commits for cleaner diffs.

---

Results for Golden Replay 1:

• MSG_NETWORK_MESSAGES: 56985
• MSG_LOGIC_CRC: 11601 (doesn't include 1785 for playback)
• MSG_DO_MOVETO: 6888
• MSG_AREA_SELECTION_DEPRECATED: 4476
• MSG_QUEUE_UNIT_CREATE: 2225
• MSG_DESTROY_SELECTED_GROUP: 17366
• MSG_CREATE_SELECTED_GROUP_NO_SOUND: 317, new group: 88
• MSG_CREATE_SELECTED_GROUP: 8299, new group: 8114
• MSG_DESTROY_SELECTED_GROUP before new group creation: 8901 (more than 8114 + 88, because this message is used twice sometimes ).

Just the MSG_DESTROY_SELECTED_GROUP message before the creation of a new group accounts for 15% (8901 / 56985) of all network game messages in this replay. That doesn't even take into account how many times this message was used when no units were selected.

---

TODO:

• Replicate in Generals.
• Do more thorough testing.

[greptile-apps]

Greptile Summary

This PR reduces the volume of MSG_DESTROY_SELECTED_GROUP messages sent during gameplay via two complementary mechanisms: (1) deselectAllDrawables() now only emits the message if objects were actually selected, and (2) a new isRedundantMessage() check in propagateMessages() drops any MSG_DESTROY_SELECTED_GROUP that is immediately followed by a message that inherently replaces the current selection (MSG_CREATE_SELECTED_GROUP with newGroup=true, MSG_CREATE_SELECTED_GROUP_NO_SOUND with newGroup=true, a duplicate MSG_DESTROY_SELECTED_GROUP, or any MSG_SELECT_TEAM*).

• The Bool postMsg = true parameter is removed from deselectAllDrawables(), which simplifies all call sites (the false case in onMetaSelectTeam is now handled implicitly by the redundancy filter since MSG_SELECT_TEAM* always follows).
• selectSingleDrawableWithoutSound is refactored so deselection and visual selection only occur when draw->getObject() is non-null, avoiding a silent client-side-only UI state change when clicking an object-less drawable.
• The local deselectAll() wrapper in SelectionXlat.cpp is removed since it was a no-argument pass-through to TheInGameUI->deselectAllDrawables().

Confidence Score: 5/5

Safe to merge; the optimisation is well-scoped and the multiplayer test coverage described in the PR description covers the main risk surface.

The two mechanisms (guard in deselectAllDrawables and redundancy filter in propagateMessages) are independently correct and compose safely. The redundancy filter handles chains of multiple consecutive MSG_DESTROY_SELECTED_GROUP messages correctly. The onMetaSelectTeam call-site change is sound because the emitted MSG_DESTROY_SELECTED_GROUP is always immediately followed by a MSG_SELECT_TEAM* and is stripped by the filter before any translator sees it.

No files require special attention.

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/Common/MessageStream.cpp	Adds isRedundantMessage() and integrates it into the propagateMessages() inner loop; logic handles all expected MSG_DESTROY chains correctly.
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp	Removes postMsg parameter, adds hadSelectedDrawables guard so MSG_DESTROY_SELECTED_GROUP is only appended when something was actually selected.
Core/GameEngine/Source/GameClient/MessageStream/SelectionXlat.cpp	Removes the deselectAll() wrapper and updates all call sites. Refactors selectSingleDrawableWithoutSound so deselect/select only happen when the drawable has an associated game object.
GeneralsMD/Code/GameEngine/Include/Common/MessageStream.h	Adds const overloads for next()/prev() and declares isRedundantMessage as a protected const method.
GeneralsMD/Code/GameEngine/Include/GameClient/InGameUI.h	Removes Bool postMsg = true parameter from deselectAllDrawables() declaration.
Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarBeacon.cpp	Removes the now-gone TRUE argument from deselectAllDrawables(). Behaviour is identical since TRUE was the old default.
Core/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp	Comment text normalisation and minor whitespace cleanup. No logic changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[deselectAllDrawables called] --> B{hadSelectedDrawables?}
    B -- No --> C[No message emitted]
    B -- Yes --> D[Append MSG_DESTROY_SELECTED_GROUP]
    D --> E[propagateMessages loop]
    E --> F{isRedundantMessage?}
    F -- Yes: next is MSG_DESTROY_SELECTED_GROUP --> G[Delete duplicate destroy]
    F -- Yes: next is MSG_CREATE_SELECTED_GROUP newGroup=true --> H[Delete CREATE implies new group]
    F -- Yes: next is MSG_SELECT_TEAM* --> I[Delete team select implies deselect]
    F -- No: standalone deselect or add-to-group --> J[Pass to translators]
    J --> K{Translator disposition}
    K -- DESTROY_MESSAGE --> L[Delete message]
    K -- Other --> M[Forward to CommandList / network]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[deselectAllDrawables called] --> B{hadSelectedDrawables?}
    B -- No --> C[No message emitted]
    B -- Yes --> D[Append MSG_DESTROY_SELECTED_GROUP]
    D --> E[propagateMessages loop]
    E --> F{isRedundantMessage?}
    F -- Yes: next is MSG_DESTROY_SELECTED_GROUP --> G[Delete duplicate destroy]
    F -- Yes: next is MSG_CREATE_SELECTED_GROUP newGroup=true --> H[Delete CREATE implies new group]
    F -- Yes: next is MSG_SELECT_TEAM* --> I[Delete team select implies deselect]
    F -- No: standalone deselect or add-to-group --> J[Pass to translators]
    J --> K{Translator disposition}
    K -- DESTROY_MESSAGE --> L[Delete message]
    K -- Other --> M[Forward to CommandList / network]

Loading

_{Reviews (11): Last reviewed commit: "Added function 'MessageStream::isRedunda..." | Re-trigger Greptile}

[xezon]

How confident are we that this will cause no gameplay issues?

[xezon]

Needs rebasing.

[xezon]

I also considered this previously but I did not execute on simplifying that because of MessageStream::insertMessage

[Caball009]

Good point. I didn't think about that. I'll change it back.

[Caball009]

I changed it back but with a small refactor to make both delete branches identical:

next = msg->next();
deleteInstance(msg);
continue;

[Caball009]

Needs rebasing.

I just rebased. I held off on rebasing because I wanted to make it obvious to you (and myself) what the new changes were.