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

• 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.

Runtime verification

Tested in-engine with eight real game processes connected through Direct Connect.

Test setup:

• Eight human clients on separate loopback addresses.
• Each player name contained 12 CJK characters (36 UTF-8 bytes): a unique first character followed by eleven repeated 界 characters.
• Combined untruncated player-name data was 288 bytes.
• Map: Twilight Flame Revealed

Results:

• Final eight-player host payload: 660 bytes before truncation → 399 bytes after truncation.
• Across the complete run, oversized payloads were rebuilt to 398–400 bytes.
• An output of exactly 400 bytes was observed and accepted.
• The synchronized names shortened only at complete UTF-8 character boundaries; no partial characters or replacement glyphs appeared.
• The match successfully started and ran for 30 seconds before I exited.

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 on oversized player names by rebuilding oversized payloads with UTF-8-safe name limits.

• Separates payload construction from player-name allocation and truncation.
• Rejects payloads that cannot fit within the 400-byte LAN limit.
• Adds a shared UTF-8 truncation helper and permits exactly 400-byte payloads.
• Verifies at compile time that the destination buffer has room for the terminator.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameNetwork/GameInfo.cpp	Reworks LAN serialization to allocate a bounded name budget, preserve complete UTF-8 characters, and reject any payload still exceeding the protocol limit.
Core/Libraries/Source/WWVegas/WWLib/utf8.cpp	Adds a truncation-length helper that backs a cut point away from UTF-8 continuation bytes.
Core/Libraries/Source/WWVegas/WWLib/utf8.h	Exposes and documents the UTF-8-safe truncation helper.
Core/GameEngine/Source/GameNetwork/LANAPI.cpp	Updates the request assertion to accept a payload exactly equal to the 400-byte limit.
Core/GameEngine/Include/GameNetwork/LANAPI.h	Adds a compile-time check that the options buffer includes space beyond the serialized payload limit.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Convert human names to UTF-8] --> B[Build complete LAN payload]
    B --> C{Payload at most 400 bytes?}
    C -- Yes --> D[Return payload]
    C -- No --> E[Calculate fixed-field length]
    E --> F[Allocate remaining bytes among human names]
    F --> G[Truncate names at UTF-8 boundaries]
    G --> H{Allocation succeeded?}
    H -- No --> I[Return empty payload]
    H -- Yes --> J[Rebuild payload]
    J --> K{Rebuilt payload at most 400 bytes?}
    K -- Yes --> D
    K -- No --> I

Loading

_{Reviews (4): Last reviewed commit: "refactor(network): Address LAN serialize..." | 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.

[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]

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?

[bobtista]

The approach is fine. In fact, it follows the same two-pass structure xezon proposed in #1119: serialize normally, calculate the exact excess only when oversized, truncate names, then rebuild.
All CI configurations pass. I also tested eight connected game clients with 36-byte CJK names. The host payload was reduced from 660 to 399 bytes, outputs across the run remained between 398 and 400 bytes, an exact 400-byte payload was accepted, no invalid UTF-8 appeared, and the match started and ran successfully.
If there is a specific remaining correctness or maintainability concern, I'm happy to address it.
Or close it. I saw that it was an open issue and thought I'd help take a grunt work task to clear the way for people working on important things like camera, pathfinding, the stuff that makes the game more playable. If this is better than it was, merge it. It's been a month

[bobtista]

I reviewed the current code and tested it myself. With eight connected clients using 36-byte CJK names, the host payload stayed between 398 and 400 bytes, remained valid UTF-8, and the match started and ran.

[coderabbitai]

Walkthrough

The change adds UTF-8-safe name truncation and shared byte budgeting to LAN game-information serialization. It also updates LAN option length validation and adds a compile-time buffer-capacity assertion.

Changes

LAN UTF-8 serialization

Layer / File(s)	Summary
UTF-8 boundary support Core/Libraries/Source/WWVegas/WWLib/utf8.h, Core/Libraries/Source/WWVegas/WWLib/utf8.cpp	Adds Utf8_Truncate_Len, which returns a truncation length that does not split a UTF-8 sequence.
Game information name budgeting Core/GameEngine/Source/GameNetwork/GameInfo.cpp	Prepares UTF-8 names, applies the shared LAN byte budget, rebuilds the payload when truncation is needed, and returns an empty string when the payload still exceeds the limit.
LAN length validation Core/GameEngine/Include/GameNetwork/LANAPI.h, Core/GameEngine/Source/GameNetwork/LANAPI.cpp	Adds a compile-time buffer-capacity check and permits option strings whose length equals m_lanMaxOptionsLength.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant LANGameRoom
  participant GameInfoToAsciiString
  participant Utf8_Truncate_Len
  participant LANAPI
  LANGameRoom->>GameInfoToAsciiString: serialize player names and game fields
  GameInfoToAsciiString->>Utf8_Truncate_Len: calculate UTF-8-safe name lengths
  Utf8_Truncate_Len-->>GameInfoToAsciiString: return complete-character lengths
  GameInfoToAsciiString->>LANAPI: validate serialized game options
  LANAPI-->>LANGameRoom: accept length at or below the LAN maximum

Loading

Merge Risk: 🟡 Moderate · up to 845fa

Some oversized lobby configurations can publish an empty game-options announcement, causing clients to remove the lobby rather than showing valid state. This should be corrected before merge.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary change: preventing the LAN lobby hang caused by long player names.
Description check	✅ Passed	The description directly explains the LAN lobby hang, the UTF-8-safe truncation fix, the 400-byte limit, and verification results.
Linked Issues check	✅ Passed	The PR addresses issue #79. GameInfoToAsciiString rebuilds oversized LAN options with bounded human names, preserves UTF-8 boundaries, reserves space for required names, and rejects results that rem…
Out of Scope Changes check	✅ Passed	The changes remain within issue #79. The UTF-8 truncation utility, compile-time LAN buffer check, exact-limit handling, and serialization changes directly support safe options serialization within the…

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

⚠️ Outside the diff (1)

🟡 Minor · Keep Utf8_Truncate_Len consistent with its documented contract.

Core/Libraries/Source/WWVegas/WWLib/utf8.cpp:291-304
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep Utf8_Truncate_Len consistent with its documented contract. For {'A', 0xC2, 0xFF} with maxLen == 2, it returns 2, although Utf8_Decode rejects the retained incomplete sequence. Validate the retained prefix or narrow the helper's contract.

The current LAN path does not produce this malformed input: WideCharStringToMultiByte uses Wide_To_Utf8, whose Wide_Read replaces unrepresentable values with U+FFFD before serialization.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: a142095d-3428-4030-acb2-8a7b4f5b2b85

📥 Commits

Reviewing files that changed from the base of the PR and between c5f6fad and 845faf4.

📒 Files selected for processing (1)

• Core/GameEngine/Source/GameNetwork/GameInfo.cpp

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '870,1110p' Core/GameEngine/Source/GameNetwork/GameInfo.cpp
rg -n 'GameInfoToAsciiString|RequestGameOptions|GameOptions|options\[' Core/GameEngine/Source/GameNetwork Core/GameEngine/Include/GameNetwork
sed -n '240,280p' Core/GameEngine/Include/GameNetwork/LANAPI.h
sed -n '800,860p' Core/GameEngine/Source/GameNetwork/LANAPI.cpp

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 17398

---

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- LANGameInfo.cpp ---'
sed -n '230,315p' Core/GameEngine/Source/GameNetwork/LANGameInfo.cpp
printf '%s\n' '--- LANAPI.cpp publication path ---'
sed -n '690,735p' Core/GameEngine/Source/GameNetwork/LANAPI.cpp
sed -n '825,875p' Core/GameEngine/Source/GameNetwork/LANAPI.cpp
printf '%s\n' '--- LANAPIhandlers.cpp relevant callers/consumer ---'
sed -n '45,160p' Core/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp
sed -n '180,210p' Core/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp
sed -n '420,455p' Core/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp
sed -n '515,545p' Core/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp
sed -n '660,690p' Core/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp
sed -n '710,730p' Core/GameEngine/Source/GameNetwork/LANAPIhandlers.cpp
printf '%s\n' '--- LANAPICallbacks.cpp relevant callers ---'
sed -n '110,140p' Core/GameEngine/Source/GameNetwork/LANAPICallbacks.cpp
sed -n '450,475p' Core/GameEngine/Source/GameNetwork/LANAPICallbacks.cpp
sed -n '500,540p' Core/GameEngine/Source/GameNetwork/LANAPICallbacks.cpp
sed -n '580,600p' Core/GameEngine/Source/GameNetwork/LANAPICallbacks.cpp

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 15987

---

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- GameInfo parser ---'
sed -n '1090,1325p' Core/GameEngine/Source/GameNetwork/GameInfo.cpp
printf '%s\n' '--- LANGameInfo parser return ---'
sed -n '245,330p' Core/GameEngine/Source/GameNetwork/LANGameInfo.cpp
printf '%s\n' '--- all direct publication call sites ---'
rg -n -C 4 'GameInfoToAsciiString\\(|GenerateGameOptionsString\\(' Core/GameEngine/Source/GameNetwork

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 9908

---

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- parser return and required fields ---'
rg -n -C 8 'sawMap|sawSlotlist|optionsOk|return ' Core/GameEngine/Source/GameNetwork/GameInfo.cpp | tail -n 180
printf '%s\n' '--- direct serialization callers ---'
rg -n -F -C 3 'GameInfoToAsciiString(' Core/GameEngine/Source/GameNetwork
printf '%s\n' '--- generated-options callers ---'
rg -n -F -C 3 'GenerateGameOptionsString(' Core/GameEngine/Source/GameNetwork

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 18314

---

Reject empty game-options payloads before LAN publication. When GameInfoToAsciiString cannot fit the payload, RequestGameAnnounce and handleRequestGameInfo copy the empty result into MSG_GAME_ANNOUNCE and send it. handleGameAnnounce rejects the empty string because the required fields are missing, then removes the discovered game. If that game already exists, this removes valid lobby state. Guard the empty result before sending the announcement or game-options update.

[xezon]

Ok. The Rabbit has complaints.