Skip to content

refactor: split Game/index.ts god file into cohesive modules #132

Description

@nodots

Problem

src/Game/index.ts is a 2491-line god file: ~40 static methods on a single Game class spanning at least six distinct concerns (lifecycle, turn flow, robot orchestration, cube/doubling, undo, guards/queries). It is hard to navigate, hard to review, and mixes unrelated responsibilities.

This issue tracks splitting the file into cohesive modules without changing behavior or the public API. It is the structural half of the Game/index.ts remediation epic; the type-safety half (removing as any casts) is tracked in #96.

Key facts (established by reading the code)

  • All ~40 statics are Game.-qualified and none use this — extraction to free functions is mechanical (body moves verbatim; each internal Game.foo(...) becomes a direct import of foo).
  • Game is instantiated with new Game() (Services/PositionReconstructor.ts:180, GameDataAdapter.test.ts), and the gnuPositionId getter (index.ts:71) is consumed in ai/api. So the facade must remain a class keeping its instance fields and the getter — it cannot become a bare namespace.
  • External surface is ~140 Game.<method> static calls across api/ai/client. The class name and every static signature must stay identical.
  • export * from '../index' (index.ts:44) is load-bearing and must be preserved in the facade.
  • executeRobotTurn is already extracted to ./executeRobotTurn and re-attached — this is the exact pattern to replicate.

Proposed file layout under packages/core/src/Game/

File Contents
index.ts Facade only. Game class: instance fields + gnuPositionId getter + export * from '../index' + static assignments delegating to the modules below.
shared.ts createBaseGameProperties, incrementStateVersion, MAX_PIP_COUNT (pure helpers, no sibling deps).
guards.ts activePlayer, inactivePlayer, getPlayersForColor, findChecker, canRoll, canRollForStart, canPlayerRoll, canGetPossibleMoves.
undo.ts undoLastInActivePlay, canUndoActivePlay, plus new pushUndoSnapshot(game) / getUndoFrames(game) helpers extracted from the inline snapshot blocks (index.ts ~1131, ~1586).
cube.ts canOfferDouble, double, canAcceptDouble, acceptDouble, canRefuseDouble, refuseDouble, resign.
lifecycle.ts createNewGame, initialize (all 8 overloads), restoreState.
turnFlow.ts rollForStart, roll, switchDice, move, moveAndFinalize, toMoved, executeAndRecalculate, checkAndCompleteTurn, confirmTurn, startMove.
robot.ts handleRobotMovedState, confirmTurnWithRobotAutomation; re-export executeRobotTurn.

Dependency map (a DAG — no cycles)

shared.ts    (leaf)
guards.ts    (leaf)
undo.ts      (leaf)
cube.ts      -> shared
lifecycle.ts -> shared
turnFlow.ts  -> shared, guards, undo
robot.ts     -> turnFlow
index.ts     -> all (facade)

Two rules prevent circular imports:

  • Rule A — extracted free functions call each other directly (same-module import), never through the Game facade. Reaching back to Game.foo from ./index would create index -> turnFlow -> index. This is the single most important constraint.
  • Rule B — the undo push helper lives in undo.ts; turnFlow imports it (turnFlow -> undo), and undo.ts never imports turnFlow.

Facade pattern (concrete)

index.ts keeps the class shell and delegates:

import * as lifecycle from './lifecycle'
import * as turnFlow from './turnFlow'
import * as guards from './guards'
import * as cube from './cube'
import * as robot from './robot'
import * as undo from './undo'

export * from '../index'   // PRESERVE

export class Game {
  // instance fields (id, stateKind, players, board, cube, ...) unchanged
  get gnuPositionId(): string { /* unchanged */ }

  static createNewGame = lifecycle.createNewGame
  static initialize = lifecycle.initialize   // overloads carried through
  static roll = turnFlow.roll
  static move = turnFlow.move
  static double = cube.double
  static undoLastInActivePlay = undo.undoLastInActivePlay
  // ...one assignment per method
}

createBaseGameProperties / incrementStateVersion were private static, so moving them into shared.ts (no longer on the class) breaks no external contract.

Phased checklist (each phase compiles and is independently mergeable)

Each phase moves one group out and replaces its in-class bodies with static x = module.x. Verify each phase with: typecheck core, run packages/core/src/Game/__tests__ jest suite; final phase also builds api + ai.

  • Phase 0 — shared.ts (lowest risk; pure helpers everyone imports).
  • Phase 1 — guards.ts (pure queries).
  • Phase 2 — undo.ts (move undo methods; add pushUndoSnapshot/getUndoFrames; delete the 4 as any undo casts after promoting undo? to BasePlay in the types package — coordinate with chore: Eliminate as any casts in Game/index.ts with proper type definitions #96).
  • Phase 3 — cube.ts (targets crawford-jacoby-rules.test.ts).
  • Phase 4 — lifecycle.ts (carry all 8 initialize overloads; targets createNewGame.test.ts, roll-for-start-fix.test.ts).
  • Phase 5 — turnFlow.ts (largest, ~1250 lines; rewire internal Game.* to local imports; verify turn-passing*.test.ts, move-and-finalize.test.ts, isMovable-e2e.test.ts, board-persistence.test.ts).
  • Phase 6 — robot.ts.
  • Phase 7 — finalize facade; confirm new Game() and gnuPositionId still resolve; build api + ai to confirm the ~140 external Game.<method> calls are unbroken.

Coordination with #96 (as any remediation)

Both issues touch this file. Recommended order: land the undo/offeredThisTurnBy/stateVersion type additions from #96 first (or the undo.ts centralization step here), so the extraction lands into typed modules. Phase 2 here consolidates the scattered (game.activePlay as any)?.undo casts into a single undo.ts location.

Type change is in-scope (authorized). The undo casts exist only because undo?: { frames: any[] } sits on BackgammonPlayMoving (packages/types/src/play.ts:111) rather than on the BasePlay/BackgammonPlay union, so activePlay.undo isn't visible on the union type. Fix: promote undo? up to BasePlay and type frames as BackgammonGameMoving[].

Correction to an earlier note: this does not create a types→core dependency. BackgammonGameMoving is defined in the same types package (packages/types/src/game.ts:193). It only forms a play.tsgame.ts file cycle within the types package, and that cycle is harmless for a type-only import — import type { BackgammonGameMoving } from './game' is erased at compile time (no runtime cycle; tsc resolves mutually-referential types fine). So the casts can be deleted outright rather than centralized-then-deferred.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions