# 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