|
| 1 | +# Delta-V: Architectural Patterns & Refactoring Guide |
| 2 | + |
| 3 | +This document captures recommendations for reducing complexity in the Delta-V codebase and making it easier to get the whole thing working really well. The codebase is already well-structured — the pure functional engine in `shared/`, the extraction of helper modules on the client side, the typed message protocol — these are solid foundations. The suggestions below are about pushing further in the same direction, not replacing anything. |
| 4 | + |
| 5 | +## What's working well |
| 6 | + |
| 7 | +Before diving into changes, it's worth naming the patterns that are already strong: |
| 8 | + |
| 9 | +- **Pure functional game engine.** `shared/game-engine.ts` takes state + actions and returns new state + events with no side effects. This is exactly right for a turn-based game and makes the rules unit-testable in isolation. |
| 10 | +- **The "derive plan, then execute" pattern.** Files like `game-client-phase.ts`, `game-client-messages.ts`, and `game-client-phase-entry.ts` return plain data objects describing what should happen, and the caller executes them. This keeps logic testable and side effects contained. The `setState` method's entry plan execution is the best example. |
| 11 | +- **Shared types as the contract.** `types.ts` as the single source of truth for `GameState`, `Ship`, network messages, etc. ensures client and server never drift. |
| 12 | +- **Decomposed renderer.** At ~1000 lines with well-extracted sub-modules (`renderer-combat.ts`, `renderer-entities.ts`, `renderer-vectors.ts`, etc.), the renderer is doing what a game renderer should. |
| 13 | + |
| 14 | +The overarching theme of this document is: you don't need a framework, you need to shrink the surface area of `GameClient` by pulling state and logic out of the class and into composable pure functions and a thin transport layer. You're already doing this — just keep going. |
| 15 | + |
| 16 | +--- |
| 17 | + |
| 18 | +## Priority 1: Pull PlanningState out of the Renderer |
| 19 | + |
| 20 | +This is the sneakiest source of complexity in the codebase. `PlanningState` lives on the `Renderer`, but it's mutated directly by three different systems: |
| 21 | + |
| 22 | +- `InputHandler` writes to it on clicks (burns, combat targets, torpedo acceleration) |
| 23 | +- `main.ts` writes to it from keyboard actions, state transitions, combat flow, and ordnance |
| 24 | +- Renderer sub-modules read it each frame |
| 25 | + |
| 26 | +```typescript |
| 27 | +// main.ts does this constantly: |
| 28 | +this.renderer.planningState.selectedShipId = null; |
| 29 | +this.renderer.planningState.burns.clear(); |
| 30 | +this.renderer.planningState.queuedAttacks.push(attack); |
| 31 | +``` |
| 32 | + |
| 33 | +Three systems reaching into the same mutable bag is where "who changed this and when" bugs come from. |
| 34 | + |
| 35 | +### Proposed change |
| 36 | + |
| 37 | +Pull `PlanningState` out as a standalone object that `GameClient` owns. Pass it to both the renderer and input handler as a read reference. Mutations go through helper functions (some already exist, like `createClearedCombatPlan`). The renderer just reads it each frame. |
| 38 | + |
| 39 | +```typescript |
| 40 | +// Owned by GameClient, not Renderer |
| 41 | +const planning: PlanningState = createInitialPlanningState(); |
| 42 | + |
| 43 | +// Renderer receives it as a read dependency |
| 44 | +const renderer = new Renderer(canvas, planning); |
| 45 | + |
| 46 | +// InputHandler produces commands instead of mutating directly |
| 47 | +const input = new InputHandler(canvas, camera); |
| 48 | +input.onCommand = (cmd) => dispatch(cmd); |
| 49 | +``` |
| 50 | + |
| 51 | +This eliminates the coupling between input, orchestration, and rendering around a shared mutable object. It also makes it straightforward to snapshot planning state for debugging or undo. |
| 52 | + |
| 53 | +--- |
| 54 | + |
| 55 | +## Priority 2: Transport adapter for local vs network play |
| 56 | + |
| 57 | +There are parallel code paths throughout `main.ts` for local and network play: |
| 58 | + |
| 59 | +```typescript |
| 60 | +if (this.isLocalGame) { |
| 61 | + this.localProcessCombat(attacks); |
| 62 | +} else { |
| 63 | + this.send({ type: 'combat', attacks }); |
| 64 | +} |
| 65 | +``` |
| 66 | + |
| 67 | +This pattern repeats for astrogation, ordnance, skip ordnance, skip combat, begin combat, fleet ready, and rematch — roughly 8 places. |
| 68 | + |
| 69 | +### Proposed change |
| 70 | + |
| 71 | +Define a `GameTransport` interface: |
| 72 | + |
| 73 | +```typescript |
| 74 | +interface GameTransport { |
| 75 | + submitAstrogation(orders: AstrogationOrder[]): void; |
| 76 | + submitCombat(attacks: CombatAttack[]): void; |
| 77 | + submitOrdnance(launches: OrdnanceLaunch[]): void; |
| 78 | + submitFleetReady(purchases: FleetPurchase[]): void; |
| 79 | + skipOrdnance(): void; |
| 80 | + skipCombat(): void; |
| 81 | + beginCombat(): void; |
| 82 | + requestRematch(): void; |
| 83 | +} |
| 84 | +``` |
| 85 | + |
| 86 | +Then implement `WebSocketTransport` and `LocalTransport`. `GameClient` calls `this.transport.submitCombat(attacks)` and never branches on `isLocalGame`. The transport handles the mechanics — the WebSocket version serialises and sends, the local version calls the engine directly and feeds results back through a callback or event. |
| 87 | + |
| 88 | +This also makes it trivial to add new transports later (e.g. replay playback, spectator mode, or test harnesses that inject canned server responses). |
| 89 | + |
| 90 | +--- |
| 91 | + |
| 92 | +## Priority 3: Command/dispatch instead of method soup |
| 93 | + |
| 94 | +`GameClient` has ~30 private methods that are action handlers (`undoSelectedShipBurn`, `confirmOrders`, `queueAttack`, `fireAllAttacks`, `sendOrdnanceLaunch`, `sendSkipCombat`, etc.). The keyboard handler is already halfway there — `deriveKeyboardAction` returns a `KeyboardAction` discriminated union, and `handleKeyboardAction` switches on it. But the execution side is still scattered across methods that directly mutate `this`. |
| 95 | + |
| 96 | +### Proposed change |
| 97 | + |
| 98 | +Define a unified command type: |
| 99 | + |
| 100 | +```typescript |
| 101 | +type GameCommand = |
| 102 | + | { type: 'confirmOrders' } |
| 103 | + | { type: 'undoBurn' } |
| 104 | + | { type: 'queueAttack' } |
| 105 | + | { type: 'fireAllAttacks' } |
| 106 | + | { type: 'launchOrdnance'; ordType: 'mine' | 'torpedo' | 'nuke' } |
| 107 | + | { type: 'emplaceBase' } |
| 108 | + | { type: 'skipOrdnance' } |
| 109 | + | { type: 'skipCombat' } |
| 110 | + | { type: 'adjustCombatStrength'; delta: number } |
| 111 | + | { type: 'resetCombatStrength' } |
| 112 | + | { type: 'clearCombatSelection' } |
| 113 | + | { type: 'cycleShip'; direction: 1 | -1 } |
| 114 | + | { type: 'focusNearestEnemy' } |
| 115 | + | { type: 'focusOwnFleet' } |
| 116 | + | { type: 'panCamera'; dx: number; dy: number } |
| 117 | + | { type: 'zoomCamera'; factor: number } |
| 118 | + | { type: 'toggleLog' } |
| 119 | + | { type: 'toggleHelp' } |
| 120 | + | { type: 'toggleMute' } |
| 121 | + | { type: 'requestRematch' } |
| 122 | + | { type: 'exitToMenu' }; |
| 123 | +``` |
| 124 | + |
| 125 | +Route all inputs through a single `dispatch(cmd: GameCommand)` method. The keyboard handler, UI callbacks, and potentially the AI flow all go through this one bottleneck. This gives you: |
| 126 | + |
| 127 | +- One place to add logging/tracing |
| 128 | +- One place to add guard conditions (e.g. "ignore commands during animation") |
| 129 | +- A clear, greppable list of everything the game can do |
| 130 | +- Easy integration with an undo stack if you ever want one |
| 131 | + |
| 132 | +The existing `KeyboardAction` type can map directly into `GameCommand` — it's almost the same union already. |
| 133 | + |
| 134 | +--- |
| 135 | + |
| 136 | +## Priority 4: Centralise mutable client state |
| 137 | + |
| 138 | +State is currently spread across multiple locations on `GameClient`: |
| 139 | + |
| 140 | +- `this.state` — the client phase (`ClientState`) |
| 141 | +- `this.gameState` — the authoritative game state |
| 142 | +- `this.renderer.planningState` — input/planning UI state |
| 143 | +- `this.ws`, `this.reconnectAttempts`, `this.reconnectTimer` — connection state |
| 144 | +- `this.pingInterval`, `this.lastPingSent`, `this.latencyMs` — ping state |
| 145 | +- `this.turnStartTime`, `this.turnTimerInterval`, `this.timerWarningPlayed` — timer state |
| 146 | +- `this.playerId`, `this.gameCode`, `this.inviteLink`, `this.scenario` — session state |
| 147 | +- `this.isLocalGame`, `this.aiDifficulty` — mode state |
| 148 | +- `this.combatWatchInterval`, `this.lastLoggedTurn` — miscellaneous |
| 149 | + |
| 150 | +### Proposed change |
| 151 | + |
| 152 | +Group into a single typed context: |
| 153 | + |
| 154 | +```typescript |
| 155 | +interface ClientContext { |
| 156 | + // Core |
| 157 | + clientState: ClientState; |
| 158 | + gameState: GameState | null; |
| 159 | + planning: PlanningState; |
| 160 | + |
| 161 | + // Session |
| 162 | + playerId: number; |
| 163 | + gameCode: string | null; |
| 164 | + inviteLink: string | null; |
| 165 | + scenario: string; |
| 166 | + isLocalGame: boolean; |
| 167 | + aiDifficulty: AIDifficulty; |
| 168 | + |
| 169 | + // Connection |
| 170 | + connection: { |
| 171 | + ws: WebSocket | null; |
| 172 | + reconnectAttempts: number; |
| 173 | + latencyMs: number; |
| 174 | + }; |
| 175 | + |
| 176 | + // Timers (managed externally) |
| 177 | + turnStartTime: number; |
| 178 | + lastLoggedTurn: number; |
| 179 | +} |
| 180 | +``` |
| 181 | + |
| 182 | +You don't need Redux or a state management library — just a plain object that you pass to your pure derivation functions instead of pulling fields off `this`. This makes it possible to snapshot the entire client state for debugging, and makes the derivation functions' dependencies explicit in their signatures. |
| 183 | + |
| 184 | +--- |
| 185 | + |
| 186 | +## Priority 5: Typed event bus for UI callbacks |
| 187 | + |
| 188 | +`UIManager` has ~15 nullable callback properties that `main.ts` wires up in the constructor: |
| 189 | + |
| 190 | +```typescript |
| 191 | +onSelectScenario: ((scenario: string) => void) | null = null; |
| 192 | +onSinglePlayer: ((scenario: string, difficulty: ...) => void) | null = null; |
| 193 | +onJoin: ((code: string, playerToken?: string | null) => void) | null = null; |
| 194 | +onUndo: (() => void) | null = null; |
| 195 | +onConfirm: (() => void) | null = null; |
| 196 | +onLaunchOrdnance: ((type: 'mine' | 'torpedo' | 'nuke') => void) | null = null; |
| 197 | +// ... ~10 more |
| 198 | +``` |
| 199 | + |
| 200 | +This works but makes the relationship between UI events and game actions invisible unless you read the constructor. |
| 201 | + |
| 202 | +### Proposed change |
| 203 | + |
| 204 | +Define a typed UI event union: |
| 205 | + |
| 206 | +```typescript |
| 207 | +type UIEvent = |
| 208 | + | { type: 'selectScenario'; scenario: string } |
| 209 | + | { type: 'startSinglePlayer'; scenario: string; difficulty: AIDifficulty } |
| 210 | + | { type: 'join'; code: string; playerToken?: string | null } |
| 211 | + | { type: 'confirm' } |
| 212 | + | { type: 'undo' } |
| 213 | + | { type: 'launchOrdnance'; ordType: 'mine' | 'torpedo' | 'nuke' } |
| 214 | + | { type: 'emplaceBase' } |
| 215 | + | { type: 'skipOrdnance' } |
| 216 | + | { type: 'attack' } |
| 217 | + | { type: 'fireAll' } |
| 218 | + | { type: 'skipCombat' } |
| 219 | + | { type: 'fleetReady'; purchases: FleetPurchase[] } |
| 220 | + | { type: 'rematch' } |
| 221 | + | { type: 'exit' } |
| 222 | + | { type: 'selectShip'; shipId: string }; |
| 223 | +``` |
| 224 | + |
| 225 | +`UIManager` fires events through a single typed emitter. `GameClient` subscribes once. The events can feed into the same `dispatch` function from Priority 3, closing the loop: UI events → commands → dispatch → state changes. |
| 226 | + |
| 227 | +--- |
| 228 | + |
| 229 | +## Priority 6: InputHandler produces commands, not mutations |
| 230 | + |
| 231 | +Currently `InputHandler` takes a `Camera` and `PlanningState` in its constructor and mutates both directly. It also has a bare `onConfirm` callback. |
| 232 | + |
| 233 | +### Proposed change |
| 234 | + |
| 235 | +The input handler should translate pointer/touch events into world-space interactions and emit commands — the same `GameCommand` type from Priority 3. Clicks and keyboard actions both produce the same command type, giving you one place to handle game input regardless of source. |
| 236 | + |
| 237 | +```typescript |
| 238 | +class InputHandler { |
| 239 | + onCommand: ((cmd: GameCommand) => void) | null = null; |
| 240 | + |
| 241 | + private handleClick(screenX: number, screenY: number) { |
| 242 | + // ... translate to hex, determine interaction ... |
| 243 | + this.onCommand?.({ type: 'setBurnDirection', shipId, direction }); |
| 244 | + } |
| 245 | +} |
| 246 | +``` |
| 247 | + |
| 248 | +The input handler no longer needs a reference to `PlanningState` at all — it just needs enough context to figure out what the click means (game state, map, current phase). The command consumer applies the planning state changes. |
| 249 | + |
| 250 | +--- |
| 251 | + |
| 252 | +## Priority 7: Async AI turn loop |
| 253 | + |
| 254 | +`processAIPhases` is a recursive chain via callbacks: |
| 255 | + |
| 256 | +```typescript |
| 257 | +this.handleLocalResolution( |
| 258 | + resolveAstrogationStep(...), |
| 259 | + () => this.processAIPhases(), // recursion via callback |
| 260 | + ... |
| 261 | +); |
| 262 | +``` |
| 263 | + |
| 264 | +The AI turn is a state machine disguised as mutual recursion through `setTimeout` and animation callbacks. It works but is hard to follow and harder to debug. |
| 265 | + |
| 266 | +### Proposed change |
| 267 | + |
| 268 | +An explicit async loop: |
| 269 | + |
| 270 | +```typescript |
| 271 | +private async runAITurn() { |
| 272 | + while (this.gameState && this.gameState.phase !== 'gameOver') { |
| 273 | + const plan = deriveAIActionPlan( |
| 274 | + this.gameState, this.playerId, this.map, this.aiDifficulty |
| 275 | + ); |
| 276 | + if (plan.kind === 'none' || plan.kind === 'transition') break; |
| 277 | + |
| 278 | + for (const entry of plan.logEntries ?? []) { |
| 279 | + this.ui.logText(entry); |
| 280 | + } |
| 281 | + |
| 282 | + await this.executeAIResolution(plan); |
| 283 | + } |
| 284 | + this.localCheckGameEnd(); |
| 285 | + if (this.gameState?.phase !== 'gameOver') { |
| 286 | + this.transitionToPhase(); |
| 287 | + } |
| 288 | +} |
| 289 | + |
| 290 | +private executeAIResolution(plan: AIActionPlan): Promise<void> { |
| 291 | + return new Promise((resolve) => { |
| 292 | + this.handleLocalResolution( |
| 293 | + resolveStep(plan), |
| 294 | + resolve, |
| 295 | + plan.errorPrefix, |
| 296 | + ); |
| 297 | + }); |
| 298 | +} |
| 299 | +``` |
| 300 | + |
| 301 | +This makes the AI turn readable as a sequence rather than a callback graph. The animation still happens — `handleLocalResolution` still calls `presentMovementResult` with a callback — but the callback resolves a promise instead of recursing. |
| 302 | + |
| 303 | +--- |
| 304 | + |
| 305 | +## Priority 8: Serialisation codec |
| 306 | + |
| 307 | +`deserializeState` is called in `handleMessage` to handle the `Map` types in `GameState` and `SolarSystemMap` that don't survive JSON round-trips. This is a potential landmine when adding new `Map` fields. |
| 308 | + |
| 309 | +### Proposed change |
| 310 | + |
| 311 | +Create a `codec.ts` module with explicit serialise/deserialise functions: |
| 312 | + |
| 313 | +```typescript |
| 314 | +// shared/codec.ts |
| 315 | +export function serializeGameState(state: GameState): SerializedGameState { ... } |
| 316 | +export function deserializeGameState(raw: SerializedGameState): GameState { ... } |
| 317 | +``` |
| 318 | + |
| 319 | +Add a round-trip test that creates a full game state with all possible fields populated, serialises it, deserialises it, and asserts deep equality. This catches any new `Map` or `Set` fields that get added to the types without updating the codec. |
| 320 | + |
| 321 | +--- |
| 322 | + |
| 323 | +## Priority 9: Reduce InputHandler's knowledge |
| 324 | + |
| 325 | +The input handler currently knows about the camera, game state, map, player ID, and planning state. It uses all of these to determine what a click means in context. |
| 326 | + |
| 327 | +### Proposed change (longer-term) |
| 328 | + |
| 329 | +Instead of giving `InputHandler` references to everything, have it produce raw spatial events: |
| 330 | + |
| 331 | +```typescript |
| 332 | +type InputEvent = |
| 333 | + | { type: 'clickHex'; hex: HexCoord } |
| 334 | + | { type: 'clickMinimap'; worldPos: PixelCoord } |
| 335 | + | { type: 'doubleClickWorld'; worldPos: PixelCoord } |
| 336 | + | { type: 'pan'; dx: number; dy: number } |
| 337 | + | { type: 'zoom'; cx: number; cy: number; factor: number }; |
| 338 | +``` |
| 339 | + |
| 340 | +A separate interpretation layer (a pure function) maps `InputEvent` + current state → `GameCommand`. This makes the input handler trivially testable (it just translates coordinates) and puts all the game-aware click logic in a pure function that's also easy to test. |
| 341 | + |
| 342 | +This is lower priority because the current structure works and the click logic is already partially extracted into `game-client-input.ts`, `game-client-combat.ts`, etc. But it's the natural endpoint of the patterns you're already using. |
| 343 | + |
| 344 | +--- |
| 345 | + |
| 346 | +## Suggested order of work |
| 347 | + |
| 348 | +1. **Pull `PlanningState` out of the renderer** — removes the most coupling with the least disruption |
| 349 | +2. **Transport adapter for local/network** — eliminates the `isLocalGame` branching throughout `main.ts` |
| 350 | +3. **Command dispatch** — unifies keyboard, UI, and input handling into one flow |
| 351 | +4. **Centralise client state** — makes derivation functions' dependencies explicit |
| 352 | +5. **Typed UI event bus** — cleans up the callback wiring |
| 353 | +6. **Async AI loop** — makes the AI turn readable |
| 354 | +7. **Serialisation codec** — prevents future bugs |
| 355 | +8. **Input handler produces commands** — the long-term clean endpoint |
| 356 | + |
| 357 | +Each step is independent and can be done incrementally. The first two remove the most accidental complexity. The command dispatch is the one that makes everything else easier to add. |
0 commit comments