[tintinhamans]

Fixes #292
Fixes #2013

DisconnectManager::processDisconnectPlayer removes a player that quits from the disconnect screen on whatever frame each player got the leave on, and nobody sends a destroy for it. A player the leave misses waits for a disconnect command only the packet router sends. So players can be stuck on the disconnect screen for good (the DC bug), remove the quitter on different frames, and its objects stay.

Every player that stays now sends the destroy, and the next packet router passes the leave on to players stalled on its frame. A disconnect command also has to come from the address of the player it claims to be from, so a faked leave can't destroy someone's objects. This is retail compatible.

Non-retail builds go further. A leave only marks the player, and the acting packet router removes it on one frame for everyone, same as it already does for a timed out or voted out player.

Scripted 4 player LAN games, 3 runs per case:

Case	Retail	Retail compatible	Non-retail compatible
Crash + quit, leave to one player lost	0/3, DC bug	3/3, 20 s	3/3, 6 s
Crash + quit	3/3, 6 s, objects stay	3/3, 6 s	3/3, 6 s
Host ahead quits, leave to one player lost	0/3, DC bug	0/3, DC bug	3/3, 7 s
Host ahead quits	3/3, 6 s, objects stay	3/3, 6 s	3/3, 7 s
Joiner ahead quits, leave to one player lost	0/3, DC bug	0/3, DC bug	3/3, 7 s
90 s lag + quit	3/3, 6 s, objects stay	3/3, 6 s	3/3, 6 s

[coderabbitai]

Walkthrough

The networking code records the sender slot for incoming packets and uses it to validate disconnect commands. DisconnectManager adds non-retail leaver tracking and retail-compatible relay handling. It also updates voting, frame selection, player cleanup, and disconnect-screen progression.

Changes

Network Disconnect Handling

Layer / File(s)	Summary
Track incoming packet senders Core/GameEngine/Include/GameNetwork/ConnectionManager.h, Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp	ConnectionManager exposes the current sender slot and resolves incoming packet addresses and ports to connected slots. It sets the slot while processing incoming packets and resets it afterward.
Validate and route disconnect commands Core/GameEngine/Include/GameNetwork/DisconnectManager.h, Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp	DisconnectManager validates commands against the current sender, rejects already-disconnected non-local targets, and sends each player-destruction command at most once per slot. Non-retail removals from a non-acting router can be held with their sender and frame.
Track non-retail leavers and update disconnect state Core/GameEngine/Include/GameNetwork/DisconnectManager.h, Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp	Non-retail builds track leavers, notify other players, and clear the leaver flag when a keepalive arrives. The code updates timeout and vote calculations, maximum-frame selection, and disconnect-screen progression while a leaver remains connected.
Relay retail-compatible disconnect commands Core/GameEngine/Include/GameNetwork/DisconnectManager.h, Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp	Retail-compatible builds store pending relay frames and recipients. The update path sends relays to connected players whose received disconnect frame matches the stored frame.

Priority: Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Peer
  participant ConnectionManager
  participant DisconnectManager
  participant OtherPeers
  Peer->>ConnectionManager: Send packet with address and port
  ConnectionManager->>ConnectionManager: Resolve sender slot
  ConnectionManager->>DisconnectManager: Process disconnect command with current sender slot
  alt Non-retail networking
    DisconnectManager->>OtherPeers: Notify peers of a leave
  else Retail-compatible networking
    DisconnectManager->>OtherPeers: Send pending relay when the disconnect frame matches
  end

Loading

Possibly related PRs

• TheSuperHackers/GeneralsGameCode#3612: Updates the disconnect-screen path to send a queued leave immediately; this change processes incoming leave commands.

Suggested reviewers: bobtista, xezon

Merge Risk: Moderate · up to 31331

A late disconnect command can leave players with inconsistent match state. Resolve that frame-handling issue before merging; malformed disconnect packets can also interrupt debug sessions.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Linked Issues check		[#2013] The change addresses asset removal. processDisconnectPlayer validates the sender, and the staying players send sendPlayerDestruct for the quitting player. [#292] The retail-compatible path…	Fix the retail-compatible lost-leave path for host and joiner cases. Prevent ALT from starting the disconnect countdown when ALT is used for unit waypoints. Add automated regression coverage for the missed-leave and asset-removal cases.

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title clearly identifies the main change: fixing disconnect-screen failures and leftover objects when a player quits.
Description check		The description directly explains the disconnect handling changes, affected networking modes, security validation, and test results.
Out of Scope Changes check		The changes remain within [#292] and [#2013]. Sender validation protects disconnect processing. Leave relays and synchronized removal address the stuck disconnect screen. sendPlayerDestruct addresse…

Full details: Out of Scope Changes check

Explanation

The changes remain within [#292] and [#2013]. Sender validation protects disconnect processing. Leave relays and synchronized removal address the stuck disconnect screen. sendPlayerDestruct addresses leftover player assets. Non-retail frame and packet-router handling supports the same disconnect flow. No unrelated product behavior is shown in the available change summary.

Full details: Linked Issues check

Explanation

[#2013] The change addresses asset removal. processDisconnectPlayer validates the sender, and the staying players send sendPlayerDestruct for the quitting player. [#292] The retail-compatible path remains incomplete. The supplied results report 0/3 success when a lost leave targets a host or joiner that is ahead, so the disconnect screen can still remain stuck. The ALT-triggered disconnect countdown is also not addressed. The available validation is manual LAN testing; no automated regression test is identified.

Resolution

Fix the retail-compatible lost-leave path for host and joiner cases. Prevent ALT from starting the disconnect countdown when ALT is used for unit waypoints. Add automated regression coverage for the missed-leave and asset-removal cases.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new blocking issue was found.

Summary

Fixes cleanup and recovery when a player quits from the disconnect screen.

• Retail-compatible games clean up quitters and pass missed leaves to stalled peers.
• Non-retail games coordinate quit removal through the acting packet router.
• Disconnect commands must come from the player they name.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Receive disconnect command] --> B{Sender address matches?}
  B -->|No| C[Ignore command]
  B -->|Yes| D{Networking mode}
  D -->|Retail-compatible| E[Remove player and send cleanup]
  E --> F[Relay leave to players on the same frame]
  D -->|Non-retail| G{Leave or removal?}
  G -->|Leave| H[Mark player and share notice]
  H --> I[Agree on removal frame]
  G -->|Removal| J{Sender is acting router?}
  J -->|Yes| K[Remove player]
  J -->|No| L[Save removal]
  L --> M[Check sender and frame again]
  M --> K

Loading

_{Reviews (3) · Last reviewed commit: "bugfix(disconnectmanager): Remove a leav..." · Reviewed by Greptile}

[coderabbitai]

Actionable comments posted: 4

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9d4db0d0-aa0b-41c9-bafa-5df1a2f560d4

Commits

Reviewing files that changed from the base of the PR and between 68d1668 and 2706f0b.

Files selected for processing: 4

• Core/GameEngine/Include/GameNetwork/ConnectionManager.h
• Core/GameEngine/Include/GameNetwork/DisconnectManager.h
• Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp
• Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[coderabbitai]

Actionable comments posted: 1

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: c5cfb5ae-04da-4049-8603-10e8d6cb92ad

Commits

Reviewing files that changed from the base of the PR and between d777a0c and 49e4900.

Files selected for processing: 2

• Core/GameEngine/Include/GameNetwork/DisconnectManager.h
• Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[coderabbitai]

Actionable comments posted: 2

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7783430f-8dfa-4186-9bac-90d160df8e60

Commits

Reviewing files that changed from the base of the PR and between 49e4900 and 31331c7.

Files selected for processing: 2

• Core/GameEngine/Include/GameNetwork/DisconnectManager.h
• Core/GameEngine/Source/GameNetwork/DisconnectManager.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.