latest state
This commit is contained in:
363
IMPROVEMENTS_CHECKLIST.md
Normal file
363
IMPROVEMENTS_CHECKLIST.md
Normal file
@ -0,0 +1,363 @@
|
||||
# Tetris SDL3 - Improvements Checklist
|
||||
|
||||
Quick reference for implementing the recommendations from CODE_ANALYSIS.md
|
||||
|
||||
---
|
||||
|
||||
## 🔴 High Priority (Critical)
|
||||
|
||||
### 1. Smart Pointer Wrapper for SDL Resources
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 2-3 hours
|
||||
**Impact:** Prevents memory leaks, improves safety
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `src/utils/SDLPointers.h` with smart pointer wrappers
|
||||
- [ ] Replace raw `SDL_Texture*` in `MenuState.h` (lines 17-21)
|
||||
- [ ] Replace raw `SDL_Texture*` in `PlayingState.h`
|
||||
- [ ] Update `main.cpp` texture loading
|
||||
- [ ] Test all states to ensure no regressions
|
||||
|
||||
**Code Template:**
|
||||
```cpp
|
||||
// src/utils/SDLPointers.h
|
||||
#pragma once
|
||||
#include <SDL3/SDL.h>
|
||||
#include <memory>
|
||||
|
||||
struct SDL_TextureDeleter {
|
||||
void operator()(SDL_Texture* tex) const {
|
||||
if (tex) SDL_DestroyTexture(tex);
|
||||
}
|
||||
};
|
||||
|
||||
struct SDL_SurfaceDeleter {
|
||||
void operator()(SDL_Surface* surf) const {
|
||||
if (surf) SDL_DestroySurface(surf);
|
||||
}
|
||||
};
|
||||
|
||||
using SDL_TexturePtr = std::unique_ptr<SDL_Texture, SDL_TextureDeleter>;
|
||||
using SDL_SurfacePtr = std::unique_ptr<SDL_Surface, SDL_SurfaceDeleter>;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 2. Remove Debug File I/O
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 30 minutes
|
||||
**Impact:** Performance, code cleanliness
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Remove or wrap `fopen("tetris_trace.log")` calls in `MenuState.cpp`
|
||||
- [ ] Remove or wrap similar calls in other files
|
||||
- [ ] Replace with SDL_LogTrace or conditional compilation
|
||||
- [ ] Delete `tetris_trace.log` from repository
|
||||
|
||||
**Files to Update:**
|
||||
- `src/states/MenuState.cpp` (lines 182-184, 195-203, 277-278, 335-337)
|
||||
- `src/main.cpp` (if any similar patterns exist)
|
||||
|
||||
**Replacement Pattern:**
|
||||
```cpp
|
||||
// Before:
|
||||
FILE* f = fopen("tetris_trace.log", "a");
|
||||
if (f) { fprintf(f, "MenuState::render entry\n"); fclose(f); }
|
||||
|
||||
// After:
|
||||
SDL_LogTrace(SDL_LOG_CATEGORY_APPLICATION, "MenuState::render entry");
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 3. Improve Error Handling in Asset Loading
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 2 hours
|
||||
**Impact:** Better debugging, prevents crashes
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Update `loadTextureFromImage` to return error information
|
||||
- [ ] Add validation for all asset loads in `main.cpp`
|
||||
- [ ] Create fallback assets for missing resources
|
||||
- [ ] Add startup asset validation
|
||||
|
||||
**Example:**
|
||||
```cpp
|
||||
struct AssetLoadResult {
|
||||
SDL_TexturePtr texture;
|
||||
std::string error;
|
||||
bool success;
|
||||
};
|
||||
|
||||
AssetLoadResult loadTextureFromImage(SDL_Renderer* renderer,
|
||||
const std::string& path);
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Medium Priority (Important)
|
||||
|
||||
### 4. Extract Common Patterns
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 3-4 hours
|
||||
**Impact:** Reduces code duplication
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `ExitPopupHelper` class in `StateContext.h`
|
||||
- [ ] Update `MenuState.cpp` to use helper
|
||||
- [ ] Update `PlayingState.cpp` to use helper
|
||||
- [ ] Update `OptionsState.cpp` to use helper
|
||||
|
||||
---
|
||||
|
||||
### 5. Move Magic Numbers to Config.h
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 1 hour
|
||||
**Impact:** Maintainability
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Add menu button constants to `Config::UI`
|
||||
- [ ] Add rendering constants to appropriate namespace
|
||||
- [ ] Update `MenuState.cpp` to use config constants
|
||||
- [ ] Update `UIRenderer.cpp` to use config constants
|
||||
|
||||
**Constants to Add:**
|
||||
```cpp
|
||||
namespace Config::UI {
|
||||
constexpr float MENU_BUTTON_WIDTH = 200.0f;
|
||||
constexpr float MENU_BUTTON_HEIGHT = 70.0f;
|
||||
constexpr float MENU_BUTTON_Y_FRACTION = 0.865f;
|
||||
constexpr float MENU_BUTTON_SPACING = 210.0f;
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 6. Add Unit Tests
|
||||
**Status:** ⚠️ Minimal (only GravityTests)
|
||||
**Effort:** 8-10 hours
|
||||
**Impact:** Code quality, regression prevention
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `tests/GameLogicTests.cpp`
|
||||
- [ ] Test piece spawning
|
||||
- [ ] Test rotation
|
||||
- [ ] Test collision detection
|
||||
- [ ] Test line clearing
|
||||
- [ ] Test scoring
|
||||
- [ ] Create `tests/ScoreManagerTests.cpp`
|
||||
- [ ] Test score submission
|
||||
- [ ] Test high score detection
|
||||
- [ ] Test persistence
|
||||
- [ ] Create `tests/StateTransitionTests.cpp`
|
||||
- [ ] Test state transitions
|
||||
- [ ] Test state lifecycle (onEnter/onExit)
|
||||
- [ ] Update CMakeLists.txt to include new tests
|
||||
|
||||
---
|
||||
|
||||
## 🟢 Low Priority (Nice to Have)
|
||||
|
||||
### 7. Add Doxygen Documentation
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 4-6 hours
|
||||
**Impact:** Developer onboarding
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `Doxyfile` configuration
|
||||
- [ ] Add class-level documentation to core classes
|
||||
- [ ] Add function-level documentation to public APIs
|
||||
- [ ] Generate HTML documentation
|
||||
- [ ] Add to build process
|
||||
|
||||
---
|
||||
|
||||
### 8. Performance Profiling
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 4-6 hours
|
||||
**Impact:** Depends on findings
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Profile with Visual Studio Profiler / Instruments
|
||||
- [ ] Identify hotspots
|
||||
- [ ] Optimize critical paths
|
||||
- [ ] Add performance benchmarks
|
||||
|
||||
---
|
||||
|
||||
### 9. Standardize Member Variable Naming
|
||||
**Status:** ⚠️ Inconsistent
|
||||
**Effort:** 2-3 hours
|
||||
**Impact:** Code consistency
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Decide on naming convention (recommend `m_` prefix for private members)
|
||||
- [ ] Update all class member variables
|
||||
- [ ] Update documentation to reflect convention
|
||||
|
||||
**Convention Recommendation:**
|
||||
```cpp
|
||||
class Example {
|
||||
public:
|
||||
int publicValue; // No prefix for public members
|
||||
|
||||
private:
|
||||
int m_privateValue; // m_ prefix for private members
|
||||
float m_memberVariable; // Consistent across all classes
|
||||
};
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 📋 Code Quality Improvements
|
||||
|
||||
### 10. Add .clang-format
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 15 minutes
|
||||
**Impact:** Consistent formatting
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `.clang-format` file in project root
|
||||
- [ ] Run formatter on all source files
|
||||
- [ ] Add format check to CI/CD
|
||||
|
||||
**Suggested .clang-format:**
|
||||
```yaml
|
||||
BasedOnStyle: LLVM
|
||||
IndentWidth: 4
|
||||
ColumnLimit: 120
|
||||
PointerAlignment: Left
|
||||
AllowShortFunctionsOnASingleLine: Empty
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 11. Add README.md
|
||||
**Status:** ❌ Missing
|
||||
**Effort:** 1 hour
|
||||
**Impact:** Project documentation
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `README.md` with:
|
||||
- [ ] Project description
|
||||
- [ ] Screenshots/GIF
|
||||
- [ ] Build instructions
|
||||
- [ ] Dependencies
|
||||
- [ ] Controls
|
||||
- [ ] License
|
||||
|
||||
---
|
||||
|
||||
### 12. Set Up CI/CD
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 2-3 hours
|
||||
**Impact:** Automated testing
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `.github/workflows/build.yml`
|
||||
- [ ] Add Windows build job
|
||||
- [ ] Add macOS build job
|
||||
- [ ] Add test execution
|
||||
- [ ] Add artifact upload
|
||||
|
||||
---
|
||||
|
||||
## 🔧 Refactoring Opportunities
|
||||
|
||||
### 13. Create Asset Manager
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 4-5 hours
|
||||
**Impact:** Better resource management
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `src/core/assets/AssetManager.h`
|
||||
- [ ] Implement texture caching
|
||||
- [ ] Implement font caching
|
||||
- [ ] Update states to use AssetManager
|
||||
- [ ] Add asset preloading
|
||||
|
||||
---
|
||||
|
||||
### 14. Implement Event System
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 6-8 hours
|
||||
**Impact:** Decoupling, flexibility
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `src/core/events/EventBus.h`
|
||||
- [ ] Define event types
|
||||
- [ ] Replace callbacks with events
|
||||
- [ ] Update Game class to publish events
|
||||
- [ ] Update Audio system to subscribe to events
|
||||
|
||||
---
|
||||
|
||||
### 15. Component-Based UI
|
||||
**Status:** ❌ Not Started
|
||||
**Effort:** 8-10 hours
|
||||
**Impact:** UI maintainability
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create `src/ui/components/Button.h`
|
||||
- [ ] Create `src/ui/components/Panel.h`
|
||||
- [ ] Create `src/ui/components/Label.h`
|
||||
- [ ] Refactor MenuState to use components
|
||||
- [ ] Refactor OptionsState to use components
|
||||
|
||||
---
|
||||
|
||||
## 📊 Progress Tracking
|
||||
|
||||
| Category | Total Items | Completed | In Progress | Not Started |
|
||||
|----------|-------------|-----------|-------------|-------------|
|
||||
| High Priority | 3 | 0 | 0 | 3 |
|
||||
| Medium Priority | 3 | 0 | 0 | 3 |
|
||||
| Low Priority | 3 | 0 | 0 | 3 |
|
||||
| Code Quality | 3 | 0 | 0 | 3 |
|
||||
| Refactoring | 3 | 0 | 0 | 3 |
|
||||
| **TOTAL** | **15** | **0** | **0** | **15** |
|
||||
|
||||
---
|
||||
|
||||
## 🎯 Suggested Implementation Order
|
||||
|
||||
### Week 1: Critical Fixes
|
||||
1. Remove debug file I/O (30 min)
|
||||
2. Smart pointer wrapper (2-3 hours)
|
||||
3. Improve error handling (2 hours)
|
||||
|
||||
### Week 2: Code Quality
|
||||
4. Move magic numbers to Config.h (1 hour)
|
||||
5. Extract common patterns (3-4 hours)
|
||||
6. Add .clang-format (15 min)
|
||||
7. Add README.md (1 hour)
|
||||
|
||||
### Week 3: Testing
|
||||
8. Add GameLogicTests (4 hours)
|
||||
9. Add ScoreManagerTests (2 hours)
|
||||
10. Add StateTransitionTests (2 hours)
|
||||
|
||||
### Week 4: Documentation & CI
|
||||
11. Set up CI/CD (2-3 hours)
|
||||
12. Add Doxygen documentation (4-6 hours)
|
||||
|
||||
### Future Iterations:
|
||||
13. Performance profiling
|
||||
14. Asset Manager
|
||||
15. Event System
|
||||
16. Component-Based UI
|
||||
|
||||
---
|
||||
|
||||
## 📝 Notes
|
||||
|
||||
- Mark items as completed by changing ❌ to ✅
|
||||
- Update progress table as you complete items
|
||||
- Feel free to reorder based on your priorities
|
||||
- Some items can be done in parallel
|
||||
- Consider creating GitHub issues for tracking
|
||||
|
||||
---
|
||||
|
||||
**Last Updated:** 2025-12-03
|
||||
**Next Review:** After completing High Priority items
|
||||
Reference in New Issue
Block a user