bugfix(network): Prevent LAN lobby hang with long player names

[bobtista]

• Closes Network Game Room hangs if 8 players with long nicknames join #79
• Supersedes [ZH] Prevent hang in network lobby with long player names #1119; the original investigation and fix are by @slurmlord
• Adds WWLib/utf8.h, as does feat(string): Implement UTF-8 string conversion and validation functions #2528; whichever lands second can rebase

GameInfoToAsciiString serializes the LAN lobby state into a string with a 400-byte limit. The existing code truncates each player name while appending its slot:

int lenRem = m_lanMaxOptionsLength - lenCur;   // can go negative
int lenMax = lenRem / (MAX_SLOTS-i);           // can go negative
while( name.getLength() > lenMax )
    name.removeLastChar();

Once the fixed portion of the options string consumes the remaining budget, lenMax becomes negative. The loop removes the entire name, after which AsciiString::removeLastChar becomes a no-op. Because 0 > lenMax remains true, the host spins forever.

The serializer now builds the complete payload with full player names first. If it exceeds 400 bytes, a second pass:

• Calculates the exact number of bytes occupied by the fixed fields.
• Reserves at least one complete UTF-8 character for every human player.
• Divides the remaining name budget among the players, carrying unused space from shorter names forward.
• Rebuilds the payload with the bounded names.
• Returns an empty payload if the fixed fields and minimum complete names cannot fit.

A final guard rejects any result that remains oversized. Therefore every non-empty LAN options payload returned by GameInfoToAsciiString is at most 400 bytes.

Truncation cuts only at UTF-8 character boundaries through Utf8_Truncate_Len in WWLib/utf8.h, keeping encoding rules outside GameInfo. Supporting changes add a compile-time check that the GameOptions.options buffer exceeds m_lanMaxOptionsLength and allow a payload of exactly 400 bytes, which fits the 401-byte null-terminated buffer.

Truncation affects only the serialized LAN payload. The host retains the full player names, while remote clients may display their truncated forms. Names that truncate to the same prefix are left as-is; distinguishing them would require additional collision handling.

Verification for this revision:

• GameInfo.cpp and utf8.cpp pass targeted syntax compilation against the macOS integration stack.
• 100,000 randomized allocations using ASCII and two-, three-, and four-byte UTF-8 characters produced no oversized result or split sequence.
• A payload of exactly 400 bytes is accepted.

Follow-up: cache each converted player name in GameSlot so it is not rebuilt on every room refresh, as suggested in #1119.

Todo:

• A lobby of eight long names, including multibyte names, serializes without hanging
• Every non-empty LAN options payload is at most 400 bytes
• A payload of exactly 400 bytes is accepted
• Replicate to Generals — N/A, the implementation is shared through Core
• Resolve the outstanding review points inherited from [ZH] Prevent hang in network lobby with long player names #1119
• Preserve UTF-8 validity during truncation

[greptile-apps]

Greptile Summary

The PR prevents LAN lobby serialization from hanging when long player names exceed the 400-byte options limit.

• Builds the complete payload first and performs a bounded second-pass truncation only when necessary.
• Preserves UTF-8 character boundaries and deterministically resolves names that collide after truncation.
• Allows exactly 400 payload bytes while retaining space for the null terminator.
• Returns an empty payload when names cannot provide enough removable bytes.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified.

The truncation path removes at least the required number of bytes while preserving UTF-8 boundaries and one complete character per non-empty name, and the exactly-400-byte payload safely fits the existing null-terminated buffers.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameNetwork/GameInfo.cpp	Adds bounded UTF-8-aware player-name truncation, deterministic collision handling, and a second serialization pass for oversized LAN payloads.
Core/GameEngine/Source/GameNetwork/LANAPI.cpp	Correctly permits a payload of exactly 400 bytes, which fits the existing 401-byte null-terminated destination.
Core/GameEngine/Include/GameNetwork/LANAPI.h	Adds a compile-time assertion confirming that the LAN options buffer includes capacity beyond the payload limit.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Build UTF-8 player names] --> B[Serialize full GameInfo]
    B --> C{Payload exceeds 400 bytes?}
    C -- No --> D[Return payload]
    C -- Yes --> E[Calculate required byte reduction]
    E --> F{Enough removable name bytes?}
    F -- No --> G[Return empty string]
    F -- Yes --> H[Truncate names at UTF-8 boundaries]
    H --> I[Resolve truncated-name collisions]
    I --> J[Rebuild serialized GameInfo]
    J --> D

Loading

_{Reviews (1): Last reviewed commit: "bugfix(network): Prevent LAN lobby hang ..." | Re-trigger Greptile}

[Skyaero42]

This feels very complicated for what is eventually just a hack. FIxing the 400 byte gameinfo byte limit should be the true goal.

Something as simple as: count the number of available bytes for player names and divide that by the number of players - this gives the number of bytes each player name can have. Yes, it is not exact (if there a players with shorter names, that would also allow players with longer names than the threshold).

In general, there is a lot of stuff added in GameInfo that doesn't belong there. Specific byte counts of characters belong in Asciistring. Such function can probably also be generalized instead of using First and Last in functions.

[Skyaero42]

pi could be confused with the number pi. Maybe just use i?

[bobtista]

used playerIndex

[Skyaero42]

This should be an Asciistring function. It does not belong in GameInfo

[bobtista]

AsciiString is the wrong home for the three UTF-8 byte helpers. They're UTF-8 encoding rules, not string operations, and putting them on AsciiString implies AsciiString knows its own encoding, which it doesn't. Probably best is WWLib/utf8.h, which is what #2528 adds. Maybe wait til after that lands?

[Skyaero42]

A separate encoding class would definitely have my favour.

[bobtista]

Updated just now - Utf8_Truncate_Len lives in WWLib/utf8.h now, GameInfo just keeps the empty-on-zero-budget bit since that's the actual hang fix. I didn't wait for #2528, its utf8.h is a wide<->utf8 transcoder with no truncation in it anyway, so waiting wouldn't have saved writing this. Both PRs add WWLib/utf8.h, whichever lands second can rebase.

Checked the new helper against the old loop on ascii, 2/3/4 byte sequences cut mid char and on boundaries, malformed input and zero budget. Same results.

[greptile-apps]

Greptile has paused reviews on this repository — it used its 100 free open-source review credits for this billing period. Reviews resume automatically on August 26. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.

[bobtista]

FIxing the 400 byte gameinfo byte limit should be the true goal.

Agreed, but retail still needs something, even if it's hacky. The 400-byte limit is part of the packed retail LAN wire layout: LANMessage is sent by size and cast directly by receivers, so enlarging the options array would change field offsets and break retail compat. Removing that limit requires a versioned or chunked protocol extension and should be a separate change. This PR keeps the existing wire format and fixes the current infinite loop; it would remain necessary as the retail-compatible fallback even after an extended protocol is introduced.

[xezon]

This change looks different from the original one by Slurmlord. Why?

[bobtista]

Originally I used Slurmlord’s two-pass implementation from 1119 (truncate after serializing, with collision handling). Skyaero noted it was too complicated for what is ultimately a hack around the 400-byte limit, so I narrowed it to keep the retail per-slot budgeting and only make the truncation itself bounded and UTF-8-safe.

The hang is that lenMax can go non-positive, and removeLastChar on an empty string is a no-op, so the old loop never exits. truncatePlayerName empties the name in that case instead. The total-length bounding and collision handling from 1119 are intentionally left out. The legacy budget can still exceed 400 bytes, and bounding the total belongs with the wider fix for the limit.

[xezon]

But Slurmlord already implemented all this logic, why not use it then?

[bobtista]

I reused Slurmlord’s diagnosis, but not his whole implementation for correctness and scope. eg Its one-byte minimum can leave a partial UTF-8 character, and its collision pass overwrites the last byte of a name, which can also corrupt UTF-8. Supporting that pass additionally requires mutable indexing on the shared string classes.
This PR preserves the retail allocation policy and fixes the nonterminating truncation with a boundary-safe UTF-8 helper.
That said, Slurmlord’s version guarantees the final payload fits within 400 bytes, and this one does not. If we want that guarantee in this PR, I would reuse the two-pass approach but reimplement the truncation safely rather than use 1119 as-is.

[bobtista]

I just pushed this and updated the description

[xezon]

All of this new code is AI generated right? So the human reviewer would now need to check that the code was generated with a good prompt right?

[xezon]

Can be one line

[bobtista]

done

[xezon]

Maybe array size should specify MAX_SLOTS ?

[bobtista]

done, and on both truncatePlayerNames and buildGameInfoAsciiString