mirror of
https://github.com/tiennm99/gomoku.git
synced 2026-07-16 22:17:00 +00:00
fix(rematch): sync both player goroutines on GameReset via RematchCh
Bug: after a PVP game ends, clicking "Play Again" cleared the board on
both clients but subsequent moves did nothing — the game was frozen.
Root cause: both players land in gameoverState, each goroutine blocked
on its own CmdCh. When Player A sent GameResetRequest, A's goroutine
reset the room and transitioned to gamePvpState — but Player B's
goroutine was still blocked in gameoverState. B's moves arrived at
CmdCh, gameoverState switch-case didn't match GameMoveRequest, and the
default branch silently dropped them. The board was reset (visible via
the broadcast) but only A was actually in the game on the server.
Fix: add lobby.Room.RematchCh (pattern mirrors StartCh in waiting state).
First goroutine entering gameoverState lazily creates it. gameoverState
now selects on CmdCh OR RematchCh. handleGameReset closes RematchCh
after resetting the room, waking the peer goroutine so both transition
to gamePvpState in lockstep.
Tests (new):
- server/state/flow_test.go:
* TestFlow_HomeCreateLeaveCreate_OneRoomVisible — drives state machine
home→create→waiting→exit→home→create and asserts the lobby store
contains exactly 1 room (guards against ghost rooms after leave)
* TestFlow_RoomListReflectsCreateLeaveCycle — explicit create/leave/
create assertion at each step
* TestFlow_HomeExit_DoesNotKillSession — verify home.ClientExit stays
in home and emits ClientExitResponse
* TestFlow_Rematch_BothPlayersTransition — two player goroutines in
gameoverState, one sends GameReset, both must reach gamePvpState
within 500ms (regression guard for the reported bug)
* TestFlow_Rematch_NewGameAcceptsMovesFromBoth — after rematch, feed
a GameMove and verify the board is mutated and a GameMoveSuccess
response is emitted
- server/lobby/lobby_flow_test.go:
* 7 flow-level tests covering create/leave/create/find cycles, PVE
leave cleanup, two-player leave preserving the room, idempotent
leave, and list consistency across concurrent users
All tests green: go vet + go test ./... + npm run build.
NOTE on "ghost room after leave": the failing-case tests in this suite
prove the server store + state machine correctly delete empty rooms on
leave. The lobby package never showed a ghost. If the symptom persists
after rebuilding, the residue is client-side DOM caching and needs a
browser refresh.
This commit is contained in:
@@ -0,0 +1,220 @@
|
||||
package lobby
|
||||
|
||||
import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
// Flow-level tests exercising create/leave/find cycles reported as buggy.
|
||||
// If any of these fail, the bug is in the lobby package.
|
||||
// If they all pass, the bug is upstream (state machine or client).
|
||||
|
||||
// TestFlow_CreateLeaveCreate_OneRoomVisible reproduces the user-reported
|
||||
// scenario: create a room, leave it, create another, list rooms — expect
|
||||
// only the second (active) room. Prior bug reports showed a "ghost" room.
|
||||
func TestFlow_CreateLeaveCreate_OneRoomVisible(t *testing.T) {
|
||||
resetStore()
|
||||
alice := RegisterPlayer("alice")
|
||||
|
||||
// Create Room 1 and join it.
|
||||
r1, err := CreatePvpRoom(alice)
|
||||
if err != nil {
|
||||
t.Fatalf("CreatePvpRoom 1: %v", err)
|
||||
}
|
||||
if err := JoinRoom(r1.ID, alice); err != nil {
|
||||
t.Fatalf("JoinRoom r1: %v", err)
|
||||
}
|
||||
|
||||
// Leave Room 1 — should delete it (sole occupant).
|
||||
LeaveRoom(alice)
|
||||
|
||||
if _, ok := GetRoom(r1.ID); ok {
|
||||
t.Errorf("Room %d still in store after leave as sole occupant", r1.ID)
|
||||
}
|
||||
if alice.RoomID != 0 {
|
||||
t.Errorf("alice.RoomID = %d after leave, want 0", alice.RoomID)
|
||||
}
|
||||
|
||||
// Create Room 2 and join it.
|
||||
r2, err := CreatePvpRoom(alice)
|
||||
if err != nil {
|
||||
t.Fatalf("CreatePvpRoom 2: %v", err)
|
||||
}
|
||||
if err := JoinRoom(r2.ID, alice); err != nil {
|
||||
t.Fatalf("JoinRoom r2: %v", err)
|
||||
}
|
||||
if r2.ID == r1.ID {
|
||||
t.Errorf("Room 2 reused ID %d — expected monotonic nextRoomID", r1.ID)
|
||||
}
|
||||
|
||||
// GetAllRooms should return exactly one room (the active r2).
|
||||
rooms := GetAllRooms()
|
||||
if len(rooms) != 1 {
|
||||
ids := make([]int64, 0, len(rooms))
|
||||
for _, r := range rooms {
|
||||
ids = append(ids, r.ID)
|
||||
}
|
||||
t.Fatalf("expected 1 room after create→leave→create, got %d rooms: %v", len(rooms), ids)
|
||||
}
|
||||
if rooms[0].ID != r2.ID {
|
||||
t.Errorf("expected room ID %d, got %d", r2.ID, rooms[0].ID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_CreateLeave_DeletesRoom: basic sanity check that a sole-occupant
|
||||
// leave removes the room from the store.
|
||||
func TestFlow_CreateLeave_DeletesRoom(t *testing.T) {
|
||||
resetStore()
|
||||
alice := RegisterPlayer("alice")
|
||||
|
||||
r, _ := CreatePvpRoom(alice)
|
||||
_ = JoinRoom(r.ID, alice)
|
||||
|
||||
if len(GetAllRooms()) != 1 {
|
||||
t.Fatalf("expected 1 room after create+join, got %d", len(GetAllRooms()))
|
||||
}
|
||||
|
||||
LeaveRoom(alice)
|
||||
|
||||
if len(GetAllRooms()) != 0 {
|
||||
t.Errorf("expected 0 rooms after sole-occupant leave, got %d", len(GetAllRooms()))
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_CreateLeaveLeaveLeave_NoGhost: repeated create/leave cycles must
|
||||
// not leak rooms.
|
||||
func TestFlow_CreateLeaveLeaveLeave_NoGhost(t *testing.T) {
|
||||
resetStore()
|
||||
alice := RegisterPlayer("alice")
|
||||
|
||||
for i := 0; i < 5; i++ {
|
||||
r, err := CreatePvpRoom(alice)
|
||||
if err != nil {
|
||||
t.Fatalf("cycle %d: CreatePvpRoom: %v", i, err)
|
||||
}
|
||||
if err := JoinRoom(r.ID, alice); err != nil {
|
||||
t.Fatalf("cycle %d: JoinRoom: %v", i, err)
|
||||
}
|
||||
LeaveRoom(alice)
|
||||
|
||||
rooms := GetAllRooms()
|
||||
if len(rooms) != 0 {
|
||||
ids := make([]int64, 0, len(rooms))
|
||||
for _, r := range rooms {
|
||||
ids = append(ids, r.ID)
|
||||
}
|
||||
t.Fatalf("cycle %d: expected 0 rooms after leave, got %d: %v", i, len(rooms), ids)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_TwoPlayers_OneLeaves_RoomStays: with 2 players in a PVP room, one
|
||||
// leaving should NOT delete the room — the other is still there.
|
||||
func TestFlow_TwoPlayers_OneLeaves_RoomStays(t *testing.T) {
|
||||
resetStore()
|
||||
alice := RegisterPlayer("alice")
|
||||
bob := RegisterPlayer("bob")
|
||||
|
||||
r, _ := CreatePvpRoom(alice)
|
||||
_ = JoinRoom(r.ID, alice)
|
||||
_ = JoinRoom(r.ID, bob)
|
||||
|
||||
LeaveRoom(alice)
|
||||
|
||||
if _, ok := GetRoom(r.ID); !ok {
|
||||
t.Fatal("room deleted after only one of two players left")
|
||||
}
|
||||
r2, _ := GetRoom(r.ID)
|
||||
if len(r2.Players) != 1 {
|
||||
t.Errorf("expected 1 remaining player, got %d", len(r2.Players))
|
||||
}
|
||||
if r2.OwnerID != bob.ID {
|
||||
t.Errorf("ownership should have transferred to bob (%d), got %d", bob.ID, r2.OwnerID)
|
||||
}
|
||||
|
||||
// Bob leaves too — now room should be deleted.
|
||||
LeaveRoom(bob)
|
||||
if _, ok := GetRoom(r.ID); ok {
|
||||
t.Error("room not deleted after both players left")
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_CreatePveLeave_DeletesRoom: PVE rooms also get deleted on leave.
|
||||
func TestFlow_CreatePveLeave_DeletesRoom(t *testing.T) {
|
||||
resetStore()
|
||||
alice := RegisterPlayer("alice")
|
||||
|
||||
r, err := CreatePveRoom(alice, 2)
|
||||
if err != nil {
|
||||
t.Fatalf("CreatePveRoom: %v", err)
|
||||
}
|
||||
_ = JoinRoom(r.ID, alice)
|
||||
|
||||
if len(GetAllRooms()) != 1 {
|
||||
t.Fatalf("expected 1 PVE room, got %d", len(GetAllRooms()))
|
||||
}
|
||||
|
||||
LeaveRoom(alice)
|
||||
|
||||
if len(GetAllRooms()) != 0 {
|
||||
t.Errorf("expected 0 rooms after PVE leave, got %d", len(GetAllRooms()))
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_GetAllRooms_AfterLeaveReflectsStore: list retrieved from GetAllRooms
|
||||
// must not include a room the caller just left. Catches stale-snapshot bugs.
|
||||
func TestFlow_GetAllRooms_AfterLeaveReflectsStore(t *testing.T) {
|
||||
resetStore()
|
||||
alice := RegisterPlayer("alice")
|
||||
bob := RegisterPlayer("bob")
|
||||
|
||||
// Alice creates her own room.
|
||||
ra, _ := CreatePvpRoom(alice)
|
||||
_ = JoinRoom(ra.ID, alice)
|
||||
|
||||
// Bob creates his own room.
|
||||
rb, _ := CreatePvpRoom(bob)
|
||||
_ = JoinRoom(rb.ID, bob)
|
||||
|
||||
if len(GetAllRooms()) != 2 {
|
||||
t.Fatalf("expected 2 rooms, got %d", len(GetAllRooms()))
|
||||
}
|
||||
|
||||
// Alice leaves.
|
||||
LeaveRoom(alice)
|
||||
|
||||
rooms := GetAllRooms()
|
||||
if len(rooms) != 1 {
|
||||
t.Fatalf("expected 1 room after alice leaves, got %d", len(rooms))
|
||||
}
|
||||
if rooms[0].ID != rb.ID {
|
||||
t.Errorf("expected remaining room to be bob's (%d), got %d", rb.ID, rooms[0].ID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_CreateRoom_CreateAnother_OldOneDoesNotPersist: implicit test that
|
||||
// a create without an intervening leave leaves the first room around (because
|
||||
// it still has the player). The "leave" is what should delete it.
|
||||
func TestFlow_CreateRoom_LeaveWhilePlayerIsStillMember_DeletesProperly(t *testing.T) {
|
||||
resetStore()
|
||||
alice := RegisterPlayer("alice")
|
||||
|
||||
r, _ := CreatePvpRoom(alice)
|
||||
_ = JoinRoom(r.ID, alice)
|
||||
// Sanity: alice.RoomID is set.
|
||||
if alice.RoomID != r.ID {
|
||||
t.Fatalf("alice.RoomID = %d, want %d", alice.RoomID, r.ID)
|
||||
}
|
||||
|
||||
LeaveRoom(alice)
|
||||
|
||||
// RoomID must be cleared so a follow-up LeaveRoom is a safe no-op.
|
||||
if alice.RoomID != 0 {
|
||||
t.Errorf("alice.RoomID = %d after leave, want 0", alice.RoomID)
|
||||
}
|
||||
|
||||
// Idempotent leave: calling again should not panic or create side effects.
|
||||
LeaveRoom(alice)
|
||||
if len(GetAllRooms()) != 0 {
|
||||
t.Errorf("expected 0 rooms after idempotent leave, got %d", len(GetAllRooms()))
|
||||
}
|
||||
}
|
||||
@@ -80,6 +80,12 @@ type Room struct {
|
||||
// Created in CreatePvpRoom; nil for PVE rooms (single player, no concurrent goroutine).
|
||||
GameOverCh chan struct{}
|
||||
|
||||
// RematchCh is closed by whichever player's gameoverState accepts a GameReset
|
||||
// request first, signalling the other player's gameoverState to follow into
|
||||
// the fresh game. Lazily created by the first goroutine entering gameoverState.
|
||||
// Only used for PVP rooms; PVE has a single goroutine so no cross-sync needed.
|
||||
RematchCh chan struct{}
|
||||
|
||||
CreatedAt time.Time
|
||||
LastActive time.Time
|
||||
}
|
||||
|
||||
@@ -0,0 +1,323 @@
|
||||
package state
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/tiennm99/gomoku/server/consts"
|
||||
"github.com/tiennm99/gomoku/server/game"
|
||||
"github.com/tiennm99/gomoku/server/lobby"
|
||||
"github.com/tiennm99/gomoku/server/protocol"
|
||||
)
|
||||
|
||||
// End-to-end flow tests that drive the state machine through realistic user
|
||||
// journeys. These catch bugs that unit tests on individual states miss — in
|
||||
// particular ghosted rooms left behind after a leave, and broken rematch
|
||||
// synchronisation between two player goroutines.
|
||||
|
||||
// runState invokes a state's Next on the player and returns (nextID, err).
|
||||
// Used where callers need to drive the machine one step at a time.
|
||||
func runState(id consts.StateID, p *lobby.Player) (consts.StateID, error) {
|
||||
s, ok := registry[id]
|
||||
if !ok {
|
||||
return 0, ErrClientExit
|
||||
}
|
||||
return s.Next(p)
|
||||
}
|
||||
|
||||
// TestFlow_HomeCreateLeaveCreate_OneRoomVisible simulates the exact reported
|
||||
// bug: from home, create a PVP room, leave it from waiting state, create
|
||||
// another, then assert lobby.GetAllRooms returns only the second room.
|
||||
// If a "ghost" room is left behind anywhere in the state machine this fails.
|
||||
func TestFlow_HomeCreateLeaveCreate_OneRoomVisible(t *testing.T) {
|
||||
alice := makeRegisteredPlayer(t, "alice")
|
||||
|
||||
// home → create room 1 → waiting
|
||||
alice.CmdCh <- &protocol.Request{
|
||||
Payload: &protocol.Request_CreateRoom{CreateRoom: &protocol.CreateRoomRequest{}},
|
||||
}
|
||||
next, err := runState(consts.StateHome, alice)
|
||||
if err != nil {
|
||||
t.Fatalf("home create: %v", err)
|
||||
}
|
||||
if next != consts.StateWaiting {
|
||||
t.Fatalf("expected StateWaiting, got %d", next)
|
||||
}
|
||||
|
||||
// waiting → client exit → home (room 1 should be deleted)
|
||||
alice.CmdCh <- &protocol.Request{
|
||||
Payload: &protocol.Request_ClientExit{ClientExit: &protocol.ClientExitRequest{}},
|
||||
}
|
||||
next, err = runState(consts.StateWaiting, alice)
|
||||
if err != nil {
|
||||
t.Fatalf("waiting exit: %v", err)
|
||||
}
|
||||
if next != consts.StateHome {
|
||||
t.Fatalf("expected StateHome after exit, got %d", next)
|
||||
}
|
||||
|
||||
// Critical check: after leaving, the store should contain 0 rooms.
|
||||
rooms := lobby.GetAllRooms()
|
||||
if len(rooms) != 0 {
|
||||
ids := make([]int64, 0, len(rooms))
|
||||
for _, r := range rooms {
|
||||
ids = append(ids, r.ID)
|
||||
}
|
||||
t.Fatalf("after leave: expected 0 rooms, got %d: %v", len(rooms), ids)
|
||||
}
|
||||
|
||||
// home → create room 2 → waiting
|
||||
alice.CmdCh <- &protocol.Request{
|
||||
Payload: &protocol.Request_CreateRoom{CreateRoom: &protocol.CreateRoomRequest{}},
|
||||
}
|
||||
next, err = runState(consts.StateHome, alice)
|
||||
if err != nil {
|
||||
t.Fatalf("home create 2: %v", err)
|
||||
}
|
||||
if next != consts.StateWaiting {
|
||||
t.Fatalf("expected StateWaiting, got %d", next)
|
||||
}
|
||||
|
||||
// GetRooms must return exactly 1 room (the fresh room 2).
|
||||
rooms = lobby.GetAllRooms()
|
||||
if len(rooms) != 1 {
|
||||
ids := make([]int64, 0, len(rooms))
|
||||
for _, r := range rooms {
|
||||
ids = append(ids, r.ID)
|
||||
}
|
||||
t.Fatalf("expected 1 room after second create, got %d: %v", len(rooms), ids)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_RoomListReflectsCreateLeaveCycle verifies that after drive the
|
||||
// state machine through create→leave→create, a follow-up GetAllRooms returns
|
||||
// exactly the one currently-active room. Directly targets Bug 2.
|
||||
func TestFlow_RoomListReflectsCreateLeaveCycle(t *testing.T) {
|
||||
alice := makeRegisteredPlayer(t, "alice")
|
||||
|
||||
// Baseline.
|
||||
if n := len(lobby.GetAllRooms()); n != 0 {
|
||||
t.Fatalf("precondition: store should be empty, got %d rooms", n)
|
||||
}
|
||||
|
||||
// Create → 1 room.
|
||||
alice.CmdCh <- &protocol.Request{Payload: &protocol.Request_CreateRoom{CreateRoom: &protocol.CreateRoomRequest{}}}
|
||||
if _, err := runState(consts.StateHome, alice); err != nil {
|
||||
t.Fatalf("home create: %v", err)
|
||||
}
|
||||
if n := len(lobby.GetAllRooms()); n != 1 {
|
||||
t.Fatalf("after create 1: expected 1 room, got %d", n)
|
||||
}
|
||||
|
||||
// Leave → 0 rooms.
|
||||
alice.CmdCh <- &protocol.Request{Payload: &protocol.Request_ClientExit{ClientExit: &protocol.ClientExitRequest{}}}
|
||||
if _, err := runState(consts.StateWaiting, alice); err != nil {
|
||||
t.Fatalf("waiting exit: %v", err)
|
||||
}
|
||||
if n := len(lobby.GetAllRooms()); n != 0 {
|
||||
t.Fatalf("after leave: expected 0 rooms, got %d", n)
|
||||
}
|
||||
|
||||
// Create again → 1 room (not 2).
|
||||
alice.CmdCh <- &protocol.Request{Payload: &protocol.Request_CreateRoom{CreateRoom: &protocol.CreateRoomRequest{}}}
|
||||
if _, err := runState(consts.StateHome, alice); err != nil {
|
||||
t.Fatalf("home create 2: %v", err)
|
||||
}
|
||||
if n := len(lobby.GetAllRooms()); n != 1 {
|
||||
t.Fatalf("after create 2: expected 1 room, got %d — Bug 2 reproduced", n)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_HomeExit_DoesNotKillSession verifies that ClientExit from home
|
||||
// keeps the session alive (returns StateHome, not ErrClientExit) so the
|
||||
// client isn't stuck after pressing a Back/Leave button in lobby.
|
||||
func TestFlow_HomeExit_DoesNotKillSession(t *testing.T) {
|
||||
alice := makeRegisteredPlayer(t, "alice")
|
||||
alice.CmdCh <- &protocol.Request{
|
||||
Payload: &protocol.Request_ClientExit{ClientExit: &protocol.ClientExitRequest{}},
|
||||
}
|
||||
|
||||
next, err := runState(consts.StateHome, alice)
|
||||
if err != nil {
|
||||
t.Fatalf("home exit should not return error, got %v", err)
|
||||
}
|
||||
if next != consts.StateHome {
|
||||
t.Fatalf("expected StateHome, got %d", next)
|
||||
}
|
||||
|
||||
// A ClientExitResponse should have been queued so the client knows to redraw.
|
||||
found := false
|
||||
for _, r := range drainSend(alice) {
|
||||
if _, ok := r.Payload.(*protocol.Response_ClientExit); ok {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Error("expected ClientExitResponse from home.ClientExit path")
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_Rematch_BothPlayersTransition is the guard for Bug 1.
|
||||
// Two players finish a game and both land in gameoverState; only one clicks
|
||||
// Play Again. Both player goroutines must transition to gamePvpState —
|
||||
// otherwise the peer is stuck and cannot move when the game restarts.
|
||||
func TestFlow_Rematch_BothPlayersTransition(t *testing.T) {
|
||||
black, white, room := setupFinishedPvpRoom(t)
|
||||
|
||||
// Assert preconditions: room is finished, both players wired.
|
||||
if room.Status != lobby.RoomStatusFinished {
|
||||
t.Fatalf("precondition: room should be Finished, got %d", room.Status)
|
||||
}
|
||||
|
||||
// Black clicks Play Again.
|
||||
black.CmdCh <- &protocol.Request{
|
||||
Payload: &protocol.Request_GameReset{GameReset: &protocol.GameResetRequest{}},
|
||||
}
|
||||
|
||||
// Start white's gameoverState.Next in a goroutine — it should unblock via RematchCh.
|
||||
whiteDone := make(chan consts.StateID, 1)
|
||||
whiteErr := make(chan error, 1)
|
||||
go func() {
|
||||
next, err := runState(consts.StateGameOver, white)
|
||||
whiteDone <- next
|
||||
whiteErr <- err
|
||||
}()
|
||||
|
||||
// Let white's goroutine enter gameoverState (and create/publish RematchCh).
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
|
||||
// Black processes its GameReset.
|
||||
blackNext, err := runState(consts.StateGameOver, black)
|
||||
if err != nil {
|
||||
t.Fatalf("black gameover: %v", err)
|
||||
}
|
||||
if blackNext != consts.StateGamePvp {
|
||||
t.Fatalf("black expected StateGamePvp, got %d", blackNext)
|
||||
}
|
||||
|
||||
// White must unblock and transition to gamePvp as well.
|
||||
select {
|
||||
case next := <-whiteDone:
|
||||
if next != consts.StateGamePvp {
|
||||
t.Errorf("white expected StateGamePvp after rematch, got %d", next)
|
||||
}
|
||||
case <-time.After(500 * time.Millisecond):
|
||||
t.Fatal("white goroutine did not transition to StateGamePvp within 500ms — rematch sync broken")
|
||||
}
|
||||
if err := <-whiteErr; err != nil {
|
||||
t.Errorf("white error: %v", err)
|
||||
}
|
||||
|
||||
// Room state must be reset and ready for a fresh game.
|
||||
room.RLock()
|
||||
status := room.Status
|
||||
nMoves := len(room.MoveHistory)
|
||||
room.RUnlock()
|
||||
if status != lobby.RoomStatusPlaying {
|
||||
t.Errorf("room.Status after rematch = %d, want Playing", status)
|
||||
}
|
||||
if nMoves != 0 {
|
||||
t.Errorf("room.MoveHistory should be empty after reset, got %d moves", nMoves)
|
||||
}
|
||||
|
||||
// Both players should have received a fresh GameStartingResponse.
|
||||
for name, p := range map[string]*lobby.Player{"black": black, "white": white} {
|
||||
found := false
|
||||
for _, r := range drainSend(p) {
|
||||
if _, ok := r.Payload.(*protocol.Response_GameStarting); ok {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Errorf("%s player did not receive GameStartingResponse after rematch", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestFlow_Rematch_NewGameAcceptsMovesFromBoth verifies that after rematch,
|
||||
// the fresh game actually accepts moves. Regression guard for the reported
|
||||
// bug where the board cleared but clicks did nothing.
|
||||
//
|
||||
// We drive black's gamePvpState in a goroutine, feed it a move, and then
|
||||
// shut it down via a game-over signal so the goroutine exits cleanly.
|
||||
func TestFlow_Rematch_NewGameAcceptsMovesFromBoth(t *testing.T) {
|
||||
black, white, room := setupFinishedPvpRoom(t)
|
||||
_ = white
|
||||
|
||||
// Trigger rematch: reset room + broadcast fresh GameStartingResponse.
|
||||
if _, err := handleGameReset(room); err != nil {
|
||||
t.Fatalf("handleGameReset: %v", err)
|
||||
}
|
||||
|
||||
// Drain the GameStartingResponse sent via broadcast.
|
||||
drainSend(black)
|
||||
drainSend(white)
|
||||
|
||||
// Precondition: fresh board, Playing status, Black's turn.
|
||||
room.RLock()
|
||||
if room.Status != lobby.RoomStatusPlaying {
|
||||
room.RUnlock()
|
||||
t.Fatalf("room not Playing after reset")
|
||||
}
|
||||
if room.CurrentTurn != game.Black {
|
||||
room.RUnlock()
|
||||
t.Errorf("CurrentTurn = %v, want Black", room.CurrentTurn)
|
||||
}
|
||||
if room.Board.MoveCount() != 0 {
|
||||
room.RUnlock()
|
||||
t.Errorf("Board.MoveCount = %d, want 0", room.Board.MoveCount())
|
||||
}
|
||||
room.RUnlock()
|
||||
|
||||
// Run black's gamePvpState in a goroutine. Feed it one GameMove, then
|
||||
// trigger game-over to make the goroutine return. If the rematch-broken
|
||||
// bug were still present, the state would silently drop the move.
|
||||
done := make(chan consts.StateID, 1)
|
||||
go func() {
|
||||
next, _ := runState(consts.StateGamePvp, black)
|
||||
done <- next
|
||||
}()
|
||||
|
||||
black.CmdCh <- &protocol.Request{
|
||||
Payload: &protocol.Request_GameMove{GameMove: &protocol.GameMoveRequest{Row: 7, Col: 7}},
|
||||
}
|
||||
|
||||
// Give the goroutine a moment to process the move, then close the game
|
||||
// so gamePvpState.Next returns (via GameOverCh select).
|
||||
time.Sleep(50 * time.Millisecond)
|
||||
signalGameOver(room)
|
||||
|
||||
select {
|
||||
case next := <-done:
|
||||
if next != consts.StateGameOver {
|
||||
t.Errorf("expected StateGameOver after signalGameOver, got %d", next)
|
||||
}
|
||||
case <-time.After(500 * time.Millisecond):
|
||||
t.Fatal("black's gamePvpState did not exit within 500ms after signalGameOver")
|
||||
}
|
||||
|
||||
// Move was applied: (7,7) has a black stone, turn flipped to White.
|
||||
room.RLock()
|
||||
piece := room.Board.GetPiece(7, 7)
|
||||
nMoves := room.Board.MoveCount()
|
||||
room.RUnlock()
|
||||
if piece != game.Black {
|
||||
t.Errorf("after move, (7,7) piece = %v, want Black", piece)
|
||||
}
|
||||
if nMoves != 1 {
|
||||
t.Errorf("Board.MoveCount = %d, want 1", nMoves)
|
||||
}
|
||||
|
||||
// Black should have received exactly one GameMoveSuccessResponse.
|
||||
gotMoveAck := 0
|
||||
for _, r := range drainSend(black) {
|
||||
if _, ok := r.Payload.(*protocol.Response_GameMoveSuccess); ok {
|
||||
gotMoveAck++
|
||||
}
|
||||
}
|
||||
if gotMoveAck != 1 {
|
||||
t.Errorf("black received %d GameMoveSuccessResponse, want 1", gotMoveAck)
|
||||
}
|
||||
}
|
||||
+49
-18
@@ -4,16 +4,23 @@ import (
|
||||
"math/rand"
|
||||
|
||||
"github.com/tiennm99/gomoku/server/consts"
|
||||
"github.com/tiennm99/gomoku/server/lobby"
|
||||
"github.com/tiennm99/gomoku/server/game"
|
||||
"github.com/tiennm99/gomoku/server/lobby"
|
||||
"github.com/tiennm99/gomoku/server/pkg/log"
|
||||
"github.com/tiennm99/gomoku/server/protocol"
|
||||
)
|
||||
|
||||
// gameOverState waits for the player to request a rematch or exit.
|
||||
// On GameResetRequest: resets the board, broadcasts GameStartingResponse,
|
||||
// and transitions back to the appropriate game state (PVP or PVE).
|
||||
// On ClientExit: removes from room and goes home.
|
||||
//
|
||||
// In PVP both player goroutines enter this state after the game ends. When
|
||||
// EITHER player sends GameResetRequest, the room is reset and both goroutines
|
||||
// must transition to gamePvpState together — otherwise one player would keep
|
||||
// acting in the fresh game while the other was still stuck in gameover,
|
||||
// silently dropping their moves.
|
||||
//
|
||||
// The cross-goroutine sync uses room.RematchCh (pattern mirrors StartCh in
|
||||
// waiting state). The first goroutine to enter gameover creates the channel;
|
||||
// whoever accepts the GameReset closes it, waking the other goroutine.
|
||||
type gameOverState struct{}
|
||||
|
||||
func (*gameOverState) Next(player *lobby.Player) (consts.StateID, error) {
|
||||
@@ -23,28 +30,47 @@ func (*gameOverState) Next(player *lobby.Player) (consts.StateID, error) {
|
||||
return consts.StateHome, nil
|
||||
}
|
||||
|
||||
// Lazily create RematchCh on first entry so both goroutines share it.
|
||||
room.Lock()
|
||||
if room.RematchCh == nil {
|
||||
room.RematchCh = make(chan struct{})
|
||||
}
|
||||
rematchCh := room.RematchCh
|
||||
roomType := room.RoomType
|
||||
room.Unlock()
|
||||
|
||||
for {
|
||||
req, reqOk := <-player.CmdCh
|
||||
if !reqOk {
|
||||
leaveRoom(player, room)
|
||||
return 0, ErrClientExit
|
||||
}
|
||||
select {
|
||||
case <-rematchCh:
|
||||
// Peer accepted a rematch — follow them into the new game.
|
||||
if roomType == lobby.RoomTypePve {
|
||||
return consts.StateGamePve, nil
|
||||
}
|
||||
return consts.StateGamePvp, nil
|
||||
|
||||
switch req.Payload.(type) {
|
||||
case *protocol.Request_GameReset:
|
||||
return handleGameReset(room)
|
||||
case req, reqOk := <-player.CmdCh:
|
||||
if !reqOk {
|
||||
leaveRoom(player, room)
|
||||
return 0, ErrClientExit
|
||||
}
|
||||
|
||||
case *protocol.Request_ClientExit:
|
||||
leaveRoom(player, room)
|
||||
return consts.StateHome, nil
|
||||
switch req.Payload.(type) {
|
||||
case *protocol.Request_GameReset:
|
||||
return handleGameReset(room)
|
||||
|
||||
default:
|
||||
log.Errorf("[gameover] player %d: unexpected %T, ignoring\n", player.ID, req.Payload)
|
||||
case *protocol.Request_ClientExit:
|
||||
leaveRoom(player, room)
|
||||
return consts.StateHome, nil
|
||||
|
||||
default:
|
||||
log.Errorf("[gameover] player %d: unexpected %T, ignoring\n", player.ID, req.Payload)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// handleGameReset resets the room and starts a fresh game.
|
||||
// handleGameReset resets the room, broadcasts a fresh GameStartingResponse,
|
||||
// and closes RematchCh so the peer gameover goroutine transitions in lockstep.
|
||||
func handleGameReset(room *lobby.Room) (consts.StateID, error) {
|
||||
seed := rand.Int63()
|
||||
room.Lock()
|
||||
@@ -56,6 +82,11 @@ func handleGameReset(room *lobby.Room) (consts.StateID, error) {
|
||||
// Fresh GameOverCh for the new game round so player goroutines can sync again.
|
||||
room.GameOverCh = make(chan struct{})
|
||||
}
|
||||
// Wake the peer gameover goroutine so both transition into the new game.
|
||||
if room.RematchCh != nil {
|
||||
close(room.RematchCh)
|
||||
room.RematchCh = nil
|
||||
}
|
||||
room.Unlock()
|
||||
|
||||
// Broadcast fresh GameStartingResponse to all players.
|
||||
|
||||
Reference in New Issue
Block a user