Rework of base interactables and managed behaviors
This commit is contained in:
173
docs/singleton_instance_timing_fix.md
Normal file
173
docs/singleton_instance_timing_fix.md
Normal file
@@ -0,0 +1,173 @@
|
||||
# Singleton Instance Timing Fix - Summary
|
||||
|
||||
## Issue Identified
|
||||
|
||||
**Critical Bug**: Singleton managers were setting their `_instance` field in `OnManagedAwake()` instead of Unity's `Awake()`, causing race conditions when managers tried to access each other during initialization.
|
||||
|
||||
## Root Cause
|
||||
|
||||
The ManagedBehaviour lifecycle calls `OnManagedAwake()` in priority order AFTER boot completion. However, if Manager A (lower priority) tries to access Manager B's instance during its `OnManagedAwake()`, but Manager B has a higher priority number and hasn't run yet, `Manager B.Instance` would be null!
|
||||
|
||||
### Example Bug Scenario
|
||||
|
||||
```csharp
|
||||
// SceneManagerService (Priority 15) - runs FIRST
|
||||
protected override void OnManagedAwake()
|
||||
{
|
||||
_instance = this; // ❌ Set here
|
||||
|
||||
// Try to access LoadingScreenController
|
||||
_loadingScreen = LoadingScreenController.Instance; // ❌ NULL! Not set yet!
|
||||
}
|
||||
|
||||
// LoadingScreenController (Priority 45) - runs LATER
|
||||
protected override void OnManagedAwake()
|
||||
{
|
||||
_instance = this; // ❌ Too late! SceneManagerService already tried to access it
|
||||
}
|
||||
```
|
||||
|
||||
## Solution
|
||||
|
||||
**Move all `_instance` assignments to Unity's `Awake()` method.**
|
||||
|
||||
This guarantees that ALL singleton instances are available BEFORE any `OnManagedAwake()` calls begin, regardless of priority ordering.
|
||||
|
||||
### Correct Pattern
|
||||
|
||||
```csharp
|
||||
private new void Awake()
|
||||
{
|
||||
// Set instance immediately so it's available before OnManagedAwake() is called
|
||||
_instance = this;
|
||||
}
|
||||
|
||||
protected override void OnManagedAwake()
|
||||
{
|
||||
// ✓ Now safe to access other managers' instances
|
||||
// ✓ Priority controls initialization order, not instance availability
|
||||
}
|
||||
```
|
||||
|
||||
**Note**: The `new` keyword is required because we're intentionally hiding ManagedBehaviour's internal `Awake()` method.
|
||||
|
||||
## Files Modified
|
||||
|
||||
All ManagedBehaviour-based singleton managers were updated:
|
||||
|
||||
### Core Systems
|
||||
1. **GameManager.cs** (Priority 10)
|
||||
2. **SceneManagerService.cs** (Priority 15)
|
||||
3. **SaveLoadManager.cs** (Priority 20)
|
||||
4. **QuickAccess.cs** (Priority 5)
|
||||
|
||||
### Infrastructure
|
||||
5. **InputManager.cs** (Priority 25)
|
||||
6. **AudioManager.cs** (Priority 30)
|
||||
7. **LoadingScreenController.cs** (Priority 45)
|
||||
8. **UIPageController.cs** (Priority 50)
|
||||
9. **PauseMenu.cs** (Priority 55)
|
||||
10. **SceneOrientationEnforcer.cs** (Priority 70)
|
||||
|
||||
### Game Systems
|
||||
11. **CardSystemManager.cs** (Priority 60)
|
||||
12. **ItemManager.cs** (Priority 75)
|
||||
13. **PuzzleManager.cs** (Priority 80)
|
||||
14. **CinematicsManager.cs** (Priority 170)
|
||||
|
||||
## Design Principles
|
||||
|
||||
### Separation of Concerns
|
||||
|
||||
**Unity's Awake()**: Singleton registration only
|
||||
- Sets `_instance = this`
|
||||
- Guarantees instance availability
|
||||
- Runs in non-deterministic order (Unity's choice)
|
||||
|
||||
**OnManagedAwake()**: Initialization logic only
|
||||
- Accesses other managers safely
|
||||
- Runs in priority order (controlled by us)
|
||||
- Performs actual setup work
|
||||
|
||||
### Why This Matters
|
||||
|
||||
1. **Deterministic Access**: Any manager can safely access any other manager's instance during `OnManagedAwake()`, regardless of priority.
|
||||
|
||||
2. **Priority Controls Initialization, Not Availability**: Priority determines WHEN initialization happens, not WHEN instances become available.
|
||||
|
||||
3. **No Hidden Dependencies**: You don't need to worry about priority ordering just to access an instance - only for initialization sequencing.
|
||||
|
||||
## Best Practices Going Forward
|
||||
|
||||
### For New ManagedBehaviour Singletons
|
||||
|
||||
Always use this pattern:
|
||||
|
||||
```csharp
|
||||
public class MyManager : ManagedBehaviour
|
||||
{
|
||||
private static MyManager _instance;
|
||||
public static MyManager Instance => _instance;
|
||||
|
||||
public override int ManagedAwakePriority => 50; // Choose appropriate priority
|
||||
|
||||
private new void Awake()
|
||||
{
|
||||
// ALWAYS set instance in Awake()
|
||||
_instance = this;
|
||||
}
|
||||
|
||||
protected override void OnManagedAwake()
|
||||
{
|
||||
// Safe to access other manager instances here
|
||||
var someManager = SomeOtherManager.Instance; // ✓ Always available
|
||||
|
||||
// Do initialization work
|
||||
InitializeMyStuff();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Priority Guidelines
|
||||
|
||||
- **0-10**: Very early (QuickAccess, GameManager)
|
||||
- **10-20**: Core infrastructure (SceneManager, SaveLoad)
|
||||
- **20-40**: Input/Audio infrastructure
|
||||
- **40-60**: UI systems
|
||||
- **60-100**: Game systems (Cards, Items, Puzzles)
|
||||
- **100+**: Scene-specific or specialized systems
|
||||
|
||||
### Common Mistake to Avoid
|
||||
|
||||
❌ **DON'T** set instance in OnManagedAwake():
|
||||
```csharp
|
||||
protected override void OnManagedAwake()
|
||||
{
|
||||
_instance = this; // ❌ WRONG! Causes race conditions
|
||||
}
|
||||
```
|
||||
|
||||
✓ **DO** set instance in Awake():
|
||||
```csharp
|
||||
private new void Awake()
|
||||
{
|
||||
_instance = this; // ✓ CORRECT! Always available
|
||||
}
|
||||
```
|
||||
|
||||
## Testing Checklist
|
||||
|
||||
When adding a new singleton manager:
|
||||
|
||||
- [ ] Instance set in `Awake()`, not `OnManagedAwake()`
|
||||
- [ ] Used `new` keyword on Awake method
|
||||
- [ ] Priority chosen based on when initialization needs to run
|
||||
- [ ] Can safely access other manager instances in `OnManagedAwake()`
|
||||
- [ ] No null reference exceptions on manager access
|
||||
|
||||
## Related Documentation
|
||||
|
||||
- **managed_bejavior.md**: Full ManagedBehaviour lifecycle documentation
|
||||
- **lifecycle_implementation_roadmap.md**: Migration roadmap
|
||||
- **levelswitch_fixes_summary.md**: Related fix for scene loading and input issues
|
||||
|
||||
Reference in New Issue
Block a user