Optimize 1v1 Disconnect Grace Period (Refined #3945)

[berkelmali]

Description:

Add a 30-second grace period before declaring a disconnect winner in 1v1 ranked games. Previously, a momentary WiFi blip would instantly declare the opponent as winner. Now the system waits 300 ticks (30s) and resets the timer if the player reconnects.

This is a highly optimized version of PR #3945. It utilizes a zero-allocation single-pass loop in WinCheckExecution.ts to track connected and disconnected humans instead of performing multiple array filters every 10 ticks, significantly reducing Garbage Collection overhead in the core game loop.

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

barfires

[coderabbitai]

Walkthrough

The pull request adds a 1v1 ranked disconnect grace period preventing immediate wins when players disconnect, re-checks server-side winner consensus on disconnect with tightened voting thresholds, converts game end handling to async error logging, and enforces Bearer token authentication on singleplayer game archival.

Changes

1v1 Ranked Disconnect Grace Period

Layer / File(s)	Summary
Grace timer state and core logic src/core/execution/WinCheckExecution.ts	Adds DISCONNECT_GRACE_TICKS (300 ticks / 30s) constant and disconnectGraceTick instance field. Replaces immediate winner selection logic with conditional grace timer that counts total and connected human players, starts or resumes the grace timer when one or both humans disconnect, declares a winner only after grace duration elapses, and resets the timer on reconnection.
Grace period behavior validation tests/core/executions/WinCheckExecution.test.ts	Adds and updates six test cases covering: single disconnect does not immediately declare winner; grace expiry declares remaining human winner; reconnect resets grace timer; both humans connected keeps execution active; both disconnect and grace expiry declares winner by tile ownership; single human without opponent does not trigger grace logic.

Server-Side Winner Consensus & Validation

Layer / File(s)	Summary
Winner consensus check on disconnect src/server/GameServer.ts	In WebSocket close handler, calls new checkWinnerConsensus() method when game has started. Iterates winnerVotes, identifies candidates exceeding strictly greater than 2:1 IP majority, sets this.winner, logs outcome, and archives the game.
Desync and vote threshold tightening src/server/GameServer.ts	Desync logic now treats all clients as out-of-sync only when out-of-sync count is a strict majority (> half) rather than at least half. Winner-vote acceptance requires strictly greater than 2:1 IP majority, stricter than the prior equality case.

Game Finalization & Singleplayer Authentication

Layer / File(s)	Summary
Async game end phase transition src/server/GameManager.ts	Changes GameManager.tick() to call game.end() as a promise with .catch() error logging instead of synchronous try/catch during GamePhase.Finished transition.
Singleplayer archive authentication and validation src/server/Worker.ts	Adds Bearer token authentication to /api/archive_singleplayer_game endpoint. Extracts authenticated persistentId via verifyClientToken and returns 401 if missing or invalid. Validates authenticated user's persistentID matches the single-player record's persistentID and returns 403 on mismatch.

Sequence Diagrams

sequenceDiagram
  participant Game
  participant WinCheck as WinCheckExecution
  participant Player1 as Player 1
  participant Player2 as Player 2
  Player1->>WinCheck: connected
  Player2->>WinCheck: connected
  Player2->>WinCheck: disconnect
  WinCheck->>WinCheck: start grace timer (tick 0)
  loop tick < 300
    Game->>WinCheck: checkWinnerFFA()
    WinCheck->>WinCheck: grace elapsed < 300 ticks
    WinCheck->>Game: no winner yet
  end
  loop tick >= 300
    Game->>WinCheck: checkWinnerFFA()
    WinCheck->>Game: setWinner(Player1)
  end
  Player2->>WinCheck: reconnect
  WinCheck->>WinCheck: reset grace timer

Loading

sequenceDiagram
  participant WS as WebSocket Client
  participant GS as GameServer
  participant Archive
  WS->>GS: close / disconnect
  alt game started
    GS->>GS: checkWinnerConsensus()
    GS->>GS: scan winnerVotes
    GS->>GS: check > 2:1 IP majority
    alt threshold met
      GS->>GS: set this.winner
      GS->>Archive: archive game
      GS->>GS: log winner decision
    else below threshold
      GS->>GS: no action
    end
  else game not started
    GS->>GS: skip consensus check
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🎮 A disconnect no longer means defeat so fast—
Grace ticks count down like seconds in the past,
Consensus votes now tighter, thresholds refined,
While auth guards singleplayer, no foes combined. ⏳

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: a 1v1 disconnect grace period optimization. It references the refined version of PR #3945 and highlights the performance improvement aspect.
Description check	✅ Passed	The description is directly related to the changeset. It explains the 30-second grace period for 1v1 ranked games, mentions the optimization approach using zero-allocation loops, and covers the reconnect reset behavior.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/server/GameServer.ts (1)

1248-1267: 💤 Low value

Edge case: winner declared when no active clients remain.

When all clients disconnect, activeUniqueIPs.size becomes 0. The condition potentialWinner.ips.size * 2 > 0 is true for any vote count ≥ 1, so a winner is declared based on prior votes.

This is probably fine (honoring votes cast before disconnect), but verify this matches intended behavior. If no winner should be declared when zero clients remain, add a guard:

  private checkWinnerConsensus() {
    if (this.winner !== null) {
      return;
    }

    const activeUniqueIPs = new Set(this.activeClients.map((c) => c.ip));
+   if (activeUniqueIPs.size === 0) {
+     return;
+   }

    for (const [winnerKey, potentialWinner] of this.winnerVotes.entries()) {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/GameServer.ts` around lines 1248 - 1267, The checkWinnerConsensus
method can declare a winner when activeUniqueIPs.size is 0; add a guard at the
start of checkWinnerConsensus to return early if there are no active unique IPs
to avoid declaring winners after all clients disconnect. Update the function
(checkWinnerConsensus) to check activeClients/activeUniqueIPs size and skip the
winnerVotes loop (and avoid calling archiveGame or setting this.winner) when
activeUniqueIPs.size === 0 so prior votes don't trigger a win in that edge case.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/server/GameServer.ts`:
- Around line 1248-1267: The checkWinnerConsensus method can declare a winner
when activeUniqueIPs.size is 0; add a guard at the start of checkWinnerConsensus
to return early if there are no active unique IPs to avoid declaring winners
after all clients disconnect. Update the function (checkWinnerConsensus) to
check activeClients/activeUniqueIPs size and skip the winnerVotes loop (and
avoid calling archiveGame or setting this.winner) when activeUniqueIPs.size ===
0 so prior votes don't trigger a win in that edge case.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8c351041-0cc9-4185-b845-de204531d296

📥 Commits

Reviewing files that changed from the base of the PR and between 0ace428 and 64d0c0d.

📒 Files selected for processing (5)

• src/core/execution/WinCheckExecution.ts
• src/server/GameManager.ts
• src/server/GameServer.ts
• src/server/Worker.ts
• tests/core/executions/WinCheckExecution.test.ts