From 40d07ee68c98b1fa21d347d6e643c2d0246a000f Mon Sep 17 00:00:00 2001
From: Pierre Wessman <4029607+pierrewessman@users.noreply.github.com>
Date: Thu, 2 Oct 2025 08:11:21 +0200
Subject: [PATCH] ...

---
 .claude/settings.local.json |   9 +
 REVIEW.md                   | 336 ++++++++++++++++++++++++++++++++++++
 src/config/constants.ts     |   1 +
 src/game/main.ts            |   4 +-
 4 files changed, 348 insertions(+), 2 deletions(-)
 create mode 100644 .claude/settings.local.json
 create mode 100644 REVIEW.md

diff --git a/.claude/settings.local.json b/.claude/settings.local.json
new file mode 100644
index 0000000..2908062
--- /dev/null
+++ b/.claude/settings.local.json
@@ -0,0 +1,9 @@
+{
+  "permissions": {
+    "allow": [
+      "Bash(find:*)"
+    ],
+    "deny": [],
+    "ask": []
+  }
+}
\ No newline at end of file
diff --git a/REVIEW.md b/REVIEW.md
new file mode 100644
index 0000000..3d768a5
--- /dev/null
+++ b/REVIEW.md
@@ -0,0 +1,336 @@
+# REVIEW.md
+
+This document provides a systematic checklist for conducting periodic code reviews of the hockey-manager-2 codebase. Use this to maintain code quality, consistency, and adherence to best practices.
+
+## Review Frequency
+
+- **Minor Review**: After completing each phase (see [PLAN.md](PLAN.md))
+- **Major Review**: Every 2-3 phases or when accumulating significant technical debt
+- **Pre-Release Review**: Before any major release or milestone
+
+## Review Checklist
+
+### 1. Code Organization & Architecture
+
+#### File Structure
+- [ ] Files are in correct directories according to their responsibility
+- [ ] No circular dependencies between modules
+- [ ] Clear separation of concerns (entities, systems, config, scenes)
+- [ ] Phaser-specific code isolated from game logic where possible
+
+#### Naming Conventions
+- [ ] Classes use PascalCase (`Player`, `BehaviorTree`)
+- [ ] Files match class names (`Player.ts`, `GameScene.ts`)
+- [ ] Constants use SCREAMING_SNAKE_CASE (`RINK_WIDTH`, `TARGET_FPS`)
+- [ ] Variables and functions use camelCase (`currentPosition`, `calculateDistance()`)
+- [ ] Meaningful, descriptive names (avoid `temp`, `data`, `handler` without context)
+
+### 2. DRY (Don't Repeat Yourself)
+
+#### Code Duplication
+- [ ] No duplicate logic across files (extract to shared utilities)
+- [ ] No duplicate constants (all magic numbers in [constants.ts](src/config/constants.ts))
+- [ ] No duplicate type definitions (centralize in types file)
+- [ ] Similar entity behaviors extracted to base classes or systems
+
+#### Common Patterns to Extract
+- [ ] Distance calculations → utility function
+- [ ] Position validation/clamping → utility function
+- [ ] Phaser object creation → factory functions
+- [ ] Coordinate conversions (meters ↔ pixels) → utility functions
+
+### 3. KISS (Keep It Simple, Stupid)
+
+#### Simplicity Checks
+- [ ] Functions do one thing and do it well
+- [ ] No overly clever solutions that sacrifice readability
+- [ ] Complex algorithms have explanatory comments
+- [ ] Prefer explicit over implicit behavior
+- [ ] Avoid premature optimization
+
+#### Function Complexity
+- [ ] Functions < 50 lines (guideline, not hard rule)
+- [ ] Nested conditionals < 3 levels deep
+- [ ] Function parameters < 5 (consider config objects for more)
+- [ ] No "god functions" that do too much
+
+### 4. Type Safety
+
+#### TypeScript Usage
+- [ ] No `any` types (use `unknown` if truly needed)
+- [ ] Interfaces defined for complex objects
+- [ ] Return types explicitly declared on public methods
+- [ ] Proper null/undefined handling with optional chaining
+- [ ] Enums or union types for fixed sets of values
+
+#### Type Definitions
+- [ ] Shared types in dedicated types file
+- [ ] No inline type definitions used multiple times
+- [ ] Entity state properly typed (position, velocity, attributes)
+
+### 5. Performance
+
+#### Hot Path Optimization
+- [ ] `update()` loops avoid unnecessary allocations
+- [ ] Object pooling for frequently created/destroyed objects (bullets, particles)
+- [ ] Minimize garbage collection pressure (reuse objects where possible)
+- [ ] Expensive calculations cached when appropriate
+
+#### Phaser Best Practices
+- [ ] Sprites reused via object pools
+- [ ] Physics bodies properly cleaned up
+- [ ] Textures loaded once, referenced many times
+- [ ] Update loops only process active entities
+
+### 6. Constants & Configuration
+
+#### Centralization
+- [ ] All magic numbers extracted to [constants.ts](src/config/constants.ts)
+- [ ] Related constants grouped logically (RINK_*, PLAYER_*, PUCK_*)
+- [ ] Constants documented with units (meters, pixels, seconds)
+- [ ] Derived constants calculated from base constants
+
+#### Tunable Values
+- [ ] Game balance values clearly marked for tuning
+- [ ] Physics values (speed, acceleration) easy to adjust
+- [ ] AI behavior probabilities configurable
+- [ ] Debug flags for visualization toggles
+
+### 7. Error Handling
+
+#### Robustness
+- [ ] Invalid positions clamped/validated
+- [ ] Division by zero checks
+- [ ] Null/undefined guards on external data
+- [ ] Graceful degradation for missing resources
+
+#### Debugging
+- [ ] Meaningful console warnings for unexpected states
+- [ ] Debug visualization available (toggle in constants)
+- [ ] Assert-like checks in development builds
+
+### 8. Comments & Documentation
+
+#### Code Comments
+- [ ] Complex algorithms explained
+- [ ] Non-obvious decisions justified
+- [ ] Public APIs documented (JSDoc style)
+- [ ] TODOs tracked and dated
+
+#### Project Documentation
+- [ ] [CLAUDE.md](CLAUDE.md) reflects current architecture
+- [ ] [PLAN.md](PLAN.md) updated with completion status
+- [ ] [NOTES.md](NOTES.md) captures important design decisions
+- [ ] README accurate for development setup
+
+### 9. Testing Readiness
+
+#### Testability
+- [ ] Pure functions separated from side effects
+- [ ] Game logic decoupled from Phaser where possible
+- [ ] Dependency injection for systems (not hardcoded singletons)
+- [ ] State machines/behavior trees unit-testable
+
+#### Manual Testing
+- [ ] Visual debug modes available
+- [ ] Hot reload works correctly
+- [ ] No console errors in normal operation
+- [ ] Performance profiling points identified
+
+### 10. Git Hygiene
+
+#### Commit Quality
+- [ ] Logical, atomic commits
+- [ ] Clear commit messages
+- [ ] No commented-out code blocks committed
+- [ ] No debug console.logs in production code
+
+#### Branch Management
+- [ ] Dead branches cleaned up
+- [ ] Feature branches merged or rebased
+- [ ] Master/main branch stable
+
+## Review Process
+
+### Before Starting Review
+
+1. **Create a review branch** (optional but recommended)
+   ```bash
+   git checkout -b review/phase-N
+   ```
+
+2. **Run the build**
+   ```bash
+   pnpm run build
+   ```
+   Ensure no errors before starting review.
+
+3. **Document findings** in a `REVIEW_NOTES.md` or GitHub issues
+
+### During Review
+
+1. **Scan each section systematically** using checklist above
+2. **Note issues** with file path and line number
+3. **Categorize by priority**:
+   - **Critical**: Bugs, type safety issues, performance problems
+   - **Important**: DRY violations, architectural issues
+   - **Nice-to-have**: Better names, additional comments
+
+4. **Create refactor tasks** for important findings
+
+### After Review
+
+1. **Prioritize fixes** (critical → important → nice-to-have)
+2. **Refactor incrementally** (small, testable changes)
+3. **Update documentation** if architecture changed
+4. **Commit refactors separately** from feature work
+
+## Specific Code Smells to Watch For
+
+### Hockey Manager Specific
+
+- **Hardcoded positions**: All positions should derive from constants
+- **Pixel/meter confusion**: Ensure conversions are correct and consistent
+- **Missing coordinate validation**: Positions should stay within rink bounds
+- **Attribute magic numbers**: Player attributes (0-20 scale) should be typed
+- **Decision probabilities**: AI chances should be in constants, not inline
+
+### Phaser Specific
+
+- **Memory leaks**: Sprites/physics bodies not destroyed
+- **Update loop bloat**: Too much computation in GameScene.update()
+- **Texture management**: Multiple loads of same texture
+- **Scale confusion**: Mixing world coordinates with screen coordinates
+
+### TypeScript Specific
+
+- **Type assertions**: Excessive use of `as` keyword
+- **Implicit any**: Missing type annotations
+- **Non-null assertions**: Overuse of `!` operator
+- **Enum misuse**: Strings where enums would be clearer
+
+## Refactoring Patterns
+
+### Extract Utility Function
+
+**Before:**
+```typescript
+const dist = Math.sqrt(
+  Math.pow(x2 - x1, 2) + Math.pow(y2 - y1, 2)
+);
+```
+
+**After:**
+```typescript
+// In utils/math.ts
+export function distance(x1: number, y1: number, x2: number, y2: number): number {
+  return Math.sqrt(Math.pow(x2 - x1, 2) + Math.pow(y2 - y1, 2));
+}
+```
+
+### Extract Constants
+
+**Before:**
+```typescript
+if (player.speed > 8.5) { // max skating speed
+```
+
+**After:**
+```typescript
+// In constants.ts
+export const MAX_SKATING_SPEED = 8.5; // m/s
+
+// In code
+if (player.speed > MAX_SKATING_SPEED) {
+```
+
+### Extract Type
+
+**Before:**
+```typescript
+function updatePlayer(data: { x: number; y: number; team: string }) {
+```
+
+**After:**
+```typescript
+// In types.ts
+interface Position {
+  x: number;
+  y: number;
+}
+
+type Team = 'home' | 'away';
+
+interface PlayerData {
+  position: Position;
+  team: Team;
+}
+
+function updatePlayer(data: PlayerData) {
+```
+
+### Simplify Nested Conditionals
+
+**Before:**
+```typescript
+if (hasPuck) {
+  if (inShootingRange) {
+    if (skill > 15) {
+      shoot();
+    }
+  }
+}
+```
+
+**After:**
+```typescript
+const canShoot = hasPuck && inShootingRange && skill > 15;
+if (canShoot) {
+  shoot();
+}
+```
+
+## Tools & Commands
+
+### Find Duplicated Code
+```bash
+# Search for duplicate function patterns
+grep -r "function calculateDistance" src/
+
+# Find magic numbers (numbers not in constants)
+grep -rE "\b[0-9]+\.?[0-9]*\b" src/ --exclude-dir=config
+```
+
+### Count Lines Per File
+```bash
+# Find large files that might need splitting
+find src -name "*.ts" -exec wc -l {} + | sort -rn
+```
+
+### TypeScript Strict Checks
+Ensure `tsconfig.json` has:
+```json
+{
+  "compilerOptions": {
+    "strict": true,
+    "noImplicitAny": true,
+    "strictNullChecks": true
+  }
+}
+```
+
+## Success Criteria
+
+A successful review results in:
+
+- ✅ All critical issues resolved
+- ✅ No obvious code duplication
+- ✅ Constants properly centralized
+- ✅ Type safety throughout
+- ✅ Documentation reflects current state
+- ✅ Build passes without warnings
+- ✅ Code runs smoothly in dev mode
+
+---
+
+**Remember**: The goal is maintainable, readable, and performant code. Don't sacrifice clarity for cleverness. When in doubt, be explicit.
diff --git a/src/config/constants.ts b/src/config/constants.ts
index 56aa674..e6a9714 100644
--- a/src/config/constants.ts
+++ b/src/config/constants.ts
@@ -25,6 +25,7 @@ export const COLOR_GOAL_CREASE = 0x87ceeb;
 
 // Game settings
 export const FPS = 60;
+export const DEBUG = true;
 
 // Player movement
 export const PLAYER_ROTATION_SPEED = 1; // radians per second
diff --git a/src/game/main.ts b/src/game/main.ts
index 4862273..d542050 100644
--- a/src/game/main.ts
+++ b/src/game/main.ts
@@ -1,6 +1,6 @@
 import Phaser from 'phaser';
 import { GameScene } from './GameScene';
-import { RINK_LENGTH, RINK_WIDTH, SCALE, FPS } from '../config/constants';
+import { RINK_LENGTH, RINK_WIDTH, SCALE, FPS, DEBUG } from '../config/constants';
 
 const config: Phaser.Types.Core.GameConfig = {
   type: Phaser.AUTO,
@@ -12,7 +12,7 @@ const config: Phaser.Types.Core.GameConfig = {
     default: 'arcade',
     arcade: {
       gravity: { x: 0, y: 0 },
-      debug: true
+      debug: DEBUG
     }
   },
   fps: {