From 9d52885a6e346de10a4e64c2dbac56bcb51763b1 Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Thu, 18 Jun 2026 10:18:44 +0200 Subject: [PATCH] fix(matchmaking): re-enqueue opens a new game, not the caller's own A second "random opponent" enqueue with the same variant and per-turn rule, while the caller's first game was still open (awaiting an opponent), returned that same open game, so a player could never start a fresh random game while one was still searching. Drop the own-open short-circuit (step 1) in store.OpenOrJoin: a re-enqueue now joins another player's open game or opens a fresh one. Accumulation stays bounded by MaxActiveQuickGames, which counts open games. Update the matchmaker/service/store doc comments and ARCHITECTURE.md, and flip the pinning test to assert the new behavior. --- backend/internal/game/service.go | 9 +++--- backend/internal/game/store.go | 40 +++++++++----------------- backend/internal/inttest/lobby_test.go | 17 +++++++---- backend/internal/lobby/matchmaker.go | 7 +++-- docs/ARCHITECTURE.md | 6 ++-- 5 files changed, 39 insertions(+), 40 deletions(-) diff --git a/backend/internal/game/service.go b/backend/internal/game/service.go index 310b882..522c3c0 100644 --- a/backend/internal/game/service.go +++ b/backend/internal/game/service.go @@ -266,10 +266,11 @@ func (svc *Service) Create(ctx context.Context, params CreateParams) (Game, erro // OpenOrJoin enters accountID into auto-match for the variant and per-turn rule in // params and returns the game they land in immediately: another waiting player's open -// game (joined=true), the caller's own still-open game on a re-enqueue, or a fresh open -// game seating only the caller with an empty opponent seat that a human or the reaper's -// robot fills later. openDeadline is when the reaper substitutes a robot into a freshly -// opened game (ignored when joining one). The bag seed defaults to random; params.Seed +// game (joined=true), or a fresh open game seating only the caller with an empty +// opponent seat that a human or the reaper's robot fills later. A re-enqueue while the +// caller is already waiting opens another game rather than returning their own. +// openDeadline is when the reaper substitutes a robot into a freshly opened game +// (ignored when joining one). The bag seed defaults to random; params.Seed // pins it. First-move fairness comes from seating the caller at seat 0 or seat 1 // (derived from the seed): seated at seat 1, the still-empty seat 0 moves first, so the // caller just waits for the opponent. It backs the lobby auto-match enqueue. diff --git a/backend/internal/game/store.go b/backend/internal/game/store.go index d04eb8e..f726f27 100644 --- a/backend/internal/game/store.go +++ b/backend/internal/game/store.go @@ -188,37 +188,25 @@ func openMatchKey(variant string, multipleWords bool) int64 { } // OpenOrJoin atomically resolves an auto-match enqueue for accountID into the game it -// lands in: it re-uses the caller's own still-open game (joined=false, created=false, -// a re-enqueue is idempotent), joins another player's waiting open game and flips it -// active (joined=true), or opens a fresh game seating the caller with an empty -// opponent seat (created=true). ins supplies the new game's immutable fields and is -// used only when a game is created. A transaction-scoped advisory lock on the -// (variant, rule) bucket serialises concurrent enqueues so two callers pair rather -// than each opening a game. seats is the two-seat arrangement (the caller and uuid.Nil -// for the still-empty opponent, in the chosen order) used only when a game is created; -// callerName is the caller's display-name snapshot, stamped on their seat whether they -// open a fresh game or fill another player's open one. +// lands in: it joins another player's waiting open game and flips it active +// (joined=true), or opens a fresh game seating the caller with an empty opponent seat +// (created=true). A re-enqueue while the caller already has an open game in the bucket +// opens another fresh game (or joins a different player's) rather than returning the +// caller's own, so tapping "random opponent" again always starts a new search. ins +// supplies the new game's immutable fields and is used only when a game is created. A +// transaction-scoped advisory lock on the (variant, rule) bucket serialises concurrent +// enqueues so two callers pair rather than each opening a game. seats is the two-seat +// arrangement (the caller and uuid.Nil for the still-empty opponent, in the chosen +// order) used only when a game is created; callerName is the caller's display-name +// snapshot, stamped on their seat whether they open a fresh game or fill another +// player's open one. func (s *Store) OpenOrJoin(ctx context.Context, accountID uuid.UUID, callerName string, ins gameInsert, seats []seatInsert) (gameID uuid.UUID, joined, created bool, err error) { err = withTx(ctx, s.db, func(tx *sql.Tx) error { if _, e := tx.ExecContext(ctx, `SELECT pg_advisory_xact_lock($1)`, openMatchKey(ins.variant, ins.multipleWordsPerTurn)); e != nil { return fmt.Errorf("open match lock: %w", e) } - // 1. The caller's own still-open game for this bucket — a re-enqueue is idempotent. - var own uuid.UUID - switch e := tx.QueryRowContext(ctx, - `SELECT g.game_id FROM backend.games g - JOIN backend.game_players p ON p.game_id = g.game_id - WHERE g.status = 'open' AND g.variant = $1 AND g.multiple_words_per_turn = $2 AND p.account_id = $3 - LIMIT 1`, - ins.variant, ins.multipleWordsPerTurn, accountID).Scan(&own); { - case e == nil: - gameID = own - return nil - case !errors.Is(e, sql.ErrNoRows): - return fmt.Errorf("find own open game: %w", e) - } - // 2. Another player's open game waiting for an opponent — fill its seat and start it. + // 1. Another player's open game waiting for an opponent — fill its seat and start it. var other uuid.UUID switch e := tx.QueryRowContext(ctx, `SELECT g.game_id FROM backend.games g @@ -237,7 +225,7 @@ func (s *Store) OpenOrJoin(ctx context.Context, accountID uuid.UUID, callerName case !errors.Is(e, sql.ErrNoRows): return fmt.Errorf("find open game: %w", e) } - // 3. None waiting — open a fresh game seating the caller (the other seat empty). + // 2. None waiting — open a fresh game seating the caller (the other seat empty). if e := insertGameTx(ctx, tx, ins, seats); e != nil { return e } diff --git a/backend/internal/inttest/lobby_test.go b/backend/internal/inttest/lobby_test.go index 5a2a2e5..4f237a4 100644 --- a/backend/internal/inttest/lobby_test.go +++ b/backend/internal/inttest/lobby_test.go @@ -74,9 +74,11 @@ func TestMatchmakingOpensThenJoins(t *testing.T) { } } -// TestMatchmakingReEnqueueReturnsOwnOpenGame checks a re-enqueue is idempotent: the -// caller gets their existing open game rather than a second one. -func TestMatchmakingReEnqueueReturnsOwnOpenGame(t *testing.T) { +// TestMatchmakingReEnqueueOpensNewGame checks a re-enqueue opens a fresh open game +// rather than returning the caller's existing one: tapping "random opponent" again while +// still waiting starts a new search instead of bouncing the player back into the pending +// game. Both games stay open until an opponent (a human or the reaper's robot) joins each. +func TestMatchmakingReEnqueueOpensNewGame(t *testing.T) { ctx := context.Background() clearOpenGames(t) mm := newMatchmaker(t, newRobotService(t, newGameService()), 90*time.Second, 90*time.Second) @@ -90,8 +92,13 @@ func TestMatchmakingReEnqueueReturnsOwnOpenGame(t *testing.T) { if err != nil { t.Fatalf("re-enqueue: %v", err) } - if r2.Game.ID != r1.Game.ID || r2.Matched { - t.Fatalf("re-enqueue = (game %s, matched %v), want the same open game %s unmatched", r2.Game.ID, r2.Matched, r1.Game.ID) + if r2.Matched || r2.Game.ID == r1.Game.ID { + t.Fatalf("re-enqueue = (game %s, matched %v), want a new open game distinct from %s, unmatched", r2.Game.ID, r2.Matched, r1.Game.ID) + } + for _, g := range []uuid.UUID{r1.Game.ID, r2.Game.ID} { + if _, _, status, err := newGameService().Participants(ctx, g); err != nil || status != "open" { + t.Fatalf("game %s status = %q err %v, want open", g, status, err) + } } } diff --git a/backend/internal/lobby/matchmaker.go b/backend/internal/lobby/matchmaker.go index 030c1b8..8bf1646 100644 --- a/backend/internal/lobby/matchmaker.go +++ b/backend/internal/lobby/matchmaker.go @@ -85,9 +85,10 @@ type EnqueueResult struct { // Enqueue resolves an auto-match request for accountID under variant and the per-turn // word rule (multipleWords) into the game they enter immediately — a freshly opened -// game awaiting an opponent, the caller's own still-open game (a re-enqueue is -// idempotent), or another player's open game they just joined. When the caller joins -// an existing game, opponent_joined is pushed to that game's waiting starter. +// game awaiting an opponent, or another player's open game they just joined. A +// re-enqueue while already waiting opens another game rather than returning the +// caller's own. When the caller joins an existing game, opponent_joined is pushed to +// that game's waiting starter. func (m *Matchmaker) Enqueue(ctx context.Context, accountID uuid.UUID, variant engine.Variant, multipleWords bool) (EnqueueResult, error) { g, joined, err := m.games.OpenOrJoin(ctx, accountID, autoMatchParams(variant, multipleWords), m.openDeadline()) if err != nil { diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2f30438..489754c 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -466,8 +466,10 @@ disguised robot stays indistinguishable from a person. caller with an **empty opponent seat** (status `open`, §9), or — when another player is already waiting for the same `variant` and per-turn rule — seats the caller into that open game and starts it; which seat the caller takes is randomised for - first-move fairness, and a re-enqueue returns the caller's own still-open game - (idempotent). Matchmaking state is therefore the **open games in the database** (not + first-move fairness, and a re-enqueue while already waiting opens **another** game + (or joins a different player's) rather than returning the caller's own, so choosing + "random opponent" again always starts a new search (bounded by the simultaneous + quick-game cap, §9). Matchmaking state is therefore the **open games in the database** (not an in-memory pool), so it survives a restart and stays anonymous (no block check); concurrent enqueues for one bucket are serialised by a transaction-scoped advisory lock so two callers pair rather than each opening a game. A background **reaper**