tweak(network): Increase message buffer and max packet sizes to reduce connection issues

[Mauller]

Closes: #203

This PR increases the send and receive buffer sizes, this can help to alleviate network pressure that can lead to the DC menu appearing when the game is waiting for more network data or has to wait for a resend.

The PR also implements a non retail change to increase the amount of data stored per UDP packet.

The retail game was designed with Dialup limitations in mind, which tended to limit the MTU side to 576 bytes.
Internally they chose 512 bytes as their packet size limit and then incorrectly calculated a lower max UDP payload than necessary on top of this.

The fix allows the game to use 1200 Bytes as the UDP payload for the greatest compatibility between different types of network connections. This should take into account overhead of PPPOE, VPN's and IPV6 on supporting networks without causing fragmentation.

At 1200 bytes per packets we are now handling ~2.5x the amount of data per packet compared to retail, this should allow a reduction of network traffic by up to 2/3's depending on how game commands are being generated.

When testing this on GO with a full 8 player Mayhem game, we did not see a single DC screen popup.
All players were creating large armies and using QQA repeatedly to move them.

[greptile-apps]

Greptile Overview

Greptile Summary

Doubled the message buffer capacity (MAX_MESSAGES: 128 → 256) and conditionally increased max packet payload size (476 → 1094 bytes when RETAIL_COMPATIBLE_NETWORKING is disabled) to reduce network pressure and connection issues in 8-player games.

Key changes:

• Introduced RETAIL_COMPATIBLE_NETWORKING feature flag in GameDefines.h (defaults to enabled for backward compatibility)
• Increased MAX_MESSAGES from 128 to 256, doubling the size of Transport::m_inBuffer/m_outBuffer arrays
• When non-retail networking is enabled, sets MAX_UDP_PAYLOAD_SIZE to 1100 bytes (accommodating PPPOE/VPN/IPv6 overhead) and MAX_PACKET_SIZE to 1094 bytes (payload minus 6-byte TransportMessageHeader)
• Preserved legacy LAN API compatibility by introducing MAX_LANAPI_PACKET_SIZE constant
• Updated all send/receive operations to use MAX_NETWORK_MESSAGE_LEN instead of the deprecated MAX_MESSAGE_LEN macro

Note: Previous review threads have already identified issues with prologue author tags, incorrect comment math, increased buffer footprint, and undefined macro concerns. The PR description claims "1200 Bytes" but the code implements 1100 bytes (MAX_UDP_PAYLOAD_SIZE).

Confidence Score: 4/5

• This PR is generally safe to merge with awareness of the known issues already flagged in previous threads
• The changes are well-structured and tested (8-player Mayhem game with no DC screens). The doubled buffer size (MAX_MESSAGES) and increased packet payload are straightforward optimizations. Score of 4 (not 5) reflects: (1) previous threads identified prologue violations, math errors in comments, and potential undefined macro behavior; (2) the feature is gated behind RETAIL_COMPATIBLE_NETWORKING which defaults to enabled, minimizing risk; (3) the increased memory footprint from doubled buffers is intentional but should be monitored
• Pay attention to NetworkDefs.h for the issues flagged in previous threads (prologue author tags, incorrect math comment, macro definition dependencies)

Important Files Changed

Filename	Overview
Core/GameEngine/Include/Common/GameDefines.h	Added RETAIL_COMPATIBLE_NETWORKING feature flag to control packet size optimizations
Core/GameEngine/Include/GameNetwork/NetworkDefs.h	Increased MAX_MESSAGES from 128 to 256 and introduced conditional packet sizing; contains prologue violations and potential description/code mismatch
Core/GameEngine/Source/GameNetwork/Transport.cpp	Updated send/receive buffer operations to use MAX_NETWORK_MESSAGE_LEN; added clarifying comments about UDP payload sizing

Sequence Diagram

sequenceDiagram
    participant Game as Game Logic
    participant Net as Network Layer
    participant Transport as Transport Layer
    participant UDP as UDP Socket
    participant Remote as Remote Peer

    Note over Game,Remote: Sending Messages (doSend)
    Game->>Net: Queue game commands
    Net->>Transport: queueSend(data, MAX_PACKET_SIZE)
    Transport->>Transport: Store in m_outBuffer[MAX_MESSAGES]
    Transport->>Transport: Add TransportMessageHeader (6 bytes)
    Transport->>UDP: Write(bytesToSend = length + header)
    UDP->>Remote: Send UDP packet (≤MAX_NETWORK_MESSAGE_LEN)

    Note over Game,Remote: Receiving Messages (doRecv)
    Remote->>UDP: Incoming UDP packet
    UDP->>Transport: Read(buf, MAX_NETWORK_MESSAGE_LEN)
    Transport->>Transport: Decrypt and validate (isGeneralsPacket)
    Transport->>Transport: Store in m_inBuffer[MAX_MESSAGES]
    Transport->>Net: Process received message
    Net->>Game: Deliver game commands

    Note over Transport: Buffer Sizes
    Note over Transport: Retail: MAX_MESSAGES=256, MAX_PACKET_SIZE=476
    Note over Transport: Non-retail: MAX_MESSAGES=256, MAX_PACKET_SIZE=1094

