-
Notifications
You must be signed in to change notification settings - Fork 5
Open
Description
Problem
The codebase has heavy dependencies on global state, particularly the global stars vector and other global arrays. This creates tight coupling, makes testing difficult, and violates modern C++ architectural principles.
Current Global State Issues
Global Dependencies
// Current problematic pattern throughout codebase:
extern std::vector<Star> stars; // Global dependency
extern std::vector<Race> races; // Global dependency
extern std::vector<Planet> planets; // Global dependency
// Used directly in many files:
stars[snum].get_name() // Tight coupling
races[player].name // Direct accessFiles with Heavy Global Dependencies
- Almost all command files (gb/commands/*.cc)
- Core game logic (gb/doturncmd.cc, gb/GB_server.cc)
- Various subsystems throughout the codebase
Vision for Improvement
Better Architecture (Long-term goal):
// Instead of global state, dependency injection:
class GameContext {
private:
std::vector<Star> stars;
std::vector<Race> races;
std::vector<Planet> planets;
public:
Star& getStar(starnum_t num);
Race& getRace(player_t num);
Planet& getPlanet(starnum_t star, planetnum_t planet);
// ... proper encapsulation
};
// Commands would receive context:
void command_func(const command_t& argv, GameObj& g, GameContext& context) {
auto& star = context.getStar(g.snum);
// Clean, testable access
}Benefits of Refactoring
Testability
- Mockable Dependencies: Easy to create test contexts
- Isolated Testing: Commands can be tested with controlled state
- Unit Testing: Individual components testable in isolation
Maintainability
- Clear Dependencies: What each function needs is explicit
- Encapsulation: Internal state changes don't break external code
- Modularity: Components can be developed and modified independently
Performance
- Cache Locality: Better memory layout control
- Threading: Easier to make thread-safe
- Resource Management: Better control over object lifetimes
Implementation Challenges
Scope of Change
This is a major architectural refactoring that would affect:
- All command implementations (~50+ files)
- Core game loop and turn processing
- Database persistence layer
- Client/server communication
- Save/load systems
Migration Strategy
This cannot be done all at once. Suggested phased approach:
Phase 1: Preparation (Current Issues #18-#23)
- Complete current modernization efforts
- Standardize access patterns
- Remove direct global array access
Phase 2: Interface Design
- Design
GameContextor similar abstraction - Define clean interfaces for game state access
- Create transition plan
Phase 3: Gradual Migration
- Introduce context alongside existing globals
- Migrate command by command
- Maintain backward compatibility during transition
Phase 4: Global Cleanup
- Remove global state variables
- Complete the architectural transition
Priority and Timeline
Priority: Low (Future)
This is a significant architectural change that should be:
- Planned carefully with full design review
- Implemented after current modernization is complete
- Done incrementally to avoid breaking existing functionality
- Considered for major version upgrade
Prerequisites
- Complete issues Replace direct stars[] array access with getstar()/putstar() pattern #18-Standardize std::optional error checking patterns #23 first
- Establish comprehensive test suite
- Design new architecture thoroughly
- Get community consensus on approach
Success Criteria (Long-term)
- No global state variables in core game logic
- All game state accessed through well-defined interfaces
- Commands receive dependencies explicitly
- Easy to create isolated test environments
- Thread-safe architecture (if desired)
- Clear separation of concerns
- Comprehensive test coverage
Related Issues
- Replace direct stars[] array access with getstar()/putstar() pattern #18 Replace direct stars[] array access (prerequisite)
- #19 Fix missing persistence calls (prerequisite)
- Modernize planet name access patterns #20 Modernize planet name access (prerequisite)
- Standardize database access layer on free functions #21 Standardize database access layer (prerequisite)
Discussion Points
- What should the new architecture look like?
- How to maintain compatibility during transition?
- What's the migration timeline?
- Should this wait for a major version?
- Community input on architectural direction?
This issue is for planning and discussion rather than immediate implementation.
Reactions are currently unavailable