Skip to content
Draft
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
13 changes: 13 additions & 0 deletions Core/GameEngine/Include/Common/AsciiString.h
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,19 @@ class AsciiString
AsciiString& operator=(const AsciiString& stringSrc); ///< the same as set()
AsciiString& operator=(const char* s); ///< the same as set()

const Char& operator[](Int index) const
{
DEBUG_ASSERTCRASH(index >= 0 && index < getLength(), ("bad index in AsciiString::operator[]"));
return peek()[index];
}

Char& operator[](Int index)
{
DEBUG_ASSERTCRASH(index >= 0 && index < getLength(), ("bad index in AsciiString::operator[]"));
ensureUniqueBufferOfSize(m_data->m_numCharsAllocated, true, NULL, NULL);
return peek()[index];
}
Comment on lines +380 to +391
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Missing TheSuperHackers comment for the new operator[] implementations. All new features require documentation:

// TheSuperHackers @feature author DD/MM/YYYY Added array subscript operator for convenient character access

Copilot generated this review using guidance from repository custom instructions.

void debugIgnoreLeaks();

};
Expand Down
13 changes: 13 additions & 0 deletions Core/GameEngine/Include/Common/UnicodeString.h
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,19 @@ class UnicodeString

UnicodeString& operator=(const UnicodeString& stringSrc); ///< the same as set()
UnicodeString& operator=(const WideChar* s); ///< the same as set()

const WideChar& operator[](Int index) const
{
DEBUG_ASSERTCRASH(index >= 0 && index < getLength(), ("bad index in UnicodeString::operator[]"));
return peek()[index];
}

WideChar& operator[](Int index)
{
DEBUG_ASSERTCRASH(index >= 0 && index < getLength(), ("bad index in UnicodeString::operator[]"));
ensureUniqueBufferOfSize(m_data->m_numCharsAllocated, true, NULL, NULL);
return peek()[index];
}
Comment on lines +332 to +343
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Missing TheSuperHackers comment for the new operator[] implementations. All new features require documentation:

// TheSuperHackers @feature author DD/MM/YYYY Added array subscript operator for convenient character access

Copilot generated this review using guidance from repository custom instructions.
};


Expand Down
1 change: 1 addition & 0 deletions Core/GameEngine/Include/GameNetwork/LANAPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ struct LANMessage
{
char options[m_lanMaxOptionsLength+1];
} GameOptions;
static_assert(ARRAY_SIZE(GameOptions.options) > m_lanMaxOptionsLength, "GameOptions.options buffer must be larger than m_lanMaxOptionsLength");
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

This static_assert will fail to compile. The condition checks if ARRAY_SIZE(GameOptions.options) > m_lanMaxOptionsLength, but GameOptions.options is defined as char options[m_lanMaxOptionsLength+1], which means ARRAY_SIZE(GameOptions.options) equals m_lanMaxOptionsLength+1. The correct assertion should be:

static_assert(ARRAY_SIZE(GameOptions.options) > m_lanMaxOptionsLength, "GameOptions.options buffer must be larger than m_lanMaxOptionsLength");

However, this is already correct (the array size is m_lanMaxOptionsLength+1, which is greater than m_lanMaxOptionsLength). The real issue is that this static_assert is placed inside a union member definition, which is not valid C++ syntax. Static assertions cannot be placed as members of unions. This declaration needs to be moved outside the union, after the closing brace of the LANMessage struct.

Copilot uses AI. Check for mistakes.

};
};
Expand Down
194 changes: 171 additions & 23 deletions Core/GameEngine/Source/GameNetwork/GameInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
#include "GameNetwork/LANAPI.h" // for testing packet size
#include "GameNetwork/LANAPICallbacks.h" // for testing packet size
#include "strtok_r.h"
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Missing include for WWMath::Div_Ceil which is used on line 968. Add #include "WWMath/wwmath.h" or the appropriate path to the wwmath header.

Suggested change
#include "strtok_r.h"
#include "strtok_r.h"
#include "WWMath/wwmath.h"

