refactor(messaging): replace GameMessage argument linked list with std::vector#2700
refactor(messaging): replace GameMessage argument linked list with std::vector#2700RikuAnt wants to merge 12 commits into
Conversation
…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
|
| 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
| // TheSuperHackers @refactor RiQQ 11/05/2026 Replaced linked list with std::vector for argument storage | ||
| GameMessage::Type m_type; | ||
| GameMessageArgument *m_argList, *m_argTail; | ||
| std::vector<GameMessageArgument*> m_argList; |
There was a problem hiding this comment.
Can this be std::vector<const GameMessageArgument*>?
There was a problem hiding this comment.
This collides with the pooled object delete functionality deleteInstance in NetGameCommandMsg destructor. We can use I made the change in favor of const_cast over the reinterpret_cast that the AI first recommendedconst_cast if this is the preferred way?
There was a problem hiding this comment.
I see. The const_cast is not ideal, but that's price we've to pay, I suppose. I think the current way with <const T*> is preferable, but I'll let this conversation stay unresolved for now.
(const_cast is right here; I'm not sure whether reinterpret_cast would even compile here).
… 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
| // TheSuperHackers @refactor RiQQ 11/05/2026 Replaced linked list with std::vector for argument storage | ||
| GameMessage::Type m_type; | ||
| GameMessageArgument *m_argList, *m_argTail; | ||
| std::vector<GameMessageArgument*> m_argList; |
There was a problem hiding this comment.
I see. The const_cast is not ideal, but that's price we've to pay, I suppose. I think the current way with <const T*> is preferable, but I'll let this conversation stay unresolved for now.
(const_cast is right here; I'm not sure whether reinterpret_cast would even compile here).
|
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. |
|
Is Caballs review addressed? |
…erHackers#2630) (cherry picked from commit 0c8438a88926ff2a2797854f9f17d94ce6daf177)
|
Addressed now @xezon Also included the mirrored changes into Generals/ side |
Fixes #2630
GameMessageandNetGameCommandMsgstored 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 astd::vector, removing the manual pointer threading and simplifying all related code. ADEBUG_ASSERTCRASHis 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: