Skip to content

Post-match lobby cleanup and network concede fixes#10410

Merged
tool4ever merged 5 commits intoCard-Forge:masterfrom
MostCromulent:post-match-cleanup
Apr 17, 2026
Merged

Post-match lobby cleanup and network concede fixes#10410
tool4ever merged 5 commits intoCard-Forge:masterfrom
MostCromulent:post-match-cleanup

Conversation

@MostCromulent
Copy link
Copy Markdown
Contributor

Summary

Addresses Phase 0.5 of the network draft/sealed implementation plan — hardening existing network match lifecycle before adding new game modes. Also fixes several pre-existing network concede bugs observed during testing. See discussion at Card-Forge/forge#9861 (comment) and follow-up.

Issues addressed

  • No lobby cleanup after network match — the previous match's state lingered in the lobby after returning. Added onMatchOver callback to reset lobby state (null hostedMatch, un-ready slots, clear player GUIs, push lobby update to clients).

  • Client concede dialog never appearedinstanceof PlayerControllerHuman excluded NetGameController, so the concede confirmation was silently skipped. Broadened the check to include network controllers.

  • Client concede force-quit both players — client sent a premature nextGameDecision(QUIT) before the server processed the concede, ending the match for both. Network games now let the server drive the game-end flow.

  • Game thread blocked after concede — only the conceding player's input latch was released, but the game thread could be waiting on the other player's input queue. Now releases all players' input latches.

  • Remote client never saw win/lose screen — game-end events were buffered in GameEventForwarder and never flushed. Added explicit flush after the game loop exits.

  • NPEs from late-arriving messages — protocol messages arriving after cleanup caused NullPointerException on the server. Added null guards in the protocol handler with null reply for non-void methods.

Why these changes are safe

  • Lobby cleanup only runs when a match ends via NextGameDecision.QUIT — the same path that already calls endCurrentGame(). It adds cleanup after existing teardown, doesn't change teardown order.
  • Concede dialog fix only affects network clients. Local games still go through the PlayerControllerHuman path with the per-player outcome check. No behavior change for single-player or local multiplayer.
  • Skipping premature QUIT only applies to network games (isNetGame() guard). Local games still send nextGameDecision synchronously as before.
  • Input latch release is idempotent — CountDownLatch.countDown() on an already-released latch is a no-op.
  • Protocol null guards only fire after cleanup when controllers are already gone. During normal gameplay, getToInvoke() and getClient() always return non-null.

🤖 Generated with Claude Code

MostCromulent and others added 2 commits April 15, 2026 07:14
When a match ends, the server now cleans up properly: nulls
hostedMatch, resets slot ready states, removes forwarder observers
from InputQueues, clears cached RemoteClientGuiGame instances, and
broadcasts a fresh LobbyUpdateEvent so clients refresh their lobby UI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Network concede was broken: the client's concede dialog never appeared
(instanceof check excluded NetGameController), the game thread stayed
blocked after concede (only the conceding player's input queue was
released), and game-end events weren't flushed to remote clients.

- AbstractGuiGame.concede(): include network controllers in concede
  check; don't send premature nextGameDecision for network games
- PlayerControllerHuman.concede(): release all players' input latches
  after game ends, not just the conceding player's
- HostedMatch: flush GameEventForwarder after game loop exits so
  remote clients receive GameEventGameFinished
- GameProtocolHandler: null-guard getToInvoke() for messages arriving
  after cleanup, with null reply for non-void methods
- GameServerHandler: null-guard getClient() for deregistered clients

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread forge-gui/src/main/java/forge/gamemodes/match/HostedMatch.java
Comment thread forge-gui/src/main/java/forge/player/PlayerControllerHuman.java
MostCromulent and others added 2 commits April 16, 2026 19:09
…comment

Fold the new observer-deletion loop into the existing per-controller
cleanup loop in HostedMatch.endCurrentGame(). Each iteration now captures
the forwarder, deletes observers, then calls shutdownForwarder — the
ordering invariant is enforced by adjacency rather than loop split.

Update the input-latch-release comment in PlayerControllerHuman.concede()
to explain the actual reason the loop is needed: remote-client controllers
on the server have no FControlGameEventHandler subscribed, so their input
queues are not released by the normal event path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread forge-gui/src/main/java/forge/gamemodes/match/GameLobby.java Outdated
GameLobby.onMatchOver unconditionally cleared isReady on every slot,
which broke LocalLobby: the human slot starts ready and has no UI to
re-ready, so the second match failed with "Player X is not ready".
Move the reset into ServerGameLobby.onMatchOver so network clients
still re-ready between matches while local play is unaffected.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@tool4ever tool4ever merged commit 4ca70fd into Card-Forge:master Apr 17, 2026
2 checks passed
@MostCromulent MostCromulent deleted the post-match-cleanup branch April 17, 2026 06:15
MostCromulent added a commit to MostCromulent/forge that referenced this pull request Apr 17, 2026
Master added an onMatchOver override on ServerGameLobby for post-match
cleanup (PR <!-- -->Card-Forge#10410). Conflicted with this branch's event-lifecycle
additions at the end of the class body. Resolved by keeping both: our
createEvent/selectEventForMatch/getDraftHost block + master's
onMatchOver override.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants