docs: map existing codebase
- STACK.md - Technologies and dependencies - ARCHITECTURE.md - System design and patterns - STRUCTURE.md - Directory layout - CONVENTIONS.md - Code style and patterns - TESTING.md - Test structure - INTEGRATIONS.md - External services - CONCERNS.md - Technical debt and issues
This commit is contained in:
Thomas Richter
205
.planning/codebase/CONCERNS.md
Normal file
205
.planning/codebase/CONCERNS.md
Normal file
@@ -0,0 +1,205 @@
|
||||
# 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*
|
||||
Reference in New Issue
Block a user