From 5efb9b80097682e4d1a64aa815929239d6499d9e Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Fri, 20 Jun 2025 12:29:16 +0200 Subject: [PATCH 01/11] Add estimates for required name truncation to prevent game info string overflow. --- .../Source/GameNetwork/GameInfo.cpp | 45 ++++++++++++++++--- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index 8d6b22755f..33c99fb2c4 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -923,7 +923,7 @@ AsciiString GameInfoToAsciiString( const GameInfo *game ) newMapName.concat(token); mapName.nextToken(&token, "\\/"); } - DEBUG_LOG(("Map name is %s\n", mapName.str())); + DEBUG_LOG(("Map name is %s\n", newMapName.str())); } AsciiString optionsString; @@ -934,6 +934,35 @@ AsciiString GameInfoToAsciiString( const GameInfo *game ) //add player info for each slot optionsString.concat(slotListID); optionsString.concat('='); + + // The longest options string a human player can have looks like ",7fffffff,8088,FT,-1,-1,-1,-1,64:" + const int MaxPerHumanOptionsLength = 33 + 1; // Include the prefix 'H' + // The longest options string an AI can have looks like "CH,-1,-1,-1,-1:" + const int MaxPerAIOptionsLength = 15; + + // Determine the average worst case name length we need to enforce to fit in the network packet + int availableSpaceInPacket = m_lanMaxOptionsLength - optionsString.getLength() - 1; // Include the trailing ';' + int numHumans = 0; + for (Int i = 0; i < MAX_SLOTS; ++i) + { + const GameSlot* slot = game->getConstSlot(i); + if (slot && slot->isHuman()) + { + numHumans++; + availableSpaceInPacket -= MaxPerHumanOptionsLength; + } + else if (slot && slot->isAI()) + { + availableSpaceInPacket -= MaxPerAIOptionsLength; + } + else + { + // "O:" or "X:" + availableSpaceInPacket -= 2; + } + } + + int maxAvgNameLength = availableSpaceInPacket / numHumans; for (Int i=0; igetConstSlot(i); @@ -949,14 +978,17 @@ AsciiString GameInfoToAsciiString( const GameInfo *game ) slot->getColor(), slot->getPlayerTemplate(), slot->getStartPos(), slot->getTeamNumber(), slot->getNATBehavior() ); + // Adjust from estimated worst-case length. MaxPerHumanOptionsLength includes prefix 'H' so we need to subtract it here, too. + availableSpaceInPacket += MaxPerHumanOptionsLength - tmp.getLength() - 1; + maxAvgNameLength = availableSpaceInPacket / numHumans--; //make sure name doesn't cause overflow of m_lanMaxOptionsLength - int lenCur = tmp.getLength() + optionsString.getLength() + 2; //+2 for H and trailing ; - int lenRem = m_lanMaxOptionsLength - lenCur; //length remaining before overflowing - int lenMax = lenRem / (MAX_SLOTS-i); //share lenRem with all remaining slots AsciiString name = WideCharStringToMultiByte(slot->getName().str()).c_str(); - while( name.getLength() > lenMax ) + while (name.getLength() > maxAvgNameLength) + { name.removeLastChar(); //what a horrible way to truncate. I hate AsciiString. - + } + + availableSpaceInPacket -= name.getLength(); str.format( "H%s%s", name.str(), tmp.str() ); } else if (slot && slot->isAI()) @@ -971,6 +1003,7 @@ AsciiString GameInfoToAsciiString( const GameInfo *game ) str.format("C%c,%d,%d,%d,%d:", c, slot->getColor(), slot->getPlayerTemplate(), slot->getStartPos(), slot->getTeamNumber()); + availableSpaceInPacket += MaxPerAIOptionsLength - str.getLength(); } else if (slot && slot->getState() == SLOT_OPEN) { From ca773e06f10b0896df1afbe8cc78c5fc1c7733c9 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Fri, 20 Jun 2025 13:14:21 +0200 Subject: [PATCH 02/11] Rename index var to si to avoid colliding with i. --- GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index 33c99fb2c4..75b5b37219 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -943,9 +943,9 @@ AsciiString GameInfoToAsciiString( const GameInfo *game ) // Determine the average worst case name length we need to enforce to fit in the network packet int availableSpaceInPacket = m_lanMaxOptionsLength - optionsString.getLength() - 1; // Include the trailing ';' int numHumans = 0; - for (Int i = 0; i < MAX_SLOTS; ++i) + for (Int si = 0; si < MAX_SLOTS; ++si) { - const GameSlot* slot = game->getConstSlot(i); + const GameSlot* slot = game->getConstSlot(si); if (slot && slot->isHuman()) { numHumans++; From b7eafeb9ffcf08d3d9e2b25eedcd932fc9314f43 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Fri, 20 Jun 2025 21:31:23 +0200 Subject: [PATCH 03/11] Move name truncation to separate method and make 2 serialization passes if required. --- .../Source/GameNetwork/GameInfo.cpp | 129 +++++++++++------- 1 file changed, 76 insertions(+), 53 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index 75b5b37219..a7b8878a6a 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -897,7 +897,46 @@ Bool GameInfo::isSandbox(void) static const char slotListID = 'S'; -AsciiString GameInfoToAsciiString( const GameInfo *game ) +static Bool BuildPlayerNamesWithTruncate(AsciiStringVec& playerNames, const GameInfo& game, UnsignedInt truncateAmount) +{ + for (Int i = 0; i < MAX_SLOTS; ++i) + { + const GameSlot* slot = game.getConstSlot(i); + if (slot && slot->isHuman()) + { + playerNames[i] = WideCharStringToMultiByte(slot->getName().str()).c_str(); + } + else + { + playerNames[i] = AsciiString::TheEmptyString; + } + } + + while (truncateAmount > 0) + { + Bool didTruncate = false; + for (Int i = 0; i < MAX_SLOTS && truncateAmount > 0; ++i) + { + // we won't truncate any names to shorter than 2 characters + if (playerNames[i].getLength() > 2) + { + playerNames[i].removeLastChar(); + truncateAmount--; + didTruncate = true; + } + } + + if (!didTruncate) + { + // iterated through all names without finding any to truncate. + return false; + } + } + + return true; +} + +AsciiString GameInfoToAsciiString(const GameInfo *game, const AsciiStringVec& playerNames) { if (!game) return AsciiString::TheEmptyString; @@ -934,35 +973,6 @@ AsciiString GameInfoToAsciiString( const GameInfo *game ) //add player info for each slot optionsString.concat(slotListID); optionsString.concat('='); - - // The longest options string a human player can have looks like ",7fffffff,8088,FT,-1,-1,-1,-1,64:" - const int MaxPerHumanOptionsLength = 33 + 1; // Include the prefix 'H' - // The longest options string an AI can have looks like "CH,-1,-1,-1,-1:" - const int MaxPerAIOptionsLength = 15; - - // Determine the average worst case name length we need to enforce to fit in the network packet - int availableSpaceInPacket = m_lanMaxOptionsLength - optionsString.getLength() - 1; // Include the trailing ';' - int numHumans = 0; - for (Int si = 0; si < MAX_SLOTS; ++si) - { - const GameSlot* slot = game->getConstSlot(si); - if (slot && slot->isHuman()) - { - numHumans++; - availableSpaceInPacket -= MaxPerHumanOptionsLength; - } - else if (slot && slot->isAI()) - { - availableSpaceInPacket -= MaxPerAIOptionsLength; - } - else - { - // "O:" or "X:" - availableSpaceInPacket -= 2; - } - } - - int maxAvgNameLength = availableSpaceInPacket / numHumans; for (Int i=0; igetConstSlot(i); @@ -970,26 +980,13 @@ AsciiString GameInfoToAsciiString( const GameInfo *game ) AsciiString str; if (slot && slot->isHuman()) { - AsciiString tmp; //all this data goes after name - tmp.format( ",%X,%d,%c%c,%d,%d,%d,%d,%d:", - slot->getIP(), slot->getPort(), - (slot->isAccepted()?'T':'F'), - (slot->hasMap()?'T':'F'), + str.format( "H%s,%X,%d,%c%c,%d,%d,%d,%d,%d:", + playerNames[i].str(), slot->getIP(), + slot->getPort(), (slot->isAccepted() ? 'T' : 'F'), + (slot->hasMap() ? 'T' : 'F'), slot->getColor(), slot->getPlayerTemplate(), slot->getStartPos(), slot->getTeamNumber(), - slot->getNATBehavior() ); - // Adjust from estimated worst-case length. MaxPerHumanOptionsLength includes prefix 'H' so we need to subtract it here, too. - availableSpaceInPacket += MaxPerHumanOptionsLength - tmp.getLength() - 1; - maxAvgNameLength = availableSpaceInPacket / numHumans--; - //make sure name doesn't cause overflow of m_lanMaxOptionsLength - AsciiString name = WideCharStringToMultiByte(slot->getName().str()).c_str(); - while (name.getLength() > maxAvgNameLength) - { - name.removeLastChar(); //what a horrible way to truncate. I hate AsciiString. - } - - availableSpaceInPacket -= name.getLength(); - str.format( "H%s%s", name.str(), tmp.str() ); + slot->getNATBehavior()); } else if (slot && slot->isAI()) { @@ -1003,7 +1000,6 @@ AsciiString GameInfoToAsciiString( const GameInfo *game ) str.format("C%c,%d,%d,%d,%d:", c, slot->getColor(), slot->getPlayerTemplate(), slot->getStartPos(), slot->getTeamNumber()); - availableSpaceInPacket += MaxPerAIOptionsLength - str.getLength(); } else if (slot && slot->getState() == SLOT_OPEN) { @@ -1022,13 +1018,40 @@ AsciiString GameInfoToAsciiString( const GameInfo *game ) } optionsString.concat(';'); - DEBUG_ASSERTCRASH(!TheLAN || (optionsString.getLength() < m_lanMaxOptionsLength), - ("WARNING: options string is longer than expected! Length is %d, but max is %d!\n", - optionsString.getLength(), m_lanMaxOptionsLength)); - return optionsString; } +AsciiString GameInfoToAsciiString(const GameInfo* game) +{ + AsciiStringVec playerNames(MAX_SLOTS); + if (!game || !BuildPlayerNamesWithTruncate(playerNames, *game, 0)) + { + return AsciiString::TheEmptyString; + } + + AsciiString infoString = GameInfoToAsciiString(game, playerNames); + + // TheSuperHackers @bugfix Safely truncate the game info string by + // scrapping characters off of player names if the overall length is too large. + if (infoString.getLength() > m_lanMaxOptionsLength) + { + const UnsignedInt truncateAmount = infoString.getLength() - m_lanMaxOptionsLength; + if (!BuildPlayerNamesWithTruncate(playerNames, *game, truncateAmount)) + { + DEBUG_LOG(("GameInfoToAsciiString - unable to truncate player names by %u characters.\n", truncateAmount)); + return AsciiString::TheEmptyString; + } + + infoString = GameInfoToAsciiString(game, playerNames); + } + + DEBUG_ASSERTCRASH(!TheLAN || (infoString.getLength() < m_lanMaxOptionsLength), + ("WARNING: options string is longer than expected! Length is %d, but max is %d!\n", + infoString.getLength(), m_lanMaxOptionsLength)); + + return infoString; +} + static Int grabHexInt(const char *s) { char tmp[5] = "0xff"; From d5a41504d146e255465474155048c1e03f0ec439 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Fri, 20 Jun 2025 21:51:46 +0200 Subject: [PATCH 04/11] Split Player name list creation and truncation into separate functions. --- .../GameEngine/Source/GameNetwork/GameInfo.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index a7b8878a6a..071853c72c 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -897,7 +897,7 @@ Bool GameInfo::isSandbox(void) static const char slotListID = 'S'; -static Bool BuildPlayerNamesWithTruncate(AsciiStringVec& playerNames, const GameInfo& game, UnsignedInt truncateAmount) +static void BuildPlayerNames(AsciiStringVec& playerNames, const GameInfo& game) { for (Int i = 0; i < MAX_SLOTS; ++i) { @@ -911,11 +911,14 @@ static Bool BuildPlayerNamesWithTruncate(AsciiStringVec& playerNames, const Game playerNames[i] = AsciiString::TheEmptyString; } } +} +static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncateAmount) +{ while (truncateAmount > 0) { Bool didTruncate = false; - for (Int i = 0; i < MAX_SLOTS && truncateAmount > 0; ++i) + for (Int i = 0; i < playerNames.size() && truncateAmount > 0; ++i) { // we won't truncate any names to shorter than 2 characters if (playerNames[i].getLength() > 2) @@ -1023,12 +1026,13 @@ AsciiString GameInfoToAsciiString(const GameInfo *game, const AsciiStringVec& pl AsciiString GameInfoToAsciiString(const GameInfo* game) { - AsciiStringVec playerNames(MAX_SLOTS); - if (!game || !BuildPlayerNamesWithTruncate(playerNames, *game, 0)) + if (!game) { return AsciiString::TheEmptyString; } + AsciiStringVec playerNames(MAX_SLOTS); + BuildPlayerNames(playerNames, *game); AsciiString infoString = GameInfoToAsciiString(game, playerNames); // TheSuperHackers @bugfix Safely truncate the game info string by @@ -1036,7 +1040,7 @@ AsciiString GameInfoToAsciiString(const GameInfo* game) if (infoString.getLength() > m_lanMaxOptionsLength) { const UnsignedInt truncateAmount = infoString.getLength() - m_lanMaxOptionsLength; - if (!BuildPlayerNamesWithTruncate(playerNames, *game, truncateAmount)) + if (!TruncatePlayerNames(playerNames, truncateAmount)) { DEBUG_LOG(("GameInfoToAsciiString - unable to truncate player names by %u characters.\n", truncateAmount)); return AsciiString::TheEmptyString; From 1ae987679266fdc34c8f253790fe9b96679027e0 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Tue, 1 Jul 2025 21:56:24 +0200 Subject: [PATCH 05/11] Add removeLastNChars method to AsciiString. --- .../Code/GameEngine/Include/Common/AsciiString.h | 6 ++++++ .../GameEngine/Source/Common/System/AsciiString.cpp | 11 +++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/Common/AsciiString.h b/GeneralsMD/Code/GameEngine/Include/Common/AsciiString.h index 9b304aa980..ab36892b8f 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/AsciiString.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/AsciiString.h @@ -240,6 +240,12 @@ class AsciiString */ void removeLastChar(); + /** + Remove the final N characters in the string. If the string is empty, + do nothing. + */ + void removeLastNChars(UnsignedInt chars); + /** Analogous to sprintf() -- this formats a string according to the given sprintf-style format string (and the variable argument list) diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/AsciiString.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/AsciiString.cpp index 7250d53b5a..873f5294d5 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/AsciiString.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/AsciiString.cpp @@ -331,15 +331,22 @@ void AsciiString::toLower() // ----------------------------------------------------- void AsciiString::removeLastChar() +{ + removeLastNChars(1U); +} + +// ----------------------------------------------------- +void AsciiString::removeLastNChars(UnsignedInt chars) { validate(); - if (m_data) + if (m_data && chars > 0) { int len = strlen(peek()); if (len > 0) { ensureUniqueBufferOfSize(len+1, true, NULL, NULL); - peek()[len - 1] = 0; + chars = chars > len ? len : chars; + peek()[len - chars] = 0; } } validate(); From cdbf4b73a865a0020ba2db8ee4a12b72dbfa5f5b Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Tue, 1 Jul 2025 22:05:04 +0200 Subject: [PATCH 06/11] Ensure game options are null terminated. --- GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp index 05fd55fde5..f3cf367e41 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp @@ -195,6 +195,7 @@ void LANAPI::handleRequestGameInfo( LANMessage *msg, UnsignedInt senderIP ) AsciiString gameOpts = GameInfoToAsciiString(m_currentGame); strncpy(reply.GameInfo.options,gameOpts.str(),m_lanMaxOptionsLength); + reply.GameInfo.options[m_lanMaxOptionsLength] = 0; wcsncpy(reply.GameInfo.gameName, m_currentGame->getName().str(), g_lanGameNameLength); reply.GameInfo.gameName[g_lanGameNameLength] = 0; reply.GameInfo.inProgress = m_currentGame->isGameInProgress(); From aab7f02493d1122ae90a5631defdb4954bd68c2e Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Tue, 1 Jul 2025 22:06:25 +0200 Subject: [PATCH 07/11] Ensure we accept the maximum length game options. --- GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPI.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPI.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPI.cpp index 79ba8dc625..2c7d8b4845 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPI.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/LANAPI.cpp @@ -837,7 +837,7 @@ void LANAPI::RequestGameStartTimer( Int seconds ) void LANAPI::RequestGameOptions( AsciiString gameOptions, Bool isPublic, UnsignedInt ip /* = 0 */ ) { - DEBUG_ASSERTCRASH(gameOptions.getLength() < m_lanMaxOptionsLength, ("Game options string is too long!")); + DEBUG_ASSERTCRASH(gameOptions.getLength() <= m_lanMaxOptionsLength, ("Game options string is too long!")); if (!m_currentGame) return; From f8600c6a763e31c2d45b534bb2abeeb62b23151c Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Tue, 1 Jul 2025 22:26:04 +0200 Subject: [PATCH 08/11] Fairer truncation algo, rewarding short names. --- .../Source/GameNetwork/GameInfo.cpp | 85 ++++++++++++++----- 1 file changed, 62 insertions(+), 23 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index 071853c72c..a29b260495 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -44,6 +44,8 @@ #include "GameNetwork/LANAPI.h" // for testing packet size #include "GameNetwork/LANAPICallbacks.h" // for testing packet size #include "strtok_r.h" +#include +#include #ifdef RTS_INTERNAL // for occasional debugging... @@ -897,42 +899,82 @@ Bool GameInfo::isSandbox(void) static const char slotListID = 'S'; -static void BuildPlayerNames(AsciiStringVec& playerNames, const GameInfo& game) +static AsciiStringVec BuildPlayerNames(const GameInfo& game) { + AsciiStringVec playerNames; + playerNames.reserve(MAX_SLOTS); + for (Int i = 0; i < MAX_SLOTS; ++i) { const GameSlot* slot = game.getConstSlot(i); - if (slot && slot->isHuman()) + if (slot->isHuman()) { - playerNames[i] = WideCharStringToMultiByte(slot->getName().str()).c_str(); + playerNames.push_back(WideCharStringToMultiByte(slot->getName().str()).c_str()); } else { - playerNames[i] = AsciiString::TheEmptyString; + playerNames.push_back(AsciiString::TheEmptyString); } } + + return playerNames; } static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncateAmount) { + // wont truncate any name to below this length + const Int MinimumNameLength = 2; + UnsignedInt availableForTruncation = 0; + + // make length+index pairs for the player names + std::vector> lengthIndex; + lengthIndex.reserve(playerNames.size()); + for (size_t pi = 0; pi < playerNames.size(); ++pi) + { + Int playerNameLength = playerNames[pi].getLength(); + lengthIndex.push_back(std::make_pair(playerNameLength, pi)); + availableForTruncation += std::max(0, playerNameLength - MinimumNameLength); + } + + if (truncateAmount > availableForTruncation) + { + DEBUG_LOG(("TruncatePlayerNames - Requested to truncate %u chars from player names, but only %u were available for truncation.\n", truncateAmount, availableForTruncation)); + return false; + } + + // sort based on length in descending order + std::sort(lengthIndex.begin(), lengthIndex.end(), std::greater>()); + + // determine how long each of the player names should be + Int currentTargetLength = lengthIndex[0].first - 1; while (truncateAmount > 0) { - Bool didTruncate = false; - for (Int i = 0; i < playerNames.size() && truncateAmount > 0; ++i) + currentTargetLength = std::min(lengthIndex[0].first - 1, currentTargetLength); + for (size_t i = 0; i < lengthIndex.size(); ++i) { - // we won't truncate any names to shorter than 2 characters - if (playerNames[i].getLength() > 2) + if (lengthIndex[i].first > currentTargetLength) { - playerNames[i].removeLastChar(); - truncateAmount--; - didTruncate = true; + Int truncateCurrent = std::min(truncateAmount, lengthIndex[i].first - currentTargetLength); + lengthIndex[i].first -= truncateCurrent; + truncateAmount -= truncateCurrent; + } + + if (truncateAmount == 0) + { + break; } } + } - if (!didTruncate) + // truncate each name to its new length + for (size_t ti = 0; ti < lengthIndex.size(); ++ti) + { + int charsToRemove = playerNames[lengthIndex[ti].second].getLength() - lengthIndex[ti].first; + if (charsToRemove > 0) { - // iterated through all names without finding any to truncate. - return false; + DEBUG_LOG(("TruncatePlayerNames - truncating '%s' by %d chars to ", playerNames[lengthIndex[ti].second].str(), charsToRemove)); + playerNames[lengthIndex[ti].second].removeLastNChars(charsToRemove); + DEBUG_LOG(("'%s' (target length=%d).\n", playerNames[lengthIndex[ti].second].str(), lengthIndex[ti].first)); } } @@ -1031,28 +1073,25 @@ AsciiString GameInfoToAsciiString(const GameInfo* game) return AsciiString::TheEmptyString; } - AsciiStringVec playerNames(MAX_SLOTS); - BuildPlayerNames(playerNames, *game); + AsciiStringVec playerNames = BuildPlayerNames(*game); AsciiString infoString = GameInfoToAsciiString(game, playerNames); // TheSuperHackers @bugfix Safely truncate the game info string by - // scrapping characters off of player names if the overall length is too large. - if (infoString.getLength() > m_lanMaxOptionsLength) + // stripping characters off of player names if the overall length is too large. + if (TheLAN && (infoString.getLength() > m_lanMaxOptionsLength)) { const UnsignedInt truncateAmount = infoString.getLength() - m_lanMaxOptionsLength; if (!TruncatePlayerNames(playerNames, truncateAmount)) { - DEBUG_LOG(("GameInfoToAsciiString - unable to truncate player names by %u characters.\n", truncateAmount)); + DEBUG_ASSERTCRASH(infoString.getLength() <= m_lanMaxOptionsLength, + ("WARNING: options string is longer than expected! Length is %d, but max is %d. Attempted to truncate player names by %u characters, but was unsuccessful!\n", + infoString.getLength(), m_lanMaxOptionsLength, truncateAmount)); return AsciiString::TheEmptyString; } infoString = GameInfoToAsciiString(game, playerNames); } - DEBUG_ASSERTCRASH(!TheLAN || (infoString.getLength() < m_lanMaxOptionsLength), - ("WARNING: options string is longer than expected! Length is %d, but max is %d!\n", - infoString.getLength(), m_lanMaxOptionsLength)); - return infoString; } From ca15e22299e7f097b2e11fd984177b0784964b97 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Tue, 1 Jul 2025 22:43:18 +0200 Subject: [PATCH 09/11] VC6 compat. --- GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index a29b260495..4183d96a42 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -927,7 +927,7 @@ static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncat UnsignedInt availableForTruncation = 0; // make length+index pairs for the player names - std::vector> lengthIndex; + std::vector > lengthIndex; lengthIndex.reserve(playerNames.size()); for (size_t pi = 0; pi < playerNames.size(); ++pi) { @@ -943,7 +943,7 @@ static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncat } // sort based on length in descending order - std::sort(lengthIndex.begin(), lengthIndex.end(), std::greater>()); + std::sort(lengthIndex.begin(), lengthIndex.end(), std::greater >()); // determine how long each of the player names should be Int currentTargetLength = lengthIndex[0].first - 1; From 60d1b56c47d69de7bfbefbac9ef3b475a386bd55 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Wed, 2 Jul 2025 21:32:15 +0200 Subject: [PATCH 10/11] Rename removeLastNChars to truncate. --- .../Code/GameEngine/Include/Common/AsciiString.h | 4 ++-- .../GameEngine/Source/Common/System/AsciiString.cpp | 12 ++++++------ .../Code/GameEngine/Source/GameNetwork/GameInfo.cpp | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/Common/AsciiString.h b/GeneralsMD/Code/GameEngine/Include/Common/AsciiString.h index ab36892b8f..8c4b26271c 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/AsciiString.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/AsciiString.h @@ -241,10 +241,10 @@ class AsciiString void removeLastChar(); /** - Remove the final N characters in the string. If the string is empty, + Remove the final charCount characters in the string. If the string is empty, do nothing. */ - void removeLastNChars(UnsignedInt chars); + void truncate(UnsignedInt charCount); /** Analogous to sprintf() -- this formats a string according to the diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/AsciiString.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/AsciiString.cpp index 873f5294d5..25fb37b394 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/AsciiString.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/AsciiString.cpp @@ -332,21 +332,21 @@ void AsciiString::toLower() // ----------------------------------------------------- void AsciiString::removeLastChar() { - removeLastNChars(1U); + truncate(1U); } // ----------------------------------------------------- -void AsciiString::removeLastNChars(UnsignedInt chars) +void AsciiString::truncate(UnsignedInt charCount) { validate(); - if (m_data && chars > 0) + if (m_data && charCount > 0) { - int len = strlen(peek()); + size_t len = strlen(peek()); if (len > 0) { ensureUniqueBufferOfSize(len+1, true, NULL, NULL); - chars = chars > len ? len : chars; - peek()[len - chars] = 0; + charCount = min(charCount, len); + peek()[len - charCount] = 0; } } validate(); diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index 4183d96a42..53b60be20d 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -973,7 +973,7 @@ static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncat if (charsToRemove > 0) { DEBUG_LOG(("TruncatePlayerNames - truncating '%s' by %d chars to ", playerNames[lengthIndex[ti].second].str(), charsToRemove)); - playerNames[lengthIndex[ti].second].removeLastNChars(charsToRemove); + playerNames[lengthIndex[ti].second].truncate(charsToRemove); DEBUG_LOG(("'%s' (target length=%d).\n", playerNames[lengthIndex[ti].second].str(), lengthIndex[ti].first)); } } From 87b552e86da98b7bfe9f769b6f2c15b4d54b81bd Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Wed, 2 Jul 2025 22:56:33 +0200 Subject: [PATCH 11/11] Improve truncation target length selection and move away from std::pair. --- .../GameEngine/Include/GameNetwork/GameInfo.h | 14 +++++ .../Source/GameNetwork/GameInfo.cpp | 51 +++++++++++-------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GameInfo.h b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GameInfo.h index abb37a4da5..20d402e339 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameNetwork/GameInfo.h +++ b/GeneralsMD/Code/GameEngine/Include/GameNetwork/GameInfo.h @@ -279,6 +279,20 @@ void GameInfo::setOldFactionsOnly( Bool oldFactionsOnly ) { m_oldFactions AsciiString GameInfoToAsciiString( const GameInfo *game ); Bool ParseAsciiStringToGameInfo( GameInfo *game, AsciiString options ); +struct LengthIndexPair +{ + Int Length; + size_t Index; + friend bool operator<(const LengthIndexPair& lhs, const LengthIndexPair& rhs) + { + if (lhs.Length == rhs.Length) + return lhs.Index < rhs.Index; + return lhs.Length < rhs.Length; + } + friend bool operator>(const LengthIndexPair& lhs, const LengthIndexPair& rhs) { return rhs < lhs; } + friend bool operator<=(const LengthIndexPair& lhs, const LengthIndexPair& rhs) { return !(lhs > rhs); } + friend bool operator>=(const LengthIndexPair& lhs, const LengthIndexPair& rhs) { return !(lhs < rhs); } +}; /** * The SkirmishGameInfo class holds information about the skirmish game and diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index ad94d36141..c15af710ab 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -904,18 +904,14 @@ static const char slotListID = 'S'; static AsciiStringVec BuildPlayerNames(const GameInfo& game) { AsciiStringVec playerNames; - playerNames.reserve(MAX_SLOTS); + playerNames.resize(MAX_SLOTS); for (Int i = 0; i < MAX_SLOTS; ++i) { const GameSlot* slot = game.getConstSlot(i); if (slot->isHuman()) { - playerNames.push_back(WideCharStringToMultiByte(slot->getName().str()).c_str()); - } - else - { - playerNames.push_back(AsciiString::TheEmptyString); + playerNames[i] = WideCharStringToMultiByte(slot->getName().str()).c_str(); } } @@ -925,16 +921,17 @@ static AsciiStringVec BuildPlayerNames(const GameInfo& game) static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncateAmount) { // wont truncate any name to below this length - const Int MinimumNameLength = 2; + CONSTEXPR const Int MinimumNameLength = 2; UnsignedInt availableForTruncation = 0; // make length+index pairs for the player names - std::vector > lengthIndex; - lengthIndex.reserve(playerNames.size()); + std::vector lengthIndex; + lengthIndex.resize(playerNames.size()); for (size_t pi = 0; pi < playerNames.size(); ++pi) { Int playerNameLength = playerNames[pi].getLength(); - lengthIndex.push_back(std::make_pair(playerNameLength, pi)); + lengthIndex[pi].Length = playerNameLength; + lengthIndex[pi].Index = pi; availableForTruncation += std::max(0, playerNameLength - MinimumNameLength); } @@ -945,19 +942,18 @@ static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncat } // sort based on length in descending order - std::sort(lengthIndex.begin(), lengthIndex.end(), std::greater >()); + std::sort(lengthIndex.begin(), lengthIndex.end(), std::greater()); // determine how long each of the player names should be - Int currentTargetLength = lengthIndex[0].first - 1; + Int currentTargetLength = lengthIndex[0].Length - 1; while (truncateAmount > 0) { - currentTargetLength = std::min(lengthIndex[0].first - 1, currentTargetLength); for (size_t i = 0; i < lengthIndex.size(); ++i) { - if (lengthIndex[i].first > currentTargetLength) + if (lengthIndex[i].Length > currentTargetLength) { - Int truncateCurrent = std::min(truncateAmount, lengthIndex[i].first - currentTargetLength); - lengthIndex[i].first -= truncateCurrent; + Int truncateCurrent = std::min(truncateAmount, lengthIndex[i].Length - currentTargetLength); + lengthIndex[i].Length -= truncateCurrent; truncateAmount -= truncateCurrent; } @@ -965,18 +961,30 @@ static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncat { break; } + + if (lengthIndex[i].Length < currentTargetLength) + { + // set target length to either the length of position i, or the remaining amount to truncate divided across all the previous entries, rounding upwards. + currentTargetLength = std::max(static_cast(lengthIndex[i].Length), lengthIndex[0].Length - ((truncateAmount + i) / (i + 1))); + // start over again with new target length + i = -1; + continue; + } } + + // All entries are of equal length, or we're finished. Figure out the length of all entries if truncated by the same amount, rounding upwards. + currentTargetLength = lengthIndex[0].Length - ((truncateAmount + lengthIndex.size() - 1) / lengthIndex.size()); } // truncate each name to its new length for (size_t ti = 0; ti < lengthIndex.size(); ++ti) { - int charsToRemove = playerNames[lengthIndex[ti].second].getLength() - lengthIndex[ti].first; + int charsToRemove = playerNames[lengthIndex[ti].Index].getLength() - lengthIndex[ti].Length; if (charsToRemove > 0) { - DEBUG_LOG(("TruncatePlayerNames - truncating '%s' by %d chars to ", playerNames[lengthIndex[ti].second].str(), charsToRemove)); - playerNames[lengthIndex[ti].second].truncate(charsToRemove); - DEBUG_LOG(("'%s' (target length=%d).\n", playerNames[lengthIndex[ti].second].str(), lengthIndex[ti].first)); + DEBUG_LOG(("TruncatePlayerNames - truncating '%s' by %d chars to ", playerNames[lengthIndex[ti].Index].str(), charsToRemove)); + playerNames[lengthIndex[ti].Index].truncate(charsToRemove); + DEBUG_LOG(("'%s' (target length=%d).\n", playerNames[lengthIndex[ti].Index].str(), lengthIndex[ti].Length)); } } @@ -1085,8 +1093,7 @@ AsciiString GameInfoToAsciiString(const GameInfo* game) const UnsignedInt truncateAmount = infoString.getLength() - m_lanMaxOptionsLength; if (!TruncatePlayerNames(playerNames, truncateAmount)) { - DEBUG_ASSERTCRASH(infoString.getLength() <= m_lanMaxOptionsLength, - ("WARNING: options string is longer than expected! Length is %d, but max is %d. Attempted to truncate player names by %u characters, but was unsuccessful!\n", + DEBUG_CRASH(("WARNING: options string is longer than expected! Length is %d, but max is %d. Attempted to truncate player names by %u characters, but was unsuccessful!\n", infoString.getLength(), m_lanMaxOptionsLength, truncateAmount)); return AsciiString::TheEmptyString; }