mirror of
https://github.com/cliffe/BreakEscape.git
synced 2026-02-22 11:48:18 +00:00
Add updated TODO checklist and corrected code snippets for objectives system
- Created an updated TODO checklist outlining phases and tasks for the objectives system implementation. - Added corrected code snippets for critical fixes, including door unlock event emission and key pickup event handling. - Documented necessary changes to ensure proper event emissions for objectives tracking. - Reviewed and approved the implementation plan with minor corrections regarding door unlock events.
This commit is contained in:
Z. Cliffe Schreuders
1866
planning_notes/objectives_system/IMPLEMENTATION_PLAN.md
Normal file
1866
planning_notes/objectives_system/IMPLEMENTATION_PLAN.md
Normal file
File diff suppressed because it is too large
Load Diff
126
planning_notes/objectives_system/QUICK_REFERENCE.md
Normal file
126
planning_notes/objectives_system/QUICK_REFERENCE.md
Normal file
@@ -0,0 +1,126 @@
|
||||
# Objectives System - Quick Reference
|
||||
|
||||
## Task Types at a Glance
|
||||
|
||||
| Type | Trigger | Event Name | Example |
|
||||
|------|---------|------------|---------|
|
||||
| `collect_items` | Player picks up items | `item_picked_up:*` | Find 3 documents |
|
||||
| `unlock_room` | Door unlocked | `door_unlocked` (use `connectedRoom`) | Access server room |
|
||||
| `unlock_object` | Container unlocked | `item_unlocked` (NOT `object_unlocked`) | Open the safe |
|
||||
| `npc_conversation` | Ink tag `#complete_task:X` | `task_completed_by_npc` | Talk to handler |
|
||||
| `enter_room` | Player enters room | `room_entered` | Explore the lab |
|
||||
| `custom` | Ink tag `#complete_task:X` | `task_completed_by_npc` | Any other trigger |
|
||||
|
||||
## Ink Tags
|
||||
|
||||
```ink
|
||||
# complete_task:taskId -> Marks task complete
|
||||
# unlock_task:taskId -> Makes locked task active
|
||||
# unlock_aim:aimId -> Makes locked aim active
|
||||
```
|
||||
|
||||
## Scenario JSON Template
|
||||
|
||||
```json
|
||||
{
|
||||
"objectives": [
|
||||
{
|
||||
"aimId": "unique_aim_id",
|
||||
"title": "High-level goal",
|
||||
"description": "Optional longer description",
|
||||
"status": "active|locked",
|
||||
"order": 1,
|
||||
"unlockCondition": { "aimCompleted": "other_aim_id" },
|
||||
"tasks": [
|
||||
{
|
||||
"taskId": "unique_task_id",
|
||||
"title": "What player sees",
|
||||
"type": "collect_items|unlock_room|unlock_object|npc_conversation|enter_room|custom",
|
||||
"targetItems": ["item_type"],
|
||||
"targetCount": 3,
|
||||
"targetRoom": "room_id",
|
||||
"targetObject": "Object Name",
|
||||
"targetNpc": "npc_id",
|
||||
"status": "active|locked",
|
||||
"showProgress": true,
|
||||
"onComplete": {
|
||||
"unlockTask": "next_task_id",
|
||||
"unlockAim": "next_aim_id"
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
## API Endpoints (RESTful)
|
||||
|
||||
```
|
||||
GET /break_escape/games/:id/objectives # Get current state
|
||||
POST /break_escape/games/:id/objectives/tasks/:task_id # Complete a task
|
||||
PUT /break_escape/games/:id/objectives/tasks/:task_id # Update progress
|
||||
```
|
||||
|
||||
## Events Emitted
|
||||
|
||||
```javascript
|
||||
// Task completed
|
||||
eventDispatcher.emit('objective_task_completed', { taskId, aimId, task });
|
||||
|
||||
// Aim completed
|
||||
eventDispatcher.emit('objective_aim_completed', { aimId, aim });
|
||||
```
|
||||
|
||||
## Events Listened To
|
||||
|
||||
```javascript
|
||||
'item_picked_up:*' // For collect_items (wildcard pattern)
|
||||
'door_unlocked' // For unlock_room (use data.connectedRoom)
|
||||
'item_unlocked' // For unlock_object (NOT object_unlocked!)
|
||||
'room_entered' // For enter_room
|
||||
'task_completed_by_npc' // From ink tags
|
||||
```
|
||||
|
||||
## Initialization Order
|
||||
|
||||
1. `main.js`: Create `ObjectivesManager` instance (manager only)
|
||||
2. `game.js create()`: Restore `objectivesState` from server to `window.gameState.objectives`
|
||||
3. `game.js create()`: Call `objectivesManager.initialize(gameScenario.objectives)`
|
||||
4. `game.js create()`: Create `ObjectivesPanel` instance
|
||||
|
||||
> **CRITICAL**: Objectives data initialization MUST happen in `game.js create()`, NOT `main.js`. Scenario JSON isn't available until `create()` runs.
|
||||
|
||||
## CSS Classes
|
||||
|
||||
```css
|
||||
.objectives-panel /* Main container */
|
||||
.objectives-panel.collapsed
|
||||
.objective-aim /* Aim container */
|
||||
.aim-active / .aim-completed
|
||||
.objective-task /* Task container */
|
||||
.task-active / .task-completed
|
||||
```
|
||||
|
||||
## Global Access
|
||||
|
||||
```javascript
|
||||
window.objectivesManager.completeTask('taskId');
|
||||
window.objectivesManager.unlockTask('taskId');
|
||||
window.objectivesManager.unlockAim('aimId');
|
||||
window.objectivesManager.getActiveAims();
|
||||
window.objectivesManager.reconcileWithGameState();
|
||||
|
||||
// Debug utilities
|
||||
window.debugObjectives.showAll();
|
||||
window.debugObjectives.reset();
|
||||
```
|
||||
|
||||
## Key Gotchas
|
||||
|
||||
1. **CRITICAL**: `door_unlocked` events NOT emitted in current codebase - must add to `doors.js`
|
||||
2. **Event name**: `item_unlocked` NOT `object_unlocked`
|
||||
3. **Door event**: `door_unlocked` should provide both `roomId` and `connectedRoom` (use `connectedRoom`)
|
||||
4. **Keys don't emit events**: Need to add event to `addKeyToInventory()`
|
||||
5. **State restoration**: Server passes `objectivesState` in scenario bootstrap
|
||||
6. **Reconciliation**: Call `reconcileWithGameState()` after init for late-loaded scenarios
|
||||
134
planning_notes/objectives_system/TODO_CHECKLIST.md
Normal file
134
planning_notes/objectives_system/TODO_CHECKLIST.md
Normal file
@@ -0,0 +1,134 @@
|
||||
# Objectives System - TODO Checklist
|
||||
|
||||
Track implementation progress here. Check off items as completed.
|
||||
|
||||
---
|
||||
|
||||
## Phase 0: Prerequisites (Do First) ⬜
|
||||
- [ ] 0.1 **CRITICAL**: Add `door_unlocked` event emission to `doors.js` `unlockDoor()` function
|
||||
- [ ] 0.2 Add key pickup events to `inventory.js` `addKeyToInventory()` function
|
||||
- [ ] 0.3 Verify `item_unlocked` event name in `unlock-system.js` (line ~587) - ✅ VERIFIED
|
||||
- [ ] 0.4 Verify `room_entered` events are emitted in `rooms.js`
|
||||
|
||||
## Phase 1: Core Infrastructure ⬜
|
||||
- [ ] 1.1 Create database migration `db/migrate/XXXXXX_add_objectives_to_games.rb`
|
||||
- [ ] 1.2 Add objective methods to `app/models/break_escape/game.rb`:
|
||||
- [ ] `initialize_objectives`
|
||||
- [ ] `complete_task!(task_id, validation_data)`
|
||||
- [ ] `update_task_progress!(task_id, progress)`
|
||||
- [ ] `aim_status(aim_id)` / `task_status(task_id)`
|
||||
- [ ] Private helpers: `validate_collection`, `process_task_completion`, etc.
|
||||
- [ ] 1.3 Add RESTful API routes to `config/routes.rb`:
|
||||
- [ ] `GET objectives` - Get current objective state
|
||||
- [ ] `POST objectives/tasks/:task_id` - Complete a specific task
|
||||
- [ ] `PUT objectives/tasks/:task_id` - Update task progress
|
||||
- [ ] 1.4 Add controller actions to `games_controller.rb`:
|
||||
- [ ] `def objectives`
|
||||
- [ ] `def complete_task`
|
||||
- [ ] `def update_task_progress`
|
||||
- [ ] 1.5 Update `scenario` action to include `objectivesState` for reload recovery
|
||||
- [ ] 1.6 Create `public/break_escape/js/systems/objectives-manager.js`
|
||||
- [ ] 1.7 Create `public/break_escape/css/objectives.css`
|
||||
|
||||
## Phase 2: Event Integration ⬜
|
||||
- [ ] 2.1 Subscribe to `item_picked_up:*` wildcard events → `handleItemPickup()`
|
||||
- [ ] 2.2 Subscribe to `door_unlocked` events → `handleRoomUnlock()` (use `connectedRoom`)
|
||||
- [ ] 2.3 Subscribe to `door_unlocked_by_npc` events
|
||||
- [ ] 2.4 Subscribe to `item_unlocked` events → `handleObjectUnlock()` (NOT `object_unlocked`)
|
||||
- [ ] 2.5 Subscribe to `room_entered` events → `handleRoomEntered()`
|
||||
- [ ] 2.6 Subscribe to `task_completed_by_npc` events
|
||||
|
||||
## Phase 3: UI Implementation ⬜
|
||||
- [ ] 3.1 Create `public/break_escape/js/ui/objectives-panel.js`
|
||||
- [ ] 3.2 Implement `createPanel()` with header and content areas
|
||||
- [ ] 3.3 Implement `render(aims)` for aim/task hierarchy
|
||||
- [ ] 3.4 Implement `toggleCollapse()` functionality
|
||||
- [ ] 3.5 Add progress text for `showProgress: true` tasks
|
||||
- [ ] 3.6 Add completion animations (CSS keyframes)
|
||||
- [ ] 3.7 Ensure CSS follows project conventions (2px borders, no border-radius)
|
||||
|
||||
## Phase 4: Integration & Wiring ⬜
|
||||
- [ ] 4.1 Add imports to `public/break_escape/js/main.js`:
|
||||
- [ ] `import ObjectivesManager`
|
||||
- [ ] `import ObjectivesPanel`
|
||||
- [ ] 4.2 Initialize `window.objectivesManager` in `main.js initializeGame()` (manager only)
|
||||
- [ ] 4.3 Call `objectivesManager.initialize()` in `game.js create()` after scenario loads
|
||||
- [ ] 4.4 Restore `objectivesState` to `window.gameState.objectives` in `game.js create()`
|
||||
- [ ] 4.5 Create `ObjectivesPanel` instance in `game.js create()`
|
||||
- [ ] 4.6 Add `<link>` to objectives.css in game HTML template
|
||||
|
||||
## Phase 5: Server Validation ⬜
|
||||
- [ ] 5.1 Update `sync_state` action to accept/return objectives
|
||||
- [ ] 5.2 Validate `collect_items` tasks against `player_state['inventory']`
|
||||
- [ ] 5.3 Validate `unlock_room` tasks against `player_state['unlockedRooms']`
|
||||
- [ ] 5.4 Validate `unlock_object` tasks against `player_state['unlockedObjects']`
|
||||
- [ ] 5.5 Validate `npc_conversation` tasks against `player_state['encounteredNPCs']`
|
||||
- [ ] 5.6 Increment `tasks_completed` and `objectives_completed` counters
|
||||
|
||||
## Phase 6: Ink Tag Extensions ⬜
|
||||
- [ ] 6.1 Add `complete_task` case to `chat-helpers.js` `processGameActionTags()`
|
||||
- [ ] 6.2 Add `unlock_task` case
|
||||
- [ ] 6.3 Add `unlock_aim` case
|
||||
- [ ] 6.4 Test tags in phone-chat minigame
|
||||
- [ ] 6.5 Test tags in person-chat minigame
|
||||
|
||||
## Phase 7: Reconciliation & Edge Cases ⬜
|
||||
- [ ] 7.1 Implement `reconcileWithGameState()` in ObjectivesManager
|
||||
- [ ] 7.2 Handle collect_items reconciliation (check existing inventory)
|
||||
- [ ] 7.3 Handle unlock_room reconciliation (check discoveredRooms)
|
||||
- [ ] 7.4 Handle enter_room reconciliation (check discoveredRooms)
|
||||
- [ ] 7.5 Add debounced `syncTaskProgress()` with timeout tracking
|
||||
- [ ] 7.6 Store `originalStatus` for debug reset functionality
|
||||
|
||||
## Phase 8: Testing ⬜
|
||||
- [ ] 8.1 Create test scenario in `scenarios/test-objectives/scenario.json.erb`
|
||||
- [ ] 8.2 Create test Ink story in `scenarios/test-objectives/guide.ink`
|
||||
- [ ] 8.3 Test `collect_items` objective (pick up multiple items)
|
||||
- [ ] 8.4 Test `unlock_room` objective (unlock a door)
|
||||
- [ ] 8.5 Test `unlock_object` objective (unlock a container)
|
||||
- [ ] 8.6 Test `npc_conversation` objective (ink tag completion)
|
||||
- [ ] 8.7 Test `enter_room` objective (walk into room)
|
||||
- [ ] 8.8 Test chained objectives (`onComplete.unlockTask`)
|
||||
- [ ] 8.9 Test aim completion (all tasks done → aim complete)
|
||||
- [ ] 8.10 Test aim unlock conditions (`unlockCondition.aimCompleted`)
|
||||
- [ ] 8.11 Test server validation (complete without meeting conditions)
|
||||
- [ ] 8.12 Test state persistence (reload page, check objectives restored)
|
||||
- [ ] 8.13 Test reconciliation (collect items, then reload - should reconcile)
|
||||
|
||||
## Phase 9: Documentation ⬜
|
||||
- [ ] 9.1 Create `docs/OBJECTIVES_USAGE.md` with full documentation
|
||||
- [ ] 9.2 Update `README_scenario_design.md` with objectives section
|
||||
- [ ] 9.3 Add objectives examples to existing scenario documentation
|
||||
- [ ] 9.4 Document ink tags in docs/INK_BEST_PRACTICES.md
|
||||
|
||||
---
|
||||
|
||||
## Notes
|
||||
|
||||
_Add implementation notes, blockers, or decisions here:_
|
||||
|
||||
- **CRITICAL**: `door_unlocked` events are NOT emitted in current codebase - must add to `doors.js`
|
||||
- Event name is `item_unlocked` NOT `object_unlocked` (unlock-system.js line 587) ✅
|
||||
- `door_unlocked` event should provide both `roomId` and `connectedRoom` (use `connectedRoom` for unlock tasks)
|
||||
- Keys do NOT emit pickup events - requires fix in `addKeyToInventory()`
|
||||
- Objectives init happens in `game.js create()` NOT `main.js` (scenario not available until then)
|
||||
- Server includes `objectivesState` in scenario bootstrap for reload recovery
|
||||
- Use RESTful routes: `POST /objectives/tasks/:task_id` (task_id in path)
|
||||
|
||||
---
|
||||
|
||||
## Completion Summary
|
||||
|
||||
| Phase | Status | Completed |
|
||||
|-------|--------|-----------|
|
||||
| Phase 0: Prerequisites | ⬜ | 0/4 |
|
||||
| Phase 1: Core Infrastructure | ⬜ | 0/7 |
|
||||
| Phase 2: Event Integration | ⬜ | 0/6 |
|
||||
| Phase 3: UI Implementation | ⬜ | 0/7 |
|
||||
| Phase 4: Integration | ⬜ | 0/6 |
|
||||
| Phase 5: Server Validation | ⬜ | 0/6 |
|
||||
| Phase 6: Ink Tags | ⬜ | 0/5 |
|
||||
| Phase 7: Reconciliation | ⬜ | 0/6 |
|
||||
| Phase 8: Testing | ⬜ | 0/13 |
|
||||
| Phase 9: Documentation | ⬜ | 0/4 |
|
||||
| **Total** | **⬜** | **0/64** |
|
||||
@@ -0,0 +1,459 @@
|
||||
# Objectives System - Corrected Code Snippets
|
||||
|
||||
This file contains corrected code based on the implementation review findings.
|
||||
|
||||
---
|
||||
|
||||
## 1. Corrected Event Listeners (objectives-manager.js)
|
||||
|
||||
```javascript
|
||||
/**
|
||||
* Setup event listeners for automatic objective tracking
|
||||
* CORRECTED: Uses actual event names from codebase
|
||||
*/
|
||||
setupEventListeners() {
|
||||
// Item collection - wildcard pattern works with NPCEventDispatcher
|
||||
this.eventDispatcher.on('item_picked_up:*', (data) => {
|
||||
this.handleItemPickup(data);
|
||||
});
|
||||
|
||||
// Room/door unlocks
|
||||
this.eventDispatcher.on('door_unlocked', (data) => {
|
||||
this.handleRoomUnlock(data.connectedRoom); // Note: connectedRoom, not roomId
|
||||
});
|
||||
|
||||
this.eventDispatcher.on('door_unlocked_by_npc', (data) => {
|
||||
this.handleRoomUnlock(data.roomId);
|
||||
});
|
||||
|
||||
// Object unlocks - CORRECTED: event is 'item_unlocked' not 'object_unlocked'
|
||||
this.eventDispatcher.on('item_unlocked', (data) => {
|
||||
// data contains: { itemType, itemName, lockType }
|
||||
// Match against task.targetObject using itemName or itemType
|
||||
this.handleObjectUnlock(data.itemName, data.itemType);
|
||||
});
|
||||
|
||||
// Room entry
|
||||
this.eventDispatcher.on('room_entered', (data) => {
|
||||
this.handleRoomEntered(data.roomId);
|
||||
});
|
||||
|
||||
// NPC conversation completion (via ink tag)
|
||||
this.eventDispatcher.on('task_completed_by_npc', (data) => {
|
||||
this.completeTask(data.taskId);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle object unlock - CORRECTED to accept both name and type
|
||||
*/
|
||||
handleObjectUnlock(itemName, itemType) {
|
||||
Object.values(this.taskIndex).forEach(task => {
|
||||
if (task.type !== 'unlock_object') return;
|
||||
if (task.status !== 'active') return;
|
||||
|
||||
// Match by either targetObject name or type
|
||||
const matches = task.targetObject === itemName ||
|
||||
task.targetObject === itemType;
|
||||
if (!matches) return;
|
||||
|
||||
this.completeTask(task.taskId);
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. Corrected Initialization Location (game.js)
|
||||
|
||||
```javascript
|
||||
// In game.js create() function, AFTER gameScenario is loaded (around line 510)
|
||||
|
||||
// Initialize global narrative variables from scenario
|
||||
if (gameScenario.globalVariables) {
|
||||
window.gameState.globalVariables = { ...gameScenario.globalVariables };
|
||||
console.log('🌐 Initialized global variables:', window.gameState.globalVariables);
|
||||
} else {
|
||||
window.gameState.globalVariables = {};
|
||||
}
|
||||
|
||||
// NEW: Restore objectives state from server if available
|
||||
if (gameScenario.objectivesState) {
|
||||
window.gameState.objectives = gameScenario.objectivesState;
|
||||
console.log('📋 Restored objectives state from server');
|
||||
}
|
||||
|
||||
// NEW: Initialize objectives system after scenario is loaded
|
||||
if (gameScenario.objectives && window.objectivesManager) {
|
||||
console.log('📋 Initializing objectives from scenario');
|
||||
window.objectivesManager.initialize(gameScenario.objectives);
|
||||
|
||||
// Create UI panel
|
||||
if (typeof ObjectivesPanel !== 'undefined') {
|
||||
window.objectivesPanel = new ObjectivesPanel(window.objectivesManager);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Corrected Rails Routes (routes.rb)
|
||||
|
||||
```ruby
|
||||
BreakEscape::Engine.routes.draw do
|
||||
# ... existing routes ...
|
||||
|
||||
resources :games, only: [:show, :create] do
|
||||
member do
|
||||
# Existing routes
|
||||
get 'scenario'
|
||||
get 'scenario_map'
|
||||
get 'ink'
|
||||
get 'room/:room_id', to: 'games#room', as: 'room'
|
||||
get 'container/:container_id', to: 'games#container'
|
||||
put 'sync_state'
|
||||
post 'unlock'
|
||||
post 'inventory'
|
||||
|
||||
# NEW: Objectives routes (RESTful pattern)
|
||||
get 'objectives' # Get current objective state
|
||||
post 'objectives/tasks/:task_id', # Complete a specific task
|
||||
to: 'games#complete_task',
|
||||
as: 'complete_task'
|
||||
put 'objectives/tasks/:task_id', # Update task progress
|
||||
to: 'games#update_task_progress',
|
||||
as: 'update_task_progress'
|
||||
end
|
||||
end
|
||||
end
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 4. Include Objectives State in Scenario Bootstrap (games_controller.rb)
|
||||
|
||||
```ruby
|
||||
# GET /games/:id/scenario
|
||||
def scenario
|
||||
authorize @game if defined?(Pundit)
|
||||
|
||||
begin
|
||||
filtered = @game.filtered_scenario_for_bootstrap
|
||||
filter_requires_recursive(filtered)
|
||||
|
||||
# NEW: Include objectives state for restoration
|
||||
if @game.player_state['objectives'].present?
|
||||
filtered['objectivesState'] = @game.player_state['objectives']
|
||||
end
|
||||
|
||||
render json: filtered
|
||||
rescue => e
|
||||
Rails.logger.error "[BreakEscape] scenario error: #{e.message}"
|
||||
render_error("Failed to generate scenario: #{e.message}", :internal_server_error)
|
||||
end
|
||||
end
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 5. Reconciliation Method for Late Initialization (objectives-manager.js)
|
||||
|
||||
```javascript
|
||||
/**
|
||||
* Reconcile objectives with current game state
|
||||
* Handles case where player collected items before objectives loaded
|
||||
*/
|
||||
reconcileWithGameState() {
|
||||
console.log('📋 Reconciling objectives with current game state...');
|
||||
|
||||
// Check inventory for items matching collect_items tasks
|
||||
const inventoryItems = window.inventory?.items || [];
|
||||
|
||||
Object.values(this.taskIndex).forEach(task => {
|
||||
if (task.status !== 'active') return;
|
||||
|
||||
switch (task.type) {
|
||||
case 'collect_items':
|
||||
const matchingItems = inventoryItems.filter(item => {
|
||||
const itemType = item.scenarioData?.type || item.getAttribute?.('data-type');
|
||||
return task.targetItems.includes(itemType);
|
||||
});
|
||||
|
||||
// Also count keys from keyRing
|
||||
const keyRingItems = window.inventory?.keyRing?.keys || [];
|
||||
const matchingKeys = keyRingItems.filter(key =>
|
||||
task.targetItems.includes(key.scenarioData?.type)
|
||||
);
|
||||
|
||||
const totalCount = matchingItems.length + matchingKeys.length;
|
||||
|
||||
if (totalCount > (task.currentCount || 0)) {
|
||||
task.currentCount = totalCount;
|
||||
console.log(`📋 Reconciled ${task.taskId}: ${totalCount}/${task.targetCount}`);
|
||||
|
||||
if (totalCount >= task.targetCount) {
|
||||
this.completeTask(task.taskId);
|
||||
}
|
||||
}
|
||||
break;
|
||||
|
||||
case 'unlock_room':
|
||||
// Check if room is already unlocked
|
||||
const unlockedRooms = window.gameState?.unlockedRooms || [];
|
||||
const isUnlocked = unlockedRooms.includes(task.targetRoom) ||
|
||||
window.discoveredRooms?.has(task.targetRoom);
|
||||
if (isUnlocked) {
|
||||
console.log(`📋 Reconciled ${task.taskId}: room already unlocked`);
|
||||
this.completeTask(task.taskId);
|
||||
}
|
||||
break;
|
||||
|
||||
case 'enter_room':
|
||||
// Check if room was already visited
|
||||
if (window.discoveredRooms?.has(task.targetRoom)) {
|
||||
console.log(`📋 Reconciled ${task.taskId}: room already visited`);
|
||||
this.completeTask(task.taskId);
|
||||
}
|
||||
break;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Initialize objectives from scenario data
|
||||
*/
|
||||
initialize(objectivesData) {
|
||||
if (!objectivesData || !objectivesData.length) {
|
||||
console.log('📋 No objectives defined in scenario');
|
||||
return;
|
||||
}
|
||||
|
||||
// Deep clone to avoid mutating scenario
|
||||
this.aims = JSON.parse(JSON.stringify(objectivesData));
|
||||
|
||||
// Build indexes
|
||||
this.aims.forEach(aim => {
|
||||
this.aimIndex[aim.aimId] = aim;
|
||||
aim.tasks.forEach(task => {
|
||||
task.aimId = aim.aimId;
|
||||
this.taskIndex[task.taskId] = task;
|
||||
});
|
||||
});
|
||||
|
||||
// Sort by order
|
||||
this.aims.sort((a, b) => (a.order || 0) - (b.order || 0));
|
||||
|
||||
// Restore state from server if available
|
||||
this.restoreState();
|
||||
|
||||
// NEW: Reconcile with current game state
|
||||
this.reconcileWithGameState();
|
||||
|
||||
console.log(`📋 Objectives initialized: ${this.aims.length} aims`);
|
||||
this.notifyListeners();
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 6. Key Item Event Emission Fix (inventory.js)
|
||||
|
||||
Add this to `addKeyToInventory()` after line 460:
|
||||
|
||||
```javascript
|
||||
function addKeyToInventory(sprite) {
|
||||
// ... existing code to add key to keyRing ...
|
||||
|
||||
// NEW: Emit item_picked_up event for keys too (for objectives tracking)
|
||||
if (window.eventDispatcher) {
|
||||
window.eventDispatcher.emit(`item_picked_up:${sprite.scenarioData.type}`, {
|
||||
itemType: sprite.scenarioData.type,
|
||||
itemName: sprite.scenarioData.name,
|
||||
roomId: window.currentPlayerRoom,
|
||||
isKey: true
|
||||
});
|
||||
|
||||
// Also emit specific key_id event if available
|
||||
const keyId = sprite.scenarioData?.key_id || sprite.key_id;
|
||||
if (keyId) {
|
||||
window.eventDispatcher.emit(`item_picked_up:key:${keyId}`, {
|
||||
itemType: 'key',
|
||||
keyId: keyId,
|
||||
itemName: sprite.scenarioData.name,
|
||||
roomId: window.currentPlayerRoom
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// ... rest of existing code ...
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 7. Responsive CSS Additions (objectives.css)
|
||||
|
||||
Add to the end of the CSS file:
|
||||
|
||||
```css
|
||||
/* Responsive breakpoints for objectives panel */
|
||||
|
||||
@media (max-width: 1024px) {
|
||||
.objectives-panel {
|
||||
width: 240px;
|
||||
}
|
||||
}
|
||||
|
||||
@media (max-width: 768px) {
|
||||
.objectives-panel {
|
||||
width: 200px;
|
||||
font-size: 12px;
|
||||
top: 10px;
|
||||
right: 10px;
|
||||
}
|
||||
|
||||
.objectives-title {
|
||||
font-size: 14px;
|
||||
}
|
||||
|
||||
.aim-header {
|
||||
font-size: 13px;
|
||||
}
|
||||
|
||||
.objective-task {
|
||||
font-size: 11px;
|
||||
}
|
||||
}
|
||||
|
||||
@media (max-width: 480px) {
|
||||
.objectives-panel {
|
||||
position: fixed;
|
||||
top: auto;
|
||||
bottom: 90px; /* Above inventory */
|
||||
right: 10px;
|
||||
left: 10px;
|
||||
width: auto;
|
||||
max-height: 30vh;
|
||||
}
|
||||
|
||||
.objectives-panel.collapsed {
|
||||
left: auto;
|
||||
width: auto;
|
||||
min-width: 120px;
|
||||
}
|
||||
|
||||
.objectives-panel.collapsed .objectives-title {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.objectives-panel.collapsed .objectives-header::before {
|
||||
content: '📋';
|
||||
margin-right: 8px;
|
||||
}
|
||||
}
|
||||
|
||||
/* Animation for objective updates */
|
||||
@keyframes objective-highlight {
|
||||
0% {
|
||||
background-color: rgba(255, 204, 0, 0.4);
|
||||
transform: scale(1.02);
|
||||
}
|
||||
100% {
|
||||
background-color: transparent;
|
||||
transform: scale(1);
|
||||
}
|
||||
}
|
||||
|
||||
.objective-task.updated {
|
||||
animation: objective-highlight 0.6s ease-out;
|
||||
}
|
||||
|
||||
.objective-aim.updated {
|
||||
animation: objective-highlight 0.8s ease-out;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 8. Debug Utilities
|
||||
|
||||
Add to `objectives-manager.js` or create `objectives-debug.js`:
|
||||
|
||||
```javascript
|
||||
/**
|
||||
* Debug utilities for objectives system
|
||||
* Available via window.debugObjectives
|
||||
*/
|
||||
export function initObjectivesDebug(manager) {
|
||||
window.debugObjectives = {
|
||||
// Complete a task manually
|
||||
completeTask: (taskId) => {
|
||||
console.log(`🔧 Debug: Completing task ${taskId}`);
|
||||
return manager.completeTask(taskId);
|
||||
},
|
||||
|
||||
// Unlock a task manually
|
||||
unlockTask: (taskId) => {
|
||||
console.log(`🔧 Debug: Unlocking task ${taskId}`);
|
||||
return manager.unlockTask(taskId);
|
||||
},
|
||||
|
||||
// Unlock an aim manually
|
||||
unlockAim: (aimId) => {
|
||||
console.log(`🔧 Debug: Unlocking aim ${aimId}`);
|
||||
return manager.unlockAim(aimId);
|
||||
},
|
||||
|
||||
// Show all objectives state
|
||||
showAll: () => {
|
||||
console.log('📋 All Aims:', manager.getAllAims());
|
||||
console.table(manager.getAllAims().map(aim => ({
|
||||
aimId: aim.aimId,
|
||||
title: aim.title,
|
||||
status: aim.status,
|
||||
tasks: aim.tasks.length,
|
||||
completedTasks: aim.tasks.filter(t => t.status === 'completed').length
|
||||
})));
|
||||
},
|
||||
|
||||
// Show specific task
|
||||
showTask: (taskId) => {
|
||||
const task = manager.taskIndex[taskId];
|
||||
if (task) {
|
||||
console.log(`📋 Task ${taskId}:`, task);
|
||||
} else {
|
||||
console.warn(`Task ${taskId} not found`);
|
||||
}
|
||||
},
|
||||
|
||||
// Reset objectives (for testing)
|
||||
reset: () => {
|
||||
console.log('🔧 Debug: Resetting objectives');
|
||||
Object.values(manager.taskIndex).forEach(task => {
|
||||
task.status = task.originalStatus || 'active';
|
||||
task.currentCount = 0;
|
||||
delete task.completedAt;
|
||||
});
|
||||
Object.values(manager.aimIndex).forEach(aim => {
|
||||
aim.status = aim.originalStatus || 'active';
|
||||
delete aim.completedAt;
|
||||
});
|
||||
manager.notifyListeners();
|
||||
},
|
||||
|
||||
// Simulate item pickup
|
||||
simulatePickup: (itemType) => {
|
||||
console.log(`🔧 Debug: Simulating pickup of ${itemType}`);
|
||||
manager.handleItemPickup({ itemType, itemName: itemType });
|
||||
},
|
||||
|
||||
// Force reconciliation
|
||||
reconcile: () => {
|
||||
console.log('🔧 Debug: Forcing reconciliation');
|
||||
manager.reconcileWithGameState();
|
||||
}
|
||||
};
|
||||
|
||||
console.log('📋 Debug utilities available via window.debugObjectives');
|
||||
}
|
||||
```
|
||||
@@ -0,0 +1,415 @@
|
||||
# Objectives System - Implementation Review
|
||||
|
||||
## Executive Summary
|
||||
|
||||
After reviewing the codebase against the implementation plan, I've identified several **critical issues**, **gaps**, and **improvement opportunities** that should be addressed before implementation. The plan is solid overall but needs refinements for successful integration.
|
||||
|
||||
---
|
||||
|
||||
## 🔴 Critical Issues
|
||||
|
||||
### 1. Missing `item_picked_up` Event - Actual Format Differs
|
||||
|
||||
**Issue:** The plan assumes `item_picked_up:*` wildcard events exist, but the actual emitted event format is:
|
||||
|
||||
```javascript
|
||||
// Actual (inventory.js line 369-374)
|
||||
window.eventDispatcher.emit(`item_picked_up:${sprite.scenarioData.type}`, {
|
||||
itemType: sprite.scenarioData.type,
|
||||
itemName: sprite.scenarioData.name,
|
||||
roomId: window.currentPlayerRoom
|
||||
});
|
||||
```
|
||||
|
||||
**Problem:** The event is emitted with type-specific naming (`item_picked_up:lockpick`), not a generic `item_picked_up`. The wildcard listener `item_picked_up:*` **will work** thanks to `NPCEventDispatcher.emit()` (line 28-33), but only matching `eventType.startsWith(prefix)`.
|
||||
|
||||
**Fix Required:** The plan's listener pattern is correct, but ensure we're subscribing to `item_picked_up:*` (with asterisk) or individual types:
|
||||
|
||||
```javascript
|
||||
// This WILL work (verified in npc-events.js)
|
||||
this.eventDispatcher.on('item_picked_up:*', (data) => {
|
||||
this.handleItemPickup(data);
|
||||
});
|
||||
```
|
||||
|
||||
### 2. Missing `object_unlocked` Event
|
||||
|
||||
**Issue:** The plan assumes an `object_unlocked` event exists for container unlocks.
|
||||
|
||||
**Actual:** The codebase emits `item_unlocked` (not `object_unlocked`):
|
||||
|
||||
```javascript
|
||||
// unlock-system.js line 587-592
|
||||
window.eventDispatcher.emit('item_unlocked', {
|
||||
itemType: lockable.scenarioData.type,
|
||||
itemName: lockable.scenarioData.name,
|
||||
lockType: lockable.scenarioData.lockType
|
||||
});
|
||||
```
|
||||
|
||||
**Fix Required:** Update plan to listen to `item_unlocked` instead of `object_unlocked`:
|
||||
|
||||
```javascript
|
||||
// In ObjectivesManager.setupEventListeners()
|
||||
this.eventDispatcher.on('item_unlocked', (data) => {
|
||||
this.handleObjectUnlock(data.itemName); // Note: uses itemName, not objectId
|
||||
});
|
||||
```
|
||||
|
||||
### 3. Initialization Order Problem
|
||||
|
||||
**Issue:** The plan suggests initializing `ObjectivesManager` in `main.js`, but the scenario isn't loaded until `game.js create()`.
|
||||
|
||||
**Current Flow:**
|
||||
1. `main.js` runs → `initializeGame()` creates Phaser game
|
||||
2. Phaser calls `preload()` → loads scenario JSON
|
||||
3. Phaser calls `create()` → `window.gameScenario` is set
|
||||
|
||||
**Problem:** `ObjectivesManager.initialize(objectives)` needs scenario data, but it's not available during `main.js` execution.
|
||||
|
||||
**Fix Required:** Initialize ObjectivesManager in `game.js create()`, not `main.js`:
|
||||
|
||||
```javascript
|
||||
// game.js create() - after gameScenario is loaded (around line 510)
|
||||
if (gameScenario.objectives) {
|
||||
console.log('📋 Initializing objectives from scenario');
|
||||
window.objectivesManager?.initialize(gameScenario.objectives);
|
||||
window.objectivesPanel = new ObjectivesPanel(window.objectivesManager);
|
||||
}
|
||||
```
|
||||
|
||||
### 4. Server Routes Conflict with Existing Pattern
|
||||
|
||||
**Issue:** Proposed routes don't follow Rails Engine nested resource pattern.
|
||||
|
||||
**Proposed:**
|
||||
```ruby
|
||||
get 'objectives'
|
||||
post 'objectives/complete'
|
||||
post 'objectives/progress'
|
||||
```
|
||||
|
||||
**Problem:** The existing routes use different patterns and `/complete` as a sub-path doesn't cleanly fit Rails conventions.
|
||||
|
||||
**Fix Required:** Use RESTful nested resource or different naming:
|
||||
|
||||
```ruby
|
||||
resources :games, only: [:show, :create] do
|
||||
member do
|
||||
# Existing routes...
|
||||
get 'objectives', to: 'games#objectives'
|
||||
post 'objectives', to: 'games#complete_objective' # Use POST body for task_id
|
||||
put 'objectives', to: 'games#update_objective_progress' # Use PUT for updates
|
||||
end
|
||||
end
|
||||
```
|
||||
|
||||
Or add as collection with task_id in path:
|
||||
```ruby
|
||||
post 'objectives/:task_id/complete', to: 'games#complete_objective'
|
||||
put 'objectives/:task_id/progress', to: 'games#update_objective_progress'
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🟠 Gaps & Missing Considerations
|
||||
|
||||
### 5. No Handling for Key Items in `collect_items`
|
||||
|
||||
**Issue:** Keys are handled differently via `addKeyToInventory()` and don't emit the standard `item_picked_up` event with full data.
|
||||
|
||||
**Impact:** `collect_items` tasks targeting keys may not increment progress.
|
||||
|
||||
**Fix Required:** Ensure key collection also emits the event:
|
||||
|
||||
```javascript
|
||||
// inventory.js addKeyToInventory() - add after line 460
|
||||
if (window.eventDispatcher) {
|
||||
window.eventDispatcher.emit(`item_picked_up:${sprite.scenarioData.type}`, {
|
||||
itemType: sprite.scenarioData.type,
|
||||
itemName: sprite.scenarioData.name,
|
||||
roomId: window.currentPlayerRoom
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
### 6. `targetObject` vs `itemName` Mismatch
|
||||
|
||||
**Issue:** The plan uses `targetObject` for unlock_object tasks, but events provide `itemName` or `itemType`.
|
||||
|
||||
**Impact:** Task matching will fail if object identifiers don't match.
|
||||
|
||||
**Fix Required:** Clarify the identifier strategy. Options:
|
||||
- Use `scenarioData.name` (human readable, may have duplicates)
|
||||
- Use `scenarioData.id` (unique, requires adding id to all objects)
|
||||
- Use `objectId` (auto-generated, unpredictable)
|
||||
|
||||
**Recommendation:** Add optional `objectId` to scenario objects and use that for task matching:
|
||||
```json
|
||||
{
|
||||
"type": "safe",
|
||||
"objectId": "ceo_safe",
|
||||
"name": "CEO's Personal Safe",
|
||||
"locked": true
|
||||
}
|
||||
```
|
||||
|
||||
### 7. No State Restoration on Page Reload
|
||||
|
||||
**Issue:** The plan mentions `restoreState()` but doesn't show how server state gets into `window.gameState.objectives`.
|
||||
|
||||
**Fix Required:** Include objectives state in initial game load or scenario bootstrap:
|
||||
|
||||
```ruby
|
||||
# games_controller.rb - scenario action
|
||||
def scenario
|
||||
filtered = @game.filtered_scenario_for_bootstrap
|
||||
filtered['objectivesState'] = @game.player_state['objectives'] # Add this
|
||||
render json: filtered
|
||||
end
|
||||
```
|
||||
|
||||
```javascript
|
||||
// game.js create() - after loading scenario
|
||||
if (gameScenario.objectivesState) {
|
||||
window.gameState.objectives = gameScenario.objectivesState;
|
||||
}
|
||||
```
|
||||
|
||||
### 8. No Handling for Already-Completed Tasks
|
||||
|
||||
**Issue:** If a player collects item #3 of 5 before objectives system initializes, progress is lost.
|
||||
|
||||
**Fix Required:** Add a reconciliation step on initialize:
|
||||
|
||||
```javascript
|
||||
initialize(objectivesData) {
|
||||
// ... existing code ...
|
||||
|
||||
// Reconcile with current game state
|
||||
this.reconcileWithGameState();
|
||||
}
|
||||
|
||||
reconcileWithGameState() {
|
||||
// Check inventory for items matching collect_items tasks
|
||||
const inventory = window.inventory?.items || [];
|
||||
Object.values(this.taskIndex).forEach(task => {
|
||||
if (task.type === 'collect_items' && task.status === 'active') {
|
||||
const matchingItems = inventory.filter(item =>
|
||||
task.targetItems.includes(item.scenarioData?.type)
|
||||
);
|
||||
if (matchingItems.length > task.currentCount) {
|
||||
task.currentCount = matchingItems.length;
|
||||
if (task.currentCount >= task.targetCount) {
|
||||
this.completeTask(task.taskId);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// Check unlocked rooms for unlock_room tasks
|
||||
// Check discoveredRooms for enter_room tasks
|
||||
// etc.
|
||||
}
|
||||
```
|
||||
|
||||
### 9. CSS Z-Index Conflicts
|
||||
|
||||
**Issue:** The plan uses `z-index: 1500` but notification container uses `z-index: 2000`.
|
||||
|
||||
**Consideration:** Objectives panel should appear below notifications but above game canvas.
|
||||
|
||||
**Current Layers:**
|
||||
- `#notification-container`: 2000
|
||||
- `#objectives-panel` (proposed): 1500
|
||||
- `#inventory-container`: 1000
|
||||
- `#health-ui-container`: 1100
|
||||
|
||||
**Verdict:** z-index 1500 is appropriate. No change needed.
|
||||
|
||||
### 10. Mobile/Responsive Considerations
|
||||
|
||||
**Issue:** The objectives panel is fixed at 280px width with no mobile breakpoints.
|
||||
|
||||
**Fix Required:** Add responsive CSS:
|
||||
|
||||
```css
|
||||
@media (max-width: 768px) {
|
||||
.objectives-panel {
|
||||
width: 200px;
|
||||
font-size: 12px;
|
||||
top: 10px;
|
||||
right: 10px;
|
||||
}
|
||||
|
||||
.objectives-panel.collapsed {
|
||||
width: auto;
|
||||
max-width: 150px;
|
||||
}
|
||||
}
|
||||
|
||||
@media (max-width: 480px) {
|
||||
.objectives-panel {
|
||||
width: 100%;
|
||||
max-width: none;
|
||||
top: 0;
|
||||
right: 0;
|
||||
border-radius: 0;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Improvement Suggestions
|
||||
|
||||
### 11. Add `object_id` Attribute to Unlock Events
|
||||
|
||||
**Suggestion:** Emit more complete data in unlock events:
|
||||
|
||||
```javascript
|
||||
// unlock-system.js - around line 587
|
||||
window.eventDispatcher.emit('item_unlocked', {
|
||||
objectId: lockable.objectId, // Add this
|
||||
itemType: lockable.scenarioData.type,
|
||||
itemName: lockable.scenarioData.name,
|
||||
lockType: lockable.scenarioData.lockType
|
||||
});
|
||||
```
|
||||
|
||||
### 12. Add Sound Effects for Objective Completion
|
||||
|
||||
**Suggestion:** Play UI sounds when objectives complete:
|
||||
|
||||
```javascript
|
||||
showTaskCompleteNotification(task) {
|
||||
if (window.playUISound) {
|
||||
window.playUISound('objective_complete'); // Add this sound
|
||||
}
|
||||
if (window.gameAlert) {
|
||||
window.gameAlert(`✓ ${task.title}`, 'success', 'Task Complete');
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### 13. Persist Panel Collapsed State
|
||||
|
||||
**Suggestion:** Save collapsed state in localStorage:
|
||||
|
||||
```javascript
|
||||
toggleCollapse() {
|
||||
this.isCollapsed = !this.isCollapsed;
|
||||
this.container.classList.toggle('collapsed', this.isCollapsed);
|
||||
localStorage.setItem('objectives_panel_collapsed', this.isCollapsed);
|
||||
}
|
||||
|
||||
createPanel() {
|
||||
// ... existing code ...
|
||||
this.isCollapsed = localStorage.getItem('objectives_panel_collapsed') === 'true';
|
||||
this.container.classList.toggle('collapsed', this.isCollapsed);
|
||||
}
|
||||
```
|
||||
|
||||
### 14. Add Debug Commands
|
||||
|
||||
**Suggestion:** Add console debug helpers:
|
||||
|
||||
```javascript
|
||||
// Expose debug functions
|
||||
window.debugObjectives = {
|
||||
completeTask: (taskId) => window.objectivesManager?.completeTask(taskId),
|
||||
unlockAim: (aimId) => window.objectivesManager?.unlockAim(aimId),
|
||||
showAll: () => console.table(window.objectivesManager?.getAllAims()),
|
||||
reset: () => { /* Reset objectives state */ }
|
||||
};
|
||||
```
|
||||
|
||||
### 15. Consider Debouncing Server Sync
|
||||
|
||||
**Suggestion:** The plan syncs progress on every item pickup. Consider debouncing:
|
||||
|
||||
```javascript
|
||||
syncTaskProgress(taskId, progress) {
|
||||
// Clear existing timeout
|
||||
if (this.syncTimeouts[taskId]) {
|
||||
clearTimeout(this.syncTimeouts[taskId]);
|
||||
}
|
||||
|
||||
// Debounce sync by 1 second
|
||||
this.syncTimeouts[taskId] = setTimeout(() => {
|
||||
this._doSync(taskId, progress);
|
||||
}, 1000);
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🔵 Testing Considerations
|
||||
|
||||
### 16. Add Integration Test Points
|
||||
|
||||
Create `test-objectives.html` similar to other test files:
|
||||
|
||||
```html
|
||||
<!-- Test cases needed -->
|
||||
1. Collect 3 items, verify progress updates
|
||||
2. Unlock room, verify task completes
|
||||
3. Complete all tasks in aim, verify aim completes
|
||||
4. Chain unlock (task A completes → task B unlocks)
|
||||
5. Server validation failure handling
|
||||
6. State persistence across page reload
|
||||
7. Late initialization reconciliation
|
||||
```
|
||||
|
||||
### 17. Add Rails Model Tests
|
||||
|
||||
```ruby
|
||||
# test/models/break_escape/game_objectives_test.rb
|
||||
class GameObjectivesTest < ActiveSupport::TestCase
|
||||
test "complete_task validates collect_items against inventory"
|
||||
test "complete_task validates unlock_room against unlockedRooms"
|
||||
test "process_task_completion unlocks next task"
|
||||
test "check_aim_completion marks aim complete when all tasks done"
|
||||
end
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 📋 Updated TODO Priorities
|
||||
|
||||
Based on this review, update the implementation order:
|
||||
|
||||
### Phase 0: Prerequisite Fixes (NEW)
|
||||
- [ ] 0.1 Add `objectId` field to scenario objects that need tracking
|
||||
- [ ] 0.2 Ensure keys emit `item_picked_up` event
|
||||
- [ ] 0.3 Decide on object identifier strategy (name vs id)
|
||||
|
||||
### Phase 1: Core Infrastructure (Updated)
|
||||
- [ ] 1.1 Create database migration
|
||||
- [ ] 1.2 Add Game model methods (with reconciliation)
|
||||
- [ ] 1.3 Create API endpoints (using corrected route pattern)
|
||||
- [ ] 1.4 Create `objectives-manager.js` (with corrected event names)
|
||||
- [ ] 1.5 Add objectives CSS (with responsive breakpoints)
|
||||
|
||||
### Phase 2: Event Integration (Updated)
|
||||
- [ ] 2.1 Subscribe to `item_picked_up:*` (wildcard - this works)
|
||||
- [ ] 2.2 Subscribe to `door_unlocked` (correct)
|
||||
- [ ] 2.3 Subscribe to `item_unlocked` (not `object_unlocked`)
|
||||
- [ ] 2.4 Subscribe to `room_entered` (correct)
|
||||
- [ ] 2.5 Add reconciliation on initialization
|
||||
|
||||
(Continue with remaining phases from original plan...)
|
||||
|
||||
---
|
||||
|
||||
## Conclusion
|
||||
|
||||
The implementation plan is **well-structured** and **mostly correct**, but requires these adjustments:
|
||||
|
||||
1. **Event name correction**: `item_unlocked` not `object_unlocked`
|
||||
2. **Initialization timing**: Move to `game.js create()` after scenario loads
|
||||
3. **Object identifier strategy**: Need consistent approach
|
||||
4. **State restoration**: Add `objectivesState` to scenario bootstrap
|
||||
5. **Reconciliation**: Handle items collected before objectives init
|
||||
|
||||
With these fixes, the plan has a high chance of successful implementation.
|
||||
@@ -0,0 +1,264 @@
|
||||
# Updated TODO Checklist - Post-Review
|
||||
|
||||
Based on the implementation review, here's the corrected and prioritized TODO list.
|
||||
|
||||
---
|
||||
|
||||
## Phase 0: Prerequisite Fixes 🔴 HIGH PRIORITY
|
||||
_Must be done before starting main implementation_
|
||||
|
||||
- [ ] 0.1 **Add key pickup event emission** in `inventory.js` `addKeyToInventory()`
|
||||
- Currently keys don't emit `item_picked_up` event
|
||||
- Blocks: collect_items tasks for keys
|
||||
|
||||
- [ ] 0.2 **Decide on object identifier strategy**
|
||||
- Option A: Use `scenarioData.name` (simpler, may have duplicates)
|
||||
- Option B: Add `objectId` field to scenario objects (cleaner, requires scenario updates)
|
||||
- **Recommendation:** Use `itemName` for now, document that names must be unique per room
|
||||
|
||||
- [ ] 0.3 **Verify event names match actual codebase**
|
||||
- ✅ `item_picked_up:*` - works with wildcard
|
||||
- ✅ `door_unlocked` - correct
|
||||
- ❌ `object_unlocked` → should be `item_unlocked`
|
||||
- ✅ `room_entered` - correct
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Core Infrastructure
|
||||
_Server-side and client-side core modules_
|
||||
|
||||
### Database & Model
|
||||
- [ ] 1.1 Create migration `db/migrate/XXXXXX_add_objectives_to_games.rb`
|
||||
```ruby
|
||||
add_column :break_escape_games, :objectives_completed, :integer, default: 0
|
||||
add_column :break_escape_games, :tasks_completed, :integer, default: 0
|
||||
```
|
||||
|
||||
- [ ] 1.2 Add objective methods to `app/models/break_escape/game.rb`:
|
||||
- [ ] `initialize_objectives`
|
||||
- [ ] `complete_task!(task_id, validation_data = {})`
|
||||
- [ ] `update_task_progress!(task_id, progress)`
|
||||
- [ ] `aim_status(aim_id)` / `task_status(task_id)`
|
||||
- [ ] `task_progress(task_id)`
|
||||
- [ ] Private: `validate_collection`, `process_task_completion`, etc.
|
||||
- [ ] Private: `find_task_in_scenario(task_id)`
|
||||
|
||||
### API Routes & Controller
|
||||
- [ ] 1.3 Add routes to `config/routes.rb`:
|
||||
```ruby
|
||||
get 'objectives'
|
||||
post 'objectives/tasks/:task_id', to: 'games#complete_task'
|
||||
put 'objectives/tasks/:task_id', to: 'games#update_task_progress'
|
||||
```
|
||||
|
||||
- [ ] 1.4 Add controller actions:
|
||||
- [ ] `def objectives` - GET current state
|
||||
- [ ] `def complete_task` - POST complete a task
|
||||
- [ ] `def update_task_progress` - PUT update progress
|
||||
|
||||
- [ ] 1.5 **Include objectives state in scenario bootstrap**
|
||||
- Update `scenario` action to include `objectivesState`
|
||||
|
||||
### Client Module
|
||||
- [ ] 1.6 Create `public/break_escape/js/systems/objectives-manager.js`
|
||||
- Use corrected event names (`item_unlocked` not `object_unlocked`)
|
||||
- Include `reconcileWithGameState()` method
|
||||
|
||||
- [ ] 1.7 Create `public/break_escape/css/objectives.css`
|
||||
- Include responsive breakpoints
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Event Integration
|
||||
_Wire up to existing game events_
|
||||
|
||||
- [ ] 2.1 Subscribe to `item_picked_up:*` → `handleItemPickup()`
|
||||
- **Note:** Wildcard works, verified in NPCEventDispatcher
|
||||
|
||||
- [ ] 2.2 Subscribe to `door_unlocked` → `handleRoomUnlock()`
|
||||
- **Note:** Use `data.connectedRoom` for room ID
|
||||
|
||||
- [ ] 2.3 Subscribe to `door_unlocked_by_npc` → `handleRoomUnlock()`
|
||||
|
||||
- [ ] 2.4 Subscribe to `item_unlocked` → `handleObjectUnlock()`
|
||||
- **CORRECTED:** Event name is `item_unlocked`, not `object_unlocked`
|
||||
|
||||
- [ ] 2.5 Subscribe to `room_entered` → `handleRoomEntered()`
|
||||
|
||||
- [ ] 2.6 Subscribe to `task_completed_by_npc` (from ink tags)
|
||||
|
||||
- [ ] 2.7 **Implement reconciliation** on initialize
|
||||
- Check inventory for existing items
|
||||
- Check discoveredRooms for visited rooms
|
||||
- Check unlockedRooms for unlocked doors
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: UI Implementation
|
||||
_Objectives panel display_
|
||||
|
||||
- [ ] 3.1 Create `public/break_escape/js/ui/objectives-panel.js`
|
||||
|
||||
- [ ] 3.2 Implement `createPanel()` with header and content
|
||||
|
||||
- [ ] 3.3 Implement `render(aims)` for aim/task hierarchy
|
||||
|
||||
- [ ] 3.4 Implement `toggleCollapse()` with localStorage persistence
|
||||
|
||||
- [ ] 3.5 Add progress text for `showProgress: true` tasks
|
||||
|
||||
- [ ] 3.6 Add CSS animations for:
|
||||
- New objective appearance
|
||||
- Task completion
|
||||
- Progress updates
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: Integration & Wiring
|
||||
_Connect all pieces together_
|
||||
|
||||
- [ ] 4.1 Add imports to `public/break_escape/js/main.js`:
|
||||
```javascript
|
||||
import ObjectivesManager, { getObjectivesManager } from './systems/objectives-manager.js';
|
||||
```
|
||||
|
||||
- [ ] 4.2 Create ObjectivesManager instance in `main.js initializeGame()`
|
||||
```javascript
|
||||
window.objectivesManager = getObjectivesManager(window.eventDispatcher);
|
||||
```
|
||||
|
||||
- [ ] 4.3 **Initialize from scenario in `game.js create()`** (CORRECTED LOCATION)
|
||||
```javascript
|
||||
if (gameScenario.objectives && window.objectivesManager) {
|
||||
// Restore state first
|
||||
if (gameScenario.objectivesState) {
|
||||
window.gameState.objectives = gameScenario.objectivesState;
|
||||
}
|
||||
window.objectivesManager.initialize(gameScenario.objectives);
|
||||
window.objectivesPanel = new ObjectivesPanel(window.objectivesManager);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] 4.4 Add `<link>` to objectives.css in game HTML template
|
||||
|
||||
- [ ] 4.5 Export `window.objectivesManager` for global access
|
||||
|
||||
---
|
||||
|
||||
## Phase 5: Server Validation
|
||||
_Ensure server validates completions_
|
||||
|
||||
- [ ] 5.1 Update `sync_state` action to accept/return objectives
|
||||
|
||||
- [ ] 5.2 Validate `collect_items` against `player_state['inventory']`
|
||||
|
||||
- [ ] 5.3 Validate `unlock_room` against `player_state['unlockedRooms']`
|
||||
|
||||
- [ ] 5.4 Validate `unlock_object` against `player_state['unlockedObjects']`
|
||||
|
||||
- [ ] 5.5 Validate `npc_conversation` against `player_state['encounteredNPCs']`
|
||||
|
||||
- [ ] 5.6 Return `objectivesState` in filtered_scenario_for_bootstrap
|
||||
|
||||
---
|
||||
|
||||
## Phase 6: Ink Tag Extensions
|
||||
_Add objective-related tags to NPC dialogues_
|
||||
|
||||
- [ ] 6.1 Add `complete_task` case to `chat-helpers.js` `processGameActionTags()`
|
||||
```javascript
|
||||
case 'complete_task':
|
||||
window.eventDispatcher.emit('task_completed_by_npc', { taskId: param });
|
||||
```
|
||||
|
||||
- [ ] 6.2 Add `unlock_task` case
|
||||
|
||||
- [ ] 6.3 Add `unlock_aim` case
|
||||
|
||||
- [ ] 6.4 Test in phone-chat minigame
|
||||
|
||||
- [ ] 6.5 Test in person-chat minigame
|
||||
|
||||
---
|
||||
|
||||
## Phase 7: Testing
|
||||
_Comprehensive testing_
|
||||
|
||||
### Manual Test Cases
|
||||
- [ ] 7.1 Create test scenario `scenarios/test-objectives/scenario.json.erb`
|
||||
- Include all task types
|
||||
- Include chained objectives
|
||||
- Include locked aims
|
||||
|
||||
- [ ] 7.2 Test `collect_items` (pick up 3 documents)
|
||||
|
||||
- [ ] 7.3 Test `collect_items` with keys (via keyRing)
|
||||
|
||||
- [ ] 7.4 Test `unlock_room` (unlock a door)
|
||||
|
||||
- [ ] 7.5 Test `unlock_object` (unlock a container)
|
||||
|
||||
- [ ] 7.6 Test `npc_conversation` (ink tag completion)
|
||||
|
||||
- [ ] 7.7 Test `enter_room` (walk into room)
|
||||
|
||||
- [ ] 7.8 Test chained objectives (`onComplete.unlockTask`)
|
||||
|
||||
- [ ] 7.9 Test aim completion (all tasks done)
|
||||
|
||||
- [ ] 7.10 Test aim unlock conditions (`unlockCondition.aimCompleted`)
|
||||
|
||||
### Edge Cases
|
||||
- [ ] 7.11 Test server validation rejection
|
||||
|
||||
- [ ] 7.12 Test state persistence (reload page)
|
||||
|
||||
- [ ] 7.13 Test reconciliation (collect items, then load objectives)
|
||||
|
||||
- [ ] 7.14 Test offline mode (no gameId)
|
||||
|
||||
---
|
||||
|
||||
## Phase 8: Documentation
|
||||
_Final documentation updates_
|
||||
|
||||
- [ ] 8.1 Create `docs/OBJECTIVES_USAGE.md`
|
||||
- Schema reference
|
||||
- Task types
|
||||
- Ink tags
|
||||
- Examples
|
||||
|
||||
- [ ] 8.2 Update `README_scenario_design.md` with objectives section
|
||||
|
||||
- [ ] 8.3 Add debug utilities documentation
|
||||
|
||||
- [ ] 8.4 Document ink tags in `docs/INK_BEST_PRACTICES.md`
|
||||
|
||||
---
|
||||
|
||||
## Completion Tracking
|
||||
|
||||
| Phase | Status | Items |
|
||||
|-------|--------|-------|
|
||||
| Phase 0: Prerequisites | ⬜ | 0/3 |
|
||||
| Phase 1: Core Infrastructure | ⬜ | 0/7 |
|
||||
| Phase 2: Event Integration | ⬜ | 0/7 |
|
||||
| Phase 3: UI Implementation | ⬜ | 0/6 |
|
||||
| Phase 4: Integration | ⬜ | 0/5 |
|
||||
| Phase 5: Server Validation | ⬜ | 0/6 |
|
||||
| Phase 6: Ink Tags | ⬜ | 0/5 |
|
||||
| Phase 7: Testing | ⬜ | 0/14 |
|
||||
| Phase 8: Documentation | ⬜ | 0/4 |
|
||||
| **Total** | **⬜** | **0/57** |
|
||||
|
||||
---
|
||||
|
||||
## Key Corrections from Review
|
||||
|
||||
1. ✅ Event `item_unlocked` not `object_unlocked`
|
||||
2. ✅ Initialize in `game.js create()` not `main.js`
|
||||
3. ✅ Include `objectivesState` in scenario bootstrap
|
||||
4. ✅ Add reconciliation for late initialization
|
||||
5. ✅ Emit events for key pickups
|
||||
6. ✅ Use RESTful routes with task_id in path
|
||||
7. ✅ Add responsive CSS breakpoints
|
||||
@@ -0,0 +1,374 @@
|
||||
# Corrected Code Snippets - Review 2
|
||||
|
||||
All code ready to copy-paste into implementation.
|
||||
|
||||
---
|
||||
|
||||
## Critical Fix: Door Unlock Event Emission
|
||||
|
||||
### File: `public/break_escape/js/systems/doors.js`
|
||||
|
||||
**Location**: `unlockDoor()` function (around line 585)
|
||||
|
||||
**Current Code** (MISSING event emission):
|
||||
```javascript
|
||||
function unlockDoor(doorSprite, roomData) {
|
||||
const props = doorSprite.doorProperties;
|
||||
console.log(`Unlocking door: ${props.roomId} -> ${props.connectedRoom}`);
|
||||
|
||||
// Mark door as unlocked
|
||||
props.locked = false;
|
||||
|
||||
// If roomData was provided from server unlock response, cache it
|
||||
if (roomData && window.roomDataCache) {
|
||||
console.log(`📦 Caching room data for ${props.connectedRoom} from unlock response`);
|
||||
window.roomDataCache.set(props.connectedRoom, roomData);
|
||||
}
|
||||
|
||||
// TODO: Implement unlock animation/effect
|
||||
|
||||
// Open the door
|
||||
openDoor(doorSprite);
|
||||
}
|
||||
```
|
||||
|
||||
**CORRECTED Code** (with event emission):
|
||||
```javascript
|
||||
function unlockDoor(doorSprite, roomData) {
|
||||
const props = doorSprite.doorProperties;
|
||||
console.log(`Unlocking door: ${props.roomId} -> ${props.connectedRoom}`);
|
||||
|
||||
// Mark door as unlocked
|
||||
props.locked = false;
|
||||
|
||||
// If roomData was provided from server unlock response, cache it
|
||||
if (roomData && window.roomDataCache) {
|
||||
console.log(`📦 Caching room data for ${props.connectedRoom} from unlock response`);
|
||||
window.roomDataCache.set(props.connectedRoom, roomData);
|
||||
}
|
||||
|
||||
// Emit door unlocked event for objectives system
|
||||
if (window.eventDispatcher) {
|
||||
window.eventDispatcher.emit('door_unlocked', {
|
||||
roomId: props.roomId,
|
||||
connectedRoom: props.connectedRoom,
|
||||
direction: props.direction
|
||||
});
|
||||
console.log(`📋 Emitted door_unlocked event: ${props.roomId} -> ${props.connectedRoom}`);
|
||||
}
|
||||
|
||||
// TODO: Implement unlock animation/effect
|
||||
|
||||
// Open the door
|
||||
openDoor(doorSprite);
|
||||
}
|
||||
```
|
||||
|
||||
**Why This Matters**: Without this event, `unlock_room` type objectives will never auto-complete.
|
||||
|
||||
---
|
||||
|
||||
## Already Documented Fix: Key Pickup Event Emission
|
||||
|
||||
### File: `public/break_escape/js/systems/inventory.js`
|
||||
|
||||
**Location**: `addKeyToInventory()` function (around line 433)
|
||||
|
||||
**Insert AFTER** the line: `window.inventory.keyRing.keys.push(sprite);`
|
||||
|
||||
```javascript
|
||||
// Emit item_picked_up event for keys too (for objectives tracking)
|
||||
// NOTE: Keys currently don't emit events - this is a required fix
|
||||
if (window.eventDispatcher) {
|
||||
window.eventDispatcher.emit(`item_picked_up:${sprite.scenarioData.type}`, {
|
||||
itemType: sprite.scenarioData.type,
|
||||
itemName: sprite.scenarioData.name,
|
||||
roomId: window.currentPlayerRoom,
|
||||
isKey: true
|
||||
});
|
||||
|
||||
// Also emit specific key_id event if available
|
||||
const keyId = sprite.scenarioData?.key_id || sprite.key_id;
|
||||
if (keyId) {
|
||||
window.eventDispatcher.emit(`item_picked_up:key:${keyId}`, {
|
||||
itemType: 'key',
|
||||
keyId: keyId,
|
||||
itemName: sprite.scenarioData.name,
|
||||
roomId: window.currentPlayerRoom
|
||||
});
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ObjectivesManager Event Listener Setup
|
||||
|
||||
### File: `public/break_escape/js/systems/objectives-manager.js`
|
||||
|
||||
**Function**: `setupEventListeners()`
|
||||
|
||||
**CORRECTED version** with proper door event handling:
|
||||
|
||||
```javascript
|
||||
/**
|
||||
* Setup event listeners for automatic objective tracking
|
||||
* NOTE: Event names match actual codebase implementation
|
||||
*/
|
||||
setupEventListeners() {
|
||||
// Item collection - wildcard pattern works with NPCEventDispatcher
|
||||
this.eventDispatcher.on('item_picked_up:*', (data) => {
|
||||
this.handleItemPickup(data);
|
||||
});
|
||||
|
||||
// Room/door unlocks
|
||||
// NOTE: door_unlocked provides both 'roomId' and 'connectedRoom'
|
||||
this.eventDispatcher.on('door_unlocked', (data) => {
|
||||
// Check tasks that match the connectedRoom (the room being unlocked)
|
||||
this.handleRoomUnlock(data.connectedRoom);
|
||||
});
|
||||
|
||||
this.eventDispatcher.on('door_unlocked_by_npc', (data) => {
|
||||
this.handleRoomUnlock(data.roomId);
|
||||
});
|
||||
|
||||
// Object unlocks - NOTE: event is 'item_unlocked' (not 'object_unlocked')
|
||||
this.eventDispatcher.on('item_unlocked', (data) => {
|
||||
// data contains: { itemType, itemName, lockType }
|
||||
this.handleObjectUnlock(data.itemName, data.itemType);
|
||||
});
|
||||
|
||||
// Room entry
|
||||
this.eventDispatcher.on('room_entered', (data) => {
|
||||
this.handleRoomEntered(data.roomId);
|
||||
});
|
||||
|
||||
// NPC conversation completion (via ink tag)
|
||||
this.eventDispatcher.on('task_completed_by_npc', (data) => {
|
||||
this.completeTask(data.taskId);
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
**Key Point**: Use `data.connectedRoom` for door unlocks, as that's the room being unlocked (the target room).
|
||||
|
||||
---
|
||||
|
||||
## Scenario Bootstrap with objectivesState
|
||||
|
||||
### File: `app/controllers/break_escape/games_controller.rb`
|
||||
|
||||
**Method**: `scenario`
|
||||
|
||||
**Current Code**:
|
||||
```ruby
|
||||
def scenario
|
||||
authorize @game if defined?(Pundit)
|
||||
|
||||
begin
|
||||
filtered = @game.filtered_scenario_for_bootstrap
|
||||
filter_requires_recursive(filtered)
|
||||
|
||||
render json: filtered
|
||||
rescue => e
|
||||
Rails.logger.error "[BreakEscape] scenario error: #{e.message}"
|
||||
render_error("Failed to generate scenario: #{e.message}", :internal_server_error)
|
||||
end
|
||||
end
|
||||
```
|
||||
|
||||
**CORRECTED Code**:
|
||||
```ruby
|
||||
def scenario
|
||||
authorize @game if defined?(Pundit)
|
||||
|
||||
begin
|
||||
filtered = @game.filtered_scenario_for_bootstrap
|
||||
filter_requires_recursive(filtered)
|
||||
|
||||
# Include objectives state for restoration on page reload
|
||||
if @game.player_state['objectives'].present?
|
||||
filtered['objectivesState'] = @game.player_state['objectives']
|
||||
end
|
||||
|
||||
render json: filtered
|
||||
rescue => e
|
||||
Rails.logger.error "[BreakEscape] scenario error: #{e.message}"
|
||||
render_error("Failed to generate scenario: #{e.message}", :internal_server_error)
|
||||
end
|
||||
end
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Game.js Integration
|
||||
|
||||
### File: `public/break_escape/js/core/game.js`
|
||||
|
||||
**Location**: In `create()` function, AFTER loading scenario and global variables
|
||||
|
||||
**Insert AFTER** this block:
|
||||
```javascript
|
||||
// Initialize global narrative variables from scenario
|
||||
if (gameScenario.globalVariables) {
|
||||
window.gameState.globalVariables = { ...gameScenario.globalVariables };
|
||||
console.log('🌐 Initialized global variables:', window.gameState.globalVariables);
|
||||
} else {
|
||||
window.gameState.globalVariables = {};
|
||||
}
|
||||
```
|
||||
|
||||
**Add this**:
|
||||
```javascript
|
||||
// Restore objectives state from server if available (passed via objectivesState)
|
||||
if (gameScenario.objectivesState) {
|
||||
window.gameState.objectives = gameScenario.objectivesState;
|
||||
console.log('📋 Restored objectives state from server');
|
||||
}
|
||||
|
||||
// Initialize objectives system AFTER scenario is loaded
|
||||
// This must happen in create() because gameScenario isn't available until now
|
||||
if (gameScenario.objectives && window.objectivesManager) {
|
||||
console.log('📋 Initializing objectives from scenario');
|
||||
window.objectivesManager.initialize(gameScenario.objectives);
|
||||
|
||||
// Create UI panel
|
||||
const ObjectivesPanel = await import('../ui/objectives-panel.js');
|
||||
window.objectivesPanel = new ObjectivesPanel.default(window.objectivesManager);
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Main.js Integration
|
||||
|
||||
### File: `public/break_escape/js/main.js`
|
||||
|
||||
**Location**: In `initializeGame()` function, AFTER NPC systems initialization
|
||||
|
||||
**Insert AFTER** this block:
|
||||
```javascript
|
||||
// Initialize NPC systems
|
||||
console.log('🎭 Initializing NPC systems...');
|
||||
window.eventDispatcher = new NPCEventDispatcher();
|
||||
window.barkSystem = new NPCBarkSystem();
|
||||
window.npcManager = new NPCManager(window.eventDispatcher, window.barkSystem);
|
||||
window.npcLazyLoader = new NPCLazyLoader(window.npcManager);
|
||||
console.log('✅ NPC lazy loader initialized');
|
||||
|
||||
// Start timed message system
|
||||
window.npcManager.startTimedMessages();
|
||||
|
||||
console.log('✅ NPC systems initialized');
|
||||
```
|
||||
|
||||
**Add this**:
|
||||
```javascript
|
||||
// Initialize Objectives System (manager only - data comes later in game.js)
|
||||
console.log('📋 Initializing objectives manager...');
|
||||
const { getObjectivesManager } = await import('./systems/objectives-manager.js');
|
||||
window.objectivesManager = getObjectivesManager(window.eventDispatcher);
|
||||
console.log('✅ Objectives manager initialized (awaiting scenario data)');
|
||||
```
|
||||
|
||||
**NOTE**: This uses dynamic import to avoid circular dependencies. The manager is created, but `initialize(data)` is called later in `game.js` once scenario is loaded.
|
||||
|
||||
---
|
||||
|
||||
## Ink Tag Processing
|
||||
|
||||
### File: `public/break_escape/js/minigames/helpers/chat-helpers.js`
|
||||
|
||||
**Function**: `processGameActionTags()`
|
||||
|
||||
**Add these cases to the switch statement**:
|
||||
|
||||
```javascript
|
||||
case 'complete_task':
|
||||
if (param) {
|
||||
const taskId = param;
|
||||
// Emit event for ObjectivesManager to handle
|
||||
if (window.eventDispatcher) {
|
||||
window.eventDispatcher.emit('task_completed_by_npc', { taskId });
|
||||
}
|
||||
result.success = true;
|
||||
result.message = `📋 Task completed: ${taskId}`;
|
||||
console.log('📋 Task completion tag:', taskId);
|
||||
} else {
|
||||
result.message = '⚠️ complete_task tag missing task ID';
|
||||
console.warn(result.message);
|
||||
}
|
||||
break;
|
||||
|
||||
case 'unlock_task':
|
||||
if (param) {
|
||||
const taskId = param;
|
||||
if (window.objectivesManager) {
|
||||
window.objectivesManager.unlockTask(taskId);
|
||||
}
|
||||
result.success = true;
|
||||
result.message = `🔓 Task unlocked: ${taskId}`;
|
||||
} else {
|
||||
result.message = '⚠️ unlock_task tag missing task ID';
|
||||
console.warn(result.message);
|
||||
}
|
||||
break;
|
||||
|
||||
case 'unlock_aim':
|
||||
if (param) {
|
||||
const aimId = param;
|
||||
if (window.objectivesManager) {
|
||||
window.objectivesManager.unlockAim(aimId);
|
||||
}
|
||||
result.success = true;
|
||||
result.message = `🔓 Aim unlocked: ${aimId}`;
|
||||
} else {
|
||||
result.message = '⚠️ unlock_aim tag missing aim ID';
|
||||
console.warn(result.message);
|
||||
}
|
||||
break;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Complete handleRoomUnlock Implementation
|
||||
|
||||
### File: `public/break_escape/js/systems/objectives-manager.js`
|
||||
|
||||
```javascript
|
||||
/**
|
||||
* Handle room unlock - check unlock_room tasks
|
||||
* Called when door_unlocked event fires
|
||||
*/
|
||||
handleRoomUnlock(roomId) {
|
||||
Object.values(this.taskIndex).forEach(task => {
|
||||
if (task.type !== 'unlock_room') return;
|
||||
if (task.status !== 'active') return;
|
||||
if (task.targetRoom !== roomId) return;
|
||||
|
||||
console.log(`📋 Room unlock detected: ${roomId} matches task ${task.taskId}`);
|
||||
this.completeTask(task.taskId);
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Summary of Required Changes
|
||||
|
||||
### Phase 0 Prerequisites (MUST DO FIRST)
|
||||
|
||||
1. ✅ **Add door unlock events** - `doors.js` `unlockDoor()` function
|
||||
2. ✅ **Add key pickup events** - `inventory.js` `addKeyToInventory()` function
|
||||
3. ⚠️ **Verify room_entered events** - Check if already emitted in `rooms.js`
|
||||
|
||||
### Integration Changes
|
||||
|
||||
4. ✅ **Update scenario controller** - Add `objectivesState` to bootstrap
|
||||
5. ✅ **Update game.js create()** - Initialize objectives after scenario loads
|
||||
6. ✅ **Update main.js** - Create manager instance early
|
||||
7. ✅ **Update chat-helpers.js** - Add ink tag handlers
|
||||
|
||||
### All Code Above is Production-Ready
|
||||
|
||||
Every snippet above can be copy-pasted directly into the codebase. No placeholders, no pseudocode.
|
||||
394
planning_notes/objectives_system/review2/FINAL_REVIEW.md
Normal file
394
planning_notes/objectives_system/review2/FINAL_REVIEW.md
Normal file
@@ -0,0 +1,394 @@
|
||||
# Final Implementation Plan Review (Review 2)
|
||||
|
||||
**Date**: November 25, 2025
|
||||
**Reviewer**: AI Assistant
|
||||
**Plan Version**: 1.1
|
||||
**Status**: ✅ APPROVED with minor corrections
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The implementation plan has been thoroughly reviewed against the current codebase. The plan is **well-structured and technically sound**, with all critical event names and initialization flows correctly documented. However, **one critical issue was discovered**: doors do NOT emit `door_unlocked` events in the current codebase.
|
||||
|
||||
### Critical Finding
|
||||
|
||||
**❌ MISSING: Door unlock events are NOT emitted**
|
||||
|
||||
The current codebase in `doors.js` does NOT emit any events when doors are unlocked. This will prevent `unlock_room` objectives from being tracked automatically.
|
||||
|
||||
### Recommendation
|
||||
|
||||
**REQUIRED FIX**: Add event emission to `doors.js` in the `unlockDoor()` function.
|
||||
|
||||
---
|
||||
|
||||
## Detailed Findings
|
||||
|
||||
### ✅ CORRECT: Event Names Verified
|
||||
|
||||
| Event Name | Location | Status |
|
||||
|------------|----------|--------|
|
||||
| `item_picked_up:*` | `inventory.js:369-374` | ✅ Correct (wildcard works) |
|
||||
| `item_unlocked` | `unlock-system.js:587, 621` | ✅ Correct (NOT object_unlocked) |
|
||||
| `room_entered` | `rooms.js` (via updatePlayerRoom) | ✅ Correct |
|
||||
| `door_unlocked` | **MISSING** | ❌ NOT EMITTED |
|
||||
|
||||
### ❌ CRITICAL: Door Unlock Events Missing
|
||||
|
||||
**File**: `public/break_escape/js/systems/doors.js`
|
||||
**Function**: `unlockDoor()` (lines ~585-600)
|
||||
|
||||
**Current Code** (No event emission):
|
||||
```javascript
|
||||
function unlockDoor(doorSprite, roomData) {
|
||||
const props = doorSprite.doorProperties;
|
||||
console.log(`Unlocking door: ${props.roomId} -> ${props.connectedRoom}`);
|
||||
|
||||
// Mark door as unlocked
|
||||
props.locked = false;
|
||||
|
||||
// If roomData was provided from server unlock response, cache it
|
||||
if (roomData && window.roomDataCache) {
|
||||
console.log(`📦 Caching room data for ${props.connectedRoom} from unlock response`);
|
||||
window.roomDataCache.set(props.connectedRoom, roomData);
|
||||
}
|
||||
|
||||
// TODO: Implement unlock animation/effect
|
||||
|
||||
// Open the door
|
||||
openDoor(doorSprite);
|
||||
}
|
||||
```
|
||||
|
||||
**Required Fix**:
|
||||
```javascript
|
||||
function unlockDoor(doorSprite, roomData) {
|
||||
const props = doorSprite.doorProperties;
|
||||
console.log(`Unlocking door: ${props.roomId} -> ${props.connectedRoom}`);
|
||||
|
||||
// Mark door as unlocked
|
||||
props.locked = false;
|
||||
|
||||
// If roomData was provided from server unlock response, cache it
|
||||
if (roomData && window.roomDataCache) {
|
||||
console.log(`📦 Caching room data for ${props.connectedRoom} from unlock response`);
|
||||
window.roomDataCache.set(props.connectedRoom, roomData);
|
||||
}
|
||||
|
||||
// Emit door unlocked event for objectives system
|
||||
if (window.eventDispatcher) {
|
||||
window.eventDispatcher.emit('door_unlocked', {
|
||||
roomId: props.roomId,
|
||||
connectedRoom: props.connectedRoom,
|
||||
direction: props.direction
|
||||
});
|
||||
console.log(`📋 Emitted door_unlocked event: ${props.roomId} -> ${props.connectedRoom}`);
|
||||
}
|
||||
|
||||
// TODO: Implement unlock animation/effect
|
||||
|
||||
// Open the door
|
||||
openDoor(doorSprite);
|
||||
}
|
||||
```
|
||||
|
||||
**Impact**: Without this fix, `unlock_room` type objectives will NOT auto-complete when players unlock doors.
|
||||
|
||||
### ✅ CORRECT: Key Items Do NOT Emit Events
|
||||
|
||||
**File**: `public/break_escape/js/systems/inventory.js`
|
||||
**Function**: `addKeyToInventory()` (lines 410-465)
|
||||
|
||||
**Finding**: Keys do NOT emit `item_picked_up:*` events. The plan correctly identifies this as a required fix in Phase 0.
|
||||
|
||||
**Verification**: Reviewed code confirms no event emission in `addKeyToInventory()`.
|
||||
|
||||
### ✅ CORRECT: Initialization Flow
|
||||
|
||||
**Plan States**: "ObjectivesManager created in `main.js`, but data loaded in `game.js create()` after scenario JSON available"
|
||||
|
||||
**Verification**:
|
||||
- ✅ `main.js` initializes NPC systems including `window.eventDispatcher` (lines 82-93)
|
||||
- ✅ `game.js create()` loads scenario at line 477: `window.gameScenario = this.cache.json.get('gameScenarioJSON')`
|
||||
- ✅ Plan correctly places objectives initialization AFTER scenario load
|
||||
|
||||
**Conclusion**: Initialization flow is architecturally correct.
|
||||
|
||||
### ✅ CORRECT: NPCEventDispatcher Wildcard Support
|
||||
|
||||
**File**: `public/break_escape/js/systems/npc-events.js` (lines 25-34)
|
||||
|
||||
**Code**:
|
||||
```javascript
|
||||
emit(eventType, data) {
|
||||
// exact-match listeners
|
||||
const exact = this.listeners.get(eventType) || [];
|
||||
for (const fn of exact) try { fn(data); } catch (e) { console.error(e); }
|
||||
|
||||
// wildcard-style listeners where eventType is a prefix (e.g. 'npc:')
|
||||
for (const [key, arr] of this.listeners.entries()) {
|
||||
if (key.endsWith('*')) {
|
||||
const prefix = key.slice(0, -1);
|
||||
if (eventType.startsWith(prefix)) for (const fn of arr) try { fn(data); } catch (e) { console.error(e); }
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Conclusion**: Wildcard pattern `item_picked_up:*` will work correctly.
|
||||
|
||||
### ⚠️ MINOR: door_unlocked Event Data Structure
|
||||
|
||||
**Plan States**: "door_unlocked event provides `connectedRoom` property (not `roomId`)"
|
||||
|
||||
**Current Code Analysis**: The `unlockDoor()` function has access to:
|
||||
- `props.roomId` - The current room containing the door
|
||||
- `props.connectedRoom` - The room the door leads to
|
||||
- `props.direction` - The direction of the door
|
||||
|
||||
**Issue**: The plan's example code shows listening for `data.connectedRoom`, but we should provide BOTH `roomId` and `connectedRoom` for maximum flexibility.
|
||||
|
||||
**Recommendation**: Update the proposed event emission to include both properties (already shown in the fix above).
|
||||
|
||||
### ✅ CORRECT: Server-Side Validation Pattern
|
||||
|
||||
**File**: `app/models/break_escape/game.rb`
|
||||
**Method**: `validate_unlock()` (lines ~183-245)
|
||||
|
||||
**Finding**: Server already validates:
|
||||
- Key-based unlocks via `has_key_in_inventory?`
|
||||
- Lockpick-based unlocks via `has_lockpick_in_inventory?`
|
||||
- PIN/password validation
|
||||
- NPC unlock permissions
|
||||
|
||||
**Conclusion**: The plan's server validation approach aligns with existing patterns.
|
||||
|
||||
### ✅ CORRECT: Scenario Bootstrap Pattern
|
||||
|
||||
**File**: `app/controllers/break_escape/games_controller.rb`
|
||||
**Method**: `scenario()` (lines 15-28)
|
||||
|
||||
**Current Code**:
|
||||
```ruby
|
||||
def scenario
|
||||
authorize @game if defined?(Pundit)
|
||||
|
||||
begin
|
||||
filtered = @game.filtered_scenario_for_bootstrap
|
||||
filter_requires_recursive(filtered)
|
||||
|
||||
render json: filtered
|
||||
rescue => e
|
||||
Rails.logger.error "[BreakEscape] scenario error: #{e.message}"
|
||||
render_error("Failed to generate scenario: #{e.message}", :internal_server_error)
|
||||
end
|
||||
end
|
||||
```
|
||||
|
||||
**Plan Proposes**: Adding `objectivesState` to the response for state restoration.
|
||||
|
||||
**Conclusion**: This approach is consistent with the existing pattern of including state in the scenario bootstrap.
|
||||
|
||||
### ✅ CORRECT: RESTful Routes Pattern
|
||||
|
||||
**Plan Proposes**:
|
||||
```ruby
|
||||
post 'objectives/tasks/:task_id', to: 'games#complete_task'
|
||||
put 'objectives/tasks/:task_id', to: 'games#update_task_progress'
|
||||
```
|
||||
|
||||
**Existing Pattern** (from routes analysis):
|
||||
- Resources use member routes with semantic actions
|
||||
- Task ID in path is clean and RESTful
|
||||
|
||||
**Conclusion**: Proposed routes follow Rails conventions.
|
||||
|
||||
### ✅ CORRECT: Ink Tag Processing Extension Point
|
||||
|
||||
**File**: `public/break_escape/js/minigames/helpers/chat-helpers.js`
|
||||
**Function**: `processGameActionTags()` (lines 13-436)
|
||||
|
||||
**Current Tags Supported**:
|
||||
- `unlock_door`
|
||||
- `give_item`
|
||||
- `give_npc_inventory_items`
|
||||
- `set_objective`
|
||||
|
||||
**Plan Proposes Adding**:
|
||||
- `complete_task`
|
||||
- `unlock_task`
|
||||
- `unlock_aim`
|
||||
|
||||
**Conclusion**: The switch statement pattern is already established and ready for extension.
|
||||
|
||||
---
|
||||
|
||||
## Plan Corrections Required
|
||||
|
||||
### 1. Add Phase 0 Task: Emit Door Unlock Events
|
||||
|
||||
**File**: `TODO_CHECKLIST.md`
|
||||
|
||||
Add to Phase 0:
|
||||
- [ ] 0.4 Add `door_unlocked` event emission to `doors.js` `unlockDoor()` function
|
||||
|
||||
### 2. Update Implementation Plan: Door Unlock Event
|
||||
|
||||
**File**: `IMPLEMENTATION_PLAN.md`
|
||||
|
||||
In the "6. Integration Points" or "Key Item Event Fix" section, add:
|
||||
|
||||
#### Door Unlock Event Fix
|
||||
|
||||
**File:** Update `public/break_escape/js/systems/doors.js`
|
||||
|
||||
In the `unlockDoor()` function, add event emission after marking door as unlocked:
|
||||
|
||||
```javascript
|
||||
function unlockDoor(doorSprite, roomData) {
|
||||
const props = doorSprite.doorProperties;
|
||||
console.log(`Unlocking door: ${props.roomId} -> ${props.connectedRoom}`);
|
||||
|
||||
// Mark door as unlocked
|
||||
props.locked = false;
|
||||
|
||||
// If roomData was provided from server unlock response, cache it
|
||||
if (roomData && window.roomDataCache) {
|
||||
console.log(`📦 Caching room data for ${props.connectedRoom} from unlock response`);
|
||||
window.roomDataCache.set(props.connectedRoom, roomData);
|
||||
}
|
||||
|
||||
// Emit door unlocked event for objectives system
|
||||
if (window.eventDispatcher) {
|
||||
window.eventDispatcher.emit('door_unlocked', {
|
||||
roomId: props.roomId,
|
||||
connectedRoom: props.connectedRoom,
|
||||
direction: props.direction
|
||||
});
|
||||
console.log(`📋 Emitted door_unlocked event: ${props.roomId} -> ${props.connectedRoom}`);
|
||||
}
|
||||
|
||||
// Open the door
|
||||
openDoor(doorSprite);
|
||||
}
|
||||
```
|
||||
|
||||
### 3. Update Quick Reference: Door Event Details
|
||||
|
||||
**File**: `QUICK_REFERENCE.md`
|
||||
|
||||
Update the "Events Listened To" section:
|
||||
|
||||
```javascript
|
||||
'door_unlocked' // For unlock_room
|
||||
// data: { roomId, connectedRoom, direction }
|
||||
// NOTE: Must add event emission to doors.js
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Implementation Plan Quality Assessment
|
||||
|
||||
### Strengths ✅
|
||||
|
||||
1. **Comprehensive Event Mapping**: All event names correctly researched and documented
|
||||
2. **Proper Initialization Flow**: Correctly identifies that scenario data isn't available until `game.js create()`
|
||||
3. **Reconciliation Strategy**: Includes `reconcileWithGameState()` to handle late initialization
|
||||
4. **Server Validation**: Follows existing validation patterns in the codebase
|
||||
5. **Complete Examples**: Includes three full scenario examples demonstrating all features
|
||||
6. **Debug Utilities**: Provides helpful debug functions for testing
|
||||
7. **RESTful API Design**: Proposes clean, conventional routes
|
||||
8. **Phase 0 Prerequisites**: Correctly identifies foundational fixes needed first
|
||||
|
||||
### Weaknesses ⚠️
|
||||
|
||||
1. **Missing Door Event Emission**: Plan assumes `door_unlocked` events exist, but they must be added
|
||||
2. **Minor Documentation Gap**: Could clarify that `door_unlocked` event provides multiple properties
|
||||
|
||||
### Risk Assessment
|
||||
|
||||
**Overall Risk**: 🟡 LOW-MEDIUM
|
||||
|
||||
- **Technical Risk**: LOW - Architecture is sound, event system is proven
|
||||
- **Integration Risk**: LOW - Follows existing patterns consistently
|
||||
- **Implementation Risk**: MEDIUM - Requires adding door events (not just consuming them)
|
||||
|
||||
---
|
||||
|
||||
## Recommendations
|
||||
|
||||
### Must-Have (Before Starting Implementation)
|
||||
|
||||
1. ✅ **Add door unlock events** to `doors.js` as documented above
|
||||
2. ✅ **Add key pickup events** to `inventory.js` as already documented in plan
|
||||
3. ✅ **Verify room_entered events** are actually emitted in current codebase
|
||||
|
||||
### Should-Have (During Implementation)
|
||||
|
||||
4. 📋 **Add integration tests** for event emissions after implementing
|
||||
5. 📋 **Add console warnings** if ObjectivesManager initialized before scenario loaded
|
||||
6. 📋 **Add validation** to reject objectives with invalid task types
|
||||
|
||||
### Nice-to-Have (Future Enhancement)
|
||||
|
||||
7. 💡 **Add event emission tracing** in debug mode to track objective updates
|
||||
8. 💡 **Add TypeScript definitions** for objective data structures
|
||||
9. 💡 **Add visual indicators** on doors/objects that are part of active objectives
|
||||
|
||||
---
|
||||
|
||||
## Verification Checklist
|
||||
|
||||
Before implementation starts:
|
||||
|
||||
- [x] Event names verified against actual codebase
|
||||
- [x] Initialization order verified against game lifecycle
|
||||
- [x] Server validation patterns reviewed and aligned
|
||||
- [x] API routes follow Rails conventions
|
||||
- [x] Ink tag extension point confirmed
|
||||
- [x] Wildcard event support confirmed
|
||||
- [ ] Door unlock events added (MUST DO FIRST)
|
||||
- [ ] Key pickup events added (MUST DO FIRST)
|
||||
- [ ] Room entered events verified (SHOULD VERIFY)
|
||||
|
||||
---
|
||||
|
||||
## Conclusion
|
||||
|
||||
The implementation plan is **well-researched and architecturally sound**. The discovery of missing door unlock events does not invalidate the plan—it simply adds one more prerequisite task to Phase 0.
|
||||
|
||||
**Approval**: ✅ **APPROVED FOR IMPLEMENTATION** after adding door unlock event emission.
|
||||
|
||||
**Confidence Level**: 90% - The plan will work as documented once the door event emission is added.
|
||||
|
||||
---
|
||||
|
||||
## Files to Update
|
||||
|
||||
### Planning Documents
|
||||
|
||||
1. `TODO_CHECKLIST.md` - Add task 0.4 for door events
|
||||
2. `IMPLEMENTATION_PLAN.md` - Add door unlock event fix section
|
||||
3. `QUICK_REFERENCE.md` - Add note about door event emission requirement
|
||||
|
||||
### Codebase (Prerequisites)
|
||||
|
||||
4. `public/break_escape/js/systems/doors.js` - Add event emission to `unlockDoor()`
|
||||
5. `public/break_escape/js/systems/inventory.js` - Add event emission to `addKeyToInventory()`
|
||||
|
||||
### Implementation Files (From Plan)
|
||||
|
||||
6. `db/migrate/XXX_add_objectives_to_games.rb`
|
||||
7. `app/models/break_escape/game.rb`
|
||||
8. `app/controllers/break_escape/games_controller.rb`
|
||||
9. `config/routes.rb`
|
||||
10. `public/break_escape/js/systems/objectives-manager.js`
|
||||
11. `public/break_escape/js/ui/objectives-panel.js`
|
||||
12. `public/break_escape/css/objectives.css`
|
||||
13. `public/break_escape/js/minigames/helpers/chat-helpers.js`
|
||||
14. `scenarios/test-objectives/`
|
||||
|
||||
---
|
||||
|
||||
**Next Steps**: Apply corrections to planning documents, then begin implementation with Phase 0 prerequisites.
|
||||
Reference in New Issue
Block a user