574 lines
16 KiB
Markdown
574 lines
16 KiB
Markdown
# YAZE Code Review: Critical Next Steps for Release
|
|
|
|
**Date**: January 31, 2025
|
|
**Version**: 0.3.2 (Pre-Release)
|
|
**Status**: Comprehensive Code Review Complete
|
|
|
|
---
|
|
|
|
## Executive Summary
|
|
|
|
YAZE is in a strong position for release with **90% feature parity** achieved on the develop branch and significant architectural improvements. However, several **critical issues** and **stability concerns** must be addressed before a stable release can be achieved.
|
|
|
|
### Key Metrics
|
|
- **Feature Parity**: 90% (develop branch) vs master
|
|
- **Code Quality**: 44% reduction in EditorManager code (3710 → 2076 lines)
|
|
- **Build Status**: ✅ Compiles successfully on all platforms
|
|
- **Test Coverage**: 46+ core tests, E2E framework in place
|
|
- **Known Critical Bugs**: 6 high-priority issues
|
|
- **Stability Risks**: 3 major areas requiring attention
|
|
|
|
---
|
|
|
|
## 🔴 CRITICAL: Must Fix Before Release
|
|
|
|
### 1. Tile16 Editor Palette System Issues (Priority: HIGH)
|
|
|
|
**Status**: Partially fixed, critical bugs remain
|
|
|
|
**Active Issues**:
|
|
1. **Tile8 Source Canvas Palette Issues** - Source tiles show incorrect colors
|
|
2. **Palette Button Functionality** - Buttons 0-7 don't update palettes correctly
|
|
3. **Color Alignment Between Canvases** - Inconsistent colors across canvases
|
|
|
|
**Impact**: Blocks proper tile editing workflow, users cannot preview tiles accurately
|
|
|
|
**Root Cause**: Area graphics not receiving proper palette application, palette switching logic incomplete
|
|
|
|
**Files**:
|
|
- `src/app/editor/graphics/tile16_editor.cc`
|
|
- `docs/F2-tile16-editor-palette-system.md`
|
|
|
|
**Effort**: 4-6 hours
|
|
**Risk**: Medium - Core editing functionality affected
|
|
|
|
---
|
|
|
|
### 2. Overworld Sprite Movement Bug (Priority: HIGH)
|
|
|
|
**Status**: Active bug, blocking sprite editing
|
|
|
|
**Issue**: Sprites are not responding to drag operations on overworld canvas
|
|
|
|
**Impact**: Blocks sprite editing workflow completely
|
|
|
|
**Location**: Overworld canvas interaction system
|
|
|
|
**Files**:
|
|
- `src/app/editor/overworld/overworld_map.cc`
|
|
- `src/app/editor/overworld/overworld_editor.cc`
|
|
|
|
**Effort**: 2-4 hours
|
|
**Risk**: High - Core feature broken
|
|
|
|
---
|
|
|
|
### 3. Canvas Multi-Select Intersection Drawing Bug (Priority: MEDIUM)
|
|
|
|
**Status**: Known bug with E2E test coverage
|
|
|
|
**Issue**: Selection box rendering incorrect when crossing 512px boundaries
|
|
|
|
**Impact**: Selection tool unreliable for large maps
|
|
|
|
**Location**: Canvas selection system
|
|
|
|
**Test Coverage**: E2E test exists (`canvas_selection_test`)
|
|
|
|
**Files**:
|
|
- `src/app/gfx/canvas/canvas.cc`
|
|
- `test/e2e/canvas_selection_e2e_tests.cc`
|
|
|
|
**Effort**: 3-5 hours
|
|
**Risk**: Medium - Workflow impact
|
|
|
|
---
|
|
|
|
### 4. Emulator Audio System (Priority: CRITICAL)
|
|
|
|
**Status**: Audio output broken, investigation needed
|
|
|
|
**Issue**: SDL2 audio device initialized but no sound plays
|
|
|
|
**Root Cause**: Multiple potential issues:
|
|
- Audio buffer size mismatch (fixed in recent changes)
|
|
- Format conversion problems (SPC700 → SDL2)
|
|
- Device paused state
|
|
- APU timing issues (handshake problems identified)
|
|
|
|
**Impact**: Core emulator feature non-functional
|
|
|
|
**Files**:
|
|
- `src/app/emu/emulator.cc`
|
|
- `src/app/platform/window.cc`
|
|
- `src/app/emu/audio/` (IAudioBackend)
|
|
- `docs/E8-emulator-debugging-vision.md`
|
|
|
|
**Effort**: 4-6 hours (investigation + fix)
|
|
**Risk**: High - Core feature broken
|
|
|
|
**Documentation**: Comprehensive debugging guide in `E8-emulator-debugging-vision.md`
|
|
|
|
---
|
|
|
|
### 5. Right-Click Context Menu Tile16 Display Bug (Priority: LOW)
|
|
|
|
**Status**: Intermittent bug
|
|
|
|
**Issue**: Context menu displays abnormally large tile16 preview randomly
|
|
|
|
**Impact**: UI polish issue, doesn't block functionality
|
|
|
|
**Location**: Right-click context menu
|
|
|
|
**Effort**: 2-3 hours
|
|
**Risk**: Low - Cosmetic issue
|
|
|
|
---
|
|
|
|
### 6. Overworld Map Properties Panel Popup (Priority: MEDIUM)
|
|
|
|
**Status**: Display issues
|
|
|
|
**Issue**: Modal popup positioning or rendering issues
|
|
|
|
**Similar to**: Canvas popup fixes (now resolved)
|
|
|
|
**Potential Fix**: Apply same solution as canvas popup refactoring
|
|
|
|
**Effort**: 1-2 hours
|
|
**Risk**: Low - Can use known fix pattern
|
|
|
|
---
|
|
|
|
## 🟡 STABILITY: Critical Areas Requiring Attention
|
|
|
|
### 1. EditorManager Refactoring - Manual Testing Required
|
|
|
|
**Status**: 90% feature parity achieved, needs validation
|
|
|
|
**Critical Gap**: Manual testing phase not completed (2-3 hours planned)
|
|
|
|
**Remaining Work**:
|
|
- [ ] Test all 34 editor cards open/close properly
|
|
- [ ] Verify DockBuilder layouts for all 10 editor types
|
|
- [ ] Test all keyboard shortcuts without conflicts
|
|
- [ ] Multi-session testing with independent card visibility
|
|
- [ ] Verify sidebar collapse/expand (Ctrl+B)
|
|
|
|
**Files**:
|
|
- `docs/H3-feature-parity-analysis.md`
|
|
- `docs/H2-editor-manager-architecture.md`
|
|
|
|
**Risk**: Medium - Refactoring may have introduced regressions
|
|
|
|
**Recommendation**: Run comprehensive manual testing before release
|
|
|
|
---
|
|
|
|
### 2. E2E Test Suite - Needs Updates for New Architecture
|
|
|
|
**Status**: Tests exist but need updating
|
|
|
|
**Issue**: E2E tests written for old monolithic architecture, new card-based system needs test updates
|
|
|
|
**Examples**:
|
|
- `dungeon_object_rendering_e2e_tests.cc` - Needs rewrite for DungeonEditorV2
|
|
- Old window references need updating to new card names
|
|
|
|
**Files**:
|
|
- `test/e2e/dungeon_object_rendering_e2e_tests.cc`
|
|
- `test/e2e/dungeon_editor_smoke_test.cc`
|
|
|
|
**Effort**: 4-6 hours
|
|
**Risk**: Medium - Test coverage gaps
|
|
|
|
---
|
|
|
|
### 3. Memory Management & Resource Cleanup
|
|
|
|
**Status**: Generally good, but some areas need review
|
|
|
|
**Known Issues**:
|
|
- ✅ Audio buffer allocation bug fixed (was using single value instead of array)
|
|
- ✅ Tile cache `std::move()` issues fixed (SIGBUS errors resolved)
|
|
- ⚠️ Slow shutdown noted in `window.cc` (line 146: "TODO: BAD FIX, SLOW SHUTDOWN TAKES TOO LONG NOW")
|
|
- ⚠️ Graphics arena shutdown sequence (may need optimization)
|
|
|
|
**Files**:
|
|
- `src/app/platform/window.cc` (line 146)
|
|
- `src/app/gfx/resource/arena.cc`
|
|
- `src/app/gfx/resource/memory_pool.cc`
|
|
|
|
**Effort**: 2-4 hours (investigation + optimization)
|
|
**Risk**: Low-Medium - Performance impact, not crashes
|
|
|
|
---
|
|
|
|
## 🟢 IMPLEMENTATION: Missing Features for Release
|
|
|
|
### 1. Global Search Enhancements (Priority: MEDIUM)
|
|
|
|
**Status**: Core search works, enhancements missing
|
|
|
|
**Missing Features**:
|
|
- Text/message string searching (40 min)
|
|
- Map name and room name searching (40 min)
|
|
- Memory address and label searching (60 min)
|
|
- Search result caching for performance (30 min)
|
|
|
|
**Total Effort**: 4-6 hours
|
|
**Impact**: Nice-to-have enhancement
|
|
|
|
**Files**:
|
|
- `src/app/editor/ui/ui_coordinator.cc`
|
|
|
|
---
|
|
|
|
### 2. Layout Persistence (Priority: LOW)
|
|
|
|
**Status**: Default layouts work, persistence stubbed
|
|
|
|
**Missing**:
|
|
- `SaveCurrentLayout()` method (45 min)
|
|
- `LoadLayout()` method (45 min)
|
|
- Layout presets (Developer/Designer/Modder) (2 hours)
|
|
|
|
**Total Effort**: 3-4 hours
|
|
**Impact**: Enhancement, not blocking
|
|
|
|
**Files**:
|
|
- `src/app/editor/ui/layout_manager.cc`
|
|
|
|
---
|
|
|
|
### 3. Keyboard Shortcut Rebinding UI (Priority: LOW)
|
|
|
|
**Status**: Shortcuts work, rebinding UI missing
|
|
|
|
**Missing**:
|
|
- Shortcut rebinding UI in Settings > Shortcuts card (2 hours)
|
|
- Shortcut persistence to user config file (1 hour)
|
|
- Shortcut reset to defaults (30 min)
|
|
|
|
**Total Effort**: 3-4 hours
|
|
**Impact**: Enhancement
|
|
|
|
---
|
|
|
|
### 4. ZSCustomOverworld Features (Priority: MEDIUM)
|
|
|
|
**Status**: Partial implementation
|
|
|
|
**Missing**:
|
|
- ZSCustomOverworld Main Palette support
|
|
- ZSCustomOverworld Custom Area BG Color support
|
|
- Fix sprite icon draw positions
|
|
- Fix exit icon draw positions
|
|
|
|
**Dependencies**: Custom overworld data loading (complete)
|
|
|
|
**Files**:
|
|
- `src/app/editor/overworld/overworld_map.cc`
|
|
|
|
**Effort**: 8-12 hours
|
|
**Impact**: Feature completeness for ZSCOW users
|
|
|
|
---
|
|
|
|
### 5. z3ed Agent Execution Loop (MCP) (Priority: LOW)
|
|
|
|
**Status**: Agent framework foundation complete
|
|
|
|
**Missing**: Complete agent execution loop with MCP protocol
|
|
|
|
**Dependencies**: Agent framework foundation (complete)
|
|
|
|
**Files**:
|
|
- `src/cli/service/agent/conversational_agent_service.cc`
|
|
|
|
**Effort**: 8-12 hours
|
|
**Impact**: Future feature, not blocking release
|
|
|
|
---
|
|
|
|
## 📊 Release Readiness Assessment
|
|
|
|
### ✅ Strengths
|
|
|
|
1. **Architecture**: Excellent refactoring with 44% code reduction
|
|
2. **Build System**: Stable across all platforms (Windows, macOS, Linux)
|
|
3. **CI/CD**: Comprehensive pipeline with automated testing
|
|
4. **Documentation**: Extensive documentation (48+ markdown files)
|
|
5. **Feature Parity**: 90% achieved with master branch
|
|
6. **Test Coverage**: 46+ core tests with E2E framework
|
|
|
|
### ⚠️ Concerns
|
|
|
|
1. **Critical Bugs**: 6 high-priority bugs need fixing
|
|
2. **Manual Testing**: 2-3 hours of validation not completed
|
|
3. **E2E Tests**: Need updates for new architecture
|
|
4. **Audio System**: Core feature broken (emulator)
|
|
5. **Tile16 Editor**: Palette system issues blocking workflow
|
|
|
|
### 📈 Metrics
|
|
|
|
| Category | Status | Completion |
|
|
|----------|--------|------------|
|
|
| Build Stability | ✅ | 100% |
|
|
| Feature Parity | 🟡 | 90% |
|
|
| Test Coverage | 🟡 | 70% (needs updates) |
|
|
| Critical Bugs | 🔴 | 0% (6 bugs) |
|
|
| Documentation | ✅ | 95% |
|
|
| Performance | ✅ | 95% |
|
|
|
|
---
|
|
|
|
## 🎯 Recommended Release Plan
|
|
|
|
### Phase 1: Critical Fixes (1-2 weeks)
|
|
|
|
**Must Complete Before Release**:
|
|
|
|
1. **Tile16 Editor Palette Fixes** (4-6 hours)
|
|
- Fix palette button functionality
|
|
- Fix tile8 source canvas palette application
|
|
- Align colors between canvases
|
|
|
|
2. **Overworld Sprite Movement** (2-4 hours)
|
|
- Fix drag operation handling
|
|
- Test sprite placement workflow
|
|
|
|
3. **Emulator Audio System** (4-6 hours)
|
|
- Investigate root cause
|
|
- Fix audio output
|
|
- Verify playback works
|
|
|
|
4. **Canvas Multi-Select Bug** (3-5 hours)
|
|
- Fix 512px boundary crossing
|
|
- Verify with existing E2E test
|
|
|
|
5. **Manual Testing Suite** (2-3 hours)
|
|
- Test all 34 cards
|
|
- Verify layouts
|
|
- Test shortcuts
|
|
|
|
**Total**: 15-24 hours (2-3 days full-time)
|
|
|
|
---
|
|
|
|
### Phase 2: Stability Improvements (1 week)
|
|
|
|
**Should Complete Before Release**:
|
|
|
|
1. **E2E Test Updates** (4-6 hours)
|
|
- Update tests for new card-based architecture
|
|
- Add missing test coverage
|
|
|
|
2. **Shutdown Performance** (2-4 hours)
|
|
- Optimize window shutdown sequence
|
|
- Review graphics arena cleanup
|
|
|
|
3. **Overworld Map Properties Popup** (1-2 hours)
|
|
- Apply canvas popup fix pattern
|
|
|
|
**Total**: 7-12 hours (1-2 days)
|
|
|
|
---
|
|
|
|
### Phase 3: Enhancement Features (Post-Release)
|
|
|
|
**Can Defer to Post-Release**:
|
|
|
|
1. Global Search enhancements (4-6 hours)
|
|
2. Layout persistence (3-4 hours)
|
|
3. Shortcut rebinding UI (3-4 hours)
|
|
4. ZSCustomOverworld features (8-12 hours)
|
|
5. z3ed agent execution loop (8-12 hours)
|
|
|
|
**Total**: 26-38 hours (future releases)
|
|
|
|
---
|
|
|
|
## 🔍 Code Quality Observations
|
|
|
|
### Positive
|
|
|
|
1. **Excellent Documentation**: Comprehensive guides, architecture docs, troubleshooting
|
|
2. **Modern C++**: C++23 features, RAII, smart pointers
|
|
3. **Cross-Platform**: Consistent behavior across platforms
|
|
4. **Error Handling**: absl::Status used throughout
|
|
5. **Modular Architecture**: Refactored from monolith to 8 delegated components
|
|
|
|
### Areas for Improvement
|
|
|
|
1. **TODO Comments**: 153+ TODO items tagged with `[EditorManagerRefactor]`
|
|
2. **Test Coverage**: Some E2E tests need architecture updates
|
|
3. **Memory Management**: Some shutdown sequences need optimization
|
|
4. **Audio System**: Needs investigation and debugging
|
|
|
|
---
|
|
|
|
## 📝 Specific Code Issues Found
|
|
|
|
### High Priority
|
|
|
|
1. **Tile16 Editor Palette** (`F2-tile16-editor-palette-system.md:280-297`)
|
|
- Palette buttons not updating correctly
|
|
- Tile8 source canvas showing wrong colors
|
|
|
|
2. **Overworld Sprite Movement** (`yaze.org:13-19`)
|
|
- Sprites not responding to drag operations
|
|
- Blocks sprite editing workflow
|
|
|
|
3. **Emulator Audio** (`E8-emulator-debugging-vision.md:35`)
|
|
- Audio output broken
|
|
- Comprehensive debugging guide available
|
|
|
|
### Medium Priority
|
|
|
|
1. **Canvas Multi-Select** (`yaze.org:21-27`)
|
|
- Selection box rendering issues at 512px boundaries
|
|
- E2E test exists for validation
|
|
|
|
2. **EditorManager Testing** (`H3-feature-parity-analysis.md:339-351`)
|
|
- Manual testing phase not completed
|
|
- 90% feature parity needs validation
|
|
|
|
### Low Priority
|
|
|
|
1. **Right-Click Context Menu** (`yaze.org:29-35`)
|
|
- Intermittent oversized tile16 display
|
|
- Cosmetic issue
|
|
|
|
2. **Shutdown Performance** (`window.cc:146`)
|
|
- Slow shutdown noted in code
|
|
- TODO comment indicates known issue
|
|
|
|
---
|
|
|
|
## 🚀 Immediate Action Items
|
|
|
|
### This Week
|
|
|
|
1. **Fix Tile16 Editor Palette Buttons** (4-6 hours)
|
|
- Priority: HIGH
|
|
- Blocks: Tile editing workflow
|
|
|
|
2. **Fix Overworld Sprite Movement** (2-4 hours)
|
|
- Priority: HIGH
|
|
- Blocks: Sprite editing
|
|
|
|
3. **Investigate Emulator Audio** (4-6 hours)
|
|
- Priority: CRITICAL
|
|
- Blocks: Core emulator feature
|
|
|
|
### Next Week
|
|
|
|
1. **Complete Manual Testing** (2-3 hours)
|
|
- Priority: HIGH
|
|
- Blocks: Release confidence
|
|
|
|
2. **Fix Canvas Multi-Select** (3-5 hours)
|
|
- Priority: MEDIUM
|
|
- Blocks: Workflow quality
|
|
|
|
3. **Update E2E Tests** (4-6 hours)
|
|
- Priority: MEDIUM
|
|
- Blocks: Test coverage confidence
|
|
|
|
---
|
|
|
|
## 📚 Documentation Status
|
|
|
|
### Excellent Coverage
|
|
|
|
- ✅ Architecture documentation (`H2-editor-manager-architecture.md`)
|
|
- ✅ Feature parity analysis (`H3-feature-parity-analysis.md`)
|
|
- ✅ Build troubleshooting (`BUILD-TROUBLESHOOTING.md`)
|
|
- ✅ Emulator debugging vision (`E8-emulator-debugging-vision.md`)
|
|
- ✅ Tile16 editor palette system (`F2-tile16-editor-palette-system.md`)
|
|
|
|
### Could Use Updates
|
|
|
|
- ⚠️ API documentation generation (TODO in `yaze.org:344`)
|
|
- ⚠️ User guide for ROM hackers (TODO in `yaze.org:349`)
|
|
|
|
---
|
|
|
|
## 🎯 Success Criteria for Release
|
|
|
|
### Must Have (Blocking Release)
|
|
|
|
- [ ] All 6 critical bugs fixed
|
|
- [ ] Manual testing suite completed
|
|
- [ ] Emulator audio working
|
|
- [ ] Tile16 editor palette system functional
|
|
- [ ] Overworld sprite movement working
|
|
- [ ] Canvas multi-select fixed
|
|
- [ ] No known crashes or data corruption
|
|
|
|
### Should Have (Release Quality)
|
|
|
|
- [ ] E2E tests updated for new architecture
|
|
- [ ] Shutdown performance optimized
|
|
- [ ] All 34 cards tested and working
|
|
- [ ] All 10 layouts verified
|
|
- [ ] Keyboard shortcuts tested
|
|
|
|
### Nice to Have (Post-Release)
|
|
|
|
- [ ] Global Search enhancements
|
|
- [ ] Layout persistence
|
|
- [ ] Shortcut rebinding UI
|
|
- [ ] ZSCustomOverworld features complete
|
|
|
|
---
|
|
|
|
## 📊 Estimated Timeline
|
|
|
|
### Conservative Estimate (Full-Time)
|
|
|
|
- **Phase 1 (Critical Fixes)**: 2-3 days (15-24 hours)
|
|
- **Phase 2 (Stability)**: 1-2 days (7-12 hours)
|
|
- **Total**: 3-5 days to release-ready state
|
|
|
|
### With Part-Time Development
|
|
|
|
- **Phase 1**: 1-2 weeks
|
|
- **Phase 2**: 1 week
|
|
- **Total**: 2-3 weeks to release-ready state
|
|
|
|
---
|
|
|
|
## 🔗 Related Documents
|
|
|
|
- `docs/H3-feature-parity-analysis.md` - Feature parity status
|
|
- `docs/H2-editor-manager-architecture.md` - Architecture details
|
|
- `docs/F2-tile16-editor-palette-system.md` - Tile16 editor issues
|
|
- `docs/E8-emulator-debugging-vision.md` - Emulator audio debugging
|
|
- `docs/yaze.org` - Development tracker with active issues
|
|
- `docs/BUILD-TROUBLESHOOTING.md` - Build system issues
|
|
- `.github/workflows/ci.yml` - CI/CD pipeline status
|
|
|
|
---
|
|
|
|
## ✅ Conclusion
|
|
|
|
YAZE is **very close to release** with excellent architecture and comprehensive documentation. The main blockers are:
|
|
|
|
1. **6 critical bugs** requiring 15-24 hours of focused work
|
|
2. **Manual testing validation** (2-3 hours)
|
|
3. **Emulator audio system** investigation (4-6 hours)
|
|
|
|
With focused effort on critical fixes, YAZE can achieve a stable release in **2-3 weeks** (part-time) or **3-5 days** (full-time).
|
|
|
|
**Recommendation**: Proceed with Phase 1 critical fixes immediately, then complete Phase 2 stability improvements before release. Enhancement features can be deferred to post-release updates.
|
|
|
|
---
|
|
|
|
**Document Status**: Complete
|
|
**Last Updated**: January 31, 2025
|
|
**Review Status**: Ready for implementation planning
|
|
|