diff --git a/server/internal/game/engine.go b/server/internal/game/engine.go index 36b11b0..29ee707 100644 --- a/server/internal/game/engine.go +++ b/server/internal/game/engine.go @@ -10,6 +10,8 @@ package game import ( "errors" "fmt" + "iter" + "maps" "math/bits" "slices" "time" @@ -179,6 +181,25 @@ func (e *Engine) Winner() PlayerID { return e.winner } // ChainLength reports how many words have been played, opening word included. func (e *Engine) ChainLength() int { return len(e.history) + 1 } +// LastMove reports the most recently played word, and false when none has +// been played yet. It exists so a caller that only ever wants the tail of the +// chain — the resume replay is the one — does not have to copy the whole +// history to reach it. +func (e *Engine) LastMove() (Move, bool) { + if len(e.history) == 0 { + return Move{}, false + } + return e.history[len(e.history)-1], true +} + +// UsedWords iterates every canonical word already played, the opening word +// included. It is read directly off the engine's own set rather than rebuilt +// from history on every call, which is what a bot's board was doing once per +// move. +func (e *Engine) UsedWords() iter.Seq[string] { + return maps.Keys(e.used) +} + // Submit validates a player's word and, if legal, plays it. // // The returned Move carries the canonical spelling; on rejection the reason @@ -575,11 +596,19 @@ func (e *Engine) Snapshot() State { alive[p] = e.alive[i] } + // Standings is meaningless while the game is in play, by its own doc + // comment, so it is only worth computing once the game actually has one — + // otherwise every TurnUpdate and every bot move pays for a table nobody + // reads. + var standings []Standing + if e.over { + standings = e.Standings() + } + return State{ Current: e.current, Turn: e.Turn(), Deadline: e.deadline, - History: append([]Move{}, e.history...), Scores: scores, Alive: alive, Eliminated: append([]PlayerID{}, e.outOrder...), @@ -587,6 +616,6 @@ func (e *Engine) Snapshot() State { Over: e.over, Winner: e.winner, EndReason: e.endReason, - Standings: e.Standings(), + Standings: standings, } } diff --git a/server/internal/game/engine_test.go b/server/internal/game/engine_test.go index 74b98c0..1f6ecd1 100644 --- a/server/internal/game/engine_test.go +++ b/server/internal/game/engine_test.go @@ -756,18 +756,60 @@ func TestSnapshotIsACopy(t *testing.T) { e.Submit(alice, "ngữ pháp", t0) snap := e.Snapshot() - snap.History[0].Word = "MUTATED" snap.Scores[alice] = 9999 fresh := e.Snapshot() - if fresh.History[0].Word == "MUTATED" { - t.Error("mutating a snapshot's history changed engine state") - } if fresh.Scores[alice] == 9999 { t.Error("mutating a snapshot's scores changed engine state") } } +// LastMove is the resume replay's whole reason to exist: the tail of the +// chain without copying the rest of it. +func TestLastMove(t *testing.T) { + e := newGame(t, standardDict(), "ngôn ngữ") + + if _, ok := e.LastMove(); ok { + t.Fatal("LastMove reported a move before any word was played") + } + + if _, r := e.Submit(alice, "ngữ pháp", t0); r != ReasonNone { + t.Fatalf("Submit: %s", r) + } + move, ok := e.LastMove() + if !ok { + t.Fatal("LastMove reported none after a word was played") + } + if move.Word != "ngữ pháp" { + t.Errorf("LastMove = %q, want %q", move.Word, "ngữ pháp") + } + + if _, r := e.Submit(bob, "pháp luật", t0); r != ReasonNone { + t.Fatalf("Submit: %s", r) + } + move, ok = e.LastMove() + if !ok || move.Word != "pháp luật" { + t.Errorf("LastMove after a second word = %+v, %v, want %q", move, ok, "pháp luật") + } +} + +// UsedWords is the bot's search boundary: the opening word and everything +// played since, with nothing else in it. +func TestUsedWords(t *testing.T) { + e := newGame(t, standardDict(), "ngôn ngữ") + if _, r := e.Submit(alice, "ngữ pháp", t0); r != ReasonNone { + t.Fatalf("Submit: %s", r) + } + + used := map[string]bool{} + for w := range e.UsedWords() { + used[w] = true + } + if !used["ngôn ngữ"] || !used["ngữ pháp"] || len(used) != 2 { + t.Errorf("UsedWords = %v, want exactly the opening word and the move played", used) + } +} + func TestRejectReasonStrings(t *testing.T) { // Every reason needs a distinct, non-empty description: these become // player-facing messages. diff --git a/server/internal/game/multiplayer_test.go b/server/internal/game/multiplayer_test.go index ff9b421..81d81e8 100644 --- a/server/internal/game/multiplayer_test.go +++ b/server/internal/game/multiplayer_test.go @@ -212,8 +212,8 @@ func TestAnEliminatedPlayerKeepsTheirScore(t *testing.T) { if got := e.Score(alice); got != scored { t.Errorf("score after elimination = %d, want %d", got, scored) } - if state := e.Snapshot(); len(state.History) != 1 || state.History[0].Player != alice { - t.Errorf("the eliminated player's move is missing from the history: %+v", state.History) + if last, ok := e.LastMove(); !ok || last.Player != alice { + t.Errorf("the eliminated player's move is missing from the history: %+v, %v", last, ok) } if !slices.Contains(e.Players(), alice) { t.Error("Players dropped the eliminated seat, which the transport still has to render") diff --git a/server/internal/game/state.go b/server/internal/game/state.go index 9c7f637..de42bb3 100644 --- a/server/internal/game/state.go +++ b/server/internal/game/state.go @@ -186,7 +186,6 @@ type State struct { Current string Turn PlayerID Deadline time.Time - History []Move Scores map[PlayerID]int // Alive says who is still in the game. A player who has been eliminated // keeps their score and their place in the history; they simply no longer diff --git a/server/internal/wsapi/drain_test.go b/server/internal/wsapi/drain_test.go index 240fe21..d8aee07 100644 --- a/server/internal/wsapi/drain_test.go +++ b/server/internal/wsapi/drain_test.go @@ -92,6 +92,59 @@ func TestLiveGameCountTracksGamesNotLobbies(t *testing.T) { } } +// TestDrainRefusesStartGameOnAnExistingLobby covers what newRegisteredRoom's +// own drain check cannot: a lobby that existed before the drain decision has +// no further room-creation call to refuse, so beginGame itself has to know. +func TestDrainRefusesStartGameOnAnExistingLobby(t *testing.T) { + api, url := newTestServer(t, chainDict(), Config{}) + host, guest, _ := pvpLobby(t, url) + guest.setReady(true) + host.await("room_state") + + api.StartDraining() + + host.startGame() + if got := host.await("error").GetError().GetCode(); got != "server_restarting" { + t.Errorf("error code = %q, want server_restarting", got) + } +} + +// TestDrainRefusesQuickMatchAutoStart is C3's other beginGame call site: the +// second seat filling after the drain decision must not start a game either, +// even though nobody sent StartGame for it to refuse. +func TestDrainRefusesQuickMatchAutoStart(t *testing.T) { + h := &hub{} + h.draining.Store(true) + first := &session{id: "a", ctx: t.Context(), out: make(chan []byte, 8)} + second := &session{id: "b", ctx: t.Context(), out: make(chan []byte, 8)} + + r := &room{hub: h, dict: chainDict(), turnLimit: time.Second, graceFor: time.Minute} + r.handleCreate(createInput{sess: first, autoStart: true}) + r.handleJoin(joinInput{sess: second}) + + if r.engine != nil { + t.Error("quick match auto-started a game after the drain decision") + } +} + +// TestDrainLetsALiveGameFinish is the other half of C3: draining must not cut +// a game that was already running short, which is the very outcome it exists +// to avoid. +func TestDrainLetsALiveGameFinish(t *testing.T) { + api, url := newTestServer(t, chainDict(), Config{TurnLimit: 200 * time.Millisecond}) + lead, waits, _ := pvpGame(t, url) + + api.StartDraining() + + // Nobody submits anything: the lead's own turn runs out the clock, which + // is enough to end a two-seat game, same as TestLiveGameCountTracksGamesNotLobbies. + waits.await("game_over") + lead.await("game_over") + if got := api.LiveGameCount(); got != 0 { + t.Errorf("live games = %d after the game ended during a drain, want 0", got) + } +} + // TestVersionEndpoint: GET /version answers with exactly the string the // server was configured with, in plain text, so a deploy check can diff it // against what was just built without parsing anything. diff --git a/server/internal/wsapi/room.go b/server/internal/wsapi/room.go index b0d8f1b..d2784fc 100644 --- a/server/internal/wsapi/room.go +++ b/server/internal/wsapi/room.go @@ -571,7 +571,8 @@ func (r *room) run() { // The code goes out in the RoomState the run loop broadcasts, so a client can // never be handed a code before the seat behind it exists. func (r *room) handleCreate(m createInput) { - r.seats[0] = &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq} + s := &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq} + r.seats[0] = s r.owner = "p1" r.autoStart = m.autoStart m.sess.attach(r, "p1") @@ -581,10 +582,31 @@ func (r *room) handleCreate(m createInput) { // who was also waiting in it from another attempt — one dequeue serves // both room-entry paths. r.hub.cancelQuickMatch(m.sess) + + if s.sess.ctx.Err() != nil { + r.disconnectGhostSeat(s) + return + } // Deliberately sent to a brand-new room's creator, where it is always // empty: it is what replaces the conversation a client may still be // holding from a room it was in before this one. - r.sendChatHistory(r.seats[0]) + r.sendChatHistory(s) +} + +// disconnectGhostSeat opens the seat's reconnect window the moment it is +// filled, for a connection that turns out to have already torn down. +// +// The session can die between the hub handing this room the seating message +// and the room goroutine draining it off the queue — nothing else ever learns +// that, because leaveRoom only notifies a room the session was already +// attached to, and attaching is exactly what has not happened yet. Left +// seated as if connected, allConnected() would report true and quick match's +// own auto-start (see handleJoin) could begin a game against a socket nobody +// is behind. Applying the same grace window handleDisconnect would reuses the +// one mechanism that already bounds this instead of adding a second one. +func (r *room) disconnectGhostSeat(s *seat) { + s.sess = nil + s.graceUntil = time.Now().Add(r.graceFor) } // handleResign is one player giving up on their own turn. The seat, not the @@ -660,12 +682,24 @@ func (r *room) handleStartBot(m startBotInput) { } r.strategy = strategy - r.seats[0] = &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq} + s := &seat{id: "p1", nickname: m.sess.nickname(), sess: m.sess, chatFrom: r.chatSeq} + r.seats[0] = s r.seats[1] = &seat{id: botPlayerID, nickname: "Máy"} r.owner = "p1" m.sess.attach(r, "p1") r.hub.cancelQuickMatch(m.sess) + if s.sess.ctx.Err() != nil { + // A bot room has no lobby to fall back to and no idle timer covering it + // while there is no engine yet (resetIdleTimer skips any room with a + // strategy) — a grace window here would leave the bot's own seat + // holding the room open forever with nothing left to vacate it. The + // room ends now instead, the same way a failed bot.New or beginGame + // above already does. + r.cancel() + return + } + if err := r.beginGame(); err != nil { slog.Error("could not start bot game", "room", r.code, "err", err) m.sess.send(errorMsg("game_start_failed")) @@ -703,7 +737,7 @@ func (r *room) handleJoin(m joinInput) { } id := seatIDs[free] - r.seats[free] = &seat{ + s := &seat{ id: id, nickname: distinguish(m.sess.nickname(), r.takenNicknames(id)), sess: m.sess, @@ -711,10 +745,16 @@ func (r *room) handleJoin(m joinInput) { // read. A room code is pasted into group chats by design. chatFrom: r.chatSeq, } + r.seats[free] = s m.sess.attach(r, string(id)) r.lobbyChanged = true r.hub.cancelQuickMatch(m.sess) - r.sendChatHistory(r.seats[free]) + + if s.sess.ctx.Err() != nil { + r.disconnectGhostSeat(s) + return + } + r.sendChatHistory(s) // A quick match seats both players itself rather than waiting on // readiness and StartGame — there is no owner here to press it, only two @@ -723,6 +763,14 @@ func (r *room) handleJoin(m joinInput) { // beat before GameStarted rather than jumping straight into one with no // seating frame behind it. if r.autoStart && r.seatedCount() >= minPlayers && r.allConnected() { + if r.hub.isDraining() { + // The second seat filled after the drain decision. There is no + // owner here to answer with server_restarting the way lobbyStart + // does, so both seats are told directly; the lobby view they are + // left in still shows each other, via lobbyChanged below. + r.broadcastError("server_restarting") + return + } r.autoStart = false r.lobbyChanged = false r.broadcastRoomState() @@ -776,6 +824,13 @@ func (r *room) handleLobby(m lobbyInput) { return } switch { + case r.hub.isDraining(): + // newRegisteredRoom already refuses a brand-new room once draining + // starts; this lobby existed before that point, and starting its + // game now would raise hub.liveGames after the drain decided how + // long to wait for exactly that number to reach zero. + m.sess.send(errorMsg("server_restarting")) + return case r.seatedCount() < minPlayers: m.sess.send(errorMsg("need_more_players")) return @@ -1113,7 +1168,7 @@ func (r *room) maybeScheduleBot() { // resign and disconnect the room processes while the bot thinks — and // bot.Board.Used reads engine state, so the race would be real, not // theoretical. - board := freezeBoard(r.engine, r.opening) + board := freezeBoard(r.engine) seq := r.turnSeq strategy := r.strategy @@ -1472,6 +1527,16 @@ func (r *room) handleResume(m resumeInput) { // somebody who is already back. The run loop sends it to all of them. r.lobbyChanged = true + if m.sess.ctx.Err() != nil { + // The new connection can die between the client's Hello landing and + // this resume being drained off the room's queue, the same race + // handleCreate and handleJoin guard against. Reopening the window it + // just closed leaves the seat exactly as reachable as it was before + // this resume was ever attempted. + r.disconnectGhostSeat(s) + return + } + // Before the lobby return below, not after it: a refresh in the lobby is // the commonest resume there is, and it is exactly the one that would miss // a replay hung off the end of this function. @@ -1484,8 +1549,7 @@ func (r *room) handleResume(m resumeInput) { } state := r.engine.Snapshot() r.sendGameStarted(s, state) - if len(state.History) > 0 { - last := state.History[len(state.History)-1] + if last, ok := r.engine.LastMove(); ok { r.sendTurnUpdate(s, state, &last, r.moveMeanings(&last)) } } @@ -1827,16 +1891,13 @@ type frozenBoard struct { dict game.Dictionary } -func freezeBoard(e *game.Engine, opening string) *frozenBoard { - state := e.Snapshot() - - // History omits the opening word, but the engine counts it as played. A - // board that disagreed would let the bot pick a word the engine then - // rejects as already used. - used := make(map[string]struct{}, len(state.History)+1) - used[opening] = struct{}{} - for _, m := range state.History { - used[m.Word] = struct{}{} +func freezeBoard(e *game.Engine) *frozenBoard { + // UsedWords already includes the opening word — it seeds the engine's own + // set — so there is nothing left to add here, and nothing to copy out of a + // history that grows with the game. + used := make(map[string]struct{}) + for word := range e.UsedWords() { + used[word] = struct{}{} } return &frozenBoard{legal: e.LegalMoves(), used: used, dict: e.Dict()}