whalehunting/.planning/codebase/CONCERNS.md

# Codebase Concerns

**Analysis Date:** 2026-02-04

## Architecture & State Management

**Fragile Inventory Passing System:**
- Issue: Inventory object is manually passed between scenes via `data` parameter in every scene transition. No centralized state management.
- Files: `src/scenes/IntroScene.js`, `src/scenes/ShipDeckScene.js`, `src/scenes/MapScene.js`, `src/scenes/HuntingScene.js`, `src/scenes/TransitionScene.js`
- Impact: Each scene maintains its own copy of inventory. A player could lose progress if a scene fails to properly receive or pass inventory data. Adding new inventory items requires changes across all 5 scenes. Risk of inventory corruption during rapid scene transitions.
- Fix approach: Create a centralized GameState or InventoryManager singleton that all scenes access. Example structure:
  ```javascript
  class GameState {
    static instance = null;
    static getInstance() {
      if (!this.instance) this.instance = new GameState();
      return this.instance;
    }
    constructor() {
      this.inventory = { whaleOil: 0, fuel: 100, penguins: 0 };
    }
  }
  ```
  Then all scenes reference `GameState.getInstance().inventory` instead of managing copies.

## Testing Coverage Gaps

**Unit Tests Don't Test Actual Scene Logic:**
- What's not tested: Game logic tests in `tests/unit/game-logic.test.js` only test pure functions and simple calculations. They don't test the actual scene behavior, event handlers, or UI state changes.
- Files: `tests/unit/game-logic.test.js`, `src/scenes/*.js`
- Specific gaps:
  - No tests for barrel creation/destruction logic in ShipDeckScene
  - No tests for whale spawning, movement, health tracking
  - No tests for harpoon firing and collision detection
  - No tests for scene transition data passing
  - No tests for inventory display updates
- Risk: Bugs in core game mechanics (whale health, fuel consumption, barrel positioning) can't be caught automatically. Changes to scene code could break functionality without detection.
- Priority: High - Core game loop (hunting scene) is untested

**E2E Tests Use Hard-coded Click Positions:**
- What's tested: `tests/e2e/game-flow.spec.js` relies on exact pixel coordinates for button clicks. Example: `canvas.click({ position: { x: 400, y: 400 } })`
- Problem: Any change to UI layout, button positioning, or screen resolution will break these tests without actually breaking the game
- Files: `tests/e2e/game-flow.spec.js` (lines 24, 40, 58, 66, 73, 95, 113-117, 125, 136)
- Risk: Tests become maintenance burden; false positives when layout changes
- Fix approach: Use Phaser's scene testing utilities or find elements by ID/class instead of pixel positions

**No Mobile Touch Testing:**
- What's tested: E2E includes mobile viewport test but only checks canvas visibility and scaling, not touch interaction functionality
- Files: `tests/e2e/game-flow.spec.js` (lines 144-173)
- Missing: Tests for touch event handlers, multi-touch scenarios, long-press interactions
- Risk: Touch controls may break without detection. Buttons may be non-responsive on actual devices.

## Performance & Complexity Issues

**HuntingScene is Large and Complex:**
- Issue: Single 637-line file handles whale AI, harpoon physics, collision detection, camera effects, input management, and rendering
- Files: `src/scenes/HuntingScene.js`
- Specific problem areas:
  - Lines 316-398: Whale update logic duplicates movement calculation and bounds checking
  - Lines 434-450: Harpoon update is simple loop-over-array that could become O(n²) with many harpoons
  - Lines 453-489: Collision detection recalculates screen position every frame for every harpoon
  - Lines 607-630: Camera sway uses a `time.addEvent()` with permanent loop instead of native `update()` method
- Impact: Difficult to debug, test, or modify. Camera sway timer creates memory leak potential (never cancelled). Whale movement becomes laggy with multiple whales.
- Fix approach: Extract whale logic to separate `Whale` class, harpoons to `Harpoon` class, collision logic to separate method, use `update()` for camera instead of addEvent()

**Barrel Recreation is Inefficient:**
- Issue: In `ShipDeckScene.js`, `updateInventoryDisplay()` calls `clearBarrels()` then recreates all barrels every time inventory changes (lines 294-297)
- Files: `src/scenes/ShipDeckScene.js` (lines 225-297)
- Impact: Visual flicker; hundreds of objects created/destroyed repeatedly. On low-end devices, could cause jank.
- Fix approach: Reuse barrel objects or update only visual properties (fill color, scale) instead of destroying/recreating

**Graphics Objects Never Destroyed:**
- Issue: Graphics objects created in multiple scenes are never destroyed - only drawn once
- Files:
  - `src/scenes/IntroScene.js` (lines 82-98, 101-116): waves and ship drawn once
  - `src/scenes/HuntingScene.js` (lines 144-170): crosshair graphics.generateTexture() leaves original graphics object
  - `src/scenes/TransitionScene.js` (lines 286-301): ocean graphics never destroyed
- Impact: Memory leak. Small issue now but accumulates with multiple playthroughs. Old graphics objects persist in memory even after scene changes.
- Fix approach: Call `.destroy()` on graphics objects after `.generateTexture()` or scene cleanup

## Known Behavioral Issues

**Incomplete Features Referenced in Code:**
- Issue: Code contains references to unimplemented scenes
- Files: `src/scenes/MapScene.js` (lines 197, 207)
- Problem: Pressing buttons to go to Antarctic or Port just returns to MapScene. Code comments say "Will change to 'AntarcticScene' when implemented" and "PortScene when implemented"
- Impact: Two major gameplay areas not implemented. Players can't sell whale oil or gather penguins as intended
- No fix needed for this sprint; just document that these scenes don't exist

**Penguin Cage Visibility Logic is Opaque:**
- Issue: Penguin cage shown/hidden based on `inventory.penguins > 0` but there's no way in current code to acquire penguins
- Files: `src/scenes/ShipDeckScene.js` (lines 83-94, 281-291)
- Impact: Code path never executes. Feature is stub/placeholder.
- Severity: Low - intentional incomplete feature, but makes code harder to understand

**Whale Diving Mechanic is Incomplete:**
- Issue: Whales go transparent when diving but no actual "invulnerability" is enforced - collision detection still runs (line 461 checks `diving` but collision only prevented if whale is visible or alpha check)
- Files: `src/scenes/HuntingScene.js` (lines 304-306, 333-342, 394-410, 460-463)
- Risk: Harpoons might hit whale during dive in edge cases
- Fix approach: Ensure collision detection returns early if `whale.getData('diving')`

## Security & Validation Issues

**No Inventory Bounds Checking:**
- Issue: Inventory values can go negative or exceed max without validation
- Files: All scene files, particularly `src/scenes/HuntingScene.js` (lines 586, 589)
- Example problem: If fuel goes below 0, the display shows negative fuel and barrel count calculation breaks
- Current safeguard: Line 547 checks `fuel < 2` before consuming, but nothing prevents other code paths from setting fuel to -5
- Fix approach: Create Inventory class with setters that enforce bounds:
  ```javascript
  class Inventory {
    setFuel(value) { this.fuel = Math.max(0, Math.min(100, value)); }
  }
  ```

**Hard-coded Game Constants Scattered Throughout:**
- Issue: Magic numbers for inventory limits, costs, and gameplay values are hard-coded in multiple files
- Files:
  - Fuel max: `src/scenes/ShipDeckScene.js:276`, `src/scenes/MapScene.js:149`, `src/scenes/HuntingScene.js:234`
  - Oil max: `src/scenes/ShipDeckScene.js:277`, `src/scenes/MapScene.js:150`, `src/scenes/HuntingScene.js:235`
  - Whale health: `src/scenes/HuntingScene.js:292` (3 hits)
  - Fuel cost to process: `src/scenes/HuntingScene.js:547, 586` (2 fuel)
  - Barrel size calculations: `src/scenes/ShipDeckScene.js:105-165` (hardcoded ratios)
- Impact: Changing game balance requires finding and updating 10+ locations. High risk of inconsistency.
- Fix approach: Create `GameConstants.js` file:
  ```javascript
  export const GAME_CONSTANTS = {
    INVENTORY: { MAX_FUEL: 100, MAX_OIL: 50, MAX_PENGUINS: 20 },
    WHALE: { HEALTH: 3, PROCESS_FUEL_COST: 2 },
    BARRELS: { FUEL_UNITS_PER_BARREL: 10, OIL_UNITS_PER_BARREL: 10 }
  };
  ```

## Code Quality Issues

**Duplicate Inventory Initialization:**
- Issue: Default inventory object defined in 5 different places
- Files: `src/scenes/IntroScene.js:120-124`, `src/scenes/ShipDeckScene.js:6-10`, `src/scenes/HuntingScene.js:9`, `src/scenes/MapScene.js:10`, `src/scenes/TransitionScene.js:10`
- Risk: If inventory structure changes, must update in all 5 places
- Fix: One source of truth (GameState or constants)

**Inconsistent Data Passing Pattern:**
- Issue: Scene init() method doesn't always match how data is received
- Files: `src/scenes/TransitionScene.js` (line 10) uses OR operator for defaults but other scenes (ShipDeckScene line 15) use if check
- Impact: Minor style inconsistency, slightly reduces readability

**No Error Handling for Scene Transitions:**
- Issue: All `this.scene.start()` calls don't handle failures
- Files: All scene files
- Risk: If scene key is misspelled or doesn't exist, silently fails
- Fix: Add error handling or validation

## Missing Features Blocking Full Gameplay

**No Penguin Collection Mechanic:**
- What's missing: Antarctic Island scene doesn't exist, so penguins can't be collected
- Blocks: Complete game loop where fuel can be managed through penguin burning
- Files: Referenced in `src/scenes/MapScene.js:197` but scene doesn't exist

**No Port/Trading System:**
- What's missing: Port scene doesn't exist, can't sell whale oil
- Blocks: Economic gameplay loop and victory condition
- Files: Referenced in `src/scenes/MapScene.js:207` but scene doesn't exist

**No Game Over or Win Condition:**
- What's missing: No way to fail (fuel runs out) or win (enough whale oil collected)
- Impact: Game has no clear objective or end state
- Files: N/A - logic doesn't exist anywhere

**No Persistence:**
- What's missing: Game state lost on page reload
- Impact: Can't save/load progress
- Not critical for prototype but important for release

## Scaling & Device Issues

**Hardcoded Screen Dimensions:**
- Issue: Game assumes 800x600 canvas with many hardcoded position values
- Files: `src/main.js:10-11`, all scene files
- Example: `src/scenes/IntroScene.js:21` (text at x:400, y:150), `src/scenes/HuntingScene.js:139-140` (crosshair clamped to 0-800, 0-600)
- Risk: Resizing game window breaks positioning. Mobile scaling works via Phaser but coordinates are still hardcoded
- Fix: Use relative positioning or percentage-based coordinates

**Mobile Detection Uses navigator.userAgent:**
- Issue: `src/scenes/HuntingScene.js:23` uses userAgent string matching
- Risk: Can be spoofed, unreliable on some devices
- Better approach: Use Phaser's `this.sys.game.device` API or check touch availability

## Testing Infrastructure Concerns

**Vitest Config Exposes Dev Server Publicly:**
- Issue: `vitest.config.js:21-22` sets `host: '0.0.0.0'` allowing external connections
- Files: `vitest.config.js`
- Risk: In production, test server would be exposed to internet
- Fix: Only use in development (check NODE_ENV)

**E2E Tests Have Long Arbitrary Waits:**
- Issue: Tests use `waitForTimeout(1000)` and `waitForTimeout(2000)` instead of waiting for elements/state
- Files: `tests/e2e/game-flow.spec.js` (almost every test)
- Risk: Tests are slow and flaky; may timeout on slow devices but pass on fast ones
- Fix: Use `waitForSelector()`, `waitForFunction()`, or Phaser's scene ready events

---

*Concerns audit: 2026-02-04*