Copilot uses AI. Check for mistakes.
#include <algorithm>
#include <numeric>
#include <utility>
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Missing include for std::set which is used in the EnsureUniqueNames function (line 1000). Add #include <set> with the other standard library includes.

Suggested change
#include <utility>
#include <utility>
#include <set>

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Missing include for std::vector which is used throughout the new code (e.g., std::vector<LengthIndexPair> on line 941). Add #include <vector> with the other standard library includes.

Suggested change
#include <utility>
#include <utility>
#include <vector>

Copilot uses AI. Check for mistakes.



Expand Down Expand Up @@ -892,7 +895,132 @@ Bool GameInfo::isSandbox(void)

static const char slotListID = 'S';

AsciiString GameInfoToAsciiString( const GameInfo *game )
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); }
};

static AsciiStringVec BuildPlayerNames(const GameInfo& game)
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

The type AsciiStringVec is used but not defined anywhere in the visible codebase. This will cause a compilation error. You need to add a typedef definition, likely at the top of GameInfo.cpp after the includes:

typedef std::vector<AsciiString> AsciiStringVec;

Copilot uses AI. Check for mistakes.
{
AsciiStringVec playerNames;
playerNames.resize(MAX_SLOTS);

for (Int i = 0; i < MAX_SLOTS; ++i)
{
const GameSlot* slot = game.getConstSlot(i);
if (slot->isHuman())
{
playerNames[i] = WideCharStringToMultiByte(slot->getName().str()).c_str();
}
}

return playerNames;
}

static Int SumLength(Int sum, const LengthIndexPair& l)
{
return sum + l.Length;
}

static Bool TruncatePlayerNames(AsciiStringVec& playerNames, Int truncateAmount)
{
// wont truncate any name to below this length
constexpr const Int MinimumNameLength = 1;

// make length+index pairs for the player names
std::vector<LengthIndexPair> lengthIndex;
lengthIndex.resize(playerNames.size());
for (size_t pi = 0; pi < playerNames.size(); ++pi)
{
Int playerNameLength = playerNames[pi].getLength();
lengthIndex[pi].Length = std::max(0, playerNameLength - MinimumNameLength);
lengthIndex[pi].Index = pi;
}

Int remainingNamesLength = std::accumulate(lengthIndex.begin(), lengthIndex.end(), 0, SumLength);

if (truncateAmount > remainingNamesLength)
{
DEBUG_LOG(("TruncatePlayerNames - Requested to truncate %u chars from player names, but only %u were available for truncation.",
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Format specifier mismatch: truncateAmount and remainingNamesLength are of type Int (likely int), but the format specifier %u is for unsigned int. Use %d instead for signed integers to avoid undefined behavior.

Suggested change
DEBUG_LOG(("TruncatePlayerNames - Requested to truncate %u chars from player names, but only %u were available for truncation.",
DEBUG_LOG(("TruncatePlayerNames - Requested to truncate %d chars from player names, but only %d were available for truncation.",

Copilot uses AI. Check for mistakes.
truncateAmount, remainingNamesLength));
return false;
}

// sort based on length in ascending order
std::sort(lengthIndex.begin(), lengthIndex.end());

for (size_t i = 0; i < lengthIndex.size(); ++i)
{
const Int playerIndex = lengthIndex[i].Index;
const Int playerLength = lengthIndex[i].Length;

// round avg name length up, which will penalize the final entry (longest name) as it will have to account for the roundings
const Int avgNameLength = WWMath::Div_Ceil((remainingNamesLength - truncateAmount), (playerNames.size() - i));
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Potential type mismatch: playerNames.size() - i involves a size_t subtraction, but WWMath::Div_Ceil expects const int parameters. When i is close to playerNames.size(), this could cause issues. Consider casting explicitly:

const Int avgNameLength = WWMath::Div_Ceil(
    (remainingNamesLength - truncateAmount), 
    static_cast<Int>(playerNames.size() - i));

Also ensure that playerNames.size() - i never becomes 0 to avoid division by zero.

Suggested change
const Int avgNameLength = WWMath::Div_Ceil((remainingNamesLength - truncateAmount), (playerNames.size() - i));
const Int namesLeft = static_cast<Int>(playerNames.size() - i);
if (namesLeft <= 0) {
// TheSuperHackers @bugfix agent 07/06/2024 Prevent division by zero in WWMath::Div_Ceil
continue;
}
const Int avgNameLength = WWMath::Div_Ceil((remainingNamesLength - truncateAmount), namesLeft);

Copilot uses AI. Check for mistakes.
remainingNamesLength -= playerLength;
if (playerLength <= avgNameLength)
{
continue;
}

Int truncateNameAmount = playerLength - avgNameLength;
if (i == lengthIndex.size() - 1)
{
// ensure we account for rounding errors when truncating the last, longest entry
truncateNameAmount = std::max(truncateAmount, truncateNameAmount);
}

// as the name is UTF-8, make sure we don't truncate part of a multibyte character
while (playerLength - truncateNameAmount >= MinimumNameLength
&& (playerNames[playerIndex].getCharAt(playerLength - truncateNameAmount + 1) & 0xC0) == 0x80)
{
// move back to the start of the multibyte character
++truncateNameAmount;
}
Comment on lines +982 to +988
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

The UTF-8 truncation logic has an off-by-one error. The condition checks playerNames[playerIndex].getCharAt(playerLength - truncateNameAmount + 1), but playerLength is the available length for truncation (i.e., actualLength - MinimumNameLength), not the actual string length. The code should use playerNames[playerIndex].getLength() instead. Additionally, after truncating, we need to ensure we're not checking beyond the string bounds. The correct approach would be:

const Int actualLength = playerNames[playerIndex].getLength();
while (actualLength - truncateNameAmount >= MinimumNameLength
    && actualLength - truncateNameAmount > 0
    && (playerNames[playerIndex].getCharAt(actualLength - truncateNameAmount - 1) & 0xC0) == 0x80)
{
    ++truncateNameAmount;
}

Note: We check the character at the cut point (index actualLength - truncateNameAmount - 1), not after it.

Copilot uses AI. Check for mistakes.

playerNames[playerIndex].truncateBy(truncateNameAmount);
truncateAmount -= truncateNameAmount;
}

return true;
}

static void EnsureUniqueNames(AsciiStringVec& playerNames)
{
// ensure there are no duplicates in the list of player names
std::set<AsciiString> uniqueNames;
for (size_t i = 0; i < playerNames.size(); ++i)
{
static_assert(MAX_SLOTS < 10, "Name collision avoidance assumes less than 10 players in the game.");
AsciiString& playerName = playerNames[i];

if (playerName.isEmpty())
continue;

Int charOffset = -1;
Int nameLength = playerName.getLength();
while (uniqueNames.insert(playerName).second == false)
{
// The name already exists, so change the last char to the number index of the player in the game.
// If that fails to be unique, iterate through 0-9 and change the last char to ensure differentiation.
// Guaranteed to find a unique name as the number of slots is less than 10.
char charToTry = '0' + static_cast<char>(charOffset == -1 ? i : charOffset);
playerName[nameLength - 1] = charToTry;
++charOffset;
}
}
}

AsciiString GameInfoToAsciiString(const GameInfo *game, const AsciiStringVec& playerNames)
{
if (!game)
return AsciiString::TheEmptyString;
Expand All @@ -918,7 +1046,7 @@ AsciiString GameInfoToAsciiString( const GameInfo *game )
newMapName.concat(token);
mapName.nextToken(&token, "\\/");
}
DEBUG_LOG(("Map name is %s", mapName.str()));
DEBUG_LOG(("Map name is %s", newMapName.str()));
}

AsciiString optionsString;
Expand All @@ -941,23 +1069,17 @@ 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'),
slot->getColor(), slot->getPlayerTemplate(),
slot->getStartPos(), slot->getTeamNumber(),
slot->getNATBehavior() );
//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 )
name.removeLastChar(); //what a horrible way to truncate. I hate AsciiString.

str.format( "H%s%s", name.str(), tmp.str() );
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());
}
else if (slot && slot->isAI())
{
Expand Down Expand Up @@ -989,13 +1111,39 @@ 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!",
optionsString.getLength(), m_lanMaxOptionsLength));

return optionsString;
}

AsciiString GameInfoToAsciiString(const GameInfo* game)
{
if (!game)
{
return AsciiString::TheEmptyString;
}

AsciiStringVec playerNames = BuildPlayerNames(*game);
AsciiString infoString = GameInfoToAsciiString(game, playerNames);

// TheSuperHackers @bugfix Safely truncate the game info string by
// 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_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!",
infoString.getLength(), m_lanMaxOptionsLength, truncateAmount));
Comment on lines +1134 to +1136
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Format specifier mismatch: truncateAmount is of type UnsignedInt (matches %u), but infoString.getLength() and m_lanMaxOptionsLength are likely of type Int or int. Use %d for these signed integer values. The correct format string should be:

"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!"

Copilot uses AI. Check for mistakes.
return AsciiString::TheEmptyString;
}

EnsureUniqueNames(playerNames);
infoString = GameInfoToAsciiString(game, playerNames);
}

return infoString;
}

static Int grabHexInt(const char *s)
{
char tmp[5] = "0xff";
Expand Down
2 changes: 1 addition & 1 deletion Core/GameEngine/Source/GameNetwork/LANAPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,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!"));
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

This fix is missing a TheSuperHackers comment as required by the project guidelines. Every user-facing change requires the format:

// TheSuperHackers @bugfix author DD/MM/YYYY Fixed off-by-one error in game options length validation

Copilot generated this review using guidance from repository custom instructions.

if (!m_currentGame)
return;
Expand Down
12 changes: 12 additions & 0 deletions Core/Libraries/Source/WWVegas/WWMath/wwmath.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ static WWINLINE bool Is_Valid_Double(double x);

static WWINLINE float Normalize_Angle(float angle); // Normalizes the angle to the range -PI..PI

static WWINLINE int Div_Ceil(const int num, const int den);

};

WWINLINE float WWMath::Sign(float val)
Expand Down Expand Up @@ -654,3 +656,13 @@ WWINLINE float WWMath::Normalize_Angle(float angle)
{
return angle - (WWMATH_TWO_PI * Floor((angle + WWMATH_PI) / WWMATH_TWO_PI));
}

// ----------------------------------------------------------------------------
// Ceil rounded int division
// Rounding away from 0 for positive values, towards 0 for negative values
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

The comment states this function rounds "away from 0 for positive values, towards 0 for negative values", but the implementation actually rounds towards positive infinity for positive quotients and towards 0 for negative quotients. This doesn't match typical ceil division semantics. For example:

  • Div_Ceil(-5, 2) returns -2 (correct ceiling would be -2, but the comment says "towards 0" which would be -2, so this is actually correct)
  • Div_Ceil(5, -2) returns -2 (quotient is negative, so no rounding up occurs)

The comment should be updated to accurately reflect the behavior: "Rounds quotient up (towards positive infinity) when the quotient is positive and there is a remainder, otherwise returns the quotient as-is."

Suggested change
// Rounding away from 0 for positive values, towards 0 for negative values
// Rounds quotient up (towards positive infinity) when the quotient is positive and there is a remainder, otherwise returns the quotient as-is

Copilot uses AI. Check for mistakes.
// ----------------------------------------------------------------------------
WWINLINE int WWMath::Div_Ceil(const int num, const int den)
{
const div_t res = ::div(num, den);
return (res.rem != 0 && res.quot >= 0) ? res.quot + 1 : res.quot;
}
Comment on lines +664 to +668
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Missing TheSuperHackers comment for the new Div_Ceil function. All new features require documentation:

// TheSuperHackers @feature author DD/MM/YYYY Added ceiling division function for integer division

Copilot generated this review using guidance from repository custom instructions.
Loading