Add review changes and improvements for NPC behavior implementation

- Created CHANGES_APPLIED.md to document critical clarifications and updates based on code review findings.
- Established a new review and improvements document (REVIEW_AND_IMPROVEMENTS.md) outlining 9 critical improvements and 12 enhancement opportunities for NPC behavior.
- Updated TECHNICAL_SPEC.md, IMPLEMENTATION_PLAN.md, QUICK_REFERENCE.md, and example_scenario.json with necessary changes regarding roomId tracking, sprite storage, wall collision setup, animation creation timing, and personal space behavior.
- Clarified async import patterns and depth update requirements in the implementation plan.
- Enhanced personal space behavior design to ensure NPCs back away slowly while maintaining eye contact with players.
- Documented all changes and improvements to ensure better integration with existing systems and reduce complexity in NPC behavior implementation.
This commit is contained in:
Z. Cliffe Schreuders
2025-11-09 02:43:17 +00:00
parent 2b3642f2d3
commit 1f6166ccf6
9 changed files with 4112 additions and 0 deletions

View File

@@ -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:<value>` | 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:<px>` | 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<npcId, NPCBehavior>
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)

View File

@@ -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

View File

@@ -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

File diff suppressed because it is too large Load Diff

View File

@@ -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

View File

@@ -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

View File

@@ -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"
}
]
}
}
}

View File

@@ -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)

View File

@@ -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)