tweak(gamemessage): Remove unused MSG_AREA_SELECTION message from game logic by Caball009 · Pull Request #2667 · TheSuperHackers/GeneralsGameCode

Caball009 · 2026-04-30T19:51:29Z

Similar to PR tweak(gamemessage): Remove unused argument of MSG_DESTROY_SELECTED_GROUP #2664

GameMessage::MSG_AREA_SELECTION is currently passed on to the game logic (and sent to other clients), but it isn't used for anything. This PR changes that so that it isn't sent to other clients anymore.

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

TODO:

Replicate in Generals.

greptile-apps · 2026-04-30T19:53:17Z

Greptile Summary

This PR reclassifies the MSG_AREA_SELECTION message from a networked game-logic message to a local-only meta message (MSG_META_AREA_SELECTION) in the GeneralsMD/ codebase, since the message was never consumed by game logic or needed by other clients. The old enum value is preserved as MSG_AREA_SELECTION_DEPRECATED (with a removal TODO) to keep network/replay ordinal compatibility intact. An equivalent change in Generals/ is tracked as a follow-up in the PR TODO.

Confidence Score: 5/5

Safe to merge; all four call sites are correctly updated and enum ordinal compatibility is preserved.

Only a P2 style finding (stale comment in HintSpy.cpp). Logic is sound: META messages are client-local, deprecated slot keeps binary layout stable, and the PR author verified this with two-client multiplayer testing.

No files require special attention.

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Include/Common/MessageStream.h	Adds MSG_META_AREA_SELECTION to the META (client-local) section and renames MSG_AREA_SELECTION to MSG_AREA_SELECTION_DEPRECATED to preserve enum ordinals for network/replay compatibility.
GeneralsMD/Code/GameEngine/Source/Common/MessageStream.cpp	Updates getCommandTypeAsString to register the new MSG_META_AREA_SELECTION label and rename MSG_AREA_SELECTION to MSG_AREA_SELECTION_DEPRECATED.
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/HintSpy.cpp	Updates case label from MSG_AREA_SELECTION to MSG_META_AREA_SELECTION; the adjacent comment still refers to the old name.
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/SelectionXlat.cpp	Both appendMessage call sites updated from MSG_AREA_SELECTION to MSG_META_AREA_SELECTION, correctly preventing the selection message from being forwarded to other clients.

Sequence Diagram

sequenceDiagram
    participant User
    participant SelectionXlat
    participant MessageStream
    participant HintSpy
    participant Network

    User->>SelectionXlat: drag-select / click-select
    SelectionXlat->>MessageStream: appendMessage(MSG_AREA_SELECTION_HINT)
    MessageStream->>HintSpy: MSG_AREA_SELECTION_HINT
    HintSpy->>HintSpy: beginAreaSelectHint()

    User->>SelectionXlat: release / confirm
    SelectionXlat->>MessageStream: appendMessage(MSG_META_AREA_SELECTION)
    note over MessageStream: META messages stay local
    MessageStream->>HintSpy: MSG_META_AREA_SELECTION
    HintSpy->>HintSpy: endAreaSelectHint()
    MessageStream--xNetwork: NOT forwarded (was MSG_AREA_SELECTION before)

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/HintSpy.cpp:105
**Stale comment — enum name not updated**

The comment still says `AREA_SELECTION` but the case now matches `MSG_META_AREA_SELECTION`. It's a minor inaccuracy that could confuse future readers.

```suggestion
		// An AREA_SELECTION_HINT is always followed by a MSG_META_AREA_SELECTION, so
```

_{Reviews (2): Last reviewed commit: "Addressed feedback." | Re-trigger Greptile}

xezon · 2026-04-30T20:09:44Z

 		MSG_COMBATDROP_AT_LOCATION,									///< dump out all rappellers
 		MSG_COMBATDROP_AT_OBJECT,										///< dump out all rappellers
+
+		// TheSuperHackers @todo MSG_AREA_SELECTION can be moved out of the network messages because it's currently unused by the game logic.


Can it already be moved out and then a MSG_AREA_SELECTION_DEPRECATED placed here to preserve the order?

That sounds like a good idea. I'll check it out.

Do you have a suggestion where the non-network version should go?

EDIT:

// "meta" messages should be thought of as "virtual keystrokes" -- they exist // solely to provide an abstraction layer useful for keyboard/mouse remapping. // they should NEVER be sent over the network.

I suppose it fits this description, because the client uses it for the selection of the drawables and then generates the necessary GameMessages to update the selection for the game logic.

MSG_META_SELECT_HERO, ///< selects player's hero character, if exists... MSG_META_SELECT_ALL, ///< selects all units across screen MSG_META_SELECT_ALL_AIRCRAFT, ///< selects all air units just like select all MSG_META_SCATTER,

Perhaps between MSG_META_SELECT_ALL_AIRCRAFT and MSG_META_SCATTER?

There is a MSG_AREA_SELECTION_HINT outside network messages.

That has a distinct purpose, right?

I don't know.

It's used when you start to create an area selection by dragging the mouse. When you release the mouse button a MSG_AREA_SELECTION message is created.

Why can it not be grouped together with MSG_AREA_SELECTION_HINT ?

Caball009 · 2026-04-30T23:47:23Z

-		case GameMessage::MSG_AREA_SELECTION:
+		case GameMessage::MSG_META_AREA_SELECTION:
 			TheInGameUI->endAreaSelectHint( msg );
 			break;


FYI this message is now outside the network message range, so it'll get removed eventually, and won't be sent to other clients.

Caball009 · 2026-04-30T23:50:28Z

I could make the following changes if that's cleaner:

MSG_AREA_SELECTION_HINT -> MSG_BEGIN_AREA_SELECTION_HINT
MSG_AREA_SELECTION -> MSG_END_AREA_SELECTION_HINT

xezon

The disconnect in grouping for the 2 Area select messages is confusing.

xezon · 2026-05-01T07:49:54Z


-		// TheSuperHackers @todo MSG_AREA_SELECTION can be moved out of the network messages because it's currently unused by the game logic.
-		MSG_AREA_SELECTION,													///< (pixelRegion) rectangular selection area
+		// TheSuperHackers @todo Remove MSG_AREA_SELECTION_DEPRECATED when possible.


Todo comment is not necessary.

Instead

MSG_AREA_SELECTION_DEPRECATED, ///< TheSuperHackers @tweak former MSG_AREA_SELECTION is deprecated as network message

xezon · 2026-05-01T07:52:08Z

 		MSG_COMBATDROP_AT_LOCATION,									///< dump out all rappellers
 		MSG_COMBATDROP_AT_OBJECT,										///< dump out all rappellers
+
+		// TheSuperHackers @todo MSG_AREA_SELECTION can be moved out of the network messages because it's currently unused by the game logic.


Why can it not be grouped together with MSG_AREA_SELECTION_HINT ?

xezon · 2026-05-01T07:53:08Z

@@ -104,12 +104,8 @@ GameMessageDisposition HintSpyTranslator::translateGameMessage(const GameMessage
 		//-----------------------------------------------------------------------------
 		// An AREA_SELECTION_HINT is always followed by an AREA_SELECTION, so


Is text up-to-date?

tweak(msg): Remove unused MSG_AREA_SELECTION message from game logic.

af44b5e

Caball009 added Enhancement Is new feature or request Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Apr 30, 2026

xezon reviewed Apr 30, 2026

View reviewed changes

Comment thread GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/HintSpy.cpp Outdated

Caball009 mentioned this pull request Apr 30, 2026

tweak(gamemessage): Reduce number of MSG_DESTROY_SELECTED_GROUP messages #2668

Open

1 task

xezon reviewed Apr 30, 2026

View reviewed changes

Addressed feedback.

bc50caa

Caball009 commented Apr 30, 2026

View reviewed changes

xezon reviewed May 1, 2026

View reviewed changes

		@@ -104,12 +104,8 @@ GameMessageDisposition HintSpyTranslator::translateGameMessage(const GameMessage
		//-----------------------------------------------------------------------------
		// An AREA_SELECTION_HINT is always followed by an AREA_SELECTION, so

Conversation

Caball009 commented Apr 30, 2026

TODO:

Uh oh!

greptile-apps Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Caball009 Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Caball009 commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Apr 30, 2026 •

edited

Loading

Caball009 Apr 30, 2026 •

edited

Loading

Caball009 commented Apr 30, 2026 •

edited

Loading