Loading

_{Last reviewed commit: 265fab3}

[greptile-apps]

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Increases buffer footprint

Bumping MAX_MESSAGES from 128 to 256 doubles the size of Transport::m_inBuffer/m_outBuffer (and m_delayedInBuffer in debug) because these arrays are sized by MAX_MESSAGES (see Core/GameEngine/Include/GameNetwork/Transport.h:73-78). This is a behavioral change that can increase memory pressure (and potentially stack/heap usage depending on how Transport is instantiated) even when RETAIL_COMPATIBLE_NETWORKING is kept enabled; please confirm this is intended and safe for all builds, or gate it behind the same retail-compat switch.

Prompt To Fix With AI

This is a comment left during a code review.
Path: Core/GameEngine/Include/GameNetwork/NetworkDefs.h
Line: 71:71

Comment:
**Increases buffer footprint**

Bumping `MAX_MESSAGES` from 128 to 256 doubles the size of `Transport::m_inBuffer`/`m_outBuffer` (and `m_delayedInBuffer` in debug) because these arrays are sized by `MAX_MESSAGES` (see `Core/GameEngine/Include/GameNetwork/Transport.h:73-78`). This is a behavioral change that can increase memory pressure (and potentially stack/heap usage depending on how `Transport` is instantiated) even when `RETAIL_COMPATIBLE_NETWORKING` is kept enabled; please confirm this is intended and safe for all builds, or gate it behind the same retail-compat switch.


How can I resolve this? If you propose a fix, please make it concise.

[xezon]

The presentation can be polished more.

[Mauller]

Tweaked and addressed comments

[greptile-apps]

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

Core/GameEngine/Source/GameNetwork/NetPacket.cpp
Incorrect memcpy length

NetPacket::NetPacket(TransportMessage *msg) copies MAX_PACKET_SIZE bytes (memcpy(m_packet, msg->data, MAX_PACKET_SIZE)) even though msg->length may be smaller. With this PR increasing MAX_PACKET_SIZE to 1200 in non-retail builds, this will read past the end of the received UDP payload for most packets and can cause out-of-bounds reads / crashes. This should copy msg->length (or m_packetLen) bytes instead of MAX_PACKET_SIZE.

Prompt To Fix With AI

This is a comment left during a code review.
Path: Core/GameEngine/Source/GameNetwork/NetPacket.cpp
Line: 960:964

Comment:
**Incorrect memcpy length**

`NetPacket::NetPacket(TransportMessage *msg)` copies `MAX_PACKET_SIZE` bytes (`memcpy(m_packet, msg->data, MAX_PACKET_SIZE)`) even though `msg->length` may be smaller. With this PR increasing `MAX_PACKET_SIZE` to 1200 in non-retail builds, this will read past the end of the received UDP payload for most packets and can cause out-of-bounds reads / crashes. This should copy `msg->length` (or `m_packetLen`) bytes instead of `MAX_PACKET_SIZE`.

How can I resolve this? If you propose a fix, please make it concise.

[xezon]

Perhaps add a SIZE into the name

[xezon]

Perhaps use RETAIL instead of RESTRICTED

[Mauller]

After having another quick look, i actually need to tweak this.

MAX_MESSAGE_LEN is the maximum data that goes out on the wire in the end, MAX_PACKET_SIZE is the max size a game message packet can be, but it gets encapsulated into a NetworkMessage struct with a NetworkMessageHeader prepended to the data, which adds 6 bytes to the start of the data.

So the MAX_PACKET_SIZE actually needs to be MAX_MESSAGE_LEN - Sizeof(NetworkMessageHeader) to not cause bytes to be missed at the end of the data.

The networking code is a mess...

[Mauller]

To save the confusion again i am going to look at renaming these to better suit their purpose.

[Mauller]

Tweaked the naming and added some extra comments to affected places, the issue here is the entire handling of network data. It really needs refactoring but theres no clean way to do that while properly maintaining the retail mishandling for now.

It's something i can come back to in the future with more network api tweaking.

Dismissed because the Mauller wants to do another pass

[Mauller]

After some talking with X64 i will need to adjust the packet size (data that goes onto the wire) down to 1100, the extra 100 bytes is to help with TURN relay headers and extra reliability layers That GO uses. this still gives 2.2-2.3x the packet size of retail.

This means that data on the wire, UDP payload, needs to be a max of 1100, but game messages can only be a max of 1094 due to the 6 bytes of game packet header that i overlooked originally.

We can always adjust these values in future when refactoring more of the networking.

[Mauller]

Updated now with the tweaks

[xezon]

In future, we can also introduce packet data compression to help reduce the traffic pressure.