perf(gamemessage): Optimize argument list of GameMessage by replacing linked list with vector by RikuAnt · Pull Request #2700 · TheSuperHackers/GeneralsGameCode

RikuAnt · 2026-05-11T22:03:25Z

GameMessage and NetGameCommandMsg stored their arguments in a linked list (m_argList/m_argTail/m_argCount/m_next). Since arguments are only ever appended and iterated in order, this is replaced with a std::vector, removing the manual pointer threading and simplifying all related code. A DEBUG_ASSERTCRASH is added to guard the existing 255-argument limit.

AI usage was minimal. All code has been reviewed and tested to best of my capability.

TODO:

Replicate in Generals.

…rs#2630) Simpler and cleaner control. Improves loop performance significantly via cache hits and brings random index access down to O(1)

Only affects parts that relied on the linked list functionality of the GameMessageArguments

…uperHackers#2630)

greptile-apps · 2026-05-11T22:11:25Z

Greptile Summary

This PR replaces the manual linked-list argument storage (m_argList/m_argTail/m_argCount/m_next) in GameMessage and NetGameCommandMsg with std::vector<const GameMessageArgument*>, applying the change consistently across both the Generals and GeneralsMD code paths.

GameMessageArgument::m_next is removed, and m_argList/m_argTail/m_argCount members are replaced by a single std::vector<const GameMessageArgument*> m_argList. All append, iterate, and free code is updated accordingly. const-correctness is maintained: the vector stores const GameMessageArgument*, allocArg() returns a non-const raw pointer for initialization, and const_cast is used only in the destructor where ownership is clear.
getArguments() is a new accessor returning const std::vector<const GameMessageArgument*>&, used by NetGameCommandMsg and RecorderClass to replace the old index-based loops. A DEBUG_ASSERTCRASH guards the 255-argument limit that was previously enforced implicitly by the UnsignedByte counter type.

Confidence Score: 5/5

Safe to merge. The linked-list-to-vector swap is applied consistently across all four source/header pairs and the change is straightforward.

Every allocation, iteration, and deletion site has been updated in both the Generals and GeneralsMD trees. Const-correctness is preserved throughout: the vector stores const GameMessageArgument*, allocArg() returns a mutable pointer only for initialization, and const_cast is used exclusively in the destructor where ownership is unambiguous. The previous-thread concern about a reinterpret_cast between incompatible vector specialisations does not appear in this version — the member type and the accessor return type are the same specialisation. No logic changes were made beyond the storage model.

No files require special attention.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Include/Common/MessageStream.h	Removes m_next/m_argList/m_argTail/m_argCount; adds std::vector<const GameMessageArgument*> and getArguments() accessor with correct const-correctness. Inline getArgumentCount() adds a DEBUG_ASSERTCRASH guard.
Generals/Code/GameEngine/Source/Common/MessageStream.cpp	Constructor, destructor, getArgument(), getArgumentDataType(), and allocArg() all correctly updated to use vector semantics; const_cast used appropriately in destructor.
GeneralsMD/Code/GameEngine/Include/Common/MessageStream.h	Mirror of Generals changes; same vector refactoring applied correctly for Zero Hour expansion code path.
GeneralsMD/Code/GameEngine/Source/Common/MessageStream.cpp	Mirror of Generals .cpp changes; all linked-list traversal replaced with index-based vector access.
Core/GameEngine/Include/GameNetwork/NetCommandMsg.h	m_numArgs, m_argSize, m_argList (ptr), m_argTail removed; replaced with std::vector<const GameMessageArgument*>. Forward declaration of GameMessageArgument added. std::vector is available transitively via NetworkDefs.h → MessageStream.h → STLTypedefs.h.
Core/GameEngine/Source/GameNetwork/NetCommandMsg.cpp	Constructor, copy-constructor, destructor, addArgument(), and constructGameMessage() all cleanly updated; reserve() call added when copying args from a GameMessage.
Generals/Code/GameEngine/Source/Common/Recorder.cpp	Switches from getArgumentCount()/getArgumentDataType()/getArgument() loop to getArguments() vector range-for, simplifying the write path.
GeneralsMD/Code/GameEngine/Source/Common/Recorder.cpp	Same Recorder simplification as the Generals path.

_{Reviews (7): Last reviewed commit: "fix: remove inconsistent todo leftover c..." | Re-trigger Greptile}

…ersions (TheSuperHackers#2630)

… unnecessary test before uint conversion (TheSuperHackers#2630)

- Return GameMessageArguments as const - Style fixes (remove blank space, indent and space out too packed methods) - Remove vector includes; already included from precompiled header

…et_cast (TheSuperHackers#2630)

Caball009 · 2026-05-13T01:28:22Z

Good effort.

Generals fails to compile for now, but it's probably most convenient to wait for xezon's feedback before you replicate the changes from ZH to Gen.

xezon · 2026-05-14T08:56:16Z

Is Caballs review addressed?

…erHackers#2630) (cherry picked from commit 0c8438a88926ff2a2797854f9f17d94ce6daf177)

RikuAnt · 2026-05-14T12:17:24Z

Addressed now @xezon Also included the mirrored changes into Generals/ side

…kers#2630)

Caball009 · 2026-05-14T20:28:42Z

Generals is out-of-sync with Zero Hour.

I find it much easier to replicate the changes to Generals with a script. It tends to work quite well.

@echo off FOR /F "delims=" %%b IN ('git merge-base --fork-point main') DO git diff %%b > changes.patch git apply -p2 --directory=Generals --reject --whitespace=fix changes.patch del changes.patch pause

You can save this as a batch script and execute it in the project directory.

This is very handy script, thank you 🍻 Ran the remaining inconsistencies with this script just in case, should be fully in sync now

https://github.com/TheSuperHackers/GeneralsGameCode/pull/2700/changes#diff-a55d9f9b9be90608e4b5bbd55688d80c7295f43186530c99fd4830553e513126L87

Looks like you missed this one, though. Perhaps worth a try to revert all Generals changes, and do everything with the script?

Wanted to avoid that, basically did it to get the proper patch file already though. Then again, I was hasty and made yet another commit for that single comment. If there is still some issue, or you feel it would be better to do with reverts and fully with script - I'll do that next

The comments are out of sync between Gen and ZH. Please revert all changes in the Generals code and replicate them in one go with the script.

xezon

I recommend to replicate to Generals not before approval.

xezon · 2026-05-18T19:19:50Z

-		deleteInstance(arg);
-		arg = m_argList;
+	for (size_t i = 0; i < m_argList.size(); ++i) {
+		deleteInstance(const_cast<GameMessageArgument*>(m_argList[i]));


Instead of const cast, make the vector element non-const.

xezon · 2026-05-18T19:22:12Z

@@ -127,21 +123,10 @@ NetGameCommandMsg::~NetGameCommandMsg() {
 */
 void NetGameCommandMsg::addArgument(const GameMessageArgumentDataType type, GameMessageArgumentType arg)


Since this function is small and only called once, you can also inline it in the constructor, which reduces code complexity.

xezon · 2026-05-18T19:23:01Z

-		addArgument(msg->getArgumentDataType(i), *(msg->getArgument(i)));
+
+	const std::vector<const GameMessageArgument*>& args = msg->getArguments();
+	m_argList.reserve(args.size());


If you inline code of addArgument function here, then this can do m_argList.resize(args.size()), which is even better.

How is that better?

Because vec[i] = value will do a bit less work than vec.push_back(value)

xezon · 2026-05-18T19:31:09Z

-		nextArg = arg->m_next;
-		deleteInstance(arg);
+	for( size_t i = 0; i < m_argList.size(); ++i ) {
+		deleteInstance(const_cast<GameMessageArgument*>(m_argList[i]));


Instead of const cast, make the vector element non-const.

xezon · 2026-05-18T19:32:12Z

+		); 
+		return static_cast<UnsignedByte>(m_argList.size());
+	}
+	const std::vector<const GameMessageArgument*>& getArguments() const { return m_argList ; }


Instead of giving this function that returns internal data structure, use established getArgumentCount() and getArgument() functions at call sites.

xezon · 2026-05-18T19:35:36Z

+	// TheSuperHackers @refactor RiQQ 12/05/2026 Return argument count from vector size
+	/// @todo If a GameMessage needs more than 255 arguments, it needs to be split up into multiple GameMessage's.
+	UnsignedByte getArgumentCount() const {
+		DEBUG_ASSERTCRASH(


This assert looks misplaced in this function. One would expect this assert at the time of argument creation, not when querying the size of it.

xezon · 2026-05-18T19:36:00Z


 	Type getType() const { return m_type; }					///< Return the message type
-	UnsignedByte getArgumentCount() const { return m_argCount; }	///< Return the number of arguments for this msg
+	// TheSuperHackers @refactor RiQQ 12/05/2026 Return argument count from vector size


Remove superfluous comment

xezon · 2026-05-18T19:36:57Z

-	UnsignedByte m_argCount;										///< The number of arguments of this message
-
-	GameMessageArgument *m_argList, *m_argTail;						///< This message's arguments
+	// TheSuperHackers @refactor RiQQ 12/05/2026 Replaced linked list with std::vector for argument storage


I think this comment can be removed because it is not useful for the next reader.

xezon · 2026-05-18T19:37:25Z

 protected:
-	Int m_numArgs;
-	Int m_argSize;
+	// TheSuperHackers @refactor RiQQ 11/05/2026 Replaced linked list with std::vector for argument storage


I think this comment can be removed because it is not useful for the next reader.

xezon · 2026-05-18T19:38:38Z

+	UnsignedByte getArgumentCount() const {
+		DEBUG_ASSERTCRASH(
+			m_argList.size() <= 255,
+			("GameMessage has more than 255 arguments")


"If a GameMessage needs more than 255 arguments, it needs to be split up into multiple GameMessage's."

RikuAnt added 3 commits May 12, 2026 00:04

perf: refactor message argument handling to use vector (TheSuperHacke…

0bf542d

…rs#2630) Simpler and cleaner control. Improves loop performance significantly via cache hits and brings random index access down to O(1)

perf: adjust network messaging to use vectors (TheSuperHackers#2630)

572f477

Only affects parts that relied on the linked list functionality of the GameMessageArguments

fix: revert accidental todo comment delete and add debug assert (TheS…

f627bba

…uperHackers#2630)

greptile-apps Bot reviewed May 11, 2026

View reviewed changes

Comment thread GeneralsMD/Code/GameEngine/Include/Common/MessageStream.h Outdated

Caball009 reviewed May 11, 2026

View reviewed changes

RikuAnt added 3 commits May 12, 2026 16:22

refactor: refactor auto and range based loops into c++98 compatible v…

84d7681

…ersions (TheSuperHackers#2630)

perf: reserve vector storage before iteratively populating and remove…

51bd443

… unnecessary test before uint conversion (TheSuperHackers#2630)

refactor: Make minor code review changes(TheSuperHackers#2630)

8e5eda3

- Return GameMessageArguments as const - Style fixes (remove blank space, indent and space out too packed methods) - Remove vector includes; already included from precompiled header

greptile-apps Bot reviewed May 12, 2026

View reviewed changes

Comment thread GeneralsMD/Code/GameEngine/Include/Common/MessageStream.h Outdated

refactor: refactor GameMessageArguments into const to avoid reinterpr…

6ca8832

…et_cast (TheSuperHackers#2630)

RikuAnt requested a review from Caball009 May 12, 2026 14:24

L3-M added Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour Refactor Edits the code with insignificant behavior changes, is never user facing Network Anything related to network, servers labels May 12, 2026

L3-M added this to the Code foundation build up milestone May 12, 2026

Caball009 reviewed May 13, 2026

View reviewed changes

RikuAnt added 2 commits May 14, 2026 15:10

fix: Get args by reference instead of copying (TheSuperHackers#2630)

be4126e

refactor(messaging): Mirror changes made to Zero Hour version (TheSup…

763f611

…erHackers#2630) (cherry picked from commit 0c8438a88926ff2a2797854f9f17d94ce6daf177)

refactor': remove unused m_next from GameMessageArgument (TheSuperHac…

4e7c038

…kers#2630)

Caball009 reviewed May 14, 2026

View reviewed changes

RikuAnt added 2 commits May 14, 2026 23:59

fix: remove inconsistent todo leftover comment (TheSuperHackers#2630)

b06b824

fix: remove inconsistent todo leftover comment (TheSuperHackers#2630)

2d3dc31

xezon changed the title ~~refactor(messaging): replace GameMessage argument linked list with std::vector~~ perf(gamemessage): Optimize argument list of GameMessage by replacing linked list with vector May 18, 2026

xezon added Performance Is a performance concern and removed Refactor Edits the code with insignificant behavior changes, is never user facing labels May 18, 2026

xezon added the Gen Relates to Generals label May 18, 2026

xezon reviewed May 18, 2026

View reviewed changes

		@@ -127,21 +123,10 @@ NetGameCommandMsg::~NetGameCommandMsg() {
		*/
		void NetGameCommandMsg::addArgument(const GameMessageArgumentDataType type, GameMessageArgumentType arg)

Conversation

RikuAnt commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO:

Uh oh!

greptile-apps Bot commented May 11, 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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

xezon commented May 14, 2026

Uh oh!

RikuAnt commented May 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Caball009 May 14, 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!

Caball009 May 14, 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!

Caball009 May 18, 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

RikuAnt commented May 11, 2026 •

edited

Loading

greptile-apps Bot commented May 11, 2026 •

edited

Loading

Caball009 commented May 13, 2026 •

edited

Loading

Caball009 May 14, 2026 •

edited

Loading

Caball009 May 14, 2026 •

edited

Loading

Caball009 May 18, 2026 •

edited

Loading