Skip to content

chore(network): Remove legacy Send Delay - #3007

Merged
xezon merged 4 commits into
TheSuperHackers:mainfrom
githubawn:refactor/remove-send-delay
Jul 30, 2026
Merged

chore(network): Remove legacy Send Delay#3007
xezon merged 4 commits into
TheSuperHackers:mainfrom
githubawn:refactor/remove-send-delay

Conversation

@githubawn

Copy link
Copy Markdown

What this code did:

The SendDelay setting (m_firewallSendDelay) enabled a specialized NAT traversal mode (FIREWALL_TYPE_NETGEAR_BUG) in FirewallHelper.cpp and NAT.cpp:

  1. During pre-game connection setup, it inserted artificial delays (m_timeTillNextSend = -1) on outbound UDP STUN/probe packets sent to peer players.
  2. In NAT::establishConnectionPaths(), it rearranged player connection slots into special pairs for nodes identified as having the Netgear NAT bug.
  3. In NAT::probed() and NAT::gotMangledPort(), it blocked local clients from sending reciprocal NAT probes until the remote peer probed them first.
  4. Exposed a "Send Delay" checkbox in the in-game Options Menu GUI and saved/loaded SendDelay = yes/no in Options.ini.

Background & Reason for Removal:

  • Historical Workaround: Added in 2003 specifically for early consumer routers (e.g., Netgear WGR614 v1/v2, WGT624) that violated UDP Binding Stability and Endpoint-Independent Mapping per RFC 4787 (their stateful NAT tables prematurely flushed active socket bindings if rapid UDP packets were sent to multiple STUN mangler destinations).
  • Hardware Extinction: Global IPv4 Shodan/Censys scans confirm these 20-year-old 802.11b/g routers are extinct.
  • Community Confusion: Over the last 20 years, players frequently confused "Send Delay" in the Options GUI with in-game input lag or network ping delays, leading to endless forum troubleshooting guides instructing users to manually set SendDelay = no in Options.ini. In reality, it had zero effect on in-game performance.
  • Code Cleanliness: Removing this eliminates non-standard probe delays in NAT.cpp, removes dead code across FirewallHelper, OptionPreferences, and GlobalData, and hides the obsolete setting from the Options GUI.

@githubawn githubawn changed the title refactor(network): Remove legacy Send Delay and Netgear NAT workaround refactor(network): Remove legacy Send Delay Jul 23, 2026
@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR removes the legacy SendDelay / Netgear-bug NAT workaround that was introduced in 2003 for early consumer routers that reset UDP bindings on rapid STUN traffic. With those devices extinct, the code was dead and confused players for decades.

  • Core networking (FirewallHelper.cpp, NAT.cpp): All FIREWALL_TYPE_NETGEAR_BUG detection, the special Netgear-pair ordering in establishConnectionPaths(), and the conditional probe-delay logic in probed() / gotMangledPort() are removed; the simplified path now always sends the probe immediately upon receiving the mangled port.
  • Data model (GlobalData, OptionPreferences): m_firewallSendDelay field and getSendDelay() are deleted from both the Generals and GeneralsMD code trees; FIREWALL_TYPE_NETGEAR_BUG = 8 is renamed FIREWALL_TYPE_UNUSED = 8 to preserve the bitmask slot.
  • GUI (OptionsMenu.cpp, both trees): Static checkSendDelay variables and all save/load/set-defaults calls are removed; the obsolete checkbox is hidden at runtime via #if ENABLE_GUI_HACKS without modifying the .wnd file yet.

Confidence Score: 5/5

Safe to merge — this is a clean removal of a 20-year-old workaround with no surviving callers or side-effects.

Every call site for m_firewallSendDelay, getSendDelay(), and FIREWALL_TYPE_NETGEAR_BUG is consistently removed across both the Generals and GeneralsMD trees. The m_beenProbed field, which might appear orphaned, is still read in connectionUpdate() to stop retrying mangled-port sends after the remote has probed us — so no dead state remains. The gotMangledPort() simplification (always send probe immediately) is the correct unconditional behavior now that the Netgear delay path is gone. The GUI hide-via-winHide approach under ENABLE_GUI_HACKS matches the established pattern in the codebase.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename Overview
Core/GameEngine/Source/GameNetwork/NAT.cpp Removes Netgear-pair ordering in establishConnectionPaths(), probe delay in doThisConnectionRound(), and conditional probe suppression in probed()/gotMangledPort(); m_beenProbed is still read in connectionUpdate() to stop retrying mangled-port sends, so it remains live.
Core/GameEngine/Source/GameNetwork/FirewallHelper.cpp Strips FIREWALL_TYPE_NETGEAR_BUG set-up from detectionBeginUpdate(), removes hardness/retries contributions, and cleans up dead #if(0) block; all remaining logic is unaffected.
Core/GameEngine/Include/GameNetwork/FirewallHelper.h Renames FIREWALL_TYPE_NETGEAR_BUG to FIREWALL_TYPE_UNUSED (preserving value 8 to avoid future bitmask collisions) and removes both isNetgear() overloads.
Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp Removes checkSendDelay static variable and all save/load/set-defaults calls; hides the checkbox at runtime via #if ENABLE_GUI_HACKS (always defined to 1) without touching the .wnd file yet.
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp Mirrors the Generals OptionsMenu changes: removes checkSendDelay state and save/load; hides checkbox via #if ENABLE_GUI_HACKS.
Generals/Code/GameEngine/Include/Common/GlobalData.h Drops m_firewallSendDelay field; no other member changes.
GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h Mirrors Generals GlobalData.h: drops m_firewallSendDelay field.
Core/GameEngine/Source/Common/OptionPreferences.cpp Removes getSendDelay() implementation; remaining preference accessors unchanged.
Core/GameEngine/Include/GameNetwork/NAT.h Removes already-commented-out NATCONNECTIONSTATE_NETGEARDELAY enum value; no other changes.

Sequence Diagram

sequenceDiagram
    participant Local as Local NAT
    participant Mangler as STUN Mangler
    participant Remote as Remote NAT

    Note over Local,Remote: Simplified flow after SendDelay removal

    Local->>Mangler: Probe (STUN request)
    Mangler-->>Local: Mangled port response
    Local->>Remote: gotMangledPort → sendAProbe (immediately, no delay)
    Local->>Remote: notifyTargetOfProbe
    Note over Local: state = WAITINGFORRESPONSE

    Remote->>Local: Probe packet
    Local->>Remote: "probed() → m_beenProbed=TRUE"
    Note over Local: connectionUpdate() stops retrying mangled port sends

    Remote-->>Local: Connection response
    Note over Local,Remote: Connection established (DONE)
Loading

Reviews (4): Last reviewed commit: "review feedback" | Re-trigger Greptile

@bobtista

Copy link
Copy Markdown

Left one question, otherwise this looks good

@githubawn githubawn added GUI For graphical user interface Minor Severity: Minor < Major < Critical < Blocker Network Anything related to network, servers Gen Relates to Generals ZH Relates to Zero Hour labels Jul 27, 2026

@bobtista bobtista left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@xezon xezon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How confident are we that no player never ever needs this delay thing?

Comment thread Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp Outdated
Comment thread Core/GameEngine/Source/GameNetwork/NAT.cpp
Comment thread Core/GameEngine/Source/GameNetwork/NAT.cpp Outdated
Comment thread Core/GameEngine/Include/GameNetwork/FirewallHelper.h
@githubawn

Copy link
Copy Markdown
Author

Highly confident.
All the Send Delay discussion I could find online is anecdotal, always bundled with other fixes or tools, never isolated as the actual cause.
That said, it's part of why I split the networking refactor into separate PRs (3 so far, 4 more planned), each one is independently revertible if something like this turns out to matter for someone.

@xezon

xezon commented Jul 29, 2026

Copy link
Copy Markdown

Ok. I am not opposed to removing it.

@xezon xezon changed the title refactor(network): Remove legacy Send Delay chore(network): Remove legacy Send Delay Jul 30, 2026
@xezon
xezon merged commit 1a7c1ef into TheSuperHackers:main Jul 30, 2026
17 checks passed
@githubawn
githubawn deleted the refactor/remove-send-delay branch August 3, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gen Relates to Generals GUI For graphical user interface Minor Severity: Minor < Major < Critical < Blocker Network Anything related to network, servers ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants