bugfix(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 1100 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.

As 6 bytes is required for the current network packet header, game data packets can only be up to 1094 bytes in size.

At 1094 bytes per game packet we are now handling ~2.3x 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

This PR improves network stability by doubling the message buffer size from 128 to 256 messages and introducing larger UDP packet payloads (1100 bytes) for non-retail builds. The changes maintain backward compatibility through the RETAIL_COMPATIBLE_NETWORKING flag while allowing modernized network configurations to handle ~2.3x more data per packet.

Key changes:

• Doubled MAX_MESSAGES from 128 to 256 to reduce buffer pressure during high-traffic scenarios
• Increased non-retail MAX_PACKET_SIZE from 476 to 1094 bytes (1100 byte UDP payload - 6 byte header)
• Introduced MAX_UDP_PAYLOAD_SIZE constant (1100 bytes) optimized for modern networks with PPPoE/VPN/IPv6 overhead
• Refactored packet size constants with clearer naming (RETAIL_GAME_PACKET_SIZE, MAX_LANAPI_PACKET_SIZE)
• Updated all references from deprecated MAX_MESSAGE_LEN macro to MAX_NETWORK_MESSAGE_LEN constant
• Added extensive comments explaining the confusing payload/header sizing logic

The implementation correctly accounts for the 6-byte TransportMessageHeader (CRC + magic number) when calculating payload sizes, and testing showed elimination of disconnection screens in 8-player games with large armies.

Confidence Score: 4/5

• This PR is safe to merge with minor concerns about memory footprint and comment clarity
• The implementation is sound and well-tested, with proper backward compatibility through feature flags. The score is 4 rather than 5 due to: (1) the MAX_MESSAGES increase doubles buffer memory usage for all builds including retail-compatible ones, which wasn't mentioned in previous thread discussions, and (2) some comments contain author tags in prologues that were flagged in previous reviews. The core networking logic is correct and the 6-byte header calculation matches the struct definition.
• Pay attention to NetworkDefs.h for the prologue author tag issue flagged in previous reviews

Important Files Changed

Filename	Overview
Core/GameEngine/Include/Common/GameDefines.h	Added RETAIL_COMPATIBLE_NETWORKING flag to control non-retail networking improvements
Core/GameEngine/Include/GameNetwork/NetworkDefs.h	Increased MAX_MESSAGES from 128 to 256, introduced larger UDP payload sizes (1100 bytes) conditionally, and refactored constants with better naming
Core/GameEngine/Source/GameNetwork/Transport.cpp	Updated send/receive operations to use MAX_NETWORK_MESSAGE_LEN and added clarifying comments about payload sizing logic

Flowchart

flowchart TD
    A[UDP Payload Size] -->|Retail| B[512 bytes total MTU]
    A -->|Non-Retail| C[1100 bytes UDP payload]
    
    B --> D[512 - 36 IP/UDP headers = 476 bytes]
    C --> E[1100 bytes max UDP payload]
    
    D --> F[MAX_PACKET_SIZE = 476<br/>RETAIL_GAME_PACKET_SIZE]
    E --> G[MAX_PACKET_SIZE = 1094<br/>1100 - 6 header bytes]
    
    F --> H[TransportMessageHeader: 6 bytes<br/>CRC + Magic]
    G --> H
    
    H --> I[Game Data Payload]
    
    I --> J[MAX_MESSAGES buffer<br/>128 → 256 doubled]
    
    J --> K{RETAIL_COMPATIBLE_NETWORKING?}
    K -->|Yes| L[Use 476 byte packets<br/>256 message buffers]
    K -->|No| M[Use 1094 byte packets<br/>256 message buffers<br/>~2.3x more data per packet]

Loading

_{Last reviewed commit: 671170a}

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

[Mauller]

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

Yes it would be worth investigating.

I want to overhaul the entire packet structure to get something more standardised between message types with a common header containing an enumeration for message type, then relevant payload data etc.

[xezon]

The Pull title says tweak but the comments have 2 bugfix. Should be bugfix in title?

[xezon]

Maybe reword this without "poor implementation"

Twice

[Mauller]

Tweaked the wording.

[xezon]

If MAX_PACKET_SIZE was never exceeded in practice, then why would it not be allowed to change this now?

[Mauller]

I was just wanting to minimise changes that could cause issues, in theory it shouldn't be a problem to set it to max packet size of 476, but with how bad the rest of the netcode is i didn't want to accidentally overlook something.

[Mauller]

The Pull title says tweak but the comments have 2 bugfix. Should be bugfix in title?

I changed it to say bugfix now, initially i was considering this a tweak but after more testing it does appear to be a considerable bugfix towards connection issues.