diff --git a/frontend/src/game/ThemeGuessGame.ts b/frontend/src/game/ThemeGuessGame.ts index 9931341..60e3e75 100644 --- a/frontend/src/game/ThemeGuessGame.ts +++ b/frontend/src/game/ThemeGuessGame.ts @@ -59,18 +59,26 @@ export class ThemeGuessGame { private height = 0; private categories!: CategoryStateMap; - /** `onCategoryAssigned`, when provided, fires at the end of every - * `applyColor` with the category and hex just assigned — multiplayer - * uses it to relay a bare `round:progress` and to track the local - * player's running color map for `round:submit`, without this class - * needing to know anything about the network. */ + /** Fires at the end of every `applyColor` with the category and hex + * just assigned — multiplayer uses it to relay a bare + * `round:progress` and to track the local player's running color map + * for `round:submit`, without this class needing to know anything + * about the network. Mutable (not just constructor-set) because the + * app keeps a single shared `ThemeGuessGame` instance across + * solo/matchmaking/private-room (see `game/sharedGame.ts`) — whichever + * flow is currently active swaps this in rather than a new instance + * being constructed (which would double-bind canvas/document + * listeners onto the same DOM). */ + onCategoryAssigned?: (id: CategoryId, hex: string) => void; + constructor( themeId: ThemeId, snippetIndex: number = pickRandomSnippetIndex(), - private readonly onCategoryAssigned?: (id: CategoryId, hex: string) => void, + onCategoryAssigned?: (id: CategoryId, hex: string) => void, ) { this.themeId = themeId; this.snippetIndex = snippetIndex; + this.onCategoryAssigned = onCategoryAssigned; this.build(); this.bindEvents(); requestAnimationFrame((t) => this.loop(t)); diff --git a/frontend/src/game/sharedGame.ts b/frontend/src/game/sharedGame.ts new file mode 100644 index 0000000..6376920 --- /dev/null +++ b/frontend/src/game/sharedGame.ts @@ -0,0 +1,40 @@ +// A single `ThemeGuessGame` bound to the page's one `#code-canvas` / +// `#category-panel` / `#reveal-btn` / document-level listeners for its +// entire lifetime. `ThemeGuessGame`'s constructor permanently binds +// fresh event listeners onto those shared, singleton DOM elements and +// never tears them down — constructing a *second* instance while a +// first is still alive would double-bind every click/keydown/mousemove +// handler onto the same elements, corrupting whichever mode the player +// used first (e.g. two live color pickers fighting over the same +// popover). +// +// Solo (`ui/soloConfigFlow.ts`), matchmaking (`ui/matchmakingFlow.ts`), +// and private rooms (`ui/privateRoomFlow.ts`) each start their own +// rounds/matches against the *same* canvas, so they must all share one +// instance rather than each constructing their own — this module is +// that shared instance's sole owner. Call `getSharedGame` every time a +// flow is about to start a round: on the very first call it constructs +// the instance; on every later call it reconfigures the existing one +// (`setTheme`/`setSnippet`) and swaps in the caller's own +// `onCategoryAssigned`, so exactly one set of DOM listeners ever exists, +// always dispatching to whichever flow is currently active. + +import { ThemeGuessGame } from './ThemeGuessGame'; +import type { CategoryId, ThemeId } from '../types'; + +let instance: ThemeGuessGame | null = null; + +export function getSharedGame( + themeId: ThemeId, + snippetIndex: number, + onCategoryAssigned?: (id: CategoryId, hex: string) => void, +): ThemeGuessGame { + if (!instance) { + instance = new ThemeGuessGame(themeId, snippetIndex, onCategoryAssigned); + return instance; + } + instance.setTheme(themeId); + instance.setSnippet(snippetIndex); + instance.onCategoryAssigned = onCategoryAssigned; + return instance; +} diff --git a/frontend/src/main.ts b/frontend/src/main.ts index d0c827e..c6a7b88 100644 --- a/frontend/src/main.ts +++ b/frontend/src/main.ts @@ -1,5 +1,5 @@ import './style.css'; -import { ThemeGuessGame } from './game/ThemeGuessGame'; +import { getSharedGame } from './game/sharedGame'; import { THEMES } from './data/themes'; import { sound } from './engine/sound'; import { createThemeGrid } from './ui/themeGrid'; @@ -56,8 +56,6 @@ function showView(name: ScreenName | null): void { } } -let game: ThemeGuessGame | null = null; - const themeGrid = createThemeGrid(themeGridEl, 'gruvbox', () => sound.playOpen()); const soloConfigFlow = createSoloConfigFlow( @@ -72,16 +70,7 @@ const soloConfigFlow = createSoloConfigFlow( (config) => { showView(null); runThemePreview(previewElements, config.themeId, () => { - if (game) { - game.setTheme(config.themeId); - game.setSnippet(config.snippetIndex); - } else { - game = new ThemeGuessGame( - config.themeId, - config.snippetIndex, - (id, hex) => soloConfigFlow.handleLocalAssignment(id, hex), - ); - } + getSharedGame(config.themeId, config.snippetIndex, (id, hex) => soloConfigFlow.handleLocalAssignment(id, hex)); themeNameBadge.textContent = THEMES[config.themeId].name; sound.playApply(); soloConfigFlow.startTimer(config.timeMode); diff --git a/frontend/src/ui/matchmakingFlow.ts b/frontend/src/ui/matchmakingFlow.ts index e2e1af6..0f03163 100644 --- a/frontend/src/ui/matchmakingFlow.ts +++ b/frontend/src/ui/matchmakingFlow.ts @@ -8,18 +8,15 @@ // sends them in the opposite order — see the `foundAt`/`roundStartPayload` // handling below, which treats both orderings identically. // -// Owns a single `OpponentView` / `ThemeGuessGame` / `MultiplayerMatch` -// trio for the lifetime of the page, constructed lazily on the first -// matchmaking round and reused (`setTheme`/`setSnippet`) on every -// subsequent one — mirroring how `ui/soloConfigFlow.ts` reuses a single -// `ThemeGuessGame` across solo replays. `ThemeGuessGame`'s constructor -// binds fresh DOM listeners every time it's called, so constructing a -// second one while a first is still alive (e.g. mixing solo/bot play -// with matchmaking in the same page load) would double up event -// handling; out of scope here, same single-orchestrator assumption -// `game/multiplayerMatch.ts`'s header documents. +// Owns a single `OpponentView` / `MultiplayerMatch` pair for the +// lifetime of the page, constructed lazily on the first matchmaking +// round and reused on every subsequent one. The `ThemeGuessGame` itself +// is *not* owned here — it's the one shared instance from +// `game/sharedGame.ts` (solo, matchmaking, and private rooms all start +// rounds against the same `#code-canvas`, so exactly one instance must +// ever exist; see that module's header for why). -import { ThemeGuessGame } from '../game/ThemeGuessGame'; +import { getSharedGame } from '../game/sharedGame'; import { MultiplayerMatch } from '../game/multiplayerMatch'; import { OpponentView } from '../game/opponentView'; import { THEMES } from '../data/themes'; @@ -103,7 +100,6 @@ export function createMatchmakingFlow( // every subsequent one this page load — see this module's header. let opponentView: OpponentView | null = null; let match: MultiplayerMatch | null = null; - let game: ThemeGuessGame | null = null; function clearBanTimers(): void { stopBanBanner?.(); @@ -194,14 +190,10 @@ export function createMatchmakingFlow( themeNameBadge.textContent = THEMES[payload.themeId].name; if (!opponentView) opponentView = new OpponentView('opponent-canvas', payload.snippetIndex); + const isNewMatch = !match; if (!match) match = new MultiplayerMatch(client, opponentView, handleLocalQuit); - if (!game) { - game = new ThemeGuessGame(payload.themeId, payload.snippetIndex, (id, hex) => match!.handleLocalAssignment(id, hex)); - match.bindGame(game); - } else { - game.setTheme(payload.themeId); - game.setSnippet(payload.snippetIndex); - } + const game = getSharedGame(payload.themeId, payload.snippetIndex, (id, hex) => match!.handleLocalAssignment(id, hex)); + if (isNewMatch) match.bindGame(game); hooks.showBoard(); match.startRound(payload); diff --git a/frontend/src/ui/privateRoomFlow.ts b/frontend/src/ui/privateRoomFlow.ts index bef4689..8892655 100644 --- a/frontend/src/ui/privateRoomFlow.ts +++ b/frontend/src/ui/privateRoomFlow.ts @@ -16,33 +16,17 @@ // its own listeners on the same client, so this flow must never react // to a round that belongs to a matchmaking match. // -// Session component lifetime: exactly one `ThemeGuessGame` + one -// `OpponentView` + one `MultiplayerMatch` are constructed, lazily, the -// first time a room's first `round:start` arrives, then reused for -// every subsequent match in that room's up-to-5-match series by calling -// `match.startRound()` again — `MultiplayerMatch.startRound()` already -// tears down and re-subscribes its own per-round listeners/timer, so -// it's safe to call repeatedly on the same instance (confirmed by -// reading its source; no fix needed there for this). Constructing a -// *second* `ThemeGuessGame` for a later room, or for matchmaking, would -// double-bind its canvas/document event listeners onto the single -// shared `#code-canvas` — so this module's `game`/`opponentView`/`match` -// are deliberately never rebuilt once created, only reconfigured via -// `startRound`. -// -// Known cross-flow gap (see this ticket's final report): main.ts's own -// solo-mode `game` singleton is *separately* constructed by -// `soloConfigFlow`, hardcoding its `onCategoryAssigned` callback to -// `soloConfigFlow.handleLocalAssignment` forever. If a player plays Solo -// and then enters a private room in the same page load, this module -// necessarily constructs its *own* `ThemeGuessGame`, which double-binds -// listeners on the same `#code-canvas`/`#category-panel`/`document` -// solo's instance already bound. Fixing this needs a shared, swappable -// dispatch for `game`'s callback at the main.ts composition-root level -// (mirroring how `soloConfigFlow` already multiplexes "alone" vs "vs -// bot" internally) — out of this ticket's edit scope (main.ts's -// `game`/`soloConfigFlow` construction block is off-limits here, see -// this ticket's report). +// Session component lifetime: one `OpponentView` + one `MultiplayerMatch` +// are constructed, lazily, the first time a room's first `round:start` +// arrives, then reused for every subsequent match in that room's +// up-to-5-match series by calling `match.startRound()` again — +// `MultiplayerMatch.startRound()` already tears down and re-subscribes +// its own per-round listeners/timer, so it's safe to call repeatedly on +// the same instance. The `ThemeGuessGame` itself is *not* owned here — +// it's the one shared instance from `game/sharedGame.ts` (solo, +// matchmaking, and this flow all start rounds against the same +// `#code-canvas`, so exactly one instance must ever exist across the +// whole page; see that module's header for why). import { THEMES } from '../data/themes'; import type { GameClient } from '../net/client'; @@ -56,7 +40,7 @@ import type { } from '../net/messages'; import { MultiplayerMatch } from '../game/multiplayerMatch'; import { OpponentView } from '../game/opponentView'; -import { ThemeGuessGame } from '../game/ThemeGuessGame'; +import { getSharedGame } from '../game/sharedGame'; import { openThemeVote, type ThemeVoteHandle } from './themeVote'; export interface PrivateRoomFlowElements { @@ -133,7 +117,6 @@ export function createPrivateRoomFlow( let transitionTimeoutId: number | null = null; let opponentView: OpponentView | null = null; - let game: ThemeGuessGame | null = null; let match: MultiplayerMatch | null = null; const splitView = requireEl('split-view'); @@ -335,20 +318,21 @@ export function createPrivateRoomFlow( * `multiplayerMatch.ts`'s documented two-phase-init order, then reuses * them for every later `round:start` this room sends. */ function ensureMatch(payload: RoundStartMessage): MultiplayerMatch { - if (match && game) return match; - - opponentView = new OpponentView('opponent-canvas', payload.snippetIndex); - match = new MultiplayerMatch(client, opponentView, () => { - // Local player quit mid-round: per PROTOCOL.md "Quit", the - // quitter gets no room:closed of their own (only the remaining - // player does) — so this is the definitive "I've left" signal, - // not something to wait on a server reply for. - roomActive = false; - callbacks.showMenu(); - render({ kind: 'hidden' }); - }); - game = new ThemeGuessGame(payload.themeId, payload.snippetIndex, (id, hex) => match!.handleLocalAssignment(id, hex)); - match.bindGame(game); + if (!opponentView) opponentView = new OpponentView('opponent-canvas', payload.snippetIndex); + const isNewMatch = !match; + if (!match) { + match = new MultiplayerMatch(client, opponentView, () => { + // Local player quit mid-round: per PROTOCOL.md "Quit", the + // quitter gets no room:closed of their own (only the remaining + // player does) — so this is the definitive "I've left" signal, + // not something to wait on a server reply for. + roomActive = false; + callbacks.showMenu(); + render({ kind: 'hidden' }); + }); + } + const game = getSharedGame(payload.themeId, payload.snippetIndex, (id, hex) => match!.handleLocalAssignment(id, hex)); + if (isNewMatch) match.bindGame(game); return match; }