[ZH] Prevent hang in network lobby with long player names

[slurmlord]

• Fixes Network Game Room hangs if 8 players with long nicknames join #79

The Problem

When many players with long names join a lobby, the host can hang. This is caused by an eternal loop while trying to truncate player names in order to fit the entire game info within a network packet (476 bytes, with 400 for the game info).
The eternal looping is a result of having overflowed the total packet size and thus requiring a player name to have negative length in order to fit within the packet. The truncation is done with a while loop checking if the string is longer than the required value, and if so removing the last character. Removing the last character on an AsciiString is a no-op, and thus the loop continues.

This name truncation is not implemented in Generals, and is therefore not affected by this issue either.

The Solution

Instead of attempting to truncate the player names "on the fly", first try to construct the game options string with full names. If the resulting string is within the size limits, use that string.
If the resulting string is too long, use the size difference to determine how much truncation needs to happen with the player names to fit.
Player names are sorted based on length in descending order, and the longer names are truncated first. An example of this truncation could be player A with a 10 char name and player B with a 7 char name need to be truncated by a total of 6 chars, then player A would end up with a 5 char name and B with a 6 char name.

A lower limit of 2 characters for player names has been added, meaning that no name will be truncated shorter than that. If all names are truncated to 2 characters and the string is still too long, the serialization will fail and return an empty string for the game options. This will result in the empty string being sent to the player that attempts to join, where it cannot be parsed, and joining the game times out for that player.

The host will know the full names of all the players, as it's the game options send to the other players that needs truncation. Truncation will therefore not be visible to the host.

[xezon]

The presented fix is too complicated.

Better approach (high level illustration):

std::vector<AsciiString> playerNames = BuildPlayerNames( playerNames );
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.length() > m_lanMaxOptionsLength)
{
  const UnsignedInt truncateAmount = infoString.length() - m_lanMaxOptionsLength;
  if (!TruncatePlayerNames( playerNames, truncateAmount ))
  {
    DEBUG_CRASH("Unable to scrap the required space off of player names");
    // abort or whatever.
  }
  infoString = GameInfoToAsciiString( game, playerNames );
}

This is a 2 pass build, but the second pass is an exceptional pass and not the norm. It is easier to understand and also easier to modify or remove afterwards.

Edit: Mockup improved.

[slurmlord]

Ah, didn't see the updated approach before pushing. I can split out the truncation from the building of the player name vector in a new iteration, it makes the separations of concerns even more clear.

[xezon]

should this be "scrapping" or "scraping"?

[slurmlord]

Tried looking around for opinions, and ended up with "stripping".

[xezon]

This implementation is not fair to players that have short names :-)

Player names will be truncated like so

Start

TheDude
ThePrince
HolySmokesLongAssWastefulName

First iteration NOW

TheDud
ThePrinc
HolySmokesLongAssWastefulNam

First iteration MORE FAIR

TheDude
ThePrince
HolySmokesLongAssWastefulN

I think this should be implemented in a way that it aggressively starts truncating the long names first.

To do that, you can take the max length of all player names, and then use that as the cap to start truncating on the iterations. However that is a wasteful algorithm.

A more efficient way to implement it is to sort the names first (use a local array with indices to the vector) and then start truncating from longest to smallest names. Maybe it can be implemented without excessive loops even. Note that removeLastChar is also quite a wasteful operation.

[xezon]

One more consideration here is: Will a new implementation for the truncate be compatible with Retail game clients? To test this, compile with RTS_MULTI_INSTANCE and launch one client before this change and one after, then matchmake in LAN.

Edit: On the other hand, the original implemention will hang up, so perhaps it does not matter at all.

[Mauller]

The game already limits the number of characters a Lan Name can have to 11 characters, even though it should be 12+1 based on g_lanPlayerNameLength + 1 being the size of the character array for the player name

[slurmlord]

My understanding is that the truncation only happens on the host, which then sends out the updated game options when some change happens in the game (like someone joining, selecting team/colors etc.). The clients only receive the ascii string serialization of the game options, and should not require any updated logic.

[slurmlord]

The newest iteration now "punishes" the players who join with long names, as they are truncated first.
As an example, if player A had 10 chars name and player B had 7 and the amount of truncation required was 6; player A would end up with 5 chars and B with 6.

[xezon]

So originally it would have returned the long string if it was unable to (theoretically) truncate. What happens if we return an empty string or a string that is longer than the cap?

[slurmlord]

Got a bit stuck on the new truncation algorithm, and ran out of time before being offline for about a week. In other words, expect no new updates on this for a while, but it's not abandoned either.

[slurmlord]

One aspect to note is that since the truncation happens for the outgoing packets from the host, the name truncation is not directly visible to the host, only the joining clients.
Also the truncation can result in duplicate names in the game; "longnameA" and "longnameB" can both be truncated to "longna" as an example.
Not sure if we want to do something with that as this point? The main focus so far has been to avoid the hang.

[xezon]

This should then also be replicated in UnicodeString, DisplayString.

[slurmlord]

I was thinking of doing that in a separate PR, and at the same time replace the cases where removeLastChar being used in a loop with one call to truncate instead. Also replicating to Generals, while the issue in this PR is ZH-only.
Does that sound like a reasonable approach?

[xezon]

Yes. How about we do that other change before this one? Then is clean.

[xezon]

Please do not use std::pair (or std::tuple). Prefer using a struct, because that can be given proper names for the fields.

[slurmlord]

Thanks, it felt a bit dirty. Much better now :)

[xezon]

This loop does not yet look optimal. It goes one char at a time, over all players.

It looks like it should be possible to skip ahead multiple characters by peeking into the next player name (if it is not the last one).

longplayername
dude
    ^ we already know that the first name is 10 characters longer than all others

[slurmlord]

I was a bit afraid of making it too complicated again, but the latest iteration will do greater steps in the target length. It's a bit more complex, but perhaps manageable?

[xezon]

In inquired with Chat GPT about it. Can you check?

Chat GPT

Great! To improve performance, we can avoid repeated scanning and dynamic target recomputation by switching to a leveling algorithm, inspired by how you might "flatten terrain" from the top down.

---

✅ Optimal Fair Truncation Algorithm — O(n log n)

📌 Key Ideas:

1. Sort strings by length descending.

2. Flatten in levels: repeatedly lower the longest group of strings to the height of the next group.

3. For each level:

   • Compute how many characters you'd save by flattening all strings in that level.
   • If you can afford it, flatten and continue.
   • If not, stop and perform partial truncation evenly on that level.

This avoids the repeated loop-reset logic of your original code.

---

🔧 Optimized Implementation

#include <vector>
#include <string>
#include <algorithm>
#include <cassert>
#include <iostream>

using namespace std;

// Alias types for clarity
using AsciiStringVec = std::vector<std::string>;
using Int = int;
using UnsignedInt = unsigned int;
using Bool = bool;

struct LengthIndexPair {
    Int Length;
    size_t Index;
};

static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncateAmount) {
    constexpr Int MinimumNameLength = 2;
    const size_t n = playerNames.size();
    if (n == 0 || truncateAmount == 0) return true;

    // Prepare length/index pairs and calculate available truncation budget
    vector<LengthIndexPair> entries(n);
    UnsignedInt availableForTruncation = 0;

    for (size_t i = 0; i < n; ++i) {
        Int len = static_cast<Int>(playerNames[i].size());
        entries[i] = { len, i };
        availableForTruncation += max(0, len - MinimumNameLength);
    }

    if (truncateAmount > availableForTruncation) {
        cerr << "Requested to truncate more than allowed.\n";
        return false;
    }

    // Sort by descending length
    sort(entries.begin(), entries.end(), [](const auto& a, const auto& b) {
        return a.Length > b.Length;
    });

    // Process in levels
    size_t i = 0;
    while (i < n && truncateAmount > 0) {
        Int currentLength = entries[i].Length;

        // Find the next group with a smaller length
        size_t j = i + 1;
        while (j < n && entries[j].Length == currentLength) ++j;

        // Determine target level
        Int nextLength = (j < n) ? entries[j].Length : MinimumNameLength;
        Int maxReduction = currentLength - max(nextLength, MinimumNameLength);
        size_t groupSize = j;

        UnsignedInt cost = groupSize * maxReduction;

        if (cost <= truncateAmount) {
            // Flatten whole group
            for (size_t k = 0; k < groupSize; ++k)
                entries[k].Length -= maxReduction;
            truncateAmount -= cost;
            i = j; // move to next group
        } else {
            // Partial truncation to equal target
            Int cut = truncateAmount / groupSize;
            Int extra = truncateAmount % groupSize;

            for (size_t k = 0; k < groupSize; ++k) {
                entries[k].Length -= cut;
                if (extra > 0) {
                    entries[k].Length -= 1;
                    --extra;
                }
            }
            truncateAmount = 0;
            break;
        }
    }

    // Apply truncation
    for (const auto& entry : entries) {
        Int newLength = max(entry.Length, MinimumNameLength);
        if ((Int)playerNames[entry.Index].size() > newLength) {
            playerNames[entry.Index].resize(newLength);
        }
    }

    return true;
}

---

✅ Time Complexity

• Sorting: O(n log n)
• Leveling: O(n) (each index touched once)
• Total: O(n log n) — optimal for this problem

---

🧪 Example

Input:

playerNames = {"Jonathan", "Tom", "Christopher", "Al", "Dan"};
truncateAmount = 10;

After truncation:

• Long names like "Jonathan" and "Christopher" will lose more characters than "Tom", "Dan", and "Al".
• But none of the shorter names will be longer than truncated longer names.

---

Let’s compare the performance characteristics of your original implementation and the optimized one I provided, to show exactly why the latter is more efficient.

---

🧠 1. Your Original Implementation

🔁 Key Pattern:

while (truncateAmount > 0)
{
    for (size_t i = 0; i < lengthIndex.size(); ++i)
    {
        ...
        if (condition) {
            i = -1; // restart loop
            continue;
        }
    }
}

❌ Performance Implications:

• The outer while loop may execute multiple full scans of the list (O(n) per scan).

• The inner for loop is restarted (i = -1) every time currentTargetLength changes. This is a valid but costly retry strategy.

• In the worst case, the target length changes frequently (e.g., strings with many different lengths), resulting in O(n²) time complexity.

🔄 Example Worst-Case Behavior:

If you have n strings with distinct lengths, it will:

• Attempt to lower the longest string to the second longest.

• Then restart and lower both to the third longest.

• And so on…

That’s O(n²) operations in the worst case.

---

⚡ 2. Optimized Implementation

✅ Key Strategy:

• Uses a leveling approach: rather than decrementing string lengths one-by-one, it processes all strings of the same length together.

• Each “level” flattens a group of strings down to the next shorter length (or as far as allowed).

• The algorithm processes each string exactly once in a sorted list.

📈 Performance Characteristics:

• Sort once: O(n log n)

• Single pass leveling: O(n)

• Total time complexity: O(n log n)

• No backtracking, no restarting loops, no wasted work.

---

💡 Analogy

Imagine you have towers of blocks (string lengths) and you want to remove N blocks total.

Your Approach:

"Try removing blocks from the tallest towers one by one. If we can't balance, restart the process with a new target height."

Problem: You often keep re-checking previous towers after recalculating your target — redundant work.

Optimized Approach:

"Sort the towers by height, and flatten them level by level, always removing as many blocks as possible at once."

Benefit: You never redo work; every block removal counts.

---

🧪 Example Timing Difference (Theoretical)

Strings Count	Your Impl (Worst Case)	Optimized Impl
1000	O(1,000,000) steps	O(10,000) steps
10,000	O(100,000,000) steps	O(140,000) steps

[Caball009]

An example of this truncation could be player A with a 10 char name and player B with a 7 char name need to be truncated by a total of 6 chars, then player A would end up with a 5 char name and B with a 6 char name.

It seems like your truncation code is too complex or this example oversimplified. You wouldn't need to do any sorting for this:
A: 10
B: 7
Total: 17
Target: 11

Truncated A: A / (Total / Target) = ~6
Truncated B: B / (Total / Target) = ~5

[xezon]

Better move this into the cpp if it is only used there.

[xezon]

Maybe set i = 0 here and ++i at the end.

[xezon]

In inquired with Chat GPT about it. Can you check?

Chat GPT

Great! To improve performance, we can avoid repeated scanning and dynamic target recomputation by switching to a leveling algorithm, inspired by how you might "flatten terrain" from the top down.

---

✅ Optimal Fair Truncation Algorithm — O(n log n)

📌 Key Ideas:

1. Sort strings by length descending.

2. Flatten in levels: repeatedly lower the longest group of strings to the height of the next group.

3. For each level:

   • Compute how many characters you'd save by flattening all strings in that level.
   • If you can afford it, flatten and continue.
   • If not, stop and perform partial truncation evenly on that level.

This avoids the repeated loop-reset logic of your original code.

---

🔧 Optimized Implementation

#include <vector>
#include <string>
#include <algorithm>
#include <cassert>
#include <iostream>

using namespace std;

// Alias types for clarity
using AsciiStringVec = std::vector<std::string>;
using Int = int;
using UnsignedInt = unsigned int;
using Bool = bool;

struct LengthIndexPair {
    Int Length;
    size_t Index;
};

static Bool TruncatePlayerNames(AsciiStringVec& playerNames, UnsignedInt truncateAmount) {
    constexpr Int MinimumNameLength = 2;
    const size_t n = playerNames.size();
    if (n == 0 || truncateAmount == 0) return true;

    // Prepare length/index pairs and calculate available truncation budget
    vector<LengthIndexPair> entries(n);
    UnsignedInt availableForTruncation = 0;

    for (size_t i = 0; i < n; ++i) {
        Int len = static_cast<Int>(playerNames[i].size());
        entries[i] = { len, i };
        availableForTruncation += max(0, len - MinimumNameLength);
    }

    if (truncateAmount > availableForTruncation) {
        cerr << "Requested to truncate more than allowed.\n";
        return false;
    }

    // Sort by descending length
    sort(entries.begin(), entries.end(), [](const auto& a, const auto& b) {
        return a.Length > b.Length;
    });

    // Process in levels
    size_t i = 0;
    while (i < n && truncateAmount > 0) {
        Int currentLength = entries[i].Length;

        // Find the next group with a smaller length
        size_t j = i + 1;
        while (j < n && entries[j].Length == currentLength) ++j;

        // Determine target level
        Int nextLength = (j < n) ? entries[j].Length : MinimumNameLength;
        Int maxReduction = currentLength - max(nextLength, MinimumNameLength);
        size_t groupSize = j;

        UnsignedInt cost = groupSize * maxReduction;

        if (cost <= truncateAmount) {
            // Flatten whole group
            for (size_t k = 0; k < groupSize; ++k)
                entries[k].Length -= maxReduction;
            truncateAmount -= cost;
            i = j; // move to next group
        } else {
            // Partial truncation to equal target
            Int cut = truncateAmount / groupSize;
            Int extra = truncateAmount % groupSize;

            for (size_t k = 0; k < groupSize; ++k) {
                entries[k].Length -= cut;
                if (extra > 0) {
                    entries[k].Length -= 1;
                    --extra;
                }
            }
            truncateAmount = 0;
            break;
        }
    }

    // Apply truncation
    for (const auto& entry : entries) {
        Int newLength = max(entry.Length, MinimumNameLength);
        if ((Int)playerNames[entry.Index].size() > newLength) {
            playerNames[entry.Index].resize(newLength);
        }
    }

    return true;
}

---

✅ Time Complexity

• Sorting: O(n log n)
• Leveling: O(n) (each index touched once)
• Total: O(n log n) — optimal for this problem

---

🧪 Example

Input:

playerNames = {"Jonathan", "Tom", "Christopher", "Al", "Dan"};
truncateAmount = 10;

After truncation:

• Long names like "Jonathan" and "Christopher" will lose more characters than "Tom", "Dan", and "Al".
• But none of the shorter names will be longer than truncated longer names.

---

Let’s compare the performance characteristics of your original implementation and the optimized one I provided, to show exactly why the latter is more efficient.

---

🧠 1. Your Original Implementation

🔁 Key Pattern:

while (truncateAmount > 0)
{
    for (size_t i = 0; i < lengthIndex.size(); ++i)
    {
        ...
        if (condition) {
            i = -1; // restart loop
            continue;
        }
    }
}

❌ Performance Implications:

• The outer while loop may execute multiple full scans of the list (O(n) per scan).

• The inner for loop is restarted (i = -1) every time currentTargetLength changes. This is a valid but costly retry strategy.

• In the worst case, the target length changes frequently (e.g., strings with many different lengths), resulting in O(n²) time complexity.

🔄 Example Worst-Case Behavior:

If you have n strings with distinct lengths, it will:

• Attempt to lower the longest string to the second longest.

• Then restart and lower both to the third longest.

• And so on…

That’s O(n²) operations in the worst case.

---

⚡ 2. Optimized Implementation

✅ Key Strategy:

• Uses a leveling approach: rather than decrementing string lengths one-by-one, it processes all strings of the same length together.

• Each “level” flattens a group of strings down to the next shorter length (or as far as allowed).

• The algorithm processes each string exactly once in a sorted list.

📈 Performance Characteristics:

• Sort once: O(n log n)

• Single pass leveling: O(n)

• Total time complexity: O(n log n)

• No backtracking, no restarting loops, no wasted work.

---

💡 Analogy

Imagine you have towers of blocks (string lengths) and you want to remove N blocks total.

Your Approach:

"Try removing blocks from the tallest towers one by one. If we can't balance, restart the process with a new target height."

Problem: You often keep re-checking previous towers after recalculating your target — redundant work.

Optimized Approach:

"Sort the towers by height, and flatten them level by level, always removing as many blocks as possible at once."

Benefit: You never redo work; every block removal counts.

---

🧪 Example Timing Difference (Theoretical)

Strings Count	Your Impl (Worst Case)	Optimized Impl
1000	O(1,000,000) steps	O(10,000) steps
10,000	O(100,000,000) steps	O(140,000) steps

[bobtista]

Do the names need to be unique? Is it a problem if two names get truncated into the same name?
eg
DrJellyFishy -> DrJelly
DrJellyDonut -> DrJelly

[xezon]

Do the names need to be unique? Is it a problem if two names get truncated into the same name? eg DrJellyFishy -> DrJelly DrJellyDonut -> DrJelly

It is good question. There are functions that work with player names or name identifiers, such as

Player *PlayerList::findPlayerWithNameKey(NameKeyType key)
{
	for (Int i = 0; i < m_playerCount; i++)
	{
		if (m_players[i]->getPlayerNameKey() == key)
		{
			return m_players[i];
		}
	}
	return NULL;
}