diff --git a/planning_notes/npc/npc_behaviour/IMPLEMENTATION_PLAN.md b/planning_notes/npc/npc_behaviour/IMPLEMENTATION_PLAN.md new file mode 100644 index 0000000..25d8c02 --- /dev/null +++ b/planning_notes/npc/npc_behaviour/IMPLEMENTATION_PLAN.md @@ -0,0 +1,854 @@ +# NPC Behavior System - Implementation Plan + +## Overview + +This document outlines the implementation of a modular, maintainable NPC behavior system for Break Escape. The system will enable NPCs to exhibit dynamic behaviors including player awareness, patrolling, personal space maintenance, and hostility states. + +## Goals + +1. **Modular Design**: Separate behavior logic from sprite/animation management +2. **Scenario-Driven**: Behaviors configurable via scenario JSON +3. **Ink Integration**: Behavior states controllable through Ink tags +4. **Performance**: Efficient update cycles, minimal overhead +5. **Maintainability**: Clear separation of concerns, reusable patterns from player.js +6. **Extensible**: Easy to add new behaviors in the future + +## Architecture + +### Core Components + +``` +js/systems/ +├── npc-manager.js (existing - manages NPC data, Ink stories) +├── npc-sprites.js (existing - sprite creation, animations) +├── npc-behavior.js (NEW - behavior state machine & update loop) +└── npc-game-bridge.js (existing - Ink→Game actions, extend for behavior) + +Integration Points: +- js/core/game.js (add behavior update to main game loop) +- scenarios/*.json (add behavior config to NPC definitions) +``` + +### Data Flow + +``` +Scenario JSON → NPC Manager (registers NPCs) + ↓ +NPC Sprite Manager (creates sprites) + ↓ +NPC Behavior Manager (initializes behaviors) + ↓ +Game Update Loop → Behavior Update → State Transitions + ↓ +Ink Story Tags → Behavior State Changes (hostile, influence, etc.) +``` + +--- + +## Behavior State Machine + +### States + +Each NPC has one active behavior state at a time: + +| State | Description | Priority | +|-------|-------------|----------| +| **idle** | Default standing/idle animation | 0 (lowest) | +| **face_player** | Turn towards player when in range | 1 | +| **patrol** | Random movement within area | 2 | +| **maintain_space** | Back away if player too close | 3 | +| **flee** | Run away from player (hostile fear) | 4 | +| **chase** | Move towards player (hostile aggression) | 5 (highest) | + +**Priority System**: Higher priority states override lower priority states. For example, `maintain_space` overrides `patrol` and `face_player`. + +### State Transitions + +``` +[Idle] ──player enters range──> [Face Player] + ──patrol config enabled──> [Patrol] + +[Face Player] ──player exits range──> [Idle] + ──player too close + personalSpace──> [Maintain Space] + +[Patrol] ──player in interaction range──> [Face Player] + ──collision detected──> [change direction] + ──stuck timer expires──> [random new direction] + +[Maintain Space] ──player backs away──> [Face Player/Idle] + ──hostile tag received──> [Flee] + +[Idle/Any] ──hostile tag + influence < 0──> [Flee] + ──hostile tag + influence >= threshold──> [Chase] +``` + +--- + +## NPC Configuration Schema + +### Scenario JSON Extensions + +```json +{ + "rooms": { + "room_id": { + "npcs": [ + { + "id": "guard_npc", + "displayName": "Security Guard", + "npcType": "person", + "position": { "x": 5, "y": 3 }, + + // ===== NEW BEHAVIOR FIELDS ===== + "behavior": { + "facePlayer": true, // Turn to face player when nearby (default: true) + "facePlayerDistance": 96, // Distance to start facing (default: 96px = 3 tiles) + + "patrol": { + "enabled": false, // Enable patrol mode (default: false) + "speed": 100, // Movement speed px/s (default: 100, player is 150) + "changeDirectionInterval": 3000, // Change direction every N ms (default: 3000) + "bounds": { // Optional patrol area bounds + "x": 0, "y": 0, "width": 320, "height": 288 // Relative to room + } + }, + + "personalSpace": { + "enabled": true, + "distance": 48, // Minimum distance to maintain (default: 48px = 1.5 tiles) + "backAwaySpeed": 30, // Speed when backing away (default: 30 - slow) + "backAwayDistance": 5 // Back away in 5px increments + }, + + "hostile": { + "defaultState": false, // Start hostile (default: false) + "influenceThreshold": -50, // Become hostile below this influence + "chaseSpeed": 200, // Speed when chasing (default: 200) + "fleeSpeed": 180, // Speed when fleeing (default: 180) + "aggroDistance": 160 // Distance to start chase (default: 160px = 5 tiles) + } + }, + + // Existing NPC fields... + "spriteSheet": "guard", + "storyPath": "scenarios/ink/guard.json", + "currentKnot": "start" + } + ] + } + } +} +``` + +### Default Behavior + +If `behavior` object is omitted, NPCs default to: +- `facePlayer: true` (turn towards player when nearby) +- All other behaviors disabled (idle when not facing player) + +--- + +## Ink Tag Integration + +### Tag Format + +Ink stories can control NPC behavior state using tags: + +```ink +=== confrontation === +# hostile +# influence:-25 +You've pushed me too far! +-> END + +=== make_peace === +# hostile:false +# influence:10 +Okay, I forgive you. +-> hub + +=== start_patrol === +# patrol_mode:on +I'll be walking around if you need me. +-> hub + +=== stop_patrol === +# patrol_mode:off +I'll stay right here. +-> hub + +=== personal_space_demo === +# personal_space:96 +Please keep your distance. +-> hub +``` + +### Tag Handlers (in npc-game-bridge.js) + +| Tag | Effect | Example | +|-----|--------|---------| +| `#hostile` | Set NPC hostile state to true (red tint) | `# hostile` | +| `#hostile:false` | Set NPC hostile state to false | `# hostile:false` | +| `#influence:` | Set NPC influence score | `# influence:-50` | +| `#patrol_mode:on` | Enable patrol behavior | `# patrol_mode:on` | +| `#patrol_mode:off` | Disable patrol behavior | `# patrol_mode:off` | +| `#personal_space:` | Set personal space distance | `# personal_space:64` | + +### Influence → Hostility Logic + +The `influence` value (Ink VAR) automatically affects hostility: +- **influence >= 0**: Neutral/friendly +- **influence < influenceThreshold** (default -50): Hostile + flee +- **influence < influenceThreshold AND aggression high**: Hostile + chase + +This is checked when `#influence` tags are processed. + +--- + +## Implementation Details + +### 1. npc-behavior.js Structure + +```javascript +/** + * NPCBehaviorManager - Manages all NPC behaviors + * + * Initialized once in game.js create() phase + * Updated every frame in game.js update() phase + */ +export class NPCBehaviorManager { + constructor(scene, npcManager) { + this.scene = scene; // Phaser scene reference + this.npcManager = npcManager; // NPC Manager reference + this.behaviors = new Map(); // Map + this.updateInterval = 50; // Update behaviors every 50ms + this.lastUpdate = 0; + } + + /** + * Register a behavior instance for an NPC sprite + */ + registerBehavior(npcId, sprite, config) { + const behavior = new NPCBehavior(npcId, sprite, config, this.scene); + this.behaviors.set(npcId, behavior); + } + + /** + * Main update loop (called from game.js update()) + */ + update(time, delta) { + // Throttle updates to every 50ms for performance + if (time - this.lastUpdate < this.updateInterval) return; + this.lastUpdate = time; + + const playerPos = window.player ? { x: window.player.x, y: window.player.y } : null; + + for (const [npcId, behavior] of this.behaviors) { + behavior.update(time, delta, playerPos); + } + } + + /** + * Update behavior config (called from Ink tag handlers) + */ + setBehaviorState(npcId, property, value) { + const behavior = this.behaviors.get(npcId); + if (behavior) { + behavior.setState(property, value); + } + } +} + +/** + * NPCBehavior - Individual NPC behavior instance + */ +class NPCBehavior { + constructor(npcId, sprite, config, scene) { + this.npcId = npcId; + this.sprite = sprite; + this.scene = scene; + this.config = this.parseConfig(config); + + // State + this.currentState = 'idle'; + this.direction = 'down'; // Current facing direction + this.hostile = this.config.hostile.defaultState; + this.influence = 0; + + // Patrol state + this.patrolTarget = null; + this.lastPatrolChange = 0; + this.stuckTimer = 0; + + // Personal space state + this.backingAway = false; + } + + parseConfig(config) { + // Parse and apply defaults to config + // (detailed in next section) + } + + update(time, delta, playerPos) { + // Main behavior update logic + // 1. Calculate distances to player + // 2. Determine highest priority state + // 3. Execute state behavior + // 4. Update animations + // 5. Update depth + } + + facePlayer(playerPos) { /* ... */ } + updatePatrol(time, delta) { /* ... */ } + maintainPersonalSpace(playerPos, delta) { /* ... */ } + updateHostileBehavior(playerPos, delta) { /* ... */ } + + setState(property, value) { /* ... */ } + calculateDirection(dx, dy) { /* ... */ } + playAnimation(state, direction) { /* ... */ } +} +``` + +### 2. Animation System + +Reuse the player animation pattern from `player.js`: + +**Walking animations** (8 directions): +- walk-right, walk-left (use flipX for left) +- walk-up, walk-down +- walk-up-right, walk-up-left, walk-down-right, walk-down-left + +**Idle animations** (8 directions): +- idle-right, idle-left (use flipX) +- idle-up, idle-down +- idle-up-right, idle-up-left, idle-down-right, idle-down-left + +These are created in `npc-sprites.js` during sprite setup, similar to player animations in `createPlayerAnimations()`. + +### 3. Turn Towards Player + +**Algorithm** (from player.js movement logic): + +```javascript +facePlayer(playerPos) { + if (!this.config.facePlayer || !playerPos) return; + + const dx = playerPos.x - this.sprite.x; + const dy = playerPos.y - this.sprite.y; + const distanceSq = dx * dx + dy * dy; + + // Only face player if within configured range + if (distanceSq > this.config.facePlayerDistanceSq) { + return; + } + + // Calculate direction (8-way) + const absVX = Math.abs(dx); + const absVY = Math.abs(dy); + + if (absVX > absVY * 2) { + // Mostly horizontal + this.direction = dx > 0 ? 'right' : 'left'; + } else if (absVY > absVX * 2) { + // Mostly vertical + this.direction = dy > 0 ? 'down' : 'up'; + } else { + // Diagonal + if (dy > 0) { + this.direction = dx > 0 ? 'down-right' : 'down-left'; + } else { + this.direction = dx > 0 ? 'up-right' : 'up-left'; + } + } + + // Play idle animation in that direction + this.playAnimation('idle', this.direction); + + // Set flipX for left directions + this.sprite.setFlipX(this.direction.includes('left')); +} +``` + +### 4. Patrol Behavior + +**Algorithm** (similar to player keyboard movement): + +```javascript +updatePatrol(time, delta) { + if (!this.config.patrol.enabled) return; + + // Check if it's time to change direction + if (time - this.lastPatrolChange > this.config.patrol.changeDirectionInterval) { + this.chooseRandomPatrolDirection(); + this.lastPatrolChange = time; + } + + // Move in current direction + if (this.patrolTarget) { + const dx = this.patrolTarget.x - this.sprite.x; + const dy = this.patrolTarget.y - this.sprite.y; + const distance = Math.sqrt(dx * dx + dy * dy); + + // Reached target or stuck + if (distance < 8 || this.sprite.body.blocked.none === false) { + this.stuckTimer += delta; + + // If stuck for > 500ms, choose new direction + if (this.stuckTimer > 500) { + this.chooseRandomPatrolDirection(); + this.stuckTimer = 0; + } + } else { + this.stuckTimer = 0; + + // Apply velocity + const velocityX = (dx / distance) * this.config.patrol.speed; + const velocityY = (dy / distance) * this.config.patrol.speed; + this.sprite.body.setVelocity(velocityX, velocityY); + + // Update direction and animation + this.updateDirectionFromVelocity(velocityX, velocityY); + this.playAnimation('walk', this.direction); + } + } +} + +chooseRandomPatrolDirection() { + // Pick a random point within patrol bounds + const bounds = this.config.patrol.bounds; + const roomData = window.rooms[this.roomId]; // Need to store roomId + const roomX = roomData.worldX || 0; + const roomY = roomData.worldY || 0; + + this.patrolTarget = { + x: roomX + bounds.x + Math.random() * bounds.width, + y: roomY + bounds.y + Math.random() * bounds.height + }; +} +``` + +### 5. Personal Space Behavior + +**Algorithm**: + +```javascript +maintainPersonalSpace(playerPos, delta) { + if (!this.config.personalSpace.enabled || !playerPos) return false; + + const dx = this.sprite.x - playerPos.x; // Away from player + const dy = this.sprite.y - playerPos.y; + const distanceSq = dx * dx + dy * dy; + + // If player too close, back away slowly + if (distanceSq < this.config.personalSpace.distanceSq) { + const distance = Math.sqrt(distanceSq); + + // Back away in small increments (5px at a time) to stay within interaction range + const backAwayDist = this.config.personalSpace.backAwayDistance; + const targetX = this.sprite.x + (dx / distance) * backAwayDist; + const targetY = this.sprite.y + (dy / distance) * backAwayDist; + + // Smoothly move to target + const moveSpeed = this.config.personalSpace.backAwaySpeed; + const moveX = (targetX - this.sprite.x); + const moveY = (targetY - this.sprite.y); + + this.sprite.body.setVelocity(moveX * moveSpeed, moveY * moveSpeed); + + // Face the player while backing away (maintain eye contact) + this.direction = this.calculateDirection(-dx, -dy); // Negative = face player + this.playAnimation('idle', this.direction); // Use idle, not walk + + this.isMoving = false; // Not "walking", just adjusting position + this.backingAway = true; + + return true; // Personal space behavior active + } + + this.backingAway = false; + return false; // No personal space violation +} +``` + +**Design Notes**: +- Distance: 48px (1.5 tiles) - **smaller than interaction range (64px)** +- Speed: 30 px/s - slow, subtle backing +- Increment: 5px - small adjustments to stay within interaction range +- Animation: Use 'idle' animation while backing (face player, maintain eye contact) +- **NPC backs away but remains interactive** + +### 6. Hostile Behavior + +**Visual Feedback**: + +```javascript +setHostile(hostile) { + this.hostile = hostile; + + if (hostile) { + // Red tint (0xff0000 with 50% strength) + this.sprite.setTint(0xff6666); + } else { + // Clear tint + this.sprite.clearTint(); + } +} +``` + +**Future Chase/Flee** (stub for now): + +```javascript +updateHostileBehavior(playerPos, delta) { + if (!this.hostile || !playerPos) return false; + + const dx = playerPos.x - this.sprite.x; + const dy = playerPos.y - this.sprite.y; + const distance = Math.sqrt(dx * dx + dy * dy); + + // TODO: Implement chase/flee based on influence and distance + // For now, just apply hostile tint + console.log(`[${this.npcId}] Hostile mode active (influence: ${this.influence})`); + + return false; // Not actively chasing/fleeing yet +} +``` + +--- + +## Integration Points + +### 1. game.js Update Loop + +Add behavior update to main game loop: + +```javascript +// In js/core/game.js update() function + +export function update(time, delta) { + if (!player) return; + + // Existing updates... + updatePlayerMovement(); + updatePlayerRoom(); + + // NEW: Update NPC behaviors + if (window.npcBehaviorManager) { + window.npcBehaviorManager.update(time, delta); + } + + // Existing updates... +} +``` + +### 2. game.js Create Phase + +Initialize behavior manager after NPCs are created: + +```javascript +// In js/core/game.js create() function + +export function create() { + // Existing initialization... + initializeRooms(this); + createPlayer(this); + + // Create NPCs (existing code in npc loading) + // ... + + // NEW: Initialize behavior manager (async lazy loading - compatible with room loading pattern) + if (window.npcManager) { + const NPCBehaviorManager = await import('./systems/npc-behavior.js?v=1'); + window.npcBehaviorManager = new NPCBehaviorManager.default(this, window.npcManager); + + // Register behaviors for all sprite-based NPCs + for (const [npcId, npcData] of window.npcManager.npcs) { + if (npcData._sprite && npcData.npcType === 'person') { + const behaviorConfig = npcData.behavior || {}; // From scenario JSON + window.npcBehaviorManager.registerBehavior(npcId, npcData._sprite, behaviorConfig); + } + } + } +} +``` + +**Note**: Async import is consistent with lazy loading architecture (rooms will be web requests in future). + +### 3. npc-game-bridge.js Extensions + +Add behavior control methods: + +```javascript +// In js/systems/npc-game-bridge.js + +class NPCGameBridge { + // ... existing methods ... + + /** + * Set NPC hostile state + * @param {string} npcId - NPC identifier + * @param {boolean} hostile - Hostile state + */ + setNPCHostile(npcId, hostile) { + if (window.npcBehaviorManager) { + window.npcBehaviorManager.setBehaviorState(npcId, 'hostile', hostile); + console.log(`🔴 NPC ${npcId} hostile: ${hostile}`); + } + } + + /** + * Set NPC influence score + * @param {string} npcId - NPC identifier + * @param {number} influence - Influence value + */ + setNPCInfluence(npcId, influence) { + if (window.npcBehaviorManager) { + window.npcBehaviorManager.setBehaviorState(npcId, 'influence', influence); + console.log(`💯 NPC ${npcId} influence: ${influence}`); + } + } + + /** + * Toggle NPC patrol mode + * @param {string} npcId - NPC identifier + * @param {boolean} enabled - Patrol enabled + */ + setNPCPatrol(npcId, enabled) { + if (window.npcBehaviorManager) { + window.npcBehaviorManager.setBehaviorState(npcId, 'patrol', enabled); + console.log(`🚶 NPC ${npcId} patrol: ${enabled}`); + } + } +} +``` + +### 4. Ink Tag Processing + +Extend tag handling in conversation manager to call bridge methods: + +```javascript +// In person-chat or phone-chat minigame tag processing + +function processInkTags(tags, npcId) { + for (const tag of tags) { + if (tag === 'hostile' || tag === 'hostile:true') { + window.npcGameBridge.setNPCHostile(npcId, true); + } else if (tag === 'hostile:false') { + window.npcGameBridge.setNPCHostile(npcId, false); + } else if (tag.startsWith('influence:')) { + const value = parseInt(tag.split(':')[1]); + window.npcGameBridge.setNPCInfluence(npcId, value); + } else if (tag === 'patrol_mode:on') { + window.npcGameBridge.setNPCPatrol(npcId, true); + } else if (tag === 'patrol_mode:off') { + window.npcGameBridge.setNPCPatrol(npcId, false); + } else if (tag.startsWith('personal_space:')) { + const distance = parseInt(tag.split(':')[1]); + window.npcGameBridge.setNPCPersonalSpace(npcId, distance); + } + } +} +``` + +--- + +## Phased Implementation + +### Phase 1: Core Infrastructure (Priority: HIGH) +- [ ] Create `npc-behavior.js` with basic structure +- [ ] Implement `NPCBehaviorManager` class +- [ ] Implement `NPCBehavior` class with state machine skeleton +- [ ] Integrate with `game.js` update loop +- [ ] Test with single NPC (idle state only) + +### Phase 2: Face Player (Priority: HIGH) +- [ ] Implement `facePlayer()` logic +- [ ] Add direction calculation (8-way) +- [ ] Test with multiple NPCs at different positions +- [ ] Verify idle animation transitions + +### Phase 3: Animations (Priority: MEDIUM) +- [ ] Extend `npc-sprites.js` to create walk animations +- [ ] Implement `playAnimation()` method +- [ ] Add animation state tracking +- [ ] Test all 8 directions + +### Phase 4: Patrol Behavior (Priority: MEDIUM) +- [ ] Implement `updatePatrol()` logic +- [ ] Add random direction selection +- [ ] Implement stuck detection and recovery +- [ ] Add collision handling +- [ ] Test with patrol bounds +- [ ] Add scenario JSON patrol configuration + +### Phase 5: Personal Space (Priority: LOW) +- [ ] Implement `maintainPersonalSpace()` logic +- [ ] Add backing-away movement +- [ ] Test with varying distances +- [ ] Add scenario JSON personal space configuration + +### Phase 6: Ink Integration (Priority: MEDIUM) +- [ ] Extend `npc-game-bridge.js` with behavior methods +- [ ] Implement tag handlers for hostile, influence, patrol +- [ ] Create test Ink story with behavior tags +- [ ] Test tag → behavior state transitions + +### Phase 7: Hostile Behavior (Priority: LOW) +- [ ] Implement hostile visual feedback (red tint) +- [ ] Add influence → hostility logic +- [ ] Stub chase/flee behaviors +- [ ] Test hostile state changes via Ink tags + +### Phase 8: Documentation & Testing (Priority: HIGH) +- [ ] Write user documentation for scenario JSON config +- [ ] Write developer documentation for extending behaviors +- [ ] Create comprehensive test scenario +- [ ] Performance testing with 10+ NPCs + +--- + +## Testing Strategy + +### Unit Tests +1. **Direction calculation**: Test 8-way direction from dx/dy +2. **Distance checks**: Verify range calculations +3. **State priority**: Ensure higher priority states override lower +4. **Config parsing**: Test default values and overrides + +### Integration Tests +1. **Face player**: NPC turns when player approaches +2. **Patrol**: NPC moves randomly and handles collisions +3. **Personal space**: NPC backs away when player too close +4. **Ink tags**: Behavior changes when tags processed +5. **Multiple NPCs**: All NPCs update independently + +### Performance Tests +1. **10 NPCs**: All idle, measure FPS impact +2. **10 NPCs**: All patrolling, measure FPS impact +3. **Update throttling**: Verify 50ms update interval + +### Test Scenario + +Create `scenarios/behavior-test.json` with: +- 1 NPC with face_player only (default) +- 1 NPC with patrol behavior +- 1 NPC with personal space behavior +- 1 NPC that starts hostile +- 1 NPC with Ink story that triggers hostile via tag + +--- + +## Future Enhancements + +### Short-term (Post-MVP) +- [ ] Chase/flee behavior implementation (hostile movement) +- [ ] Waypoint-based patrol paths (not just random) +- [ ] Group behaviors (NPCs follow each other) +- [ ] Conversation bubbles during face_player + +### Long-term +- [ ] NPC pathfinding (use EasyStar like player) +- [ ] NPC-to-NPC interactions +- [ ] Emotion system (beyond just hostile) +- [ ] Animation state blending (smooth transitions) +- [ ] Dynamic behavior scheduling (time-based state changes) + +--- + +## Performance Considerations + +1. **Update throttling**: Behaviors update every 50ms, not every frame (16ms) +2. **Distance caching**: Pre-calculate squared distances to avoid sqrt() when possible +3. **Animation checks**: Only change animation if state/direction changed +4. **Spatial partitioning**: Future enhancement if >20 NPCs in single room +5. **Behavior disable**: NPCs in non-visible rooms don't update (future) + +--- + +## Code Style & Conventions + +1. **Match player.js patterns**: Reuse direction calculation, animation logic +2. **Depth calculation**: Use same formula as player (bottomY + 0.5) +3. **Collision handling**: Use Phaser arcade physics like player +4. **Console logging**: Use emoji prefixes (🤖 for behaviors) +5. **Config defaults**: Always provide sensible defaults +6. **Error handling**: Graceful degradation if behavior config invalid + +--- + +## Dependencies + +### Existing Systems +- `npc-manager.js` - NPC data, Ink integration +- `npc-sprites.js` - Sprite creation, animations +- `player.js` - Movement/animation patterns to reuse +- `game.js` - Update loop integration +- `constants.js` - TILE_SIZE, INTERACTION_RANGE + +### New Files +- `npc-behavior.js` - Core behavior system +- `behavior-test.json` - Test scenario + +### Modified Files +- `game.js` - Add behavior update call +- `npc-game-bridge.js` - Add behavior control methods +- `person-chat-minigame.js` - Add tag processing for behavior +- `npc-sprites.js` - Add walk animation creation + +--- + +## Risk Assessment + +| Risk | Impact | Mitigation | +|------|--------|------------| +| Performance degradation with many NPCs | HIGH | Throttle updates, profile performance, add culling | +| Animation conflicts with existing NPC code | MEDIUM | Careful testing, state isolation | +| Player collision issues | MEDIUM | Reuse player collision patterns, extensive testing | +| Ink tag conflicts | LOW | Use namespaced tags (#npc_hostile vs #hostile) | +| Config schema complexity | LOW | Provide clear examples, good defaults | + +--- + +## Success Criteria + +✅ **Phase 1 Complete**: Single NPC faces player when approached +✅ **Phase 2 Complete**: NPC patrols randomly and handles collisions +✅ **Phase 3 Complete**: NPC maintains personal space from player +✅ **Phase 4 Complete**: Ink tags change NPC behavior in real-time +✅ **Phase 5 Complete**: Hostile NPC displays red tint +✅ **MVP Complete**: All behaviors work in test scenario without errors +✅ **Production Ready**: Documentation complete, performance verified + +--- + +## References + +- `js/core/player.js` - Movement, animation, depth calculation +- `js/systems/npc-sprites.js` - Sprite creation, current animation setup +- `js/systems/npc-manager.js` - NPC data management, Ink integration +- `docs/INK_BEST_PRACTICES.md` - Ink tag system usage +- `.github/copilot-instructions.md` - Project architecture, patterns + +--- + +## Questions for Review + +1. Should hostile chase/flee be part of MVP or post-MVP? + - **Recommendation**: Post-MVP (stub only for now) + +2. Should personal space back-away use pathfinding or direct movement? + - **Recommendation**: Direct movement (simpler, sufficient for MVP) + +3. Should behaviors be per-room or global? + - **Recommendation**: Global (NPCs in non-visible rooms just don't update) + +4. Should we add behavior debug visualization (show ranges, paths)? + - **Recommendation**: Yes, add debug mode toggle + +5. Integration with existing NPC talk icons system? + - **Recommendation**: Keep separate, talk icons are UI layer + +--- + +**Document Status**: Draft v1.0 +**Last Updated**: 2025-11-09 +**Author**: AI Coding Agent (GitHub Copilot) diff --git a/planning_notes/npc/npc_behaviour/QUICK_REFERENCE.md b/planning_notes/npc/npc_behaviour/QUICK_REFERENCE.md new file mode 100644 index 0000000..a025b49 --- /dev/null +++ b/planning_notes/npc/npc_behaviour/QUICK_REFERENCE.md @@ -0,0 +1,376 @@ +# NPC Behavior System - Quick Reference + +## For Scenario Designers + +### Basic Setup (Face Player Only - Default) + +```json +{ + "id": "receptionist", + "displayName": "Receptionist", + "npcType": "person", + "position": { "x": 5, "y": 3 }, + "spriteSheet": "hacker-red", + "storyPath": "scenarios/ink/receptionist.json" +} +``` + +**Result**: NPC will automatically turn to face player when player is within 3 tiles (96px). + +--- + +### Add Patrol Behavior + +```json +{ + "id": "guard", + "displayName": "Security Guard", + "npcType": "person", + "position": { "x": 5, "y": 3 }, + "behavior": { + "patrol": { + "enabled": true, + "speed": 80, + "changeDirectionInterval": 4000 + } + }, + "spriteSheet": "guard", + "storyPath": "scenarios/ink/guard.json" +} +``` + +**Result**: NPC will wander randomly in the room, changing direction every 4 seconds. Will still face player when approached. + +--- + +### Add Personal Space + +```json +{ + "id": "nervous_npc", + "displayName": "Nervous Employee", + "npcType": "person", + "position": { "x": 5, "y": 3 }, + "behavior": { + "personalSpace": { + "enabled": true, + "distance": 48, + "backAwaySpeed": 30, + "backAwayDistance": 5 + } + }, + "spriteSheet": "hacker", + "storyPath": "scenarios/ink/nervous.json" +} +``` + +**Result**: NPC will back away slowly (5px at a time) if player gets within 48px (1.5 tiles), while still facing the player. NPC remains within interaction range. + +--- + +### Start as Hostile + +```json +{ + "id": "enemy_agent", + "displayName": "Enemy Agent", + "npcType": "person", + "position": { "x": 5, "y": 3 }, + "behavior": { + "hostile": { + "defaultState": true, + "influenceThreshold": -30 + } + }, + "spriteSheet": "hacker-red", + "storyPath": "scenarios/ink/enemy.json" +} +``` + +**Result**: NPC will have red tint from start. Can be changed via Ink tags. + +--- + +## For Ink Story Writers + +### Control Hostility + +```ink +=== make_hostile === +# hostile +You've gone too far! +-> END + +=== make_friendly === +# hostile:false +Okay, I forgive you. +-> hub +``` + +### Set Influence Score + +```ink +=== gain_favour === +# influence:25 +I really appreciate your help! +-> hub + +=== lose_favour === +# influence:-50 +I can't believe you did that. +-> hub +``` + +### Toggle Patrol + +```ink +=== start_rounds === +# patrol_mode:on +I'll be making my rounds now. +-> hub + +=== stop_rounds === +# patrol_mode:off +I'll stay here for now. +-> hub +``` + +### Adjust Personal Space + +```ink +=== need_distance === +# personal_space:128 +Please keep your distance (4 tiles). +-> hub + +=== no_personal_space === +# personal_space:0 +You can come closer now. +-> hub +``` + +--- + +## For Developers + +### Register a Behavior + +```javascript +// In game.js create() phase +window.npcBehaviorManager.registerBehavior( + 'npc_id', // NPC identifier + npcSprite, // Phaser sprite reference + behaviorConfig // Config from scenario JSON +); +``` + +### Update Loop Integration + +```javascript +// In game.js update() function +export function update(time, delta) { + // ... existing updates ... + + if (window.npcBehaviorManager) { + window.npcBehaviorManager.update(time, delta); + } +} +``` + +### Control via Code + +```javascript +// Set hostile state +window.npcGameBridge.setNPCHostile('guard', true); + +// Set influence +window.npcGameBridge.setNPCInfluence('receptionist', 50); + +// Toggle patrol +window.npcGameBridge.setNPCPatrol('guard', false); +``` + +--- + +## Configuration Defaults + +| Property | Default Value | Description | +|----------|--------------|-------------| +| `facePlayer` | `true` | Turn to face player when nearby | +| `facePlayerDistance` | `96` | Distance (px) to start facing | +| `patrol.enabled` | `false` | Enable random patrolling | +| `patrol.speed` | `100` | Movement speed (px/s) | +| `patrol.changeDirectionInterval` | `3000` | Time (ms) between direction changes | +| `personalSpace.enabled` | `false` | Back away when player too close | +| `personalSpace.distance` | `48` | Min distance (px) - **smaller than interaction range** | +| `personalSpace.backAwaySpeed` | `30` | Speed when backing away (px/s) - **slow** | +| `personalSpace.backAwayDistance` | `5` | Back away distance per update (px) | +| `hostile.defaultState` | `false` | Start hostile | +| `hostile.influenceThreshold` | `-50` | Become hostile below this influence | + +**Personal Space Design**: NPCs back away slowly (5px at a time) while facing the player. Distance is 48px (1.5 tiles), which is **smaller than the interaction range (64px / 2 tiles)**, so NPCs remain interactive while maintaining comfort. + +--- + +## Distance Reference + +| Tiles | Pixels | Use Case | +|-------|--------|----------| +| 1 | 32 | Very close (touching) | +| 1.5 | 48 | **Default personal space (stays interactive)** | +| 2 | 64 | **Interaction range (player can talk to NPC)** | +| 3 | 96 | Default face player range | +| 4 | 128 | Extended personal space | +| 5 | 160 | Hostile aggro range | +| 10 | 320 | One full room width | + +--- + +## Behavior Priority + +Behaviors are evaluated in priority order (highest first): + +1. **Chase** (5) - Hostile chase (future) +2. **Flee** (4) - Hostile flee (future) +3. **Maintain Space** (3) - Back away from player +4. **Patrol** (2) - Random movement +5. **Face Player** (1) - Turn towards player +6. **Idle** (0) - Default standing + +Higher priority behaviors override lower priority behaviors. + +--- + +## Common Patterns + +### Friendly NPC That Patrols + +```json +{ + "behavior": { + "patrol": { "enabled": true, "speed": 80 } + } +} +``` + +### Skittish NPC (Personal Space) + +```json +{ + "behavior": { + "personalSpace": { + "enabled": true, + "distance": 96 + } + } +} +``` + +### Guard That Patrols Until Confronted + +```json +{ + "behavior": { + "patrol": { "enabled": true, "speed": 100 }, + "hostile": { "defaultState": false } + } +} +``` + +**Ink Story**: +```ink +=== confrontation === +# patrol_mode:off +# hostile +Stop right there! +-> combat +``` + +### NPC That Warms Up to Player + +```json +{ + "behavior": { + "personalSpace": { "enabled": true, "distance": 96 }, + "hostile": { "defaultState": false } + } +} +``` + +**Ink Story**: +```ink +=== start === +# influence:0 +# personal_space:96 +Stay back! +-> hub + +=== make_friends === +# influence:50 +# personal_space:0 +Okay, I trust you now. +-> hub +``` + +--- + +## Troubleshooting + +### NPC Not Facing Player +- Check `facePlayer: true` in config +- Verify `facePlayerDistance` is large enough +- Check player is within range (use debug mode) + +### NPC Not Patrolling +- Check `patrol.enabled: true` +- Verify NPC has collision body (immovable: false for movement) +- Check patrol bounds include NPC starting position +- Wall collisions automatically set up for patrolling NPCs + +### NPC Not Backing Away +- Check `personalSpace.enabled: true` +- Verify `distance` is 48px (or custom value smaller than interaction range) +- Note: NPC should back away slowly while still facing player +- Ensure NPC has collision body + +### NPC Backs Away Too Far +- Personal space distance should be **smaller than 64px** (interaction range) +- Default is 48px - NPC stays within interaction range +- Use `backAwayDistance: 5` for subtle 5px adjustments + +### Hostile Tint Not Showing +- Check `hostile: true` set via config or tag +- Verify sprite exists and is visible +- Check console for errors + +--- + +## Debug Mode + +Enable debug logging: + +```javascript +// In browser console +window.NPC_BEHAVIOR_DEBUG = true; +``` + +Visualize behavior ranges (future): + +```javascript +// In browser console +window.NPC_BEHAVIOR_DEBUG_VISUAL = true; +``` + +--- + +## Performance Tips + +1. **Limit NPCs**: Keep < 10 NPCs with behaviors per room +2. **Disable when not visible**: Future enhancement +3. **Use lower update rates**: Default 50ms is good balance +4. **Simple patrol bounds**: Smaller areas = less collision checks + +--- + +**Last Updated**: 2025-11-09 +**Version**: 1.0 diff --git a/planning_notes/npc/npc_behaviour/README.md b/planning_notes/npc/npc_behaviour/README.md new file mode 100644 index 0000000..7f7b7b5 --- /dev/null +++ b/planning_notes/npc/npc_behaviour/README.md @@ -0,0 +1,341 @@ +# NPC Behavior System - Planning & Implementation Guide + +## Overview + +This directory contains comprehensive planning documents for implementing dynamic NPC behaviors in Break Escape. The behavior system allows NPCs to react to the player, patrol areas, maintain personal space, and exhibit hostility states—all configurable through scenario JSON and controllable via Ink story tags. + +## Document Index + +### 1. **IMPLEMENTATION_PLAN.md** (START HERE) +**Purpose**: Complete implementation roadmap with architecture, algorithms, and phased rollout plan. + +**Contains**: +- System architecture and data flow +- Behavior state machine design +- Scenario JSON schema extensions +- Ink tag integration specifications +- Movement algorithms (face player, patrol, personal space) +- Integration points with existing systems +- 8-phase implementation plan +- Testing strategy and success criteria +- Performance considerations +- Future enhancements roadmap + +**For**: Lead developer, project planning, architecture review + +--- + +### 2. **TECHNICAL_SPEC.md** +**Purpose**: Deep technical documentation for developers implementing the system. + +**Contains**: +- Class definitions (NPCBehaviorManager, NPCBehavior) +- Complete API reference with method signatures +- Configuration schema with defaults +- Animation system specifications +- Movement algorithm implementations (with code) +- Depth calculation formulas +- Ink integration flow diagrams +- Tag handler implementations +- Performance optimization techniques +- Error handling patterns +- Testing checklist + +**For**: Developers writing npc-behavior.js, integration work + +--- + +### 3. **QUICK_REFERENCE.md** +**Purpose**: Fast lookup guide for common tasks and patterns. + +**Contains**: +- Scenario configuration examples (copy-paste ready) +- Ink tag usage examples +- Developer API quick reference +- Configuration defaults table +- Distance reference (tiles ↔ pixels) +- Behavior priority table +- Common patterns cookbook +- Troubleshooting guide + +**For**: Scenario designers, Ink writers, quick lookups + +--- + +### 4. **example_scenario.json** +**Purpose**: Test scenario demonstrating all behavior types. + +**Contains**: +- 5 NPCs with different behavior configurations: + 1. Default NPC (face player only) + 2. Patrolling Guard (patrol + face player) + 3. Shy Person (personal space + face player) + 4. Hostile Agent (hostile state + red tint) + 5. Complex NPC (all behaviors, Ink-controlled) + +**For**: Testing, reference implementation, QA + +--- + +### 5. **example_ink_complex.ink** +**Purpose**: Ink story demonstrating all behavior control tags. + +**Contains**: +- Toggle patrol (#patrol_mode:on / :off) +- Set influence (#influence:±value) +- Toggle hostile (#hostile / #hostile:false) +- Adjust personal space (#personal_space:distance) +- Interactive dialogue for testing each behavior + +**For**: Ink writers, behavior testing, tag reference + +--- + +### 6. **example_ink_hostile.ink** +**Purpose**: Focused example of hostile state and influence system. + +**Contains**: +- Hostile state initialization (#hostile) +- Influence score management (#influence:value) +- Threshold-based behavior changes +- Peace-making dialogue (hostile → friendly) + +**For**: Ink writers, hostile behavior patterns + +--- + +## Implementation Workflow + +### Phase 1: Review & Setup +1. Read **IMPLEMENTATION_PLAN.md** (sections 1-4) for system overview +2. Review **TECHNICAL_SPEC.md** class definitions +3. Set up development branch + +### Phase 2: Core Infrastructure +1. Create `js/systems/npc-behavior.js` (skeleton) +2. Implement `NPCBehaviorManager` class +3. Implement `NPCBehavior` class with state machine +4. Integrate with `game.js` update loop +5. Test with single NPC (idle state) + +### Phase 3: Face Player Behavior +1. Implement `facePlayer()` logic (TECHNICAL_SPEC.md) +2. Use **QUICK_REFERENCE.md** for distance calculations +3. Test with **example_scenario.json** default_npc + +### Phase 4: Animation System +1. Extend `npc-sprites.js` with walk animations +2. Implement `playAnimation()` method +3. Test 8-direction animations + +### Phase 5: Patrol Behavior +1. Implement `updatePatrol()` logic +2. Add collision handling and stuck recovery +3. Test with **example_scenario.json** patrol_npc + +### Phase 6: Personal Space +1. Implement `maintainPersonalSpace()` logic +2. Test with **example_scenario.json** shy_npc + +### Phase 7: Ink Integration +1. Extend `npc-game-bridge.js` (TECHNICAL_SPEC.md tag handlers) +2. Add tag processing to person-chat minigame +3. Test with **example_ink_complex.ink** + +### Phase 8: Hostile Behavior +1. Implement hostile visual feedback (red tint) +2. Add influence → hostility logic +3. Test with **example_scenario.json** hostile_npc and **example_ink_hostile.ink** + +### Phase 9: Testing & Documentation +1. Run full test suite (TECHNICAL_SPEC.md checklist) +2. Write user documentation +3. Update scenario design guides + +--- + +## Quick Start for Common Tasks + +### For Scenario Designers + +**"How do I make an NPC patrol?"** +→ See **QUICK_REFERENCE.md** → "Add Patrol Behavior" + +**"What distances should I use?"** +→ See **QUICK_REFERENCE.md** → "Distance Reference" table + +**"How do I make a hostile NPC?"** +→ See **example_scenario.json** → `hostile_npc` configuration + +### For Ink Writers + +**"What tags control NPC behavior?"** +→ See **QUICK_REFERENCE.md** → "For Ink Story Writers" section + +**"How do I make an NPC hostile via dialogue?"** +→ See **example_ink_hostile.ink** → `make_peace` knot + +**"How do I toggle patrol mode?"** +→ See **example_ink_complex.ink** → `start_patrol` / `stop_patrol` knots + +### For Developers + +**"What's the class structure?"** +→ See **TECHNICAL_SPEC.md** → "Class Definitions" + +**"How do I integrate with game.js?"** +→ See **IMPLEMENTATION_PLAN.md** → "Integration Points" + +**"What's the animation key format?"** +→ See **TECHNICAL_SPEC.md** → "Animation System" + +--- + +## Key Design Decisions + +### 1. **Why throttle updates to 50ms?** +- Balance responsiveness with performance +- 20 updates/sec sufficient for NPC behaviors +- Reduces CPU usage with many NPCs + +### 2. **Why use squared distances?** +- Avoid expensive Math.sqrt() calls +- Pre-calculate squared thresholds in config +- Significant performance gain with many NPCs + +### 3. **Why priority-based state machine?** +- Clear behavior precedence (personal space > patrol) +- Predictable behavior in complex scenarios +- Easy to extend with new behaviors + +### 4. **Why reuse player animation patterns?** +- Consistency in codebase +- Proven working implementation +- Reduced development time + +### 5. **Why Ink tags for behavior control?** +- Integrates with existing narrative system +- No new UI/controls needed +- Designers can script dynamic behaviors + +--- + +## Development Principles + +1. **Modularity**: Behavior system is self-contained module +2. **Performance**: Throttled updates, cached calculations +3. **Maintainability**: Clear separation of concerns +4. **Extensibility**: Easy to add new behaviors +5. **Robustness**: Graceful degradation on errors +6. **Documentation**: Every decision documented + +--- + +## Performance Targets + +| Metric | Target | Method | +|--------|--------|--------| +| Update frequency | 20 Hz (50ms) | Throttled update loop | +| FPS impact (10 NPCs) | < 10% | Optimized calculations | +| Distance checks | O(1) | Squared distances | +| Animation changes | Minimal | Change detection | +| Memory footprint | < 5MB | Efficient data structures | + +--- + +## Integration Checklist + +- [ ] `npc-behavior.js` created and exported +- [ ] `game.js` creates NPCBehaviorManager +- [ ] `game.js` update loop calls behavior update +- [ ] `npc-sprites.js` creates walk animations +- [ ] `npc-game-bridge.js` has behavior control methods +- [ ] `person-chat-minigame.js` processes behavior tags +- [ ] Test scenario loads without errors +- [ ] Behaviors work as documented +- [ ] Performance targets met + +--- + +## Known Limitations & Future Work + +### Current Limitations +- No pathfinding (direct line movement) +- No chase/flee implementation (stub only) +- No NPC-to-NPC interactions +- No behavior scheduling (time-based) +- No spatial culling (all NPCs update) + +### Post-MVP Enhancements +- Chase/flee hostile behaviors +- Waypoint-based patrol paths +- Group behaviors (follow leader) +- Debug visualization overlay +- Behavior scheduling system + +### Long-term Vision +- Full pathfinding integration +- Emotion system (beyond hostile) +- Dynamic behavior trees +- NPC conversation system +- Animation state blending + +--- + +## Support & Questions + +**For design questions**: Review QUICK_REFERENCE.md and example files + +**For technical questions**: Check TECHNICAL_SPEC.md API reference + +**For architecture questions**: See IMPLEMENTATION_PLAN.md + +**For troubleshooting**: See QUICK_REFERENCE.md troubleshooting section + +**For performance questions**: See TECHNICAL_SPEC.md optimization section + +--- + +## Version History + +- **v1.0** (2025-11-09): Initial planning documents + - Complete architecture and technical specifications + - Example scenario and Ink stories + - 8-phase implementation plan + +--- + +## Contributing + +When implementing behaviors: + +1. Follow patterns from `player.js` (movement, animation) +2. Use emoji prefixes in console logs (🤖 for behaviors) +3. Add comprehensive error handling +4. Write JSDoc comments for all methods +5. Update this documentation if design changes + +--- + +## File Locations + +``` +planning_notes/npc/npc_behaviour/ +├── README.md (this file) +├── IMPLEMENTATION_PLAN.md (architecture & roadmap) +├── TECHNICAL_SPEC.md (developer reference) +├── QUICK_REFERENCE.md (quick lookup guide) +├── example_scenario.json (test scenario config) +├── example_ink_complex.ink (full behavior demo) +└── example_ink_hostile.ink (hostile state demo) + +Future implementation files: +js/systems/ +└── npc-behavior.js (core behavior system) +``` + +--- + +**Document Status**: Planning Complete, Ready for Implementation +**Last Updated**: 2025-11-09 +**Maintainer**: Development Team diff --git a/planning_notes/npc/npc_behaviour/TECHNICAL_SPEC.md b/planning_notes/npc/npc_behaviour/TECHNICAL_SPEC.md new file mode 100644 index 0000000..a4c270f --- /dev/null +++ b/planning_notes/npc/npc_behaviour/TECHNICAL_SPEC.md @@ -0,0 +1,1032 @@ +# NPC Behavior Technical Specification + +## Module Architecture + +### File Structure + +``` +js/systems/ +├── npc-behavior.js (NEW - 400-500 lines) +│ ├── NPCBehaviorManager (main manager class) +│ └── NPCBehavior (individual behavior instance) +│ +├── npc-game-bridge.js (MODIFIED - add 5 methods) +│ ├── setNPCHostile() +│ ├── setNPCInfluence() +│ ├── setNPCPatrol() +│ ├── setNPCPersonalSpace() +│ └── _updateNPCBehaviorFromInfluence() +│ +└── npc-sprites.js (MODIFIED - add walk animations) + └── setupNPCAnimations() (extend to create walk anims) + +js/core/ +└── game.js (MODIFIED - 2 integration points) + ├── create() (initialize behavior manager) + └── update() (call behavior update loop) + +js/minigames/person-chat/ +└── person-chat-minigame.js (MODIFIED - tag processing) + └── processInkTags() (add behavior tag handlers) +``` + +--- + +## Class Definitions + +### NPCBehaviorManager + +**Purpose**: Singleton manager for all NPC behaviors. Initialized once in game.js. + +**Properties**: +```javascript +{ + scene: Phaser.Scene // Game scene reference + npcManager: NPCManager // NPC Manager reference + behaviors: Map // npcId → behavior instance + updateInterval: number // Update throttle (50ms) + lastUpdate: number // Last update timestamp +} +``` + +**Methods**: + +| Method | Parameters | Returns | Description | +|--------|-----------|---------|-------------| +| `constructor(scene, npcManager)` | scene, npcManager | void | Initialize manager | +| `registerBehavior(npcId, sprite, config)` | npcId, sprite, config | void | Create behavior for NPC | +| `update(time, delta)` | time, delta | void | Update all behaviors (throttled) | +| `setBehaviorState(npcId, property, value)` | npcId, property, value | void | Update behavior property | +| `getBehavior(npcId)` | npcId | NPCBehavior\|null | Get behavior instance | +| `removeBehavior(npcId)` | npcId | void | Remove behavior (optional - for future) | + +**Lifecycle**: +``` +Create Phase (game.js): + new NPCBehaviorManager(scene, npcManager) + ↓ + registerBehavior(npcId, sprite, config) for each NPC + ↓ +Update Phase (game.js): + update(time, delta) every frame + ↓ + (throttled to 50ms intervals) + ↓ + NPCBehavior.update() for each behavior +``` + +--- + +### NPCBehavior + +**Purpose**: Individual behavior state machine for one NPC. + +**Properties**: +```javascript +{ + // Identity + npcId: string // NPC identifier + sprite: Phaser.Sprite // Sprite reference + scene: Phaser.Scene // Scene reference + roomId: string // Room identifier (from npcData) + + // Configuration + config: { + facePlayer: boolean + facePlayerDistance: number + facePlayerDistanceSq: number // Cached squared distance + patrol: { + enabled: boolean + speed: number + changeDirectionInterval: number + bounds: { x, y, width, height } + } + personalSpace: { + enabled: boolean + distance: number + distanceSq: number // Cached squared distance + backAwaySpeed: number + } + hostile: { + defaultState: boolean + influenceThreshold: number + chaseSpeed: number + fleeSpeed: number + aggroDistance: number + aggroDistanceSq: number // Cached squared distance + } + } + + // Runtime state + currentState: string // 'idle', 'face_player', 'patrol', etc. + direction: string // 'down', 'up', 'left', 'right', etc. + hostile: boolean // Current hostile state + influence: number // Current influence score + + // Patrol state + patrolTarget: {x, y}|null // Current patrol destination + lastPatrolChange: number // Timestamp of last direction change + stuckTimer: number // How long NPC has been stuck + lastPosition: {x, y} // For stuck detection + + // Personal space state + backingAway: boolean // Currently backing away + + // Animation state + lastAnimationKey: string // Track animation changes + isMoving: boolean // Movement state +} +``` + +**Methods**: + +| Method | Parameters | Returns | Description | +|--------|-----------|---------|-------------| +| `constructor(npcId, sprite, config, scene)` | npcId, sprite, config, scene | void | Initialize behavior | +| `update(time, delta, playerPos)` | time, delta, playerPos | void | Main update loop | +| `parseConfig(config)` | config | Object | Parse and validate config | +| `determineState(playerPos)` | playerPos | string | Calculate highest priority state | +| `executeState(state, time, delta, playerPos)` | state, time, delta, playerPos | void | Execute behavior for state | +| `facePlayer(playerPos)` | playerPos | void | Face towards player | +| `updatePatrol(time, delta)` | time, delta | void | Patrol behavior | +| `maintainPersonalSpace(playerPos, delta)` | playerPos, delta | boolean | Personal space behavior | +| `updateHostileBehavior(playerPos, delta)` | playerPos, delta | boolean | Hostile behavior | +| `chooseRandomPatrolDirection()` | none | void | Pick random patrol target | +| `calculateDirection(dx, dy)` | dx, dy | string | Calculate 8-way direction | +| `updateDirectionFromVelocity(vx, vy)` | vx, vy | void | Update direction from velocity | +| `playAnimation(state, direction)` | state, direction | void | Play animation (idle/walk) | +| `updateDepth()` | none | void | Update sprite depth | +| `setState(property, value)` | property, value | void | Update state property | +| `setHostile(hostile)` | hostile | void | Set hostile state with tint | +| `setInfluence(influence)` | influence | void | Set influence score | + +**State Machine**: + +``` +determineState(playerPos) { + 1. Check hostile behavior (highest priority) + → If hostile + close: 'chase' or 'flee' + + 2. Check personal space + → If player too close: 'maintain_space' + + 3. Check patrol + → If patrol enabled: 'patrol' + + 4. Check face player + → If player in range: 'face_player' + + 5. Default: 'idle' +} + +executeState(state) { + switch (state) { + case 'idle': + sprite.body.setVelocity(0, 0) + playAnimation('idle', direction) + + case 'face_player': + facePlayer(playerPos) + sprite.body.setVelocity(0, 0) + + case 'patrol': + updatePatrol(time, delta) + + case 'maintain_space': + maintainPersonalSpace(playerPos, delta) + + case 'chase': + updateHostileBehavior(playerPos, delta) // Stub for now + + case 'flee': + updateHostileBehavior(playerPos, delta) // Stub for now + } +} +``` + +--- + +## Configuration Schema + +### Default Configuration + +```javascript +const DEFAULT_CONFIG = { + facePlayer: true, + facePlayerDistance: 96, // 3 tiles + facePlayerDistanceSq: 9216, // Pre-calculated + + patrol: { + enabled: false, + speed: 100, // px/s (player is 150) + changeDirectionInterval: 3000, // ms + bounds: { + x: 0, + y: 0, + width: 320, // Full room width + height: 288 // Full room height + } + }, + + personalSpace: { + enabled: false, + distance: 48, // 1.5 tiles (smaller than interaction range) + distanceSq: 2304, // Pre-calculated + backAwaySpeed: 30, // px/s (slow backing) + backAwayDistance: 5 // Only move 5px at a time + }, + + hostile: { + defaultState: false, + influenceThreshold: -50, + chaseSpeed: 200, // px/s + fleeSpeed: 180, // px/s + aggroDistance: 160, // 5 tiles + aggroDistanceSq: 25600 // Pre-calculated + } +}; +``` + +### Config Merging + +```javascript +parseConfig(userConfig) { + const config = JSON.parse(JSON.stringify(DEFAULT_CONFIG)); // Deep clone + + if (userConfig.facePlayer !== undefined) { + config.facePlayer = userConfig.facePlayer; + } + + if (userConfig.facePlayerDistance) { + config.facePlayerDistance = userConfig.facePlayerDistance; + config.facePlayerDistanceSq = config.facePlayerDistance ** 2; + } + + // Merge patrol config + if (userConfig.patrol) { + Object.assign(config.patrol, userConfig.patrol); + } + + // Calculate patrol bounds relative to NPC's room + // (done in constructor after room data available) + + // Merge personal space config + if (userConfig.personalSpace) { + Object.assign(config.personalSpace, userConfig.personalSpace); + if (userConfig.personalSpace.distance) { + config.personalSpace.distanceSq = config.personalSpace.distance ** 2; + } + } + + // Merge hostile config + if (userConfig.hostile) { + Object.assign(config.hostile, userConfig.hostile); + if (userConfig.hostile.aggroDistance) { + config.hostile.aggroDistanceSq = config.hostile.aggroDistance ** 2; + } + } + + return config; +} +``` + +--- + +## Animation System + +### Animation Key Format + +Pattern: `npc-{npcId}-{state}-{direction}` + +Examples: +- `npc-guard-idle-down` +- `npc-guard-walk-right` +- `npc-receptionist-walk-up-left` + +### Animation Creation (in npc-sprites.js) + +```javascript +export function setupNPCAnimations(scene, sprite, spriteSheet, config, npcId) { + // Create idle animations (existing code) + // ... + + // NEW: Create walk animations (8 directions) + const walkAnimations = [ + { key: 'walk-right', frames: [1, 2, 3, 4] }, + { key: 'walk-down', frames: [6, 7, 8, 9] }, + { key: 'walk-up', frames: [11, 12, 13, 14] }, + { key: 'walk-up-right', frames: [16, 17, 18, 19] }, + { key: 'walk-down-right', frames: [21, 22, 23, 24] } + ]; + + walkAnimations.forEach(anim => { + const animKey = `npc-${npcId}-${anim.key}`; + if (!scene.anims.exists(animKey)) { + scene.anims.create({ + key: animKey, + frames: scene.anims.generateFrameNumbers(spriteSheet, { + frames: anim.frames + }), + frameRate: 8, + repeat: -1 + }); + } + }); + + // Left directions use right animations with flipX + // (handled in NPCBehavior.playAnimation()) +} +``` + +### Animation Playback Logic + +```javascript +// In NPCBehavior class + +playAnimation(state, direction) { + // Map left directions to right + let animDirection = direction; + let flipX = false; + + if (direction.includes('left')) { + animDirection = direction.replace('left', 'right'); + flipX = true; + } + + const animKey = `npc-${this.npcId}-${state}-${animDirection}`; + + // Only change animation if different from current + if (this.lastAnimationKey !== animKey) { + if (this.sprite.anims.exists(animKey)) { + this.sprite.play(animKey, true); + this.lastAnimationKey = animKey; + } else { + console.warn(`Animation not found: ${animKey}`); + } + } + + // Set flipX for left-facing directions + this.sprite.setFlipX(flipX); +} +``` + +--- + +## Movement Algorithms + +### Direction Calculation (8-way) + +```javascript +calculateDirection(dx, dy) { + const absVX = Math.abs(dx); + const absVY = Math.abs(dy); + + // Threshold: if one axis is > 2x the other, consider it pure cardinal + if (absVX > absVY * 2) { + return dx > 0 ? 'right' : 'left'; + } + + if (absVY > absVX * 2) { + return dy > 0 ? 'down' : 'up'; + } + + // Diagonal + if (dy > 0) { + return dx > 0 ? 'down-right' : 'down-left'; + } else { + return dx > 0 ? 'up-right' : 'up-left'; + } +} +``` + +### Face Player Algorithm + +```javascript +facePlayer(playerPos) { + if (!this.config.facePlayer || !playerPos) return; + + const dx = playerPos.x - this.sprite.x; + const dy = playerPos.y - this.sprite.y; + const distanceSq = dx * dx + dy * dy; + + // Only face if within range + if (distanceSq > this.config.facePlayerDistanceSq) { + return; + } + + // Calculate direction + this.direction = this.calculateDirection(dx, dy); + + // Play idle animation facing player + this.playAnimation('idle', this.direction); + + // Stop movement + if (this.sprite.body) { + this.sprite.body.setVelocity(0, 0); + } +} +``` + +### Patrol Algorithm + +```javascript +updatePatrol(time, delta) { + if (!this.config.patrol.enabled) return; + + // Time to change direction? + if (!this.patrolTarget || + time - this.lastPatrolChange > this.config.patrol.changeDirectionInterval) { + this.chooseRandomPatrolDirection(); + this.lastPatrolChange = time; + this.stuckTimer = 0; + } + + // Calculate vector to target + const dx = this.patrolTarget.x - this.sprite.x; + const dy = this.patrolTarget.y - this.sprite.y; + const distance = Math.sqrt(dx * dx + dy * dy); + + // Reached target? + if (distance < 8) { + this.chooseRandomPatrolDirection(); + return; + } + + // Check if stuck (blocked by collision) + const isBlocked = this.sprite.body.blocked.none === false; + + if (isBlocked) { + this.stuckTimer += delta; + + // Stuck for > 500ms? Choose new direction + if (this.stuckTimer > 500) { + this.chooseRandomPatrolDirection(); + this.stuckTimer = 0; + } + } else { + this.stuckTimer = 0; + + // Apply velocity + const velocityX = (dx / distance) * this.config.patrol.speed; + const velocityY = (dy / distance) * this.config.patrol.speed; + this.sprite.body.setVelocity(velocityX, velocityY); + + // Update direction and animation + this.direction = this.calculateDirection(dx, dy); + this.playAnimation('walk', this.direction); + this.isMoving = true; + } +} + +chooseRandomPatrolDirection() { + // Get NPC's room data + const npcData = window.npcManager.npcs.get(this.npcId); + const roomData = window.rooms[npcData.roomId]; + + if (!roomData) { + console.warn(`Room data not found for NPC ${this.npcId}`); + return; + } + + const bounds = this.config.patrol.bounds; + const roomX = roomData.worldX || 0; + const roomY = roomData.worldY || 0; + + // Pick random point within bounds + this.patrolTarget = { + x: roomX + bounds.x + Math.random() * bounds.width, + y: roomY + bounds.y + Math.random() * bounds.height + }; + + console.log(`🚶 ${this.npcId} patrol target: (${this.patrolTarget.x}, ${this.patrolTarget.y})`); +} +``` + +### Personal Space Algorithm + +```javascript +maintainPersonalSpace(playerPos, delta) { + if (!this.config.personalSpace.enabled || !playerPos) { + return false; + } + + const dx = this.sprite.x - playerPos.x; // Away from player + const dy = this.sprite.y - playerPos.y; + const distanceSq = dx * dx + dy * dy; + + // Player too close? + if (distanceSq < this.config.personalSpace.distanceSq) { + const distance = Math.sqrt(distanceSq); + + // Back away slowly in small increments (5px at a time) + const backAwayDist = this.config.personalSpace.backAwayDistance; + const targetX = this.sprite.x + (dx / distance) * backAwayDist; + const targetY = this.sprite.y + (dy / distance) * backAwayDist; + + // Smoothly move to target + const moveSpeed = this.config.personalSpace.backAwaySpeed; + const moveX = (targetX - this.sprite.x); + const moveY = (targetY - this.sprite.y); + + this.sprite.body.setVelocity(moveX * moveSpeed, moveY * moveSpeed); + + // Still face the player while backing away + this.direction = this.calculateDirection(-dx, -dy); // Negative = face player + this.playAnimation('idle', this.direction); // Use idle, not walk + + this.isMoving = false; // Not "walking", just adjusting position + this.backingAway = true; + + return true; // Personal space behavior active + } + + this.backingAway = false; + return false; // No violation +} +``` + +**Design Notes**: +- Distance: 48px (1.5 tiles) - smaller than interaction range (64px) +- Speed: 30 px/s - slow, subtle backing +- Increment: 5px - small adjustments to stay within interaction range +- Animation: Use 'idle' animation while backing (face player, maintain eye contact) +- NPC backs away but remains interactive + +--- + +## Depth Calculation + +Reuse player depth calculation pattern: + +```javascript +updateDepth() { + if (!this.sprite || !this.sprite.body) return; + + // Get bottom of sprite (feet position) + const spriteBottomY = this.sprite.y + (this.sprite.displayHeight / 2); + + // Same formula as player: bottomY + 0.5 + const depth = spriteBottomY + 0.5; + this.sprite.setDepth(depth); +} +``` + +Called: +- Every update cycle (REQUIRED for proper Y-axis rendering order) +- Depth determines draw order as NPCs move up/down + +**Note**: Depth updates are NOT optional - they ensure NPCs render correctly relative to each other and the player as they move along the Y-axis. + +--- + +## Ink Integration + +### Tag Processing Flow + +``` +Ink Story (e.g., scenarios/ink/guard.json) + ↓ + # hostile + # influence:-25 + ↓ +Person Chat Minigame (person-chat-minigame.js) + ↓ + processInkTags(tags, npcId) + ↓ +NPC Game Bridge (npc-game-bridge.js) + ↓ + setNPCHostile(npcId, true) + setNPCInfluence(npcId, -25) + ↓ +NPC Behavior Manager (npc-behavior.js) + ↓ + setBehaviorState(npcId, 'hostile', true) + setBehaviorState(npcId, 'influence', -25) + ↓ +NPC Behavior (npc-behavior.js) + ↓ + setState('hostile', true) → setHostile(true) + setState('influence', -25) → setInfluence(-25) + ↓ +Visual Effect: sprite.setTint(0xff6666) +State Change: Update hostile state +``` + +### Tag Handler Implementation + +In `js/systems/npc-game-bridge.js`: + +```javascript +class NPCGameBridge { + // ... existing methods ... + + setNPCHostile(npcId, hostile) { + if (!window.npcBehaviorManager) { + console.warn('NPCBehaviorManager not initialized'); + return; + } + + const behavior = window.npcBehaviorManager.getBehavior(npcId); + if (behavior) { + behavior.setState('hostile', hostile); + console.log(`🔴 NPC ${npcId} hostile: ${hostile}`); + + this._logAction('setNPCHostile', { npcId, hostile }, { success: true }); + } else { + console.warn(`Behavior not found for NPC: ${npcId}`); + } + } + + setNPCInfluence(npcId, influence) { + if (!window.npcBehaviorManager) return; + + const behavior = window.npcBehaviorManager.getBehavior(npcId); + if (behavior) { + behavior.setState('influence', influence); + console.log(`💯 NPC ${npcId} influence: ${influence}`); + + // Check if influence change should trigger hostile state + this._updateNPCBehaviorFromInfluence(npcId, influence); + + this._logAction('setNPCInfluence', { npcId, influence }, { success: true }); + } + } + + setNPCPatrol(npcId, enabled) { + if (!window.npcBehaviorManager) return; + + const behavior = window.npcBehaviorManager.getBehavior(npcId); + if (behavior) { + behavior.setState('patrol', enabled); + console.log(`🚶 NPC ${npcId} patrol: ${enabled}`); + + this._logAction('setNPCPatrol', { npcId, enabled }, { success: true }); + } + } + + setNPCPersonalSpace(npcId, distance) { + if (!window.npcBehaviorManager) return; + + const behavior = window.npcBehaviorManager.getBehavior(npcId); + if (behavior) { + behavior.setState('personalSpaceDistance', distance); + console.log(`↔️ NPC ${npcId} personal space: ${distance}px`); + + this._logAction('setNPCPersonalSpace', { npcId, distance }, { success: true }); + } + } + + _updateNPCBehaviorFromInfluence(npcId, influence) { + const behavior = window.npcBehaviorManager.getBehavior(npcId); + if (!behavior) return; + + const threshold = behavior.config.hostile.influenceThreshold; + + // Auto-trigger hostile if influence drops below threshold + if (influence < threshold && !behavior.hostile) { + this.setNPCHostile(npcId, true); + console.log(`⚠️ NPC ${npcId} became hostile due to low influence (${influence} < ${threshold})`); + } + // Auto-disable hostile if influence recovers + else if (influence >= threshold && behavior.hostile) { + this.setNPCHostile(npcId, false); + console.log(`✅ NPC ${npcId} no longer hostile (influence: ${influence})`); + } + } +} +``` + +In `js/minigames/person-chat/person-chat-minigame.js`: + +```javascript +// Add to existing tag processing +function processInkTags(tags, npcId) { + for (const tag of tags) { + // ... existing tag handlers ... + + // NEW: Behavior tags + if (tag === 'hostile' || tag === 'hostile:true') { + window.npcGameBridge.setNPCHostile(npcId, true); + } else if (tag === 'hostile:false') { + window.npcGameBridge.setNPCHostile(npcId, false); + } else if (tag.startsWith('influence:')) { + const value = parseInt(tag.split(':')[1], 10); + if (!isNaN(value)) { + window.npcGameBridge.setNPCInfluence(npcId, value); + } + } else if (tag === 'patrol_mode:on') { + window.npcGameBridge.setNPCPatrol(npcId, true); + } else if (tag === 'patrol_mode:off') { + window.npcGameBridge.setNPCPatrol(npcId, false); + } else if (tag.startsWith('personal_space:')) { + const distance = parseInt(tag.split(':')[1], 10); + if (!isNaN(distance) && distance >= 0) { + window.npcGameBridge.setNPCPersonalSpace(npcId, distance); + } + } + } +} +``` + +--- + +## Performance Optimization + +### Update Throttling + +```javascript +// In NPCBehaviorManager.update() + +update(time, delta) { + // Only update every 50ms (20 updates/sec instead of 60) + if (time - this.lastUpdate < this.updateInterval) { + return; + } + this.lastUpdate = time; + + // Get player position once per update + const playerPos = window.player ? { + x: window.player.x, + y: window.player.y + } : null; + + // Update all behaviors + for (const [npcId, behavior] of this.behaviors) { + behavior.update(time, delta, playerPos); + } +} +``` + +### Distance Caching + +```javascript +// Pre-calculate squared distances in config parsing +config.facePlayerDistanceSq = config.facePlayerDistance ** 2; +config.personalSpace.distanceSq = config.personalSpace.distance ** 2; +config.hostile.aggroDistanceSq = config.hostile.aggroDistance ** 2; + +// Use squared distances in comparisons (avoid sqrt) +const distanceSq = dx * dx + dy * dy; +if (distanceSq < this.config.facePlayerDistanceSq) { + // Face player +} +``` + +### Animation Caching + +```javascript +// Only change animation if different +if (this.lastAnimationKey !== animKey) { + this.sprite.play(animKey, true); + this.lastAnimationKey = animKey; +} +``` + +### Spatial Culling (Future) + +```javascript +// Skip update if NPC not in visible room +if (this.roomId !== window.currentRoom) { + this.sprite.body.setVelocity(0, 0); + return; +} +``` + +--- + +## Error Handling + +### Graceful Degradation + +```javascript +// In NPCBehavior.update() +update(time, delta, playerPos) { + try { + // Comprehensive sprite validation (including .destroyed check) + if (!this.sprite || !this.sprite.body || this.sprite.destroyed) { + console.warn(`⚠️ Invalid sprite for ${this.npcId}, skipping update`); + return; + } + + // Main update logic... + + } catch (error) { + console.error(`Error updating NPC behavior ${this.npcId}:`, error); + // Reset to idle state on error + this.currentState = 'idle'; + if (this.sprite && this.sprite.body && !this.sprite.destroyed) { + this.sprite.body.setVelocity(0, 0); + } + } +} +``` + +### NPCBehavior Constructor + +```javascript +constructor(npcId, sprite, config, scene) { + this.npcId = npcId; + this.sprite = sprite; + this.scene = scene; + + // CRITICAL: Get roomId from NPC data at initialization + const npcData = window.npcManager?.npcs.get(npcId); + this.roomId = npcData?.roomId || null; + + if (!this.roomId) { + console.warn(`⚠️ NPC ${npcId} has no roomId - patrol bounds will be limited`); + } + + // Verify sprite reference matches npcData._sprite + if (npcData && npcData._sprite && npcData._sprite !== sprite) { + console.warn(`⚠️ Sprite reference mismatch for ${npcId}`); + } + + // Parse and merge configuration + this.config = this.parseConfig(config); + + // Initialize state + this.state = 'idle'; + this.direction = 'down'; + this.isMoving = false; + this.hostile = this.config.hostile.defaultState; + this.influence = 0; + + // Patrol state + this.patrolTarget = null; + this.stuckTimer = 0; + this.lastDirectionChange = 0; + + // Animation tracking + this.lastAnimationKey = null; + + // Apply initial hostile visual if needed + if (this.hostile) { + this.setHostile(true); + } + + console.log(`✅ Behavior registered for ${npcId} in room ${this.roomId}`); +} +``` + +**Note**: Sprite storage locations: +- `npcData._sprite` - Set in npc-sprites.js line 69 +- `roomData.npcSprites[]` - Array in rooms.js line 1894 +- Both should reference the same sprite object + +### Config Validation + +```javascript +parseConfig(userConfig) { + const config = { ...DEFAULT_CONFIG }; + + // Validate patrol speed + if (userConfig.patrol && userConfig.patrol.speed !== undefined) { + if (typeof userConfig.patrol.speed === 'number' && userConfig.patrol.speed > 0) { + config.patrol.speed = userConfig.patrol.speed; + } else { + console.warn(`Invalid patrol speed for NPC ${this.npcId}, using default`); + } + } + + // Validate distances (must be positive) + if (userConfig.personalSpace && userConfig.personalSpace.distance !== undefined) { + if (typeof userConfig.personalSpace.distance === 'number' && + userConfig.personalSpace.distance >= 0) { + config.personalSpace.distance = userConfig.personalSpace.distance; + config.personalSpace.distanceSq = config.personalSpace.distance ** 2; + } else { + console.warn(`Invalid personal space distance for NPC ${this.npcId}`); + } + } + + return config; +} +``` + +--- + +## Testing Checklist + +### Unit Tests (Manual) + +- [ ] Direction calculation (8 directions + edge cases) +- [ ] Distance calculation (squared distances) +- [ ] Config parsing (defaults, overrides, validation) +- [ ] State priority (higher priority overrides lower) +- [ ] Animation key generation (correct format) + +### Integration Tests + +- [ ] Single NPC faces player when approached +- [ ] Multiple NPCs face player independently +- [ ] NPC patrols and changes direction +- [ ] NPC handles collision while patrolling +- [ ] NPC recovers from stuck state +- [ ] NPC backs away when player too close +- [ ] Hostile state applies red tint +- [ ] Ink tag changes behavior in real-time +- [ ] Personal space + patrol interaction +- [ ] Face player + patrol interaction + +### Performance Tests + +- [ ] 1 NPC: FPS impact < 2% +- [ ] 5 NPCs: FPS impact < 5% +- [ ] 10 NPCs: FPS impact < 10% +- [ ] Update throttling working (50ms interval) +- [ ] No memory leaks after 5 minutes + +### Edge Cases + +- [ ] Player sprite missing (shouldn't crash) +- [ ] NPC sprite destroyed mid-update +- [ ] Invalid config (uses defaults) +- [ ] No patrol bounds defined (uses room size) +- [ ] Zero personal space distance +- [ ] Negative influence values +- [ ] Room change while patrolling + +--- + +## Future Enhancements + +### Short-term (Post-MVP) + +1. **Chase Behavior**: Hostile NPC moves towards player + ```javascript + updateChase(playerPos, delta) { + const dx = playerPos.x - this.sprite.x; + const dy = playerPos.y - this.sprite.y; + const distance = Math.sqrt(dx * dx + dy * dy); + + const velocityX = (dx / distance) * this.config.hostile.chaseSpeed; + const velocityY = (dy / distance) * this.config.hostile.chaseSpeed; + this.sprite.body.setVelocity(velocityX, velocityY); + + this.direction = this.calculateDirection(dx, dy); + this.playAnimation('walk', this.direction); + } + ``` + +2. **Flee Behavior**: Hostile NPC runs away from player + ```javascript + updateFlee(playerPos, delta) { + const dx = this.sprite.x - playerPos.x; // Away from player + const dy = this.sprite.y - playerPos.y; + const distance = Math.sqrt(dx * dx + dy * dy); + + const velocityX = (dx / distance) * this.config.hostile.fleeSpeed; + const velocityY = (dy / distance) * this.config.hostile.fleeSpeed; + this.sprite.body.setVelocity(velocityX, velocityY); + + this.direction = this.calculateDirection(dx, dy); + this.playAnimation('walk', this.direction); + } + ``` + +3. **Waypoint Patrol**: Follow predefined path + ```json + { + "patrol": { + "enabled": true, + "waypoints": [ + { "x": 5, "y": 3 }, + { "x": 10, "y": 3 }, + { "x": 10, "y": 8 }, + { "x": 5, "y": 8 } + ], + "loop": true + } + } + ``` + +4. **Debug Visualization**: Show ranges, paths, state + ```javascript + if (window.NPC_BEHAVIOR_DEBUG_VISUAL) { + // Draw face player range + graphics.strokeCircle(sprite.x, sprite.y, config.facePlayerDistance); + + // Draw personal space range + graphics.strokeCircle(sprite.x, sprite.y, config.personalSpace.distance); + + // Draw patrol target + graphics.fillCircle(patrolTarget.x, patrolTarget.y, 5); + } + ``` + +### Long-term + +1. **Pathfinding**: Use EasyStar.js like player +2. **Group Behaviors**: NPCs follow leader +3. **Emotion System**: Happy, sad, angry states +4. **Dynamic Scheduling**: Time-based behaviors +5. **NPC-to-NPC Interactions**: NPCs talk to each other +6. **Animation Blending**: Smooth transitions +7. **Spatial Partitioning**: Room-based culling + +--- + +**Document Status**: Technical Specification v1.0 +**Last Updated**: 2025-11-09 +**Author**: AI Coding Agent diff --git a/planning_notes/npc/npc_behaviour/example_ink_complex.ink b/planning_notes/npc/npc_behaviour/example_ink_complex.ink new file mode 100644 index 0000000..4792fae --- /dev/null +++ b/planning_notes/npc/npc_behaviour/example_ink_complex.ink @@ -0,0 +1,136 @@ +// Behavior Test - Complex NPC with multiple states +// Demonstrates all behavior controls via Ink tags + +VAR influence = 0 +VAR is_patrolling = false +VAR is_hostile = false + +=== start === +# speaker:npc +Hi! I'm the complex behavior NPC. + +I demonstrate how Ink stories can control my behavior dynamically. + +Right now: +- Personal space: ENABLED (I'll back away if you get too close) +- Patrol: {is_patrolling: ENABLED | DISABLED} +- Hostile: {is_hostile: YES | NO} +- Influence: {influence} + +-> hub + +=== hub === +* [What behaviors can you demonstrate?] + -> explain_behaviors + +* [Make you start patrolling] + -> start_patrol + +* [Make you stop patrolling] + -> stop_patrol + +* [Increase your influence (+25)] + -> gain_influence + +* [Decrease your influence (-25)] + -> lose_influence + +* [Make you hostile] + -> become_hostile + +* [Make you friendly] + -> become_friendly + +* [Disable your personal space] + -> disable_personal_space + +* [Enable your personal space] + -> enable_personal_space + ++ [Exit] #exit_conversation + # speaker:npc + Come back anytime to test more behaviors! + +-> hub + +=== explain_behaviors === +# speaker:npc +I can demonstrate several behaviors: + +**Face Player**: I always turn to face you when you're nearby. + +**Patrol**: When enabled, I walk around randomly. Use tags to toggle: #patrol_mode:on / #patrol_mode:off + +**Personal Space**: When enabled, I back away if you get too close. Use tags: #personal_space:64 + +**Hostile**: Shows a red tint. Use tags: #hostile / #hostile:false + +**Influence**: A score that affects my reactions. Use tags: #influence:25 + +Try the other dialogue options to see these in action! +-> hub + +=== start_patrol === +# speaker:npc +# patrol_mode:on +~ is_patrolling = true +Okay, I'll start patrolling around this area! + +Watch me walk around randomly. I'll still face you when you approach. +-> hub + +=== stop_patrol === +# speaker:npc +# patrol_mode:off +~ is_patrolling = false +Alright, I'll stop patrolling and stay in one place. +-> hub + +=== gain_influence === +# speaker:npc +# influence:25 +~ influence = influence + 25 +Thanks! My influence increased to {influence}. + +{influence >= 50: I really trust you now!} +-> hub + +=== lose_influence === +# speaker:npc +# influence:-25 +~ influence = influence - 25 +That wasn't nice. My influence dropped to {influence}. + +{influence <= -40: I'm getting close to my hostile threshold (-40)...} +{influence < -40: I should be hostile now based on my threshold!} +-> hub + +=== become_hostile === +# speaker:npc +# hostile +~ is_hostile = true +You've pushed me too far! I'm now hostile! (red tint) + +Note: In the future, I'll chase or flee from you when hostile. +-> hub + +=== become_friendly === +# speaker:npc +# hostile:false +~ is_hostile = false +Okay, I forgive you. I'm no longer hostile. + +My tint should return to normal now. +-> hub + +=== disable_personal_space === +# speaker:npc +# personal_space:0 +You can come as close as you want now. Personal space disabled! +-> hub + +=== enable_personal_space === +# speaker:npc +# personal_space:64 +I need some personal space again. I'll back away if you get within 2 tiles (64px). +-> hub diff --git a/planning_notes/npc/npc_behaviour/example_ink_hostile.ink b/planning_notes/npc/npc_behaviour/example_ink_hostile.ink new file mode 100644 index 0000000..d479ecf --- /dev/null +++ b/planning_notes/npc/npc_behaviour/example_ink_hostile.ink @@ -0,0 +1,62 @@ +// Behavior Test - Hostile NPC +// Demonstrates hostile state and influence-based behavior changes + +VAR influence = -50 + +=== start === +# speaker:npc +# hostile +# influence:-50 +I don't trust you. Stay back! + +(Notice my red tint - I'm hostile!) +-> hub + +=== hub === +* [Why are you hostile?] + -> explain_hostility + +* [I come in peace...] + -> make_peace + +* [Show me your influence score] + -> show_influence + ++ [Exit] #exit_conversation + # speaker:npc + {influence >= 0: Safe travels, friend. | Stay away from me.} + +-> hub + +=== explain_hostility === +# speaker:npc +I'm hostile because my influence is {influence}. + +My hostile threshold is -30. When influence drops below that, I become hostile automatically. + +My red tint is a visual indicator of my hostile state. + +In the future, I might chase or flee from you when hostile! +-> hub + +=== make_peace === +# speaker:npc +# influence:25 +# hostile:false +~ influence = 25 + +Okay... I'll give you a chance. + +My influence is now {influence}, above the threshold. + +My red tint should be gone now. I'm no longer hostile. +-> hub + +=== show_influence === +# speaker:npc +Current influence: {influence} +Hostile threshold: -30 + +{influence < -30: I'm hostile because influence < threshold.} +{influence >= -30: I'm not hostile because influence >= threshold.} +-> hub diff --git a/planning_notes/npc/npc_behaviour/example_scenario.json b/planning_notes/npc/npc_behaviour/example_scenario.json new file mode 100644 index 0000000..74ef422 --- /dev/null +++ b/planning_notes/npc/npc_behaviour/example_scenario.json @@ -0,0 +1,149 @@ +{ + "scenario_brief": "Test scenario for NPC behavior system - demonstrates all behavior types", + "endGoal": "Interact with each NPC to observe different behaviors", + "startRoom": "behavior_test_room", + + "player": { + "id": "player", + "displayName": "Agent Test", + "spriteSheet": "hacker", + "spriteTalk": "assets/characters/hacker-talk.png", + "spriteConfig": { + "idleFrameStart": 20, + "idleFrameEnd": 23 + } + }, + + "rooms": { + "behavior_test_room": { + "type": "room_office", + "connections": {}, + "npcs": [ + { + "id": "default_npc", + "displayName": "Default NPC (Face Player Only)", + "npcType": "person", + "position": { "x": 3, "y": 3 }, + "spriteSheet": "hacker", + "spriteConfig": { + "idleFrameStart": 20, + "idleFrameEnd": 23 + }, + "storyPath": "scenarios/ink/behavior-test-default.json", + "currentKnot": "start", + "_note": "No behavior config - defaults to face_player: true" + }, + + { + "id": "patrol_npc", + "displayName": "Patrolling Guard", + "npcType": "person", + "position": { "x": 6, "y": 3 }, + "spriteSheet": "hacker-red", + "spriteConfig": { + "idleFrameStart": 20, + "idleFrameEnd": 23 + }, + "storyPath": "scenarios/ink/behavior-test-patrol.json", + "currentKnot": "start", + "behavior": { + "facePlayer": true, + "facePlayerDistance": 96, + "patrol": { + "enabled": true, + "speed": 80, + "changeDirectionInterval": 4000, + "bounds": { + "x": 160, + "y": 32, + "width": 128, + "height": 160 + } + } + }, + "_note": "Patrols in small area, stops to face player when nearby" + }, + + { + "id": "shy_npc", + "displayName": "Shy Person (Personal Space)", + "npcType": "person", + "position": { "x": 2, "y": 6 }, + "spriteSheet": "hacker", + "spriteConfig": { + "idleFrameStart": 20, + "idleFrameEnd": 23 + }, + "storyPath": "scenarios/ink/behavior-test-shy.json", + "currentKnot": "start", + "behavior": { + "facePlayer": true, + "personalSpace": { + "enabled": true, + "distance": 48, + "backAwaySpeed": 30, + "backAwayDistance": 5 + } + }, + "_note": "Backs away slowly (5px at a time) if player gets within 48px (1.5 tiles), while still facing player. Stays within interaction range (64px)." + }, + + { + "id": "hostile_npc", + "displayName": "Hostile Agent (Red Tint)", + "npcType": "person", + "position": { "x": 8, "y": 6 }, + "spriteSheet": "hacker-red", + "spriteConfig": { + "idleFrameStart": 20, + "idleFrameEnd": 23 + }, + "storyPath": "scenarios/ink/behavior-test-hostile.json", + "currentKnot": "start", + "behavior": { + "facePlayer": true, + "hostile": { + "defaultState": true, + "influenceThreshold": -30, + "aggroDistance": 160 + } + }, + "_note": "Starts hostile (red tint), can be made friendly via Ink tags" + }, + + { + "id": "complex_npc", + "displayName": "Complex Behavior NPC", + "npcType": "person", + "position": { "x": 5, "y": 8 }, + "spriteSheet": "hacker", + "spriteConfig": { + "idleFrameStart": 20, + "idleFrameEnd": 23 + }, + "storyPath": "scenarios/ink/behavior-test-complex.json", + "currentKnot": "start", + "behavior": { + "facePlayer": true, + "patrol": { + "enabled": false, + "speed": 100, + "changeDirectionInterval": 3000 + }, + "personalSpace": { + "enabled": true, + "distance": 48, + "backAwaySpeed": 30, + "backAwayDistance": 5 + }, + "hostile": { + "defaultState": false, + "influenceThreshold": -40 + } + }, + "_note": "Starts with personal space (subtle backing), Ink story can enable patrol and toggle hostility" + } + ] + } + } +} \ No newline at end of file diff --git a/planning_notes/npc/npc_behaviour/review/CHANGES_APPLIED.md b/planning_notes/npc/npc_behaviour/review/CHANGES_APPLIED.md new file mode 100644 index 0000000..afa3229 --- /dev/null +++ b/planning_notes/npc/npc_behaviour/review/CHANGES_APPLIED.md @@ -0,0 +1,247 @@ +# Review Changes Applied to NPC Behavior Implementation Plan + +## Date: 2025-11-09 + +This document summarizes all changes applied to the implementation plan based on the code review findings and project-specific clarifications. + +--- + +## Critical Clarifications from Project Lead + +### 1. **Async Import Pattern** (Issue #5) +- **Clarification**: Project uses async lazy loading for rooms (will be web requests in future) +- **Decision**: Keep async import pattern in game.js - consistent with architecture +- **Status**: ✅ Updated IMPLEMENTATION_PLAN.md to document async pattern + +### 2. **Room Lifecycle** (Issue #7) +- **Clarification**: Rooms are lazy-loaded but NEVER unloaded - NPCs persist throughout session +- **Decision**: Cleanup system moved to Phase 9 (future enhancement, optional) +- **Impact**: No memory leak risk, simpler implementation +- **Status**: ✅ Updated all documents to reflect optional cleanup + +### 3. **Depth Updates** (Issue #8) +- **Clarification**: Depth MUST be updated every frame for proper Y-axis rendering order +- **Decision**: Keep depth updates in every update cycle (not conditional) +- **Impact**: No performance issue - necessary for correct visual layering +- **Status**: ✅ Updated TECHNICAL_SPEC.md with explanation + +### 4. **Personal Space Design** (Issue #15) +- **Clarification**: Personal space should be SMALLER than interaction range (64px) +- **Requirements**: + - Back away slowly (5px increments) + - Face player while backing (maintain eye contact) + - Stay within interaction range (NPC remains interactive) +- **New Values**: + - `distance: 48px` (was 64px) + - `backAwaySpeed: 30` (was 80) + - `backAwayDistance: 5` (new property) +- **Status**: ✅ Updated all documents and examples + +--- + +## Files Updated + +### 1. TECHNICAL_SPEC.md +**Changes Applied**: +- ✅ Added `roomId` property to NPCBehavior (from npcManager.npcs) +- ✅ Added sprite validation with `.destroyed` check in update loop +- ✅ Updated personal space defaults: 48px, speed 30, increment 5 +- ✅ Rewrote `maintainPersonalSpace()` algorithm for subtle backing +- ✅ Added note about depth updates being required +- ✅ Added NPCBehavior constructor code with roomId initialization +- ✅ Documented sprite storage in both locations (npcData._sprite and roomData.npcSprites) +- ✅ Updated removeBehavior() as optional for future use + +**Key Code Changes**: +```javascript +// Personal space now backs away slowly while facing player +const backAwayDist = this.config.personalSpace.backAwayDistance; // 5px +this.direction = this.calculateDirection(-dx, -dy); // Face player +this.playAnimation('idle', this.direction); // Use idle, not walk +this.isMoving = false; // Not "walking", just adjusting +``` + +### 2. IMPLEMENTATION_PLAN.md +**Changes Applied**: +- ✅ Updated personal space config defaults (48px, speed 30, increment 5) +- ✅ Updated `maintainPersonalSpace()` implementation with design notes +- ✅ Added note about async import being compatible with lazy loading +- ✅ Added roomId tracking requirement + +**Key Changes**: +- Personal space algorithm completely rewritten +- Design notes added explaining the subtle backing behavior +- Async import pattern kept with architecture justification + +### 3. QUICK_REFERENCE.md +**Changes Applied**: +- ✅ Updated personal space example (48px, speed 30, increment 5) +- ✅ Updated configuration defaults table with new values +- ✅ Added `personalSpace.backAwayDistance` property +- ✅ Updated distance reference table (added 1.5 tiles = 48px) +- ✅ Updated troubleshooting section +- ✅ Added note about NPCs remaining interactive +- ✅ Fixed patrol troubleshooting (immovable: false, not true) + +**Key Changes**: +```json +"personalSpace": { + "enabled": true, + "distance": 48, + "backAwaySpeed": 30, + "backAwayDistance": 5 +} +``` + +### 4. example_scenario.json +**Changes Applied**: +- ✅ Updated "shy_npc" personal space to 48px, speed 30, increment 5 +- ✅ Updated "complex_npc" personal space to 48px, speed 30, increment 5 +- ✅ Updated notes to explain subtle backing behavior + +### 5. TODO List +**Changes Applied**: +- ✅ Added Phase 0 task: "Modify npc-sprites.js to create walk animations" +- ✅ Updated task descriptions with critical requirements: + - roomId tracking + - sprite validation with .destroyed + - wall collision setup + - subtle personal space design + - async import pattern +- ✅ Reorganized task priorities based on review + +### 6. REVIEW_AND_IMPROVEMENTS.md +**Changes Applied**: +- ✅ Updated Issue #5 (async import) - marked as acceptable pattern +- ✅ Updated Issue #7 (cleanup) - moved to optional/Phase 9 +- ✅ Updated Issue #8 (depth) - not an issue, required for rendering +- ✅ Completely rewrote Issue #15 (personal space) - new design +- ✅ Updated priority matrix (5 critical issues, 4 non-issues) +- ✅ Updated implementation phases (Phase 0 simplified) +- ✅ Updated risk assessment (several concerns eliminated) +- ✅ Updated documentation update requirements +- ✅ Updated validation checklist +- ✅ Increased confidence level from 70% to 90% +- ✅ Updated version to 1.1 + +--- + +## Implementation Impact + +### Reduced Complexity +1. **No cleanup system needed** (Phase 1) - rooms never unload +2. **Async pattern is standard** - no import changes needed +3. **Depth updates are required** - no optimization needed +4. **Personal space stays simple** - subtle backing, not fleeing + +### Enhanced Design +1. **Personal space is more realistic** - NPCs back away slowly while maintaining eye contact +2. **Interaction preserved** - NPCs stay within range (48px < 64px) +3. **Better UX** - Subtle 5px adjustments vs jarring large movements + +### Timeline Impact +- **Original estimate**: +4-6 hours for corrections +- **New estimate**: +2-3 hours (several concerns eliminated) +- **Phase 0**: 2-3 hours (animation creation only) +- **Phase 1**: 4-6 hours (no change - cleanup removed) +- **Overall**: Faster implementation due to simplified requirements + +--- + +## Critical Issues Status + +| Issue # | Title | Status | Action | +|---------|-------|--------|--------| +| 1 | roomId tracking | ✅ RESOLVED | Added to constructor and properties | +| 2 | Sprite storage locations | ✅ DOCUMENTED | Both locations noted in spec | +| 3 | Wall collisions | ✅ DOCUMENTED | setupNPCWallCollisions() exists | +| 4 | Animation timing | ✅ PLANNED | Phase 0 task created | +| 5 | Async import | ✅ CLARIFIED | Pattern is correct for project | +| 6 | NPCs Map iteration | ✅ NON-ISSUE | Code is correct | +| 7 | Cleanup system | ✅ DEFERRED | Phase 9, optional | +| 8 | Depth updates | ✅ CLARIFIED | Required, not an issue | +| 9 | .destroyed check | ✅ RESOLVED | Added to validation | + +--- + +## Phase 0 Requirements (Before Implementation) + +### Must Complete: +1. ✅ Update all planning documents (DONE) +2. ⏳ Modify npc-sprites.js setupNPCAnimations() to create walk animations +3. ⏳ Review and sign-off on corrected plan + +### Walk Animation Frames: +```javascript +const frameMap = { + 'right': [1, 2, 3, 4], + 'down': [6, 7, 8, 9], + 'up': [11, 12, 13, 14], + 'up-right': [16, 17, 18, 19], + 'down-right': [21, 22, 23, 24] +}; +``` + +--- + +## Key Design Decisions + +### Personal Space Behavior +**Design Goal**: Create subtle, realistic backing behavior that maintains interaction + +**Implementation**: +- **Distance**: 48px (1.5 tiles) - smaller than interaction range +- **Speed**: 30 px/s - slow, deliberate movement +- **Increment**: 5px - small adjustments, not jarring +- **Animation**: idle animation (not walk) - maintains eye contact +- **Result**: NPC backs away slowly while facing player, stays interactive + +**Rationale**: Prevents NPCs from fleeing out of interaction range. Creates more natural, less jarring behavior. Players can still interact while NPC maintains comfort distance. + +### Depth Updates +**Decision**: Update depth every frame (required) + +**Rationale**: +- Y-axis movement requires depth recalculation for proper rendering order +- With 50ms throttle + 10 NPCs = only 200 calculations/sec +- Performance impact is negligible +- Alternative (conditional updates) would cause rendering bugs + +### Room Persistence +**Decision**: Rooms never unload, cleanup is optional + +**Rationale**: +- Current architecture keeps all rooms in memory +- Simplifies behavior implementation (no lifecycle management) +- Future enhancement can add cleanup if rooms become unloadable +- No memory leak risk with current design + +--- + +## Next Steps + +1. ✅ **Review this document** - Verify all changes are correct +2. ⏳ **Create Phase 0 branch** - For animation modifications +3. ⏳ **Modify npc-sprites.js** - Add walk animation creation +4. ⏳ **Begin Phase 1** - Implement core behavior system +5. ⏳ **Test personal space** - Verify subtle backing behavior works as designed + +--- + +## Confidence Assessment + +**Before Review**: 70% confidence +**After Clarifications**: 90% confidence + +**Reasons for Increased Confidence**: +1. ✅ Architecture patterns validated (async imports, room persistence) +2. ✅ Critical issues resolved or clarified (roomId, sprites, depth) +3. ✅ Design improved (subtle personal space behavior) +4. ✅ Complexity reduced (no cleanup system needed) +5. ✅ Timeline improved (fewer corrections needed) + +--- + +**Status**: All review changes applied ✅ +**Version**: Updated to match REVIEW_AND_IMPROVEMENTS.md v1.1 +**Ready for**: Phase 0 implementation (animation creation) diff --git a/planning_notes/npc/npc_behaviour/review/REVIEW_AND_IMPROVEMENTS.md b/planning_notes/npc/npc_behaviour/review/REVIEW_AND_IMPROVEMENTS.md new file mode 100644 index 0000000..90f11b9 --- /dev/null +++ b/planning_notes/npc/npc_behaviour/review/REVIEW_AND_IMPROVEMENTS.md @@ -0,0 +1,915 @@ +# NPC Behavior Implementation Plan - Review & Improvements + +## Executive Summary + +After reviewing the implementation plan against the existing codebase, I've identified **9 critical improvements** and **12 enhancement opportunities** that will significantly increase the chances of implementation success. The plan is solid architecturally, but needs adjustments for better integration with existing systems. + +--- + +## ✅ CRITICAL IMPROVEMENTS (Must Address) + +### 1. **roomId Assignment and Tracking** + +**Issue**: The plan assumes NPCs have a `roomId` property, but this is only set in `npc-lazy-loader.js` at line 39 during registration. The plan's patrol algorithm relies on accessing `roomData` via stored `roomId`. + +**Impact**: Patrol bounds calculation will fail if NPC doesn't have roomId or if room data isn't accessible. + +**Solution**: +```javascript +// In NPCBehavior constructor +constructor(npcId, sprite, config, scene) { + this.npcId = npcId; + this.sprite = sprite; + this.scene = scene; + + // CRITICAL: Get roomId from NPC data at initialization + const npcData = window.npcManager?.npcs.get(npcId); + this.roomId = npcData?.roomId || null; + + if (!this.roomId) { + console.warn(`⚠️ NPC ${npcId} has no roomId - patrol bounds will be limited`); + } + + // ... rest of constructor +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Add roomId to NPCBehavior properties +- `IMPLEMENTATION_PLAN.md` - Document roomId requirement in Phase 4 + +--- + +### 2. **NPC Sprite Array vs Individual Sprite Reference** + +**Issue**: The existing code stores NPC sprites in `roomData.npcSprites` array (rooms.js:1894), AND stores a reference in `npc._sprite` (npc-sprites.js:69). The plan only mentions `npc._sprite`. + +**Impact**: +- Room transitions need to access sprites via `roomData.npcSprites` +- Behavior manager needs to iterate all sprites +- Inconsistent access patterns could cause bugs + +**Solution**: +```javascript +// In NPCBehaviorManager.registerBehavior() +registerBehavior(npcId, sprite, config) { + // Verify sprite is in both locations + const npcData = this.npcManager.npcs.get(npcId); + + if (!npcData._sprite || npcData._sprite !== sprite) { + console.warn(`⚠️ Sprite reference mismatch for ${npcId}`); + } + + const behavior = new NPCBehavior(npcId, sprite, config, this.scene); + this.behaviors.set(npcId, behavior); +} +``` + +**Update Required**: +- `IMPLEMENTATION_PLAN.md` - Document both sprite storage locations +- Add validation in integration checklist + +--- + +### 3. **Wall Collision Setup for Moving NPCs** + +**Issue**: The existing code has `setupNPCWallCollisions()` function (npc-sprites.js:293), but plan doesn't mention this. Moving NPCs (patrol) MUST have wall collisions or they'll walk through walls. + +**Impact**: Patrolling NPCs will walk through walls and objects, breaking immersion. + +**Solution**: +```javascript +// In NPCBehavior constructor, after sprite assignment +if (this.config.patrol.enabled) { + // Ensure wall collisions are set up for moving NPCs + const NPCSpriteManager = await import('../systems/npc-sprites.js'); + if (this.roomId && NPCSpriteManager.setupNPCWallCollisions) { + NPCSpriteManager.setupNPCWallCollisions( + this.scene, + this.sprite, + this.roomId + ); + console.log(`🧱 Wall collisions enabled for patrolling NPC ${this.npcId}`); + } +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Add wall collision setup in constructor +- `IMPLEMENTATION_PLAN.md` Phase 4 - Add wall collision as prerequisite +- `QUICK_REFERENCE.md` - Add troubleshooting for NPCs walking through walls + +--- + +### 4. **NPC Animation Creation Timing** + +**Issue**: The plan says to extend `setupNPCAnimations()` in npc-sprites.js to create walk animations. However, this function is called ONCE during sprite creation (npc-sprites.js:55). If behavior is registered AFTER sprite creation, animations won't exist. + +**Impact**: Walk animations will be missing, causing `playAnimation()` to fail silently. + +**Solution - Option A (Preferred)**: Create animations during sprite creation: +```javascript +// Modify npc-sprites.js setupNPCAnimations() immediately +export function setupNPCAnimations(scene, sprite, spriteSheet, config, npcId) { + // Existing idle animation code... + + // NEW: Create walk animations (all NPCs get these, even if not moving yet) + const walkDirs = ['right', 'down', 'up', 'up-right', 'down-right']; + const frameMap = { + 'right': [1, 2, 3, 4], + 'down': [6, 7, 8, 9], + 'up': [11, 12, 13, 14], + 'up-right': [16, 17, 18, 19], + 'down-right': [21, 22, 23, 24] + }; + + walkDirs.forEach(dir => { + const animKey = `npc-${npcId}-walk-${dir}`; + if (!scene.anims.exists(animKey)) { + scene.anims.create({ + key: animKey, + frames: scene.anims.generateFrameNumbers(spriteSheet, { + frames: frameMap[dir] + }), + frameRate: 8, + repeat: -1 + }); + } + }); +} +``` + +**Solution - Option B**: Lazy-create animations in NPCBehavior: +```javascript +// In NPCBehavior.playAnimation() +playAnimation(state, direction) { + // Ensure animation exists (lazy creation) + this._ensureAnimationExists(state, direction); + // ... rest of playAnimation logic +} + +_ensureAnimationExists(state, direction) { + // Create animation if missing (implementation in TECHNICAL_SPEC) +} +``` + +**Recommendation**: Use Option A - create all animations upfront during sprite creation. Simpler, more reliable. + +**Update Required**: +- `IMPLEMENTATION_PLAN.md` Phase 3 - Change to "modify npc-sprites.js setupNPCAnimations NOW" +- `TECHNICAL_SPEC.md` - Update animation creation section +- Move animation creation to Phase 1 (infrastructure) instead of Phase 3 + +--- + +### 5. **Game.js Integration Point - Async Import Issue** + +**Issue**: The plan shows using async import in game.js create(): +```javascript +const NPCBehaviorManager = await import('./systems/npc-behavior.js?v=1'); +``` + +But `create()` is NOT an async function in the existing code (game.js:434). + +**CLARIFICATION**: The project uses async lazy loading for rooms (will be web requests in future), so making create() async is acceptable and follows the project's async pattern. + +**Impact**: No impact - async create() is compatible with the project architecture. + +**Solution**: +```javascript +// Make create() async (RECOMMENDED for this project) +export async function create() { + // ... existing code ... + + // Import and initialize behavior manager (lazy load) + const { default: NPCBehaviorManager } = await import('../systems/npc-behavior.js?v=1'); + window.npcBehaviorManager = new NPCBehaviorManager(this, window.npcManager); + + // Register behaviors for sprite-based NPCs + for (const [npcId, npcData] of window.npcManager.npcs) { + if (npcData._sprite && npcData.npcType === 'person') { + const behaviorConfig = npcData.behavior || {}; + window.npcBehaviorManager.registerBehavior(npcId, npcData._sprite, behaviorConfig); + } + } +} +``` + +**Recommendation**: Use async/await pattern - consistent with room lazy loading architecture. + +**Update Required**: +- `IMPLEMENTATION_PLAN.md` - Keep async import, add note about lazy loading +- `TECHNICAL_SPEC.md` - Document async pattern + +--- + +### 6. **NPCs Array vs NPCs Map Iteration** + +**Issue**: The plan shows iterating `window.npcManager.npcs` as an array: +```javascript +for (const [npcId, npcData] of window.npcManager.npcs) +``` + +But in npc-manager.js:8, `npcs` is a Map, not an array. However, the iteration IS correct for a Map. + +**Non-Issue**: Actually, this is correct! Just needs documentation. + +**Update Required**: +- `IMPLEMENTATION_PLAN.md` - Add comment clarifying this is Map.entries() iteration +- `TECHNICAL_SPEC.md` - Document that npcs is a Map + +--- + + + +**Issue**: The plan doesn't address what happens to NPC behaviors when rooms are loaded/unloaded. Currently, `unloadNPCSprites()` (rooms.js:1942) destroys sprites but doesn't clean up behaviors. + +**Impact**: +- Memory leaks if behaviors aren't removed when sprites destroyed +- Behavior update loop tries to access destroyed sprites +- Patrol state lost when room reloaded + +**Solution**: +```javascript +### 7. **Behavior State Persistence Across Room Changes** + +**CLARIFICATION**: Rooms are lazy-loaded but never unloaded in the current architecture. NPCs persist once created. + +**Impact**: No memory leak risk - sprites and behaviors remain in memory throughout game session. + +**Simplified Solution**: +```javascript +// In NPCBehavior.update() - just add validation +update(time, delta, playerPos) { + // Verify sprite still exists (safety check only) + if (!this.sprite || !this.sprite.body || this.sprite.destroyed) { + console.warn(`⚠️ Invalid sprite for ${this.npcId}, skipping update`); + return; + } + + // Normal update logic... +} +``` + +**Optional Enhancement for Future**: +```javascript +// In NPCBehaviorManager - add cleanup method for future use +removeBehavior(npcId) { + const behavior = this.behaviors.get(npcId); + if (behavior) { + // Stop movement + if (behavior.sprite && behavior.sprite.body) { + behavior.sprite.body.setVelocity(0, 0); + } + this.behaviors.delete(npcId); + console.log(`🧹 Removed behavior for ${npcId}`); + } +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Note that cleanup is optional (for future room unloading) +- `IMPLEMENTATION_PLAN.md` - Move cleanup to Phase 9 (future enhancement) +- Keep validation in Phase 1 +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Add cleanup() method to NPCBehavior +- `TECHNICAL_SPEC.md` - Add removeBehavior() to NPCBehaviorManager +- `IMPLEMENTATION_PLAN.md` - Add cleanup to integration points +- Add to Phase 1 (infrastructure) + +--- + +### 8. **Depth Update Frequency** + +**CLARIFICATION**: Depth MUST be updated frequently (at minimum while visible) because z-index needs updating as NPC moves along Y-axis for proper rendering order. + +**Impact**: No performance issue - depth updates are necessary for correct visual layering. + +**Solution**: +```javascript +// In NPCBehavior.update() +update(time, delta, playerPos) { + // ... state machine logic ... + + // ALWAYS update depth (required for proper rendering order) + this.updateDepth(); +} +``` + +**Performance Note**: With throttled updates (50ms) and 10 NPCs, that's only 200 depth calculations/sec, which is negligible compared to rendering overhead. + +**Update Required**: +- `TECHNICAL_SPEC.md` - Keep depth update in every cycle +- Add note explaining why it's necessary (Y-axis rendering order) + +--- + +### 9. **Missing Error Handling for Destroyed Sprites** + +**Issue**: The plan's update loop checks `if (!this.sprite || !this.sprite.body)` but doesn't check `this.sprite.destroyed` which is the Phaser way to check if a sprite is destroyed. + +**Impact**: May try to operate on destroyed sprites, causing errors. + +**Solution**: +```javascript +// In NPCBehavior.update() +update(time, delta, playerPos) { + // Comprehensive sprite validation + if (!this.sprite || !this.sprite.body || this.sprite.destroyed) { + console.warn(`⚠️ Invalid sprite for ${this.npcId}, skipping update`); + return; + } + + // ... rest of update +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Update error handling example +- Add `destroyed` check in all sprite access locations + +--- + +## 🎯 ENHANCEMENT OPPORTUNITIES (Recommended) + +### 10. **Use Existing NPC Collision System** + +**Opportunity**: The code already has `createNPCCollision()` (npc-sprites.js:206) and `setupNPCEnvironmentCollisions()` (called in rooms.js:1910). Leverage these. + +**Benefit**: Consistent collision handling, less code duplication. + +**Implementation**: Document in plan that these functions are already called during sprite creation. + +**Update Required**: +- `IMPLEMENTATION_PLAN.md` - Note these systems already exist +- `TECHNICAL_SPEC.md` - Reference existing collision setup + +--- + +### 11. **Behavior Debug Mode Integration** + +**Opportunity**: The project already has a debug system (`js/systems/debug.js`). Integrate behavior debug mode with it. + +**Benefit**: Consistent debug interface, toggle via existing debug panel. + +**Implementation**: +```javascript +// In debug.js +window.toggleNPCBehaviorDebug = function() { + window.NPC_BEHAVIOR_DEBUG = !window.NPC_BEHAVIOR_DEBUG; + console.log(`NPC Behavior Debug: ${window.NPC_BEHAVIOR_DEBUG ? 'ON' : 'OFF'}`); +}; +``` + +**Update Required**: +- Add to future enhancements section +- Document debug integration in QUICK_REFERENCE.md + +--- + +### 12. **Reuse Event Dispatcher for Behavior Events** + +**Opportunity**: The project has `window.eventDispatcher` (npc-events.js). Use it for behavior state changes. + +**Benefit**: Other systems can react to NPC behavior changes (e.g., player achievements, UI updates). + +**Implementation**: +```javascript +// In NPCBehavior.setHostile() +setHostile(hostile) { + if (this.hostile !== hostile) { + this.hostile = hostile; + + // Visual feedback + if (hostile) { + this.sprite.setTint(0xff6666); + } else { + this.sprite.clearTint(); + } + + // Emit event for other systems + if (window.eventDispatcher) { + window.eventDispatcher.emit('npc_hostile_changed', { + npcId: this.npcId, + hostile: hostile + }); + } + } +} +``` + +**Update Required**: +- Add to future enhancements +- Document event emission in TECHNICAL_SPEC.md + +--- + +### 13. **NPC Bark Integration for Patrol** + +**Opportunity**: The project has `NPCBarkSystem` (npc-barks.js). Use it for NPC ambient dialogue during patrol. + +**Benefit**: More immersive, NPCs feel alive. + +**Implementation**: +```javascript +// In patrol behavior, occasionally trigger bark +if (Math.random() < 0.01) { // 1% chance per update + if (window.barkSystem) { + window.barkSystem.showBark(this.npcId, "Just making my rounds..."); + } +} +``` + +**Update Required**: +- Add to future enhancements +- Document in QUICK_REFERENCE.md patterns + +--- + +### 14. **Sound Effects for NPC Movement** + +**Opportunity**: The project has `SoundManager` (sound-manager.js). Add footstep sounds for NPCs. + +**Benefit**: Audio feedback for NPC presence and movement. + +**Implementation**: Add to post-MVP enhancements. + +**Update Required**: +- Add to future enhancements section + +--- + +### 15. **Personal Space Behavior Design** + +**CLARIFICATION**: Personal space should be SMALLER than interaction range (64px). NPCs should back up only ~5px at a time while still facing the player, staying within interaction range. + +**Design Goals**: +- Non-hostile NPCs back away slowly (not flee) +- Stay within interaction range so player can still talk +- Maintain eye contact (face player) while backing away +- Subtle movement, not jarring + +**Improved Implementation**: +```javascript +const DEFAULT_CONFIG = { + personalSpace: { + enabled: false, + distance: 48, // CHANGE: 48px (1.5 tiles) - smaller than interaction range + distanceSq: 2304, + backAwaySpeed: 30, // CHANGE: Slow backing speed (was 80) + backAwayDistance: 5 // NEW: Only move 5px at a time + } +}; + +// In maintainPersonalSpace() +maintainPersonalSpace(playerPos, delta) { + if (!this.config.personalSpace.enabled || !playerPos) { + return false; + } + + const dx = this.sprite.x - playerPos.x; // Away from player + const dy = this.sprite.y - playerPos.y; + const distanceSq = dx * dx + dy * dy; + + // Player too close? + if (distanceSq < this.config.personalSpace.distanceSq) { + const distance = Math.sqrt(distanceSq); + + // Back away slowly in small increments + const backAwayDist = this.config.personalSpace.backAwayDistance; + const targetX = this.sprite.x + (dx / distance) * backAwayDist; + const targetY = this.sprite.y + (dy / distance) * backAwayDist; + + // Smoothly move to target + const moveSpeed = this.config.personalSpace.backAwaySpeed; + const moveX = (targetX - this.sprite.x); + const moveY = (targetY - this.sprite.y); + + this.sprite.body.setVelocity(moveX * moveSpeed, moveY * moveSpeed); + + // Still face the player while backing away + this.direction = this.calculateDirection(-dx, -dy); // Negative = face player + this.playAnimation('idle', this.direction); // Use idle, not walk + + this.isMoving = false; // Not "walking", just adjusting position + this.backingAway = true; + + return true; + } + + this.backingAway = false; + return false; +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Update personal space implementation with small increments +- `QUICK_REFERENCE.md` - Update defaults: distance=48px, speed=30, increment=5px +- Add note: "NPCs maintain eye contact while backing away slightly" + +--- + +### 16. **Patrol Bounds Default to Room Size** + +**Enhancement**: The plan mentions defaulting patrol bounds to room size, but doesn't show implementation. + +**Implementation**: +```javascript +parseConfig(userConfig) { + const config = { ...DEFAULT_CONFIG }; + + // ... other parsing ... + + // Calculate patrol bounds relative to room + if (this.roomId && window.rooms && window.rooms[this.roomId]) { + const roomData = window.rooms[this.roomId]; + const roomWidth = roomData.map?.widthInPixels || 320; + const roomHeight = roomData.map?.heightInPixels || 288; + + // Default to 80% of room size (avoid walls) + if (!userConfig.patrol?.bounds) { + config.patrol.bounds = { + x: roomWidth * 0.1, + y: roomHeight * 0.1, + width: roomWidth * 0.8, + height: roomHeight * 0.8 + }; + } + } + + return config; +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Add default bounds calculation code +- `QUICK_REFERENCE.md` - Document automatic bounds + +--- + +### 17. **Face Player Distance Validation** + +**Enhancement**: Ensure face player distance is less than aggro distance (hostile NPCs). + +**Implementation**: +```javascript +parseConfig(userConfig) { + // ... parsing ... + + // Validate: facePlayer distance should be less than aggro distance + if (config.facePlayer && config.hostile.aggroDistance) { + if (config.facePlayerDistance >= config.hostile.aggroDistance) { + console.warn( + `⚠️ facePlayerDistance (${config.facePlayerDistance}) >= ` + + `aggroDistance (${config.hostile.aggroDistance}). ` + + `This may cause unexpected behavior. Reducing facePlayerDistance.` + ); + config.facePlayerDistance = config.hostile.aggroDistance * 0.5; + config.facePlayerDistanceSq = config.facePlayerDistance ** 2; + } + } + + return config; +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Add validation logic +- Add to config validation section + +--- + +### 18. **Animation Fallback for Missing Sprites** + +**Enhancement**: If walk animations don't exist for a spritesheet, fall back to idle animations. + +**Implementation**: +```javascript +playAnimation(state, direction) { + // ... existing code ... + + const animKey = `npc-${this.npcId}-${state}-${animDirection}`; + + if (this.sprite.anims.exists(animKey)) { + this.sprite.play(animKey, true); + this.lastAnimationKey = animKey; + } else { + // Fallback: use idle animation if walk doesn't exist + if (state === 'walk') { + const idleKey = `npc-${this.npcId}-idle-${animDirection}`; + if (this.sprite.anims.exists(idleKey)) { + console.warn(`⚠️ Walk animation missing for ${this.npcId}, using idle`); + this.sprite.play(idleKey, true); + this.lastAnimationKey = idleKey; + return; + } + } + console.warn(`❌ Animation not found: ${animKey}`); + } +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Add fallback logic +- `QUICK_REFERENCE.md` - Add to troubleshooting + +--- + +### 19. **Stuck Detection Uses Position Delta** + +**Enhancement**: Current stuck detection uses `sprite.body.blocked`. Better to also check if position hasn't changed. + +**Implementation**: +```javascript +updatePatrol(time, delta) { + // ... existing code ... + + // Enhanced stuck detection + const currentPos = { x: this.sprite.x, y: this.sprite.y }; + const positionDelta = Math.sqrt( + (currentPos.x - this.lastPatrolPos.x) ** 2 + + (currentPos.y - this.lastPatrolPos.y) ** 2 + ); + + const isBlocked = this.sprite.body.blocked.none === false; + const isStuck = positionDelta < 2; // Moved less than 2px + + if (isBlocked || isStuck) { + this.stuckTimer += delta; + + if (this.stuckTimer > 500) { + this.chooseRandomPatrolDirection(); + this.stuckTimer = 0; + } + } else { + this.stuckTimer = 0; + this.lastPatrolPos = currentPos; + } +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Enhance stuck detection algorithm +- Add `lastPatrolPos` to NPCBehavior properties + +--- + +### 20. **Config Deep Clone Utility** + +**Enhancement**: The plan uses `JSON.parse(JSON.stringify())` for deep cloning. This loses functions and has issues with undefined values. Use a proper clone utility. + +**Implementation**: +```javascript +// Helper function +_deepClone(obj) { + if (obj === null || typeof obj !== 'object') return obj; + if (obj instanceof Date) return new Date(obj.getTime()); + if (Array.isArray(obj)) return obj.map(item => this._deepClone(item)); + + const cloned = {}; + for (const key in obj) { + if (obj.hasOwnProperty(key)) { + cloned[key] = this._deepClone(obj[key]); + } + } + return cloned; +} + +parseConfig(userConfig) { + const config = this._deepClone(DEFAULT_CONFIG); + // ... rest of parsing +} +``` + +**Update Required**: +- `TECHNICAL_SPEC.md` - Replace JSON clone with proper deep clone +- Add utility function + +--- + +### 21. **TypeScript Definitions (Future)** + +**Enhancement**: Consider adding JSDoc type definitions for better IDE support. + +**Example**: +```javascript +/** + * @typedef {Object} NPCBehaviorConfig + * @property {boolean} facePlayer + * @property {number} facePlayerDistance + * @property {PatrolConfig} patrol + * @property {PersonalSpaceConfig} personalSpace + * @property {HostileConfig} hostile + */ + +/** + * @param {string} npcId - NPC identifier + * @param {Phaser.Sprite} sprite - Phaser sprite reference + * @param {NPCBehaviorConfig} config - Behavior configuration + * @param {Phaser.Scene} scene - Phaser scene reference + */ +constructor(npcId, sprite, config, scene) { + // ... +} +``` + +**Update Required**: +- Add to future enhancements +- Consider for Phase 9 (documentation) + +--- + +## 📊 PRIORITY MATRIX + +| Issue # | Type | Priority | Impact | Effort | Phase | +|---------|------|----------|--------|--------|-------| +| 1 | Critical | HIGH | HIGH | LOW | 1 | +| 2 | Critical | HIGH | MEDIUM | LOW | 1 | +| 3 | Critical | HIGH | HIGH | MEDIUM | 4 | +| 4 | Critical | HIGH | HIGH | LOW | 1 | +| 5 | Critical | LOW | LOW | LOW | 1 | +| 6 | Critical | LOW | LOW | LOW | 1 | +| 7 | Critical | LOW | LOW | LOW | 9 | +| 8 | Critical | NONE | NONE | NONE | N/A | +| 9 | Critical | HIGH | MEDIUM | LOW | 1 | +| 10 | Enhancement | MEDIUM | LOW | LOW | Any | +| 11 | Enhancement | LOW | LOW | MEDIUM | Post-MVP | +| 12 | Enhancement | MEDIUM | MEDIUM | LOW | Post-MVP | +| 13 | Enhancement | LOW | LOW | MEDIUM | Post-MVP | +| 14 | Enhancement | LOW | LOW | MEDIUM | Post-MVP | +| 15 | Enhancement | HIGH | HIGH | MEDIUM | 5 | +| 16 | Enhancement | HIGH | MEDIUM | MEDIUM | 4 | +| 17 | Enhancement | LOW | LOW | LOW | 1 | +| 18 | Enhancement | MEDIUM | MEDIUM | LOW | 3 | +| 19 | Enhancement | MEDIUM | LOW | MEDIUM | 4 | +| 20 | Enhancement | LOW | LOW | LOW | 1 | +| 21 | Enhancement | LOW | LOW | HIGH | Post-MVP | + +--- + +## 🔧 RECOMMENDED IMPLEMENTATION ORDER + +### Phase 0: Pre-Implementation (NEW PHASE) + +**Before starting Phase 1, address critical issues:** + +1. ✅ Update IMPLEMENTATION_PLAN.md with corrections from issues #1 +2. ✅ Update TECHNICAL_SPEC.md with corrections from issues #2, #4, #9 +3. ✅ Update QUICK_REFERENCE.md with corrections from issues #3, #15 +4. ✅ Modify npc-sprites.js to create walk animations NOW (issue #4) +5. ✅ Review and sign-off on corrected plan + +**Estimated Time**: 2-3 hours + +### Phase 1: Core Infrastructure (UPDATED) + +**Add to existing Phase 1:** +- Add sprite validation with .destroyed check (issue #9) +- Add config validation (issue #17) +- Use async import pattern (issue #5 - clarified as acceptable) + +**Estimated Time**: 4-6 hours (no change - cleanup moved to Phase 9) + +### Phase 2-8: Continue as Planned + +**With modifications:** +- Phase 3: Animation system is already done in Phase 0 +- Phase 4: Add wall collision setup (issue #3), bounds calculation (issue #16) +- Phase 5: Implement subtle personal space behavior (issue #15) - 48px distance, 5px increments +- Phase 9: Add optional cleanup system for future room unloading (issue #7) + +--- + +## 📝 DOCUMENTATION UPDATES NEEDED + +### IMPLEMENTATION_PLAN.md + +1. Add Pre-Implementation Phase 0 +2. Keep async import pattern with note about lazy loading (issue #5) +3. Move cleanup system to Phase 9 (future enhancement) (issue #7) +4. Move animation creation to Phase 0 (issue #4) +5. Add wall collision to Phase 4 (issue #3) +6. Update Phase 5 with subtle personal space behavior (issue #15) + +### TECHNICAL_SPEC.md + +1. Add `roomId` property to NPCBehavior (issue #1) +2. Document sprite storage in both locations (issue #2) +3. Add `cleanup()` method as optional future enhancement (issue #7) +4. Update error handling examples with .destroyed check (issue #9) +5. Keep depth update in every cycle with explanation (issue #8) +6. Update animation creation section (issue #4) +7. Implement subtle personal space behavior (issue #15) - 48px, 5px increments, face player +8. Add config validation examples (issue #17) +9. Add default bounds calculation (issue #16) +10. Enhance stuck detection (issue #19) + +### QUICK_REFERENCE.md + +1. Update default personal space: distance=48px, speed=30, increment=5px (issue #15) +2. Add note about subtle backing behavior while facing player (issue #15) +3. Add troubleshooting for walking through walls (issue #3) +4. Add troubleshooting for missing animations (issue #18) +5. Document automatic patrol bounds +6. Note that cleanup is optional for future (issue #7) + +### example_scenario.json + +1. Update personal space distances to 48px with backAwayDistance: 5 +2. Add roomId comments for clarity + +### README.md + +1. Add Pre-Implementation Phase 0 +2. Update integration checklist +3. Note that room persistence means cleanup is optional + +--- + +## ✅ VALIDATION CHECKLIST + +Before starting implementation: + +- [ ] Critical issues (#1, #2, #3, #4, #9) addressed in documentation +- [ ] npc-sprites.js walk animations created +- [ ] IMPLEMENTATION_PLAN.md updated with Phase 0 +- [ ] TECHNICAL_SPEC.md updated with all corrections +- [ ] QUICK_REFERENCE.md updated with new defaults (48px personal space) +- [ ] Async import pattern documented (lazy loading compatible) +- [ ] Wall collision integration verified +- [ ] Depth update kept in every cycle (required for Y-axis ordering) +- [ ] Personal space behavior designed for subtle 5px backing +- [ ] Error handling patterns reviewed (.destroyed check added) + +--- + +## 🎓 LESSONS LEARNED + +### What Went Right +1. ✅ Solid architecture - modular, extensible design +2. ✅ Good separation of concerns +3. ✅ Comprehensive documentation structure +4. ✅ Clear phased approach +5. ✅ Performance considerations included + +### What Needs Improvement +1. ⚠️ Need to review existing code patterns more thoroughly +2. ⚠️ Integration points need more detailed analysis +3. ✅ Lifecycle management clarified - rooms persist, cleanup optional +4. ✅ Async patterns verified - lazy loading compatible +5. ⚠️ Animation timing dependencies need explicit documentation + +--- + +## 📈 RISK ASSESSMENT UPDATE + +| Risk (Original) | New Risk Level | Mitigation Status | +|----------------|----------------|-------------------| +| Performance degradation | MEDIUM → LOW | Throttling (depth updates required) | +| Animation conflicts | MEDIUM → LOW | Create animations upfront | +| Player collision issues | MEDIUM → LOW | Reuse existing systems | +| Ink tag conflicts | LOW → LOW | No change needed | +| Config schema complexity | LOW → LOW | Added validation | +| **Room transition bugs** | **NEW - NONE** | **Rooms never unload - not a concern** | +| **Import/async issues** | **NEW - NONE** | **Async lazy loading is standard pattern** | +| **Sprite lifecycle** | **NEW - HIGH → LOW** | **Added .destroyed validation** | + +--- + +## 🚀 CONFIDENCE LEVEL + +**Before Review**: 70% confidence in plan success +**After Review**: 90% confidence with corrections applied + +**Key Success Factors**: +1. ✅ Address 5 critical issues before coding (others clarified as non-issues) +2. ✅ Use async pattern consistent with lazy loading architecture +3. ✅ Use existing systems where possible (collision, events) +4. ✅ Create animations during sprite creation (not lazy) +5. ✅ Implement subtle personal space (5px backing, face player) + +--- + +## 📞 NEXT STEPS + +1. **Review this document** with team/lead developer +2. **Apply corrections** to all planning documents +3. **Create Phase 0 branch** for pre-implementation fixes +4. **Modify npc-sprites.js** to create walk animations +5. **Begin Phase 1** with updated requirements +6. **Schedule check-in** after Phase 2 completion + +--- + +**Review Status**: Complete (Updated with project clarifications) +**Recommendations**: Implement critical fixes #1-4, #9 before Phase 1 +**Estimated Additional Time**: +2-3 hours for corrections (reduced from 4-6) +**Overall Timeline Impact**: Minimal - several concerns eliminated by architecture clarifications + +--- + +**Reviewer**: AI Coding Agent (GitHub Copilot) +**Review Date**: 2025-11-09 +**Version**: 1.1 (Updated with project-specific clarifications)