|
| 1 | +# Combat Code Issues - Priority List |
| 2 | + |
| 3 | +Generated: 2025-01-27 |
| 4 | + |
| 5 | +## 🎯 **Recently Fixed Issues** |
| 6 | +- ✅ Move queuing after killing all enemies (combat state validation) |
| 7 | +- ✅ Duplicate queue cleanup logic (centralized cleanup function) |
| 8 | +- ✅ Queue cleanup not removing actions targeting dead entities |
| 9 | +- ✅ **Race Condition in Action Queue Processing** - Refactored to two-phase processing |
| 10 | +- ✅ **No Validation of Action Targets During Execution** - Added target validation checks |
| 11 | + |
| 12 | +## 🔍 **Outstanding Issues by Severity** |
| 13 | + |
| 14 | +### 🚨 **HIGH SEVERITY** |
| 15 | + |
| 16 | +*All high severity issues have been resolved! 🎉* |
| 17 | + |
| 18 | +### 🟡 **MEDIUM SEVERITY** |
| 19 | + |
| 20 | +#### 3. **Inconsistent Combat State Management** ⚡ *Medium Effort* ✅ **RESOLVED** |
| 21 | +- **Location**: Multiple files - combat state checked in various places |
| 22 | +- **Issue**: Combat state is checked/set in multiple places with different conditions |
| 23 | +- **Impact**: Possible states where combat UI shows but combat logic isn't active |
| 24 | +- **Fix**: ✅ Created centralized `CombatStateManager` system in `/home/melty/Git/LithicRivers/crates/core/src/combat_state_manager.rs` |
| 25 | +- **Code Changes**: ✅ Centralized all combat state transitions, replaced old `combat_trigger_system` usage in system_scheduler.rs |
| 26 | + |
| 27 | +#### 4. **Performance: O(n²) Queue Cleanup** ⚡ *Medium Effort* ✅ **RESOLVED** |
| 28 | +- **Location**: `cleanup_actions_targeting_dead_entity()` in `/home/melty/Git/LithicRivers/crates/core/src/systems.rs:1085` |
| 29 | +- **Issue**: Every entity death iterates through ALL entities with queues |
| 30 | +- **Impact**: Performance degrades with many entities in large battles |
| 31 | +- **Fix**: ✅ Implemented `TargetTracker` resource with reverse lookup `HashMap<Entity, HashSet<Entity>>` |
| 32 | +- **Code Changes**: ✅ Created `/home/melty/Git/LithicRivers/crates/core/src/target_tracker.rs`, optimized cleanup function reduces from O(n²) to O(1) + O(k) where k = entities targeting dead one |
| 33 | + |
| 34 | +#### 5. **Memory Leak: Dead Entities with Components** ⚡ *Low Effort* |
| 35 | +- **Location**: Entity cleanup throughout codebase |
| 36 | +- **Issue**: Dead entities keep their ActionQueue and other components until manually cleaned |
| 37 | +- **Impact**: Memory usage grows during long combat sessions |
| 38 | +- **Fix**: Add periodic cleanup system or immediate component removal on death |
| 39 | +- **Code Changes**: Extend cleanup functions to remove all unnecessary components |
| 40 | + |
| 41 | +#### 6. **No Action Priority System** ⚡ *High Effort* |
| 42 | +- **Location**: `ActionQueue` system in `/home/melty/Git/LithicRivers/crates/core/src/moves.rs:278` |
| 43 | +- **Issue**: All actions execute in timing order only, no tactical priority |
| 44 | +- **Impact**: Unrealistic combat where healing can't interrupt attacks |
| 45 | +- **Fix**: Add priority levels to actions and priority-based queue sorting |
| 46 | +- **Code Changes**: Major refactor of action queue to support priority levels |
| 47 | + |
| 48 | +### 🟢 **LOW SEVERITY** |
| 49 | + |
| 50 | +#### 7. **UI Shows Entity IDs Instead of Names** ⚡ *Low Effort* |
| 51 | +- **Location**: Combat UI in `/home/melty/Git/LithicRivers/crates/client/src/ui/panels/combat.rs:222` |
| 52 | +- **Issue**: Target display shows "Enemy 42" instead of "Goblin" or similar |
| 53 | +- **Impact**: Poor user experience, harder to track specific enemies |
| 54 | +- **Fix**: Add name component lookup in UI target display logic |
| 55 | +- **Code Changes**: Add Name component and lookup in UI rendering |
| 56 | + |
| 57 | +#### 8. **No Action Animation/Feedback Delay** ⚡ *Medium Effort* |
| 58 | +- **Location**: Action execution system |
| 59 | +- **Issue**: Actions complete instantly with no visual feedback period |
| 60 | +- **Impact**: Combat feels disconnected, hard to follow action flow |
| 61 | +- **Fix**: Add animation/feedback phase to action execution |
| 62 | +- **Code Changes**: Add animation states and timing to action system |
| 63 | + |
| 64 | +#### 9. **Hard-coded Move Validation** ⚡ *Medium Effort* |
| 65 | +- **Location**: Input handler `/home/melty/Git/LithicRivers/crates/client/src/app/input_handler.rs:677` |
| 66 | +- **Issue**: Move #3 is hard-coded as "Escape" in input handler |
| 67 | +- **Impact**: Fragile code, breaks if move list changes |
| 68 | +- **Fix**: Add move metadata for target requirements |
| 69 | +- **Code Changes**: Add `requires_target` field to Move struct |
| 70 | + |
| 71 | +#### 10. **Limited Error Recovery** ⚡ *Low Effort* |
| 72 | +- **Location**: Various action execution paths |
| 73 | +- **Issue**: Failed actions don't provide recovery mechanisms |
| 74 | +- **Impact**: Players can get stuck in invalid states |
| 75 | +- **Fix**: Add action validation and error state recovery |
| 76 | +- **Code Changes**: Add validation checks and fallback behaviors |
| 77 | + |
| 78 | +## 📊 **Summary Statistics** |
| 79 | + |
| 80 | +### By Severity: |
| 81 | +- **High**: 0 issues ✅ (All resolved!) |
| 82 | +- **Medium**: 2 issues (performance/UX problems) - 2 resolved ✅ |
| 83 | +- **Low**: 4 issues (polish/maintainability) |
| 84 | + |
| 85 | +### By Effort: |
| 86 | +- **Low Effort**: 4 issues (quick fixes) |
| 87 | +- **Medium Effort**: 5 issues (moderate refactoring) |
| 88 | +- **High Effort**: 1 issue (major feature addition) |
| 89 | + |
| 90 | +## 🎯 **Recommended Fix Order** |
| 91 | + |
| 92 | +### Phase 1 - Critical Fixes (High Impact, Lower Effort) |
| 93 | +1. **Action target validation** (High Severity, Low Effort) - Prevents crashes |
| 94 | +2. **Memory leak cleanup** (Medium Severity, Low Effort) - Performance improvement |
| 95 | +3. **UI entity names** (Low Severity, Low Effort) - Quick UX win |
| 96 | + |
| 97 | +### Phase 2 - Performance & Stability (Medium Effort) |
| 98 | +4. **Race condition fix** (High Severity, Medium Effort) - System stability |
| 99 | +5. **Queue cleanup performance** (Medium Severity, Medium Effort) - Scalability |
| 100 | +6. **Combat state centralization** (Medium Severity, Medium Effort) - Maintainability |
| 101 | + |
| 102 | +### Phase 3 - Polish & Features (Higher Effort) |
| 103 | +7. **Hard-coded move validation** (Low Severity, Medium Effort) - Code quality |
| 104 | +8. **Animation system** (Low Severity, Medium Effort) - UX improvement |
| 105 | +9. **Error recovery** (Low Severity, Low Effort) - Robustness |
| 106 | +10. **Priority system** (Medium Severity, High Effort) - Major feature |
| 107 | + |
| 108 | +## 🔧 **Technical Notes** |
| 109 | + |
| 110 | +- All high severity issues should be addressed before major feature additions |
| 111 | +- The race condition fix (#1) is the most critical as it affects system stability |
| 112 | +- Performance issues (#4) will become more apparent with larger battles |
| 113 | +- Priority system (#6) would be a good candidate for a major refactor milestone |
0 commit comments