Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Core/GameEngine/Include/GameNetwork/NetCommandMsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "GameNetwork/NetPacketStructs.h"
#include "Common/UnicodeString.h"

class GameMessageArgument;
Comment thread
Caball009 marked this conversation as resolved.
class NetCommandRef;

//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -117,10 +118,9 @@ class NetGameCommandMsg : public NetCommandMsgT<NetPacketGameCommand, SmallNetPa
virtual Select getSmallNetPacketSelect() const override;

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

GameMessage::Type m_type;
GameMessageArgument *m_argList, *m_argTail;
std::vector<const GameMessageArgument*> m_argList;
};

//-----------------------------------------------------------------------------
Expand Down
42 changes: 12 additions & 30 deletions Core/GameEngine/Source/GameNetwork/NetCommandMsg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,8 @@ Int NetCommandMsg::getSortNumber() const {
* Constructor with no argument, sets everything to default values.
*/
NetGameCommandMsg::NetGameCommandMsg() {
m_argSize = 0;
m_numArgs = 0;
m_type = (GameMessage::Type)0;
m_commandType = NETCOMMANDTYPE_GAMECOMMAND;
m_argList = nullptr;
m_argTail = nullptr;
}

/**
Expand All @@ -102,23 +98,23 @@ NetGameCommandMsg::NetGameCommandMsg() {
*/
NetGameCommandMsg::NetGameCommandMsg(GameMessage *msg) {
m_commandType = NETCOMMANDTYPE_GAMECOMMAND;

m_type = msg->getType();
Int count = msg->getArgumentCount();
for (Int i = 0; i < count; ++i) {
addArgument(msg->getArgumentDataType(i), *(msg->getArgument(i)));

const std::vector<const GameMessageArgument*>& args = msg->getArguments();
m_argList.reserve(args.size());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is that better?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I did some benchmark tests and it's tied to the cost of default construction and the vector size. It'd appear to be the better choice in this context, though.


for (size_t i = 0; i < args.size(); ++i) {
Comment thread
Caball009 marked this conversation as resolved.
const GameMessageArgument* arg = args[i];
addArgument(arg->m_type, arg->m_data);
}
}

/**
* Destructor
*/
NetGameCommandMsg::~NetGameCommandMsg() {
GameMessageArgument *arg = m_argList;
while (arg != nullptr) {
m_argList = m_argList->m_next;
deleteInstance(arg);
arg = m_argList;
for (size_t i = 0; i < m_argList.size(); ++i) {
deleteInstance(const_cast<GameMessageArgument*>(m_argList[i]));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
if (m_argTail == nullptr) {
m_argList = newInstance(GameMessageArgument);
m_argTail = m_argList;
m_argList->m_data = arg;
m_argList->m_type = type;
m_argList->m_next = nullptr;
return;
}

GameMessageArgument *newArg = newInstance(GameMessageArgument);
newArg->m_data = arg;
newArg->m_type = type;
newArg->m_next = nullptr;
m_argTail->m_next = newArg;
m_argTail = newArg;
m_argList.push_back(newArg);
}

// here's where we figure out which slot corresponds to which player
Expand Down Expand Up @@ -171,9 +156,8 @@ GameMessage *NetGameCommandMsg::constructGameMessage() const
name.format("player%d", getPlayerID());
retval->friend_setPlayerIndex( ThePlayerList->findPlayerWithNameKey(TheNameKeyGenerator->nameToKey(name))->getPlayerIndex());

GameMessageArgument *arg = m_argList;
while (arg != nullptr) {

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

case ARGUMENTDATATYPE_INTEGER:
Expand Down Expand Up @@ -211,8 +195,6 @@ GameMessage *NetGameCommandMsg::constructGameMessage() const
break;

}

arg = arg->m_next;
}
return retval;
}
Expand Down
19 changes: 12 additions & 7 deletions Generals/Code/GameEngine/Include/Common/MessageStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ class GameMessageArgument : public MemoryPoolObject
{
MEMORY_POOL_GLUE_WITH_USERLOOKUP_CREATE(GameMessageArgument, "GameMessageArgument")
public:
GameMessageArgument* m_next; ///< The next argument
GameMessageArgumentType m_data; ///< The data storage of an argument
GameMessageArgumentDataType m_type; ///< The type of the argument.
};
Expand Down Expand Up @@ -605,7 +604,16 @@ class GameMessage : public MemoryPoolObject
GameMessage *prev() { return m_prev; } ///< Return prev message in the stream

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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove superfluous comment

/// @todo If a GameMessage needs more than 255 arguments, it needs to be split up into multiple GameMessage's.
UnsignedByte getArgumentCount() const {
DEBUG_ASSERTCRASH(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

m_argList.size() <= 255,
("GameMessage has more than 255 arguments")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

const char *getCommandAsString() const; ///< returns a string representation of the command type.
static const char *getCommandTypeAsString(GameMessage::Type t);
Expand All @@ -628,7 +636,6 @@ class GameMessage : public MemoryPoolObject

/**
* Return the given argument union.
* @todo This should be a more list-like interface. Very inefficient.
*/
const GameMessageArgumentType *getArgument( Int argIndex ) const;
GameMessageArgumentDataType getArgumentDataType( Int argIndex ) const;
Expand All @@ -649,10 +656,8 @@ class GameMessage : public MemoryPoolObject

Int m_playerIndex; ///< The Player who issued the command

/// @todo If a GameMessage needs more than 255 arguments, it needs to be split up into multiple GameMessage's.
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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

std::vector<const GameMessageArgument*> m_argList; ///< This message's arguments

/// allocate a new argument, add it to list, return pointer to its data
GameMessageArgument *allocArg();
Expand Down
46 changes: 7 additions & 39 deletions Generals/Code/GameEngine/Source/Common/MessageStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ GameMessage::GameMessage( GameMessage::Type type )
{
m_playerIndex = ThePlayerList->getLocalPlayer()->getPlayerIndex();
m_type = type;
m_argList = nullptr;
m_argTail = nullptr;
m_argCount = 0;
m_list = nullptr;
}

Expand All @@ -69,12 +66,8 @@ GameMessage::GameMessage( GameMessage::Type type )
GameMessage::~GameMessage()
{
// free all arguments
GameMessageArgument *arg, *nextArg;

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

// detach message from list
Expand All @@ -84,14 +77,11 @@ GameMessage::~GameMessage()

/**
* Return the given argument union.
* @todo This should be a more list-like interface. Very inefficient.
*/
const GameMessageArgumentType *GameMessage::getArgument( Int argIndex ) const
{
int i=0;
for( GameMessageArgument *a = m_argList; a; a=a->m_next, i++ )
if (i == argIndex)
return &a->m_data;
if (static_cast<size_t>(argIndex) < m_argList.size())
return &m_argList[argIndex]->m_data;

DEBUG_CRASH(("argument not found"));
static const GameMessageArgumentType zero = { 0 };
Expand All @@ -103,17 +93,9 @@ const GameMessageArgumentType *GameMessage::getArgument( Int argIndex ) const
*/
GameMessageArgumentDataType GameMessage::getArgumentDataType( Int argIndex ) const
{
if (argIndex >= m_argCount) {
return ARGUMENTDATATYPE_UNKNOWN;
}
int i=0;
GameMessageArgument *a = m_argList;
for (; a && (i < argIndex); a=a->m_next, ++i );
if (static_cast<size_t>(argIndex) < m_argList.size())
return m_argList[argIndex]->m_type;

if (a != nullptr)
{
return a->m_type;
}
return ARGUMENTDATATYPE_UNKNOWN;
}

Expand All @@ -124,21 +106,7 @@ GameMessageArgument *GameMessage::allocArg()
{
// allocate a new argument
GameMessageArgument *arg = newInstance(GameMessageArgument);

// add to end of argument list
if (m_argTail)
m_argTail->m_next = arg;
else
{
m_argList = arg;
m_argTail = arg;
}

arg->m_next = nullptr;
m_argTail = arg;

m_argCount++;

m_argList.push_back(arg);
return arg;
}

Expand Down
13 changes: 4 additions & 9 deletions Generals/Code/GameEngine/Source/Common/Recorder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,15 +741,10 @@ void RecorderClass::writeToFile(GameMessage * msg) {
argType = argType->getNext();
}

// UnsignedByte lasttype = (UnsignedByte)ARGUMENTDATATYPE_UNKNOWN;
Int numArgs = msg->getArgumentCount();
for (Int i = 0; i < numArgs; ++i) {
// UnsignedByte type = (UnsignedByte)(msg->getArgumentDataType(i));
// if (lasttype != type) {
// fwrite(&type, sizeof(type), 1, m_file);
// lasttype = type;
// }
writeArgument(msg->getArgumentDataType(i), *(msg->getArgument(i)));
const std::vector<const GameMessageArgument*>& args = msg->getArguments();
for ( size_t i = 0; i < args.size(); ++i ) {
const GameMessageArgument *arg = args[i];
writeArgument(arg->m_type, arg->m_data);
}

deleteInstance(parser);
Expand Down
19 changes: 12 additions & 7 deletions GeneralsMD/Code/GameEngine/Include/Common/MessageStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ class GameMessageArgument : public MemoryPoolObject
{
MEMORY_POOL_GLUE_WITH_USERLOOKUP_CREATE(GameMessageArgument, "GameMessageArgument")
public:
GameMessageArgument* m_next; ///< The next argument
GameMessageArgumentType m_data; ///< The data storage of an argument
GameMessageArgumentDataType m_type; ///< The type of the argument.
};
Expand Down Expand Up @@ -639,7 +638,16 @@ class GameMessage : public MemoryPoolObject
GameMessage *prev() { return m_prev; } ///< Return prev message in the stream

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 11/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 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better move this function next to getArgument because they belong together.

DEBUG_ASSERTCRASH(
m_argList.size() <= 255,
("GameMessage has more than 255 arguments")
);
return static_cast<UnsignedByte>(m_argList.size());
}
const std::vector<const GameMessageArgument*>& getArguments() const { return m_argList ; }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


const char *getCommandAsString() const; ///< returns a string representation of the command type.
static const char *getCommandTypeAsString(GameMessage::Type t);
Expand All @@ -662,7 +670,6 @@ class GameMessage : public MemoryPoolObject

/**
* Return the given argument union.
* @todo This should be a more list-like interface. Very inefficient.
*/
const GameMessageArgumentType *getArgument( Int argIndex ) const;
GameMessageArgumentDataType getArgumentDataType( Int argIndex ) const;
Expand All @@ -683,10 +690,8 @@ class GameMessage : public MemoryPoolObject

Int m_playerIndex; ///< The Player who issued the command

/// @todo If a GameMessage needs more than 255 arguments, it needs to be split up into multiple GameMessage's.
UnsignedByte m_argCount; ///< The number of arguments of this message

GameMessageArgument *m_argList, *m_argTail; ///< This message's arguments
// TheSuperHackers @refactor RiQQ 11/05/2026 Replaced linked list with std::vector for argument storage
std::vector<const GameMessageArgument*> m_argList; ///< This message's arguments

/// allocate a new argument, add it to list, return pointer to its data
GameMessageArgument *allocArg();
Expand Down
46 changes: 7 additions & 39 deletions GeneralsMD/Code/GameEngine/Source/Common/MessageStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ GameMessage::GameMessage( GameMessage::Type type )
{
m_playerIndex = ThePlayerList->getLocalPlayer()->getPlayerIndex();
m_type = type;
m_argList = nullptr;
m_argTail = nullptr;
m_argCount = 0;
m_list = nullptr;
}

Expand All @@ -69,12 +66,8 @@ GameMessage::GameMessage( GameMessage::Type type )
GameMessage::~GameMessage()
{
// free all arguments
GameMessageArgument *arg, *nextArg;

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

// detach message from list
Expand All @@ -84,14 +77,11 @@ GameMessage::~GameMessage()

/**
* Return the given argument union.
* @todo This should be a more list-like interface. Very inefficient.
*/
const GameMessageArgumentType *GameMessage::getArgument( Int argIndex ) const
{
int i=0;
for( GameMessageArgument *a = m_argList; a; a=a->m_next, i++ )
if (i == argIndex)
return &a->m_data;
if (static_cast<size_t>(argIndex) < m_argList.size())
return &m_argList[argIndex]->m_data;

DEBUG_CRASH(("argument not found"));
static const GameMessageArgumentType zero = { 0 };
Expand All @@ -103,17 +93,9 @@ const GameMessageArgumentType *GameMessage::getArgument( Int argIndex ) const
*/
GameMessageArgumentDataType GameMessage::getArgumentDataType( Int argIndex ) const
{
if (argIndex >= m_argCount) {
return ARGUMENTDATATYPE_UNKNOWN;
}
int i=0;
GameMessageArgument *a = m_argList;
for (; a && (i < argIndex); a=a->m_next, ++i );
if (static_cast<size_t>(argIndex) < m_argList.size())
return m_argList[argIndex]->m_type;

if (a != nullptr)
{
return a->m_type;
}
return ARGUMENTDATATYPE_UNKNOWN;
}

Expand All @@ -124,21 +106,7 @@ GameMessageArgument *GameMessage::allocArg()
{
// allocate a new argument
GameMessageArgument *arg = newInstance(GameMessageArgument);

// add to end of argument list
if (m_argTail)
m_argTail->m_next = arg;
else
{
m_argList = arg;
m_argTail = arg;
}

arg->m_next = nullptr;
m_argTail = arg;

m_argCount++;

m_argList.push_back(arg);
return arg;
}

Expand Down
Loading
Loading