fix(frontend): share one ThemeGuessGame instance across solo/matchmaking/rooms

ThemeGuessGame's constructor permanently binds click/mousemove/document
listeners onto the single shared #code-canvas/#category-panel. Solo,
matchmaking, and private-room flows each independently constructed their
own instance, so using more than one mode in the same page load (e.g.
Solo then Find Match) left multiple instances double-bound to the same
DOM, corrupting whichever mode was used first.

Add game/sharedGame.ts: a single lazily-constructed instance, reused via
setTheme/setSnippet and now-mutable onCategoryAssigned across every
flow. main.ts, matchmakingFlow.ts, and privateRoomFlow.ts now call
getSharedGame() instead of `new ThemeGuessGame(...)`.

Verified via CDP DOMDebugger.getEventListeners against a live session:
exactly one click/mousemove listener on #code-canvas after playing a
Solo round followed by a real matched Matchmaking round in the same
page load (previously would have been two of each).
This commit is contained in:
Gabriel Franco 2026-09-10 14:31:59 -03:00
parent bf88a340d6
commit 36b2725498
5 changed files with 94 additions and 81 deletions

View file

@ -59,18 +59,26 @@ export class ThemeGuessGame {
private height = 0; private height = 0;
private categories!: CategoryStateMap; private categories!: CategoryStateMap;
/** `onCategoryAssigned`, when provided, fires at the end of every /** Fires at the end of every `applyColor` with the category and hex
* `applyColor` with the category and hex just assigned — multiplayer * just assigned — multiplayer uses it to relay a bare
* uses it to relay a bare `round:progress` and to track the local * `round:progress` and to track the local player's running color map
* player's running color map for `round:submit`, without this class * for `round:submit`, without this class needing to know anything
* needing to know anything about the network. */ * 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( constructor(
themeId: ThemeId, themeId: ThemeId,
snippetIndex: number = pickRandomSnippetIndex(), snippetIndex: number = pickRandomSnippetIndex(),
private readonly onCategoryAssigned?: (id: CategoryId, hex: string) => void, onCategoryAssigned?: (id: CategoryId, hex: string) => void,
) { ) {
this.themeId = themeId; this.themeId = themeId;
this.snippetIndex = snippetIndex; this.snippetIndex = snippetIndex;
this.onCategoryAssigned = onCategoryAssigned;
this.build(); this.build();
this.bindEvents(); this.bindEvents();
requestAnimationFrame((t) => this.loop(t)); requestAnimationFrame((t) => this.loop(t));

View file

@ -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;
}

View file

@ -1,5 +1,5 @@
import './style.css'; import './style.css';
import { ThemeGuessGame } from './game/ThemeGuessGame'; import { getSharedGame } from './game/sharedGame';
import { THEMES } from './data/themes'; import { THEMES } from './data/themes';
import { sound } from './engine/sound'; import { sound } from './engine/sound';
import { createThemeGrid } from './ui/themeGrid'; 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 themeGrid = createThemeGrid(themeGridEl, 'gruvbox', () => sound.playOpen());
const soloConfigFlow = createSoloConfigFlow( const soloConfigFlow = createSoloConfigFlow(
@ -72,16 +70,7 @@ const soloConfigFlow = createSoloConfigFlow(
(config) => { (config) => {
showView(null); showView(null);
runThemePreview(previewElements, config.themeId, () => { runThemePreview(previewElements, config.themeId, () => {
if (game) { getSharedGame(config.themeId, config.snippetIndex, (id, hex) => soloConfigFlow.handleLocalAssignment(id, hex));
game.setTheme(config.themeId);
game.setSnippet(config.snippetIndex);
} else {
game = new ThemeGuessGame(
config.themeId,
config.snippetIndex,
(id, hex) => soloConfigFlow.handleLocalAssignment(id, hex),
);
}
themeNameBadge.textContent = THEMES[config.themeId].name; themeNameBadge.textContent = THEMES[config.themeId].name;
sound.playApply(); sound.playApply();
soloConfigFlow.startTimer(config.timeMode); soloConfigFlow.startTimer(config.timeMode);

View file

@ -8,18 +8,15 @@
// sends them in the opposite order — see the `foundAt`/`roundStartPayload` // sends them in the opposite order — see the `foundAt`/`roundStartPayload`
// handling below, which treats both orderings identically. // handling below, which treats both orderings identically.
// //
// Owns a single `OpponentView` / `ThemeGuessGame` / `MultiplayerMatch` // Owns a single `OpponentView` / `MultiplayerMatch` pair for the
// trio for the lifetime of the page, constructed lazily on the first // lifetime of the page, constructed lazily on the first matchmaking
// matchmaking round and reused (`setTheme`/`setSnippet`) on every // round and reused on every subsequent one. The `ThemeGuessGame` itself
// subsequent one — mirroring how `ui/soloConfigFlow.ts` reuses a single // is *not* owned here — it's the one shared instance from
// `ThemeGuessGame` across solo replays. `ThemeGuessGame`'s constructor // `game/sharedGame.ts` (solo, matchmaking, and private rooms all start
// binds fresh DOM listeners every time it's called, so constructing a // rounds against the same `#code-canvas`, so exactly one instance must
// second one while a first is still alive (e.g. mixing solo/bot play // ever exist; see that module's header for why).
// 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.
import { ThemeGuessGame } from '../game/ThemeGuessGame'; import { getSharedGame } from '../game/sharedGame';
import { MultiplayerMatch } from '../game/multiplayerMatch'; import { MultiplayerMatch } from '../game/multiplayerMatch';
import { OpponentView } from '../game/opponentView'; import { OpponentView } from '../game/opponentView';
import { THEMES } from '../data/themes'; import { THEMES } from '../data/themes';
@ -103,7 +100,6 @@ export function createMatchmakingFlow(
// every subsequent one this page load — see this module's header. // every subsequent one this page load — see this module's header.
let opponentView: OpponentView | null = null; let opponentView: OpponentView | null = null;
let match: MultiplayerMatch | null = null; let match: MultiplayerMatch | null = null;
let game: ThemeGuessGame | null = null;
function clearBanTimers(): void { function clearBanTimers(): void {
stopBanBanner?.(); stopBanBanner?.();
@ -194,14 +190,10 @@ export function createMatchmakingFlow(
themeNameBadge.textContent = THEMES[payload.themeId].name; themeNameBadge.textContent = THEMES[payload.themeId].name;
if (!opponentView) opponentView = new OpponentView('opponent-canvas', payload.snippetIndex); if (!opponentView) opponentView = new OpponentView('opponent-canvas', payload.snippetIndex);
const isNewMatch = !match;
if (!match) match = new MultiplayerMatch(client, opponentView, handleLocalQuit); if (!match) match = new MultiplayerMatch(client, opponentView, handleLocalQuit);
if (!game) { const game = getSharedGame(payload.themeId, payload.snippetIndex, (id, hex) => match!.handleLocalAssignment(id, hex));
game = new ThemeGuessGame(payload.themeId, payload.snippetIndex, (id, hex) => match!.handleLocalAssignment(id, hex)); if (isNewMatch) match.bindGame(game);
match.bindGame(game);
} else {
game.setTheme(payload.themeId);
game.setSnippet(payload.snippetIndex);
}
hooks.showBoard(); hooks.showBoard();
match.startRound(payload); match.startRound(payload);

View file

@ -16,33 +16,17 @@
// its own listeners on the same client, so this flow must never react // its own listeners on the same client, so this flow must never react
// to a round that belongs to a matchmaking match. // to a round that belongs to a matchmaking match.
// //
// Session component lifetime: exactly one `ThemeGuessGame` + one // Session component lifetime: one `OpponentView` + one `MultiplayerMatch`
// `OpponentView` + one `MultiplayerMatch` are constructed, lazily, the // are constructed, lazily, the first time a room's first `round:start`
// first time a room's first `round:start` arrives, then reused for // arrives, then reused for every subsequent match in that room's
// every subsequent match in that room's up-to-5-match series by calling // up-to-5-match series by calling `match.startRound()` again —
// `match.startRound()` again — `MultiplayerMatch.startRound()` already // `MultiplayerMatch.startRound()` already tears down and re-subscribes
// tears down and re-subscribes its own per-round listeners/timer, so // its own per-round listeners/timer, so it's safe to call repeatedly on
// it's safe to call repeatedly on the same instance (confirmed by // the same instance. The `ThemeGuessGame` itself is *not* owned here —
// reading its source; no fix needed there for this). Constructing a // it's the one shared instance from `game/sharedGame.ts` (solo,
// *second* `ThemeGuessGame` for a later room, or for matchmaking, would // matchmaking, and this flow all start rounds against the same
// double-bind its canvas/document event listeners onto the single // `#code-canvas`, so exactly one instance must ever exist across the
// shared `#code-canvas` — so this module's `game`/`opponentView`/`match` // whole page; see that module's header for why).
// 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).
import { THEMES } from '../data/themes'; import { THEMES } from '../data/themes';
import type { GameClient } from '../net/client'; import type { GameClient } from '../net/client';
@ -56,7 +40,7 @@ import type {
} from '../net/messages'; } from '../net/messages';
import { MultiplayerMatch } from '../game/multiplayerMatch'; import { MultiplayerMatch } from '../game/multiplayerMatch';
import { OpponentView } from '../game/opponentView'; import { OpponentView } from '../game/opponentView';
import { ThemeGuessGame } from '../game/ThemeGuessGame'; import { getSharedGame } from '../game/sharedGame';
import { openThemeVote, type ThemeVoteHandle } from './themeVote'; import { openThemeVote, type ThemeVoteHandle } from './themeVote';
export interface PrivateRoomFlowElements { export interface PrivateRoomFlowElements {
@ -133,7 +117,6 @@ export function createPrivateRoomFlow(
let transitionTimeoutId: number | null = null; let transitionTimeoutId: number | null = null;
let opponentView: OpponentView | null = null; let opponentView: OpponentView | null = null;
let game: ThemeGuessGame | null = null;
let match: MultiplayerMatch | null = null; let match: MultiplayerMatch | null = null;
const splitView = requireEl<HTMLElement>('split-view'); const splitView = requireEl<HTMLElement>('split-view');
@ -335,20 +318,21 @@ export function createPrivateRoomFlow(
* `multiplayerMatch.ts`'s documented two-phase-init order, then reuses * `multiplayerMatch.ts`'s documented two-phase-init order, then reuses
* them for every later `round:start` this room sends. */ * them for every later `round:start` this room sends. */
function ensureMatch(payload: RoundStartMessage): MultiplayerMatch { function ensureMatch(payload: RoundStartMessage): MultiplayerMatch {
if (match && game) return match; if (!opponentView) opponentView = new OpponentView('opponent-canvas', payload.snippetIndex);
const isNewMatch = !match;
opponentView = new OpponentView('opponent-canvas', payload.snippetIndex); if (!match) {
match = new MultiplayerMatch(client, opponentView, () => { match = new MultiplayerMatch(client, opponentView, () => {
// Local player quit mid-round: per PROTOCOL.md "Quit", the // Local player quit mid-round: per PROTOCOL.md "Quit", the
// quitter gets no room:closed of their own (only the remaining // quitter gets no room:closed of their own (only the remaining
// player does) — so this is the definitive "I've left" signal, // player does) — so this is the definitive "I've left" signal,
// not something to wait on a server reply for. // not something to wait on a server reply for.
roomActive = false; roomActive = false;
callbacks.showMenu(); callbacks.showMenu();
render({ kind: 'hidden' }); render({ kind: 'hidden' });
}); });
game = new ThemeGuessGame(payload.themeId, payload.snippetIndex, (id, hex) => match!.handleLocalAssignment(id, hex)); }
match.bindGame(game); const game = getSharedGame(payload.themeId, payload.snippetIndex, (id, hex) => match!.handleLocalAssignment(id, hex));
if (isNewMatch) match.bindGame(game);
return match; return match;
} }