Remove outdated Canvas migration and refactoring documentation

- Deleted the `canvas-migration.md` and `canvas-refactor-summary.md` files as they are no longer relevant to the current codebase.
- These documents outlined previous strategies and issues related to the Canvas class, which have since been resolved through recent refactoring efforts.
This commit is contained in:
scawful
2025-09-25 15:52:05 -04:00
parent 6f906a020d
commit 6779c4cc4a
2 changed files with 0 additions and 364 deletions

View File

@@ -1,186 +0,0 @@
# Canvas Interface Migration Strategy
## Overview
This document outlines a strategy for migrating from the current complex Canvas class to a simplified version that uses pure functions while maintaining backward compatibility.
## Current Issues
1. **Duplicate Declarations**: Many methods are declared in both public and private sections
2. **Mixed Access Patterns**: Some private methods are accessed publicly through accessors
3. **Complex State Management**: Canvas state is scattered across many member variables
4. **Tight Coupling**: Drawing operations are tightly coupled to the class implementation
## Migration Strategy
### Phase 1: Create Pure Function Interface (COMPLETED)
- ✅ Created `canvas_interface.h` with pure function definitions
- ✅ Created `CanvasSimplified` class that uses pure functions
- ✅ Maintained identical public API for backward compatibility
### Phase 2: Gradual Migration
#### Step 1: Add Compatibility Layer
```cpp
// In existing Canvas class, add delegation methods
class Canvas {
public:
// Existing methods delegate to pure functions
void DrawBitmap(gfx::Bitmap& bitmap, int border_offset, float scale) override {
canvas_ops::drawing::DrawBitmap(state_, bitmap, border_offset, scale);
}
private:
canvas_ops::CanvasState state_; // Replace individual members
};
```
#### Step 2: Update Existing Code
```cpp
// Before
canvas.DrawBitmap(bitmap, 2, 1.0f);
// After (same API, different implementation)
canvas.DrawBitmap(bitmap, 2, 1.0f); // No change needed!
```
#### Step 3: Gradual Replacement
```cpp
// Option 1: Replace Canvas with CanvasSimplified
using Canvas = CanvasSimplified;
// Option 2: Use typedef for gradual migration
typedef CanvasSimplified NewCanvas;
```
### Phase 3: Benefits of New Interface
#### 1. Pure Functions
```cpp
// Testable without Canvas instance
canvas_ops::CanvasState test_state;
canvas_ops::drawing::DrawBitmap(test_state, bitmap, 2, 1.0f);
```
#### 2. Better Separation of Concerns
```cpp
// Drawing operations
canvas_ops::drawing::DrawGrid(state, 64.0f);
// Interaction handling
canvas_ops::interaction::HandleMouseInput(state);
// State management
canvas_ops::canvas_management::SetCanvasSize(state, ImVec2(512, 512));
```
#### 3. Easier Testing
```cpp
TEST(CanvasDrawing, DrawBitmap) {
canvas_ops::CanvasState state;
gfx::Bitmap bitmap;
canvas_ops::drawing::DrawBitmap(state, bitmap, 2, 1.0f);
// Assert on state changes
EXPECT_TRUE(state.refresh_graphics_);
}
```
## Implementation Plan
### Week 1: Core Infrastructure
- [ ] Implement pure function bodies in `canvas_interface.cc`
- [ ] Create `CanvasSimplified` implementation
- [ ] Add unit tests for pure functions
### Week 2: Compatibility Layer
- [ ] Add delegation methods to existing Canvas class
- [ ] Replace member variables with `CanvasState`
- [ ] Test backward compatibility
### Week 3: Gradual Migration
- [ ] Update one editor at a time (overworld, dungeon, etc.)
- [ ] Use `CanvasSimplified` in new code
- [ ] Monitor for any breaking changes
### Week 4: Cleanup
- [ ] Remove old Canvas implementation
- [ ] Rename `CanvasSimplified` to `Canvas`
- [ ] Update documentation
## Breaking Changes
### None Expected
The new interface maintains the exact same public API, so existing code should continue to work without changes.
### Optional Improvements
```cpp
// Old way (still works)
canvas.DrawBitmap(bitmap, 2, 1.0f);
// New way (optional, more explicit)
canvas_ops::drawing::DrawBitmap(canvas.GetState(), bitmap, 2, 1.0f);
```
## Testing Strategy
### Unit Tests
```cpp
TEST(CanvasOps, DrawBitmap) {
canvas_ops::CanvasState state;
gfx::Bitmap bitmap;
canvas_ops::drawing::DrawBitmap(state, bitmap, 2, 1.0f);
// Verify state changes
EXPECT_TRUE(state.refresh_graphics_);
}
```
### Integration Tests
```cpp
TEST(CanvasIntegration, OverworldEditor) {
CanvasSimplified canvas;
OverworldEditor editor(&canvas);
// Test that editor still works with new canvas
editor.Draw();
}
```
## Performance Considerations
### Minimal Overhead
- Pure functions have minimal overhead compared to method calls
- `CanvasState` is passed by reference, no copying
- Same memory layout as original class
### Potential Optimizations
```cpp
// Batch operations
canvas_ops::CanvasState& state = canvas.GetState();
canvas_ops::drawing::DrawGrid(state, 64.0f);
canvas_ops::drawing::DrawOverlay(state);
// Single state update at end
```
## Rollback Plan
If issues arise:
1. Keep both implementations available
2. Use compile-time flag to switch between them
3. Revert to original Canvas if needed
4. Gradual rollback of individual editors
## Conclusion
This migration strategy provides:
- ✅ Backward compatibility
- ✅ Better testability
- ✅ Cleaner separation of concerns
- ✅ Easier maintenance
- ✅ No breaking changes
The pure function approach makes the Canvas system more modular and maintainable while preserving all existing functionality.

View File

@@ -1,178 +0,0 @@
# Canvas Interface Refactoring
## Problem Statement
The current `Canvas` class has several issues:
1. **Duplicate Declarations**: Methods declared in both public and private sections
2. **Mixed Access Patterns**: Private methods accessed through public accessors
3. **Complex State Management**: State scattered across many member variables
4. **Tight Coupling**: Drawing operations tightly coupled to class implementation
5. **Compilation Errors**: Multiple redefinition errors preventing builds
## Solution Overview
A **pure function interface** has been created that maintains the same public API while providing better separation of concerns and eliminating the compilation issues. The refactoring is complete and integrated into the codebase.
## Files Created
### 1. `canvas_interface.h` - Pure Function Interface
- Defines `CanvasState` structure containing all canvas state
- Organizes pure functions into logical namespaces:
- `drawing::` - All drawing operations
- `interaction::` - Mouse/keyboard handling
- `selection::` - Selection management
- `context_menu::` - Context menu system
- `labels::` - Label management
- `canvas_management::` - Canvas lifecycle
- `utils::` - Utility accessors
### 2. `canvas_simplified.h` - New Canvas Class
- Maintains **identical public API** to original Canvas
- Delegates all operations to pure functions
- Uses `CanvasState` internally instead of scattered member variables
- Provides backward compatibility
### 3. `canvas_interface.cc` - Pure Function Implementations
- Implements all pure functions
- Handles state management through `CanvasState` parameter
- Provides foundation for testing and modularity
### 4. `canvas_simplified.cc` - Canvas Class Implementation
- Implements all public methods by delegating to pure functions
- Maintains exact same API as original Canvas
- No breaking changes for existing code
### 5. `canvas_example.cc` - Usage Examples
- Shows how to use the new interface
- Demonstrates migration path
- Examples of custom operations using pure functions
### 6. `canvas_migration.md` - Migration Strategy
- Detailed plan for transitioning from old to new Canvas
- Backward compatibility guarantees
- Testing strategy
- Rollback plan
## Key Benefits
### 1. **No Breaking Changes**
```cpp
// Old code continues to work unchanged
CanvasSimplified canvas("MyCanvas");
canvas.DrawBitmap(bitmap, 2, 1.0f); // Same API!
```
### 2. **Better Testability**
```cpp
// Test pure functions without Canvas instance
canvas_ops::CanvasState state;
canvas_ops::drawing::DrawBitmap(state, bitmap, 2, 1.0f);
assert(state.refresh_graphics_ == true);
```
### 3. **Cleaner Separation of Concerns**
```cpp
// Drawing operations
canvas_ops::drawing::DrawGrid(state, 64.0f);
// Interaction handling
canvas_ops::interaction::HandleMouseInput(state);
// State management
canvas_ops::canvas_management::SetCanvasSize(state, ImVec2(512, 512));
```
### 4. **Easier Maintenance**
- Pure functions are easier to understand and modify
- State is centralized in `CanvasState` structure
- No more duplicate declarations or access pattern confusion
### 5. **Extensibility**
```cpp
// Easy to add new operations without modifying Canvas class
namespace custom_operations {
void DrawCustomBorder(canvas_ops::CanvasState& state, ImColor color, float thickness);
void DrawCustomGrid(canvas_ops::CanvasState& state, ImColor color, float spacing);
}
```
## Migration Path
### Phase 1: Immediate Fix ✅ COMPLETED
1. ✅ Add `map_properties.cc` to CMake build
2. ✅ Create pure function interface
3. ✅ Create `CanvasSimplified` class
4. ✅ Maintain backward compatibility
### Phase 2: Gradual Migration ✅ COMPLETED
- ✅ Pure function interface implemented
-`CanvasSimplified` class available
- ✅ Backward compatibility maintained
### Phase 3: Full Migration ✅ COMPLETED
1. ✅ New interface integrated into codebase
2. ✅ Old Canvas implementation maintained for compatibility
3. ✅ Both interfaces available for use
## Current Status
### ✅ Build Issues Resolved
The Canvas class compilation errors have been resolved through the pure function interface.
### ✅ Build System Integration
The new interface files are integrated into the CMake build system:
- `app/gui/canvas_interface.cc`
- `app/gui/canvas_simplified.cc`
### ✅ Backward Compatibility Verified
The new interface maintains full backward compatibility:
```cpp
// This works without any changes
CanvasSimplified canvas;
canvas.DrawBitmap(bitmap, 2, 1.0f);
canvas.DrawGrid(64.0f);
canvas.DrawOverlay();
```
## Performance Considerations
- **Minimal Overhead**: Pure functions have same performance as method calls
- **No Memory Overhead**: `CanvasState` has same memory layout as original class
- **Same API**: No performance impact on existing code
## Testing Strategy
### Unit Tests
```cpp
TEST(CanvasOps, DrawBitmap) {
canvas_ops::CanvasState state;
gfx::Bitmap bitmap;
canvas_ops::drawing::DrawBitmap(state, bitmap, 2, 1.0f);
EXPECT_TRUE(state.refresh_graphics_);
}
```
### Integration Tests
```cpp
TEST(CanvasIntegration, OverworldEditor) {
CanvasSimplified canvas;
OverworldEditor editor(&canvas);
// Test that editor still works with new canvas
editor.Draw();
}
```
## Conclusion
This refactoring provides:
-**Fixes compilation errors** immediately
-**Maintains backward compatibility** completely
-**Improves code organization** significantly
-**Enables better testing** through pure functions
-**Provides migration path** without breaking changes
-**Reduces complexity** of Canvas class
The solution addresses all current issues while providing a foundation for future improvements. Existing code continues to work unchanged, and new code can take advantage of the cleaner pure function interface.