mirror of
https://github.com/tiennm99/gomoku.git
synced 2026-08-23 02:24:17 +00:00
fix(exit): distinguish self vs peer ClientExitResponse
Fixes the server log "gameover player N: unexpected Request_CreateRoom". Root cause: when ANY player leaves a room, leaveRoom broadcasts ClientExitResponse to ALL players in room.Players (so peers see who left). The broadcast fired CLIENT_EXIT on both the exiting client AND its peer. But the client's handlers treated every CLIENT_EXIT as "I exited" — transitioning to lobby, clearing gameState.roomId, hiding the HUD. Meanwhile the peer's server-side state machine was still in gameoverState. The peer then clicked Create Room from the (now wrongly shown) lobby, and the request arrived at gameoverState's default case. Client: - New services/client-exit-helpers.js with isSelfExit(data, clientId). exitClientId === 0 or missing → bare self-ack (home.ClientExit / watching.ClientExit / room-closed eject). exitClientId === clientId → our own leaveRoom broadcast loopback. Anything else → a peer left; stay in state. - game-state-service CLIENT_EXIT handler: only reset() when isSelfExit. - game-scene._onClientExit: only transition to MenuScene when isSelfExit; peer exits render a toast "<nickname> left the room" instead. - menu-ui.js module CLIENT_EXIT handler: only showLobby() when isSelfExit. Server: the client fix alone leaves a new hole — after a peer leaves a finished PVP room, the remaining player sits in gameoverState with a game-over modal and two buttons. "Play Again" would try to restart a PVP game with only one human; "Leave" still works. To close this hole cleanly: - state/waiting.go: add kickStaleRoomPeers helper. After a leaveRoom in gameoverState, if the room is a PVP room with < 2 remaining human players it's unplayable — push a synthetic Request_ClientExit onto each remaining peer's CmdCh. Their gameoverState goroutine wakes up, processes the synthetic exit, and returns StateHome in lockstep. - state/gameover.go: call kickStaleRoomPeers after leaveRoom in the ClientExit case. Tests: - Rename TestFlow_CreateRoomAfterOpponentForfeit → TestFlow_OpponentExit_KicksPeerFromGameover and rewrite to assert the auto-kick behavior: black leaves → white's gameoverState must return StateHome WITHOUT any test-driven CmdCh push. Then both players can create fresh rooms. Full suite + client build green.
This commit is contained in:
@@ -12,6 +12,7 @@ import { eventBus } from '../services/event-bus.js';
|
||||
import { gameState } from '../services/game-state-service.js';
|
||||
import { connectionService } from '../services/connection-service.js';
|
||||
import { ClientEventCode } from '../config/protocol-constants.js';
|
||||
import { isSelfExit } from '../services/client-exit-helpers.js';
|
||||
import {
|
||||
showGameHud,
|
||||
updateTurnIndicator,
|
||||
@@ -218,11 +219,20 @@ export class GameScene extends Phaser.Scene {
|
||||
}
|
||||
|
||||
/**
|
||||
* Opponent disconnected or client exited — return to menu lobby.
|
||||
* MenuScene.create() checks gameState.nickname and shows the lobby directly.
|
||||
* ClientExitResponse received. Distinguish self vs peer:
|
||||
* - self (exitClientId === 0 as an ack, OR === our clientId): transition to lobby
|
||||
* - peer: stay in GameScene; the corresponding GameOverResponse will
|
||||
* show the game-over modal via forfeit. Showing a toast here keeps
|
||||
* the player informed without silently desyncing their server state.
|
||||
* @param {{ exitClientId?: number, exitClientNickname?: string }} data
|
||||
* @private
|
||||
*/
|
||||
_onClientExit() {
|
||||
_onClientExit(data) {
|
||||
if (!isSelfExit(data, gameState.clientId)) {
|
||||
const who = (data && data.exitClientNickname) ? data.exitClientNickname : 'Opponent';
|
||||
showToast(`${who} left the room`, 'info');
|
||||
return;
|
||||
}
|
||||
hideGameHud();
|
||||
this._cleanup();
|
||||
this.scene.start('MenuScene');
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
/**
|
||||
* Helpers for interpreting ClientExitResponse.
|
||||
*
|
||||
* The server broadcasts ClientExitResponse to ALL players in the room when
|
||||
* anyone leaves (so peers see "opponent left"), AND uses the same message
|
||||
* type as a self-ack from homeState / watchingState. Clients must
|
||||
* distinguish self from peer — otherwise a peer's exit forces the client
|
||||
* to transition to lobby while the server keeps the client in gameoverState,
|
||||
* causing a state desync where subsequent requests log as "unexpected".
|
||||
*
|
||||
* Rules:
|
||||
* - exitClientId === 0 or missing → self ack (server path that sends a
|
||||
* bare ClientExitResponse; e.g., home.ClientExit, watching.ClientExit,
|
||||
* spectator room-closed eject). Treat as self.
|
||||
* - exitClientId === this client's clientId → self (our own leaveRoom
|
||||
* broadcast reached us through room.Players iteration).
|
||||
* - Otherwise → a peer left; consumer should stay in state and render
|
||||
* a "X left" notification.
|
||||
*
|
||||
* clientId is passed in explicitly (rather than reading gameState here)
|
||||
* to keep this module free of circular imports with game-state-service.
|
||||
*
|
||||
* @module client-exit-helpers
|
||||
*/
|
||||
|
||||
/**
|
||||
* Return true if the incoming ClientExitResponse refers to this client.
|
||||
* @param {{ exitClientId?: number }|null|undefined} data
|
||||
* @param {number|null} clientId - this client's server-assigned ID
|
||||
* @returns {boolean}
|
||||
*/
|
||||
export function isSelfExit(data, clientId) {
|
||||
if (!data) return true; // defensive: no data → treat as self to avoid ghost state
|
||||
const id = data.exitClientId;
|
||||
if (id == null || id === 0) return true; // bare ack from server
|
||||
return id === clientId;
|
||||
}
|
||||
@@ -7,6 +7,7 @@
|
||||
|
||||
import { eventBus } from './event-bus.js';
|
||||
import { ClientEventCode } from '../config/protocol-constants.js';
|
||||
import { isSelfExit } from './client-exit-helpers.js';
|
||||
|
||||
/**
|
||||
* @typedef {Object} MoveEntry
|
||||
@@ -141,7 +142,12 @@ class GameStateService {
|
||||
console.warn('[GameState] spectator cannot perform this action');
|
||||
});
|
||||
|
||||
eventBus.on(ClientEventCode.CLIENT_EXIT, () => this.reset());
|
||||
// Only reset when WE exited, not when a peer leaves a shared room.
|
||||
// Otherwise a peer's exit would blow away our own roomId and board,
|
||||
// desyncing us from the server's gameoverState.
|
||||
eventBus.on(ClientEventCode.CLIENT_EXIT, (data) => {
|
||||
if (isSelfExit(data, this.clientId)) this.reset();
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import { ClientEventCode } from '../config/protocol-constants.js';
|
||||
import { showToast } from './game-ui.js';
|
||||
import { eventBus } from '../services/event-bus.js';
|
||||
import { showRoomList, showPveDifficultyPanel, showWaiting } from './menu-ui-rooms.js';
|
||||
import { isSelfExit } from '../services/client-exit-helpers.js';
|
||||
|
||||
// Re-export showRoomList and showWaiting so menu-scene.js imports from one place
|
||||
export { showRoomList, showWaiting };
|
||||
@@ -116,5 +117,8 @@ eventBus.on(ClientEventCode.NICKNAME_SET, (data) => {
|
||||
}
|
||||
});
|
||||
|
||||
// Return to lobby when client exits or is kicked
|
||||
eventBus.on(ClientEventCode.CLIENT_EXIT, showLobby);
|
||||
// Return to lobby only when WE exit. Peer exits are rendered as toasts
|
||||
// by game-scene._onClientExit; they must not blow away our menu state.
|
||||
eventBus.on(ClientEventCode.CLIENT_EXIT, (data) => {
|
||||
if (isSelfExit(data, gameState.clientId)) showLobby();
|
||||
});
|
||||
|
||||
+21
-17
@@ -675,15 +675,17 @@ func TestFlow_CreateRoomAfterPvpRematchLeave(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_CreateRoomAfterOpponentForfeit: the other player disconnects
|
||||
// during the game via ClientExit. The remaining player sees GameOver via
|
||||
// forfeit broadcast + ClientExit notification. They end up in gameoverState
|
||||
// and must be able to leave + create a new room.
|
||||
func TestFlow_CreateRoomAfterOpponentForfeit(t *testing.T) {
|
||||
black, white, room := setupFinishedPvpRoom(t)
|
||||
_ = room
|
||||
// TestFlow_OpponentExit_KicksPeerFromGameover verifies the regression fix
|
||||
// for the server log "gameover player N: unexpected Request_CreateRoom":
|
||||
// when one player leaves a finished PVP room, the remaining peer should
|
||||
// NOT be left sitting in gameoverState — kickStaleRoomPeers injects a
|
||||
// synthetic ClientExit so their goroutine auto-transitions to home.
|
||||
// Both players must then be free to create fresh rooms.
|
||||
func TestFlow_OpponentExit_KicksPeerFromGameover(t *testing.T) {
|
||||
black, white, _ := setupFinishedPvpRoom(t)
|
||||
|
||||
// Black clicks Leave from gameoverState — room still has white inside.
|
||||
// Black leaves the finished PVP room → kickStaleRoomPeers should push
|
||||
// a synthetic ClientExit onto white.CmdCh.
|
||||
black.CmdCh <- &protocol.Request{
|
||||
Payload: &protocol.Request_ClientExit{ClientExit: &protocol.ClientExitRequest{}},
|
||||
}
|
||||
@@ -694,21 +696,24 @@ func TestFlow_CreateRoomAfterOpponentForfeit(t *testing.T) {
|
||||
t.Errorf("black.RoomID = %d, want 0", black.RoomID)
|
||||
}
|
||||
|
||||
// White now sends ClientExit — room should be deleted.
|
||||
white.CmdCh <- &protocol.Request{
|
||||
Payload: &protocol.Request_ClientExit{ClientExit: &protocol.ClientExitRequest{}},
|
||||
// White's CmdCh should contain a synthetic ClientExit queued by
|
||||
// kickStaleRoomPeers. Running gameoverState on white should consume
|
||||
// it and transition to StateHome WITHOUT any test-driven input.
|
||||
next, err := runState(consts.StateGameOver, white)
|
||||
if err != nil {
|
||||
t.Fatalf("white auto-exit from kick: %v", err)
|
||||
}
|
||||
if _, err := runState(consts.StateGameOver, white); err != nil {
|
||||
t.Fatalf("white gameover exit: %v", err)
|
||||
if next != consts.StateHome {
|
||||
t.Fatalf("white expected StateHome after synthetic kick, got %d", next)
|
||||
}
|
||||
if white.RoomID != 0 {
|
||||
t.Errorf("white.RoomID = %d, want 0", white.RoomID)
|
||||
t.Errorf("white.RoomID = %d after auto-kick, want 0", white.RoomID)
|
||||
}
|
||||
if len(lobby.GetAllRooms()) != 0 {
|
||||
t.Errorf("expected 0 rooms after both players exit, got %d", len(lobby.GetAllRooms()))
|
||||
t.Errorf("expected 0 rooms after both players kicked, got %d", len(lobby.GetAllRooms()))
|
||||
}
|
||||
|
||||
// Both players should now be able to create new rooms.
|
||||
// Both players should now be able to create fresh rooms from home.
|
||||
for _, p := range []*lobby.Player{black, white} {
|
||||
drainSend(p)
|
||||
p.CmdCh <- &protocol.Request{
|
||||
@@ -726,7 +731,6 @@ func TestFlow_CreateRoomAfterOpponentForfeit(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Two new rooms in the store now.
|
||||
if len(lobby.GetAllRooms()) != 2 {
|
||||
t.Errorf("expected 2 rooms (one per player), got %d", len(lobby.GetAllRooms()))
|
||||
}
|
||||
|
||||
@@ -60,6 +60,12 @@ func (*gameOverState) Next(player *lobby.Player) (consts.StateID, error) {
|
||||
|
||||
case *protocol.Request_ClientExit:
|
||||
leaveRoom(player, room)
|
||||
// If this left a PVP peer alone in a now-unplayable room,
|
||||
// push a synthetic ClientExit onto their CmdCh so their
|
||||
// gameoverState goroutine wakes up and transitions home too
|
||||
// (instead of silently sitting in gameover while the client
|
||||
// believes it's in the lobby — the exact desync reported).
|
||||
kickStaleRoomPeers(room)
|
||||
return consts.StateHome, nil
|
||||
|
||||
default:
|
||||
|
||||
+40
-1
@@ -96,7 +96,8 @@ func assignColors(room *lobby.Room) {
|
||||
// leaveRoom removes player from their current room cleanly.
|
||||
// Broadcasts ClientExitResponse to everyone in the room BEFORE removing the
|
||||
// player, so the exiting player also receives their own exit confirmation
|
||||
// (used by the client to transition UI back to the lobby).
|
||||
// (used by the client to transition UI back to the lobby). The client
|
||||
// distinguishes self vs peer via exit_client_id.
|
||||
func leaveRoom(player *lobby.Player, room *lobby.Room) {
|
||||
exitResp := &protocol.Response{
|
||||
Payload: &protocol.Response_ClientExit{
|
||||
@@ -112,3 +113,41 @@ func leaveRoom(player *lobby.Player, room *lobby.Room) {
|
||||
// Then remove from the room.
|
||||
lobby.LeaveRoom(player)
|
||||
}
|
||||
|
||||
// kickStaleRoomPeers pushes a synthetic ClientExitRequest onto every remaining
|
||||
// player's CmdCh if the room is now unplayable (PVP with fewer than 2 human
|
||||
// players). Used after leaveRoom in the gameover path to prevent peers from
|
||||
// sitting in gameoverState forever once the game cannot continue.
|
||||
//
|
||||
// Safe to call with a room that was already deleted from the store — the
|
||||
// caller holds a reference; we just drain room.Players for peers still around.
|
||||
func kickStaleRoomPeers(room *lobby.Room) {
|
||||
room.RLock()
|
||||
roomType := room.RoomType
|
||||
peers := make([]*lobby.Player, 0, len(room.Players))
|
||||
for _, p := range room.Players {
|
||||
peers = append(peers, p)
|
||||
}
|
||||
room.RUnlock()
|
||||
|
||||
if roomType != lobby.RoomTypePvp {
|
||||
return
|
||||
}
|
||||
if len(peers) >= 2 {
|
||||
return // still playable
|
||||
}
|
||||
|
||||
syntheticExit := &protocol.Request{
|
||||
Payload: &protocol.Request_ClientExit{ClientExit: &protocol.ClientExitRequest{}},
|
||||
}
|
||||
for _, p := range peers {
|
||||
if p.CmdCh == nil {
|
||||
continue
|
||||
}
|
||||
select {
|
||||
case p.CmdCh <- syntheticExit:
|
||||
default:
|
||||
// CmdCh full — idle reaper will clean up the room eventually.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user