diff --git a/.claude/settings.local.json b/.claude/settings.local.json index e6d9c2d..85be06a 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -2,7 +2,8 @@ "permissions": { "allow": [ "Bash(find:*)", - "Bash(cat:*)" + "Bash(cat:*)", + "Bash(pnpm run:*)" ], "deny": [], "ask": [] diff --git a/REFACTORING_SUMMARY.md b/REFACTORING_SUMMARY.md deleted file mode 100644 index c7aca57..0000000 --- a/REFACTORING_SUMMARY.md +++ /dev/null @@ -1,263 +0,0 @@ -# Refactoring Summary -**Date:** 2025-10-02 -**Based on:** [REVIEW_FINDINGS.md](REVIEW_FINDINGS.md) - ---- - -## Changes Completed ✅ - -All critical and important issues identified in the code review have been addressed. - -### 1. Created Utility Modules - -#### **CoordinateUtils** ([src/utils/coordinates.ts](src/utils/coordinates.ts)) -Eliminated 4 instances of coordinate conversion duplication: -- `gameToScreen()` - Convert game coords (meters) → screen coords (pixels) -- `screenToGame()` - Convert screen coords (pixels) → game coords (meters) -- `getScreenCenter()` - Get screen center position - -**Impact:** -- Removed duplicate code from Player.ts (2 instances) -- Removed duplicate code from Puck.ts (2 instances) -- Centralized coordinate system logic in one place - -#### **MathUtils** ([src/utils/math.ts](src/utils/math.ts)) -Eliminated 4+ instances of math operation duplication: -- `distance()` - Calculate distance between two points -- `distanceSquared()` - Fast distance comparison (no sqrt) -- `normalizeAngle()` - Normalize angle to [-PI, PI] -- `angleDifference()` - Calculate shortest angular difference - -**Impact:** -- Removed duplicate code from GameScene.ts (3 instances) -- Removed duplicate code from Player.ts (2 instances) -- Removed duplicate code from BehaviorTree.ts (1 instance) -- Simplified angle calculations in Player.ts - ---- - -### 2. Extracted Magic Numbers to Constants - -Added 23 new constants to [src/config/constants.ts](src/config/constants.ts): - -#### Player Constants -```typescript -PLAYER_RADIUS_GOALIE = 12 // pixels -PLAYER_RADIUS_SKATER = 10 // pixels -SPEED_SCALE_FACTOR = 10 // converts 0-100 attribute to m/s -GOAL_DECELERATION_RATE = 0.9 // after goal celebration -``` - -#### Movement Constants -```typescript -MOVEMENT_STOP_THRESHOLD = 0.1 // meters -``` - -#### Puck Constants -```typescript -PUCK_PICKUP_RADIUS = 1.5 // meters -PUCK_CARRY_DISTANCE = 1.0 // meters -MAX_PUCK_VELOCITY = 50 // m/s -SHOT_SPEED = 30 // m/s -``` - -#### Goal Constants -```typescript -GOAL_POST_THICKNESS = 0.3 // meters -GOAL_BAR_THICKNESS = 0.4 // meters -``` - -#### Rink Visual Constants -```typescript -FACEOFF_CIRCLE_RADIUS = 4.5 // meters -CENTER_DOT_RADIUS = 5 // pixels -``` - -#### AI/Behavior Constants -```typescript -SHOOTING_RANGE = 10 // meters -SHOOTING_ANGLE_THRESHOLD = Math.PI / 4 // radians (45°) -GOALIE_RANGE = 3 // meters -``` - -**Impact:** -- All magic numbers now have descriptive names -- Easy to tune game balance (all values in one place) -- Self-documenting code (names explain purpose) - ---- - -### 3. Updated All Files to Use Utilities & Constants - -#### [Player.ts](src/entities/Player.ts) -- ✅ Uses `CoordinateUtils.gameToScreen()` in constructor -- ✅ Uses `CoordinateUtils.gameToScreen()` in `setGamePosition()` -- ✅ Uses `CoordinateUtils.screenToGame()` in `update()` -- ✅ Uses `MathUtils.distance()` for target distance -- ✅ Uses `MathUtils.angleDifference()` for rotation -- ✅ Uses `MathUtils.normalizeAngle()` to clean up angle -- ✅ Uses `PLAYER_RADIUS_*` constants (2 places) -- ✅ Uses `SPEED_SCALE_FACTOR` constant -- ✅ Uses `MOVEMENT_STOP_THRESHOLD` constant -- ✅ Uses `GOAL_DECELERATION_RATE` constant - -**Lines reduced:** 211 → 212 (cleaner despite being same length) - -#### [Puck.ts](src/entities/Puck.ts) -- ✅ Uses `CoordinateUtils.gameToScreen()` in constructor -- ✅ Uses `CoordinateUtils.gameToScreen()` in `setGamePosition()` -- ✅ Uses `CoordinateUtils.screenToGame()` in `update()` -- ✅ Uses `MAX_PUCK_VELOCITY` constant - -**Lines reduced:** 126 → 120 (6 lines saved) - -#### [GameScene.ts](src/game/GameScene.ts) -- ✅ Uses `MathUtils.distance()` (3 places) -- ✅ Uses `FACEOFF_CIRCLE_RADIUS` constant -- ✅ Uses `CENTER_DOT_RADIUS` constant -- ✅ Uses `MOVEMENT_STOP_THRESHOLD` constant -- ✅ Uses `PUCK_CARRY_DISTANCE` constant -- ✅ Uses `PUCK_PICKUP_RADIUS` constant -- ✅ Uses `SHOT_SPEED` constant -- ✅ **Refactored `update()` method** (see section 4 below) - -**Lines reduced:** 234 → 247 (13 lines added, but much cleaner structure) - -#### [BehaviorTree.ts](src/systems/BehaviorTree.ts) -- ✅ Uses `MathUtils.distance()` for shot range check -- ✅ Uses `GOAL_LINE_OFFSET` constant (3 places - **CRITICAL FIX**) -- ✅ Uses `GOALIE_RANGE` constant -- ✅ Uses `SHOOTING_RANGE` constant -- ✅ Uses `SHOOTING_ANGLE_THRESHOLD` constant - -**CRITICAL:** Fixed hardcoded `±26` values that duplicated `GOAL_LINE_OFFSET` - -**Lines reduced:** 139 → 147 (8 lines added for imports, but cleaner logic) - -#### [Goal.ts](src/game/Goal.ts) -- ✅ Uses `GOAL_POST_THICKNESS` constant -- ✅ Uses `GOAL_BAR_THICKNESS` constant - -**Lines reduced:** 144 → 144 (same length, cleaner code) - ---- - -### 4. Refactored GameScene.update() - -**Before:** 56-line monolithic update loop -```typescript -update(_time: number, delta: number) { - // All logic in one massive method - // - Puck updates - // - Puck pickup checks - // - Goal checks - // - Player AI evaluation - // - Player movement - // - Puck carrier positioning -} -``` - -**After:** Clean, modular structure -```typescript -update(_time: number, delta: number) { - this.updatePuck(); - this.updatePlayers(delta); - this.checkGoals(); -} - -private updatePuck() { /* ... */ } -private updatePlayers(delta: number) { /* ... */ } -private updatePuckCarrier(player: Player) { /* ... */ } -private checkGoals() { /* ... */ } -``` - -**Benefits:** -- Each method has a single responsibility -- Easier to understand flow at a glance -- Simpler to test individual pieces -- Better code organization - ---- - -## Impact Summary - -### DRY Violations Fixed -- ✅ Coordinate conversion: 4 duplicates → 0 duplicates -- ✅ Distance calculation: 4 duplicates → 0 duplicates -- ✅ Angle normalization: 2 duplicates → 0 duplicates -- ✅ Body center calculation: Simplified with utilities - -### Magic Numbers Eliminated -- ✅ 23 magic numbers extracted to named constants -- ✅ 3 critical hardcoded values replaced with existing constants - -### Code Quality Improvements -- ✅ GameScene.update() refactored into 4 focused methods -- ✅ All files now use consistent utility functions -- ✅ Type safety maintained (strict mode, no errors) -- ✅ Build passes successfully - -### Metrics - -| Metric | Before | After | Change | -|--------|--------|-------|--------| -| **Utility modules** | 0 | 2 | +2 | -| **Named constants** | 9 | 32 | +23 | -| **Coordinate conversion duplicates** | 4 | 0 | -4 ✅ | -| **Distance calculation duplicates** | 4 | 0 | -4 ✅ | -| **GameScene.update() complexity** | 56 lines | 3 lines | -53 ✅ | -| **Hardcoded goal positions** | 3 | 0 | -3 ✅ | -| **Build status** | ✅ Pass | ✅ Pass | Stable | - ---- - -## File Changes - -### New Files Created -1. [src/utils/coordinates.ts](src/utils/coordinates.ts) - 52 lines -2. [src/utils/math.ts](src/utils/math.ts) - 55 lines - -### Modified Files -1. [src/config/constants.ts](src/config/constants.ts) - Added 23 constants -2. [src/entities/Player.ts](src/entities/Player.ts) - Uses utilities & constants -3. [src/entities/Puck.ts](src/entities/Puck.ts) - Uses utilities & constants -4. [src/game/GameScene.ts](src/game/GameScene.ts) - Uses utilities & constants, refactored update() -5. [src/systems/BehaviorTree.ts](src/systems/BehaviorTree.ts) - Uses utilities & constants -6. [src/game/Goal.ts](src/game/Goal.ts) - Uses constants - ---- - -## Next Steps (Future Improvements) - -### Performance Optimizations (Low Priority) -- [ ] Cache `gameState` object in GameScene (mutate instead of recreate) -- [ ] Use `distanceSquared()` where only comparison is needed -- [ ] Profile with 12+ players to identify bottlenecks - -### Testing Setup (Future) -- [ ] Add Jest/Vitest configuration -- [ ] Create unit tests for MathUtils -- [ ] Create unit tests for CoordinateUtils -- [ ] Create unit tests for BehaviorTree decisions -- [ ] Add integration tests for game flow - -### Further Refactoring (Optional) -- [ ] Extract pure game logic from Phaser dependencies (better testability) -- [ ] Consider base class for Player/Puck shared coordinate logic -- [ ] Add debug visualization toggle using DEBUG constant - ---- - -## Conclusion - -All critical and important issues from the code review have been resolved: -- ✅ **DRY violations eliminated** - No more duplicate coordinate/math code -- ✅ **Magic numbers extracted** - All values now in centralized constants -- ✅ **Code complexity reduced** - GameScene.update() is now clean and modular -- ✅ **Build successful** - No errors, project compiles cleanly - -The codebase is now significantly more maintainable, readable, and ready for the next development phase. Tuning game balance (speeds, ranges, thresholds) is now as simple as editing values in [constants.ts](src/config/constants.ts). - -**Refactoring Grade:** A+ ✨ - -All changes follow SOLID principles, maintain type safety, and improve code quality without breaking functionality. diff --git a/src/entities/Player.ts b/src/entities/Player.ts index af4e9bc..e2666da 100644 --- a/src/entities/Player.ts +++ b/src/entities/Player.ts @@ -10,15 +10,7 @@ import { } from '../config/constants'; import { CoordinateUtils } from '../utils/coordinates'; import { MathUtils } from '../utils/math'; - -export type PlayerPosition = 'LW' | 'C' | 'RW' | 'LD' | 'RD' | 'G'; -export type TeamSide = 'home' | 'away'; -export type PlayerState = 'offensive' | 'defensive'; - -export interface PlayerAttributes { - speed: number; // 0-100: movement speed - skill: number; // 0-100: pass/shot accuracy, decision quality -} +import type { PlayerPosition, TeamSide, PlayerState, PlayerAttributes } from '../types/game'; export class Player extends Phaser.GameObjects.Container { declare body: Phaser.Physics.Arcade.Body; diff --git a/src/entities/Puck.ts b/src/entities/Puck.ts index b3c4c90..d8be0c5 100644 --- a/src/entities/Puck.ts +++ b/src/entities/Puck.ts @@ -1,8 +1,7 @@ import Phaser from 'phaser'; import { SCALE, MAX_PUCK_VELOCITY } from '../config/constants'; import { CoordinateUtils } from '../utils/coordinates'; - -export type PuckState = 'loose' | 'carried' | 'passing' | 'shot'; +import type { PuckState, TeamSide, Position } from '../types/game'; export class Puck extends Phaser.GameObjects.Container { declare body: Phaser.Physics.Arcade.Body; @@ -13,7 +12,7 @@ export class Puck extends Phaser.GameObjects.Container { // Puck state public state: PuckState; - public possession: 'home' | 'away' | null; + public possession: TeamSide | null; public carrier: string | null; // PlayerID constructor(scene: Phaser.Scene, gameX: number = 0, gameY: number = 0) { @@ -83,7 +82,7 @@ export class Puck extends Phaser.GameObjects.Container { /** * Set puck to be carried by a player */ - public setCarrier(playerId: string, team: 'home' | 'away') { + public setCarrier(playerId: string, team: TeamSide) { this.carrier = playerId; this.possession = team; this.state = 'carried'; @@ -100,7 +99,7 @@ export class Puck extends Phaser.GameObjects.Container { /** * Get puck position in game coordinates */ - public getGamePosition(): { x: number; y: number } { + public getGamePosition(): Position { return { x: this.gameX, y: this.gameY }; } diff --git a/src/systems/BehaviorTree.ts b/src/systems/BehaviorTree.ts index 8926325..32e04c2 100644 --- a/src/systems/BehaviorTree.ts +++ b/src/systems/BehaviorTree.ts @@ -7,17 +7,7 @@ import { SHOOTING_ANGLE_THRESHOLD } from '../config/constants'; import { MathUtils } from '../utils/math'; - -export interface GameState { - puck: Puck; - allPlayers: Player[]; -} - -export interface PlayerAction { - type: 'move' | 'chase_puck' | 'skate_with_puck' | 'shoot' | 'idle'; - targetX?: number; - targetY?: number; -} +import type { GameState, PlayerAction } from '../types/game'; /** * Simple behavior tree for player decision making diff --git a/src/types/game.ts b/src/types/game.ts new file mode 100644 index 0000000..235dad4 --- /dev/null +++ b/src/types/game.ts @@ -0,0 +1,63 @@ +import type { Player } from '../entities/Player'; +import type { Puck } from '../entities/Puck'; + +/** + * Player position on the ice + */ +export type PlayerPosition = 'LW' | 'C' | 'RW' | 'LD' | 'RD' | 'G'; + +/** + * Team side (home or away) + */ +export type TeamSide = 'home' | 'away'; + +/** + * Player tactical state + */ +export type PlayerState = 'offensive' | 'defensive'; + +/** + * Puck state in the game + */ +export type PuckState = 'loose' | 'carried' | 'passing' | 'shot'; + +/** + * Player attributes (0-100 scale) + */ +export interface PlayerAttributes { + speed: number; // 0-100: movement speed + skill: number; // 0-100: pass/shot accuracy, decision quality +} + +/** + * Game state snapshot for AI decision making + */ +export interface GameState { + puck: Puck; + allPlayers: Player[]; +} + +/** + * Player action decided by behavior tree + */ +export interface PlayerAction { + type: 'move' | 'chase_puck' | 'skate_with_puck' | 'shoot' | 'idle'; + targetX?: number; + targetY?: number; +} + +/** + * Goal event data + */ +export interface GoalEvent { + team: TeamSide; + goal: 'left' | 'right'; +} + +/** + * Position in 2D space (game coordinates in meters) + */ +export interface Position { + x: number; + y: number; +}