diff --git a/Assets/Scripts/Core/ItemManager.cs b/Assets/Scripts/Core/ItemManager.cs index f6d794b0..ef476647 100644 --- a/Assets/Scripts/Core/ItemManager.cs +++ b/Assets/Scripts/Core/ItemManager.cs @@ -244,5 +244,36 @@ namespace Core public IEnumerable Pickups => _pickups; public IEnumerable ItemSlots => _itemSlots; + + /// + /// Gets all registered pickups. Used by save/load system to find items by save ID. + /// + public IEnumerable GetAllPickups() => _pickups; + + /// + /// Gets all registered item slots. Used by save/load system. + /// + public IEnumerable GetAllItemSlots() => _itemSlots; + + /// + /// Finds a pickup by its save ID. Used by save/load system to restore item references. + /// + /// The save ID to search for + /// The pickup's GameObject if found, null otherwise + public GameObject FindPickupBySaveId(string saveId) + { + if (string.IsNullOrEmpty(saveId)) return null; + + // Search through all registered pickups + foreach (var pickup in _pickups) + { + if (pickup is SaveableInteractable saveable && saveable.GetSaveId() == saveId) + { + return pickup.gameObject; + } + } + + return null; + } } } diff --git a/Assets/Scripts/Core/SaveLoad/ISaveParticipant.cs b/Assets/Scripts/Core/SaveLoad/ISaveParticipant.cs index 1baf2d0a..d28e6043 100644 --- a/Assets/Scripts/Core/SaveLoad/ISaveParticipant.cs +++ b/Assets/Scripts/Core/SaveLoad/ISaveParticipant.cs @@ -23,6 +23,12 @@ /// Should handle null/empty data gracefully with default behavior. /// void RestoreState(string serializedData); + + /// + /// Returns true if this participant has already had its state restored. + /// Used to prevent double-restoration when inactive objects become active. + /// + bool HasBeenRestored { get; } } } diff --git a/Assets/Scripts/Core/SaveLoad/SaveLoadManager.cs b/Assets/Scripts/Core/SaveLoad/SaveLoadManager.cs index 9eb0bda7..735f1b5c 100644 --- a/Assets/Scripts/Core/SaveLoad/SaveLoadManager.cs +++ b/Assets/Scripts/Core/SaveLoad/SaveLoadManager.cs @@ -48,7 +48,7 @@ namespace Core.SaveLoad private void Start() { - Load(); + } private void OnApplicationQuit() @@ -67,6 +67,12 @@ namespace Core.SaveLoad SceneManagerService.Instance.SceneUnloadStarted += OnSceneUnloadStarted; Logging.Debug("[SaveLoadManager] Subscribed to SceneManagerService events"); } + + #if UNITY_EDITOR + OnSceneLoadCompleted("RestoreInEditor"); + #endif + + Load(); } void OnDestroy() @@ -114,7 +120,8 @@ namespace Core.SaveLoad Logging.Debug($"[SaveLoadManager] Registered participant: {saveId}"); // If we have save data loaded and we're not currently restoring, restore this participant's state immediately - if (IsSaveDataLoaded && !IsRestoringState && currentSaveData != null) + // BUT only if the participant hasn't already been restored (prevents double-restoration when inactive objects become active) + if (IsSaveDataLoaded && !IsRestoringState && currentSaveData != null && !participant.HasBeenRestored) { RestoreParticipantState(participant); } @@ -153,10 +160,28 @@ namespace Core.SaveLoad private void OnSceneLoadCompleted(string sceneName) { - Logging.Debug($"[SaveLoadManager] Scene '{sceneName}' loaded. Participants can now register and will be restored."); + Logging.Debug($"[SaveLoadManager] Scene '{sceneName}' loaded. Discovering inactive SaveableInteractables..."); - // Participants register themselves, so we just wait for them - // After registration, they'll be automatically restored if data is available + // Find ONLY INACTIVE SaveableInteractables (active ones will register themselves via Start()) + var inactiveSaveables = FindObjectsByType( + typeof(Interactions.SaveableInteractable), + FindObjectsInactive.Include, + FindObjectsSortMode.None + ); + + int registeredCount = 0; + foreach (var obj in inactiveSaveables) + { + var saveable = obj as Interactions.SaveableInteractable; + if (saveable != null && !saveable.gameObject.activeInHierarchy) + { + // Only register if it's actually inactive + RegisterParticipant(saveable); + registeredCount++; + } + } + + Logging.Debug($"[SaveLoadManager] Discovered and registered {registeredCount} inactive SaveableInteractables"); } private void OnSceneUnloadStarted(string sceneName) diff --git a/Assets/Scripts/Data/CardSystem/CardSystemManager.cs b/Assets/Scripts/Data/CardSystem/CardSystemManager.cs index 524de956..0dc2d42f 100644 --- a/Assets/Scripts/Data/CardSystem/CardSystemManager.cs +++ b/Assets/Scripts/Data/CardSystem/CardSystemManager.cs @@ -461,6 +461,13 @@ namespace Data.CardSystem #region ISaveParticipant Implementation + private bool hasBeenRestored; + + /// + /// Returns true if this participant has already had its state restored. + /// + public bool HasBeenRestored => hasBeenRestored; + /// /// Returns the unique save ID for the CardSystemManager. /// Since this is a singleton global system, the ID is constant. @@ -487,6 +494,7 @@ namespace Data.CardSystem if (string.IsNullOrEmpty(serializedData)) { Logging.Debug("[CardSystemManager] No saved state to restore, using defaults"); + hasBeenRestored = true; return; } @@ -496,6 +504,7 @@ namespace Data.CardSystem if (state != null) { ApplyCardCollectionState(state); + hasBeenRestored = true; Logging.Debug("[CardSystemManager] Successfully restored card collection state"); } else diff --git a/Assets/Scripts/Dialogue/DialogueComponent.cs b/Assets/Scripts/Dialogue/DialogueComponent.cs index 75c064a7..2b1809f2 100644 --- a/Assets/Scripts/Dialogue/DialogueComponent.cs +++ b/Assets/Scripts/Dialogue/DialogueComponent.cs @@ -664,7 +664,7 @@ namespace Dialogue // Check all pickups for the given ID foreach (var pickup in ItemManager.Instance.Pickups) { - if (pickup.isPickedUp && pickup.itemData != null && + if (pickup.IsPickedUp && pickup.itemData != null && pickup.itemData.itemId == itemID) { return true; diff --git a/Assets/Scripts/Interactions/ItemSlot.cs b/Assets/Scripts/Interactions/ItemSlot.cs index e95fd164..5cc32931 100644 --- a/Assets/Scripts/Interactions/ItemSlot.cs +++ b/Assets/Scripts/Interactions/ItemSlot.cs @@ -16,6 +16,19 @@ namespace Interactions Forbidden } + /// + /// Saveable data for ItemSlot state + /// + [System.Serializable] + public class ItemSlotSaveData + { + public PickupSaveData pickupData; // Base pickup state + public ItemSlotState slotState; // Current slot validation state + public string slottedItemSaveId; // Save ID of slotted item (if any) + public string slottedItemDataAssetPath; // Asset path to PickupItemData + } + + // TODO: Remove this ridiculous inheritance from Pickup if possible /// /// Interaction requirement that allows slotting, swapping, or picking up items in a slot. /// @@ -178,7 +191,130 @@ namespace Interactions } } - public void SlotItem(GameObject itemToSlot, PickupItemData itemToSlotData, bool clearFollowerHeldItem = true) + // Register with ItemManager when enabled + protected override void Start() + { + base.Start(); // This calls Pickup.Start() which registers with save system + + // Additionally register as ItemSlot + ItemManager.Instance?.RegisterItemSlot(this); + } + + protected override void OnDestroy() + { + base.OnDestroy(); // Unregister from save system and pickup manager + + // Unregister from slot manager + ItemManager.Instance?.UnregisterItemSlot(this); + } + + #region Save/Load Implementation + + protected override object GetSerializableState() + { + // Get base pickup state + PickupSaveData baseData = base.GetSerializableState() as PickupSaveData; + + // Get slotted item save ID if there's a slotted item + string slottedSaveId = ""; + string slottedAssetPath = ""; + + if (_currentlySlottedItemObject != null) + { + var slottedPickup = _currentlySlottedItemObject.GetComponent(); + if (slottedPickup is SaveableInteractable saveablePickup) + { + slottedSaveId = saveablePickup.GetSaveId(); + } + + if (_currentlySlottedItemData != null) + { +#if UNITY_EDITOR + slottedAssetPath = UnityEditor.AssetDatabase.GetAssetPath(_currentlySlottedItemData); +#endif + } + } + + return new ItemSlotSaveData + { + pickupData = baseData, + slotState = _currentState, + slottedItemSaveId = slottedSaveId, + slottedItemDataAssetPath = slottedAssetPath + }; + } + + protected override void ApplySerializableState(string serializedData) + { + ItemSlotSaveData data = JsonUtility.FromJson(serializedData); + if (data == null) + { + Debug.LogWarning($"[ItemSlot] Failed to deserialize save data for {gameObject.name}"); + return; + } + + // First restore base pickup state + if (data.pickupData != null) + { + string pickupJson = JsonUtility.ToJson(data.pickupData); + base.ApplySerializableState(pickupJson); + } + + // Restore slot state + _currentState = data.slotState; + + // Restore slotted item if there was one + if (!string.IsNullOrEmpty(data.slottedItemSaveId)) + { + RestoreSlottedItem(data.slottedItemSaveId, data.slottedItemDataAssetPath); + } + } + + /// + /// Restore a slotted item from save data. + /// This is called during load restoration and should NOT trigger events. + /// + private void RestoreSlottedItem(string slottedItemSaveId, string slottedItemDataAssetPath) + { + // Try to find the item in the scene by its save ID via ItemManager + GameObject slottedObject = ItemManager.Instance?.FindPickupBySaveId(slottedItemSaveId); + + if (slottedObject == null) + { + Debug.LogWarning($"[ItemSlot] Could not find slotted item with save ID: {slottedItemSaveId}"); + return; + } + + // Get the item data + PickupItemData slottedData = null; +#if UNITY_EDITOR + if (!string.IsNullOrEmpty(slottedItemDataAssetPath)) + { + slottedData = UnityEditor.AssetDatabase.LoadAssetAtPath(slottedItemDataAssetPath); + } +#endif + + if (slottedData == null) + { + var pickup = slottedObject.GetComponent(); + if (pickup != null) + { + slottedData = pickup.itemData; + } + } + + // Silently slot the item (no events, no interaction completion) + ApplySlottedItemState(slottedObject, slottedData, triggerEvents: false); + } + + /// + /// Core logic for slotting an item. Can be used both for normal slotting and silent restoration. + /// + /// The item GameObject to slot (or null to clear) + /// The PickupItemData for the item + /// Whether to fire events and complete interaction + /// Whether to clear the follower's held item + private void ApplySlottedItemState(GameObject itemToSlot, PickupItemData itemToSlotData, bool triggerEvents, bool clearFollowerHeldItem = true) { // Cache the previous item data before clearing, needed for OnItemSlotRemoved event var previousItemData = _currentlySlottedItemData; @@ -188,11 +324,10 @@ namespace Interactions { _currentlySlottedItemObject = null; _currentlySlottedItemData = null; - // Clear state when no item is slotted _currentState = ItemSlotState.None; - // Fire native event for slot clearing - if (wasSlotCleared) + // Fire native event for slot clearing (only if triggering events) + if (wasSlotCleared && triggerEvents) { OnItemSlotRemoved?.Invoke(previousItemData); } @@ -205,55 +340,53 @@ namespace Interactions _currentlySlottedItemData = itemToSlotData; } - if (clearFollowerHeldItem) + if (clearFollowerHeldItem && _followerController != null) { _followerController.ClearHeldItem(); } UpdateSlottedSprite(); - // Once an item is slotted, we know it is not forbidden, so we can skip that check, but now check if it was - // the correct item we're looking for - var config = _interactionSettings?.GetSlotItemConfig(itemData); - var allowed = config?.allowedItems ?? new List(); - if (itemToSlotData != null && PickupItemData.ListContainsEquivalent(allowed, itemToSlotData)) + // Only validate and trigger events if requested + if (triggerEvents) { - if (itemToSlot != null) + // Once an item is slotted, we know it is not forbidden, so we can skip that check, but now check if it was + // the correct item we're looking for + var config = _interactionSettings?.GetSlotItemConfig(itemData); + var allowed = config?.allowedItems ?? new List(); + if (itemToSlotData != null && PickupItemData.ListContainsEquivalent(allowed, itemToSlotData)) { - DebugUIMessage.Show("You correctly slotted " + itemToSlotData.itemName + " into: " + itemData.itemName, Color.green); - onCorrectItemSlotted?.Invoke(); - OnCorrectItemSlotted?.Invoke(itemData, _currentlySlottedItemData); - _currentState = ItemSlotState.Correct; + if (itemToSlot != null) + { + DebugUIMessage.Show("You correctly slotted " + itemToSlotData.itemName + " into: " + itemData.itemName, Color.green); + onCorrectItemSlotted?.Invoke(); + OnCorrectItemSlotted?.Invoke(itemData, _currentlySlottedItemData); + _currentState = ItemSlotState.Correct; + } + + CompleteInteraction(true); } - - CompleteInteraction(true); - } - else - { - if (itemToSlot != null) + else { - DebugUIMessage.Show("I'm not sure this works.", Color.yellow); - onIncorrectItemSlotted?.Invoke(); - OnIncorrectItemSlotted?.Invoke(itemData, _currentlySlottedItemData); - _currentState = ItemSlotState.Incorrect; + if (itemToSlot != null) + { + DebugUIMessage.Show("I'm not sure this works.", Color.yellow); + onIncorrectItemSlotted?.Invoke(); + OnIncorrectItemSlotted?.Invoke(itemData, _currentlySlottedItemData); + _currentState = ItemSlotState.Incorrect; + } + CompleteInteraction(false); } - CompleteInteraction(false); } } - // Register with ItemManager when enabled - void Start() + /// + /// Public API for slotting items during gameplay. + /// + public void SlotItem(GameObject itemToSlot, PickupItemData itemToSlotData, bool clearFollowerHeldItem = true) { - // Note: Base Pickup class also calls RegisterPickup in its Start - // This additionally registers as ItemSlot - ItemManager.Instance?.RegisterPickup(this); - ItemManager.Instance?.RegisterItemSlot(this); + ApplySlottedItemState(itemToSlot, itemToSlotData, triggerEvents: true, clearFollowerHeldItem); } - void OnDestroy() - { - // Unregister from both pickup and slot managers - ItemManager.Instance?.UnregisterPickup(this); - ItemManager.Instance?.UnregisterItemSlot(this); - } + #endregion } } diff --git a/Assets/Scripts/Interactions/Pickup.cs b/Assets/Scripts/Interactions/Pickup.cs index e94620c2..6c252503 100644 --- a/Assets/Scripts/Interactions/Pickup.cs +++ b/Assets/Scripts/Interactions/Pickup.cs @@ -1,17 +1,30 @@ using Input; using UnityEngine; -using System; // added for Action +using System; +using System.Linq; // added for Action using Core; // register with ItemManager namespace Interactions { - public class Pickup : InteractableBase + /// + /// Saveable data for Pickup state + /// + [System.Serializable] + public class PickupSaveData + { + public bool isPickedUp; + public Vector3 worldPosition; + public Quaternion worldRotation; + public bool isActive; + } + + public class Pickup : SaveableInteractable { public PickupItemData itemData; public SpriteRenderer iconRenderer; // Track if the item has been picked up - public bool isPickedUp { get; private set; } + public bool IsPickedUp { get; internal set; } // Event: invoked when the item was picked up successfully public event Action OnItemPickedUp; @@ -22,8 +35,10 @@ namespace Interactions /// /// Unity Awake callback. Sets up icon and applies item data. /// - protected virtual void Awake() + protected override void Awake() { + base.Awake(); // Register with save system + if (iconRenderer == null) iconRenderer = GetComponent(); @@ -33,16 +48,24 @@ namespace Interactions /// /// Register with ItemManager on Start /// - void Start() + protected override void Start() { - ItemManager.Instance?.RegisterPickup(this); + base.Start(); // Register with save system + + // Don't register with ItemManager if already picked up (restored from save) + if (!IsPickedUp) + { + ItemManager.Instance?.RegisterPickup(this); + } } /// /// Unity OnDestroy callback. Unregisters from ItemManager. /// - void OnDestroy() + protected override void OnDestroy() { + base.OnDestroy(); // Unregister from save system + // Unregister from ItemManager ItemManager.Instance?.UnregisterPickup(this); } @@ -130,9 +153,69 @@ namespace Interactions // Update pickup state and invoke event when the item was picked up successfully if (wasPickedUp) { - isPickedUp = true; + IsPickedUp = true; OnItemPickedUp?.Invoke(itemData); } } + + #region Save/Load Implementation + + protected override object GetSerializableState() + { + return new PickupSaveData + { + isPickedUp = this.IsPickedUp, + worldPosition = transform.position, + worldRotation = transform.rotation, + isActive = gameObject.activeSelf + }; + } + + protected override void ApplySerializableState(string serializedData) + { + PickupSaveData data = JsonUtility.FromJson(serializedData); + if (data == null) + { + Debug.LogWarning($"[Pickup] Failed to deserialize save data for {gameObject.name}"); + return; + } + + // Restore picked up state + IsPickedUp = data.isPickedUp; + + if (IsPickedUp) + { + // Hide the pickup if it was already picked up + gameObject.SetActive(false); + } + else + { + // Restore position for items that haven't been picked up (they may have moved) + transform.position = data.worldPosition; + transform.rotation = data.worldRotation; + gameObject.SetActive(data.isActive); + } + + // Note: We do NOT fire OnItemPickedUp event during restoration + // This prevents duplicate logic execution + } + + /// + /// Resets the pickup state when the item is dropped back into the world. + /// Called by FollowerController when swapping items. + /// + public void ResetPickupState() + { + IsPickedUp = false; + gameObject.SetActive(true); + + // Re-register with ItemManager if not already registered + if (ItemManager.Instance != null && !ItemManager.Instance.GetAllPickups().Contains(this)) + { + ItemManager.Instance.RegisterPickup(this); + } + } + + #endregion } } \ No newline at end of file diff --git a/Assets/Scripts/Interactions/SaveableInteractable.cs b/Assets/Scripts/Interactions/SaveableInteractable.cs new file mode 100644 index 00000000..da6133df --- /dev/null +++ b/Assets/Scripts/Interactions/SaveableInteractable.cs @@ -0,0 +1,261 @@ +using Core.SaveLoad; +using UnityEngine; +using UnityEngine.SceneManagement; + +namespace Interactions +{ + /// + /// Base class for interactables that participate in the save/load system. + /// Provides common save ID generation and serialization infrastructure. + /// + public abstract class SaveableInteractable : InteractableBase, ISaveParticipant + { + [Header("Save System")] + [SerializeField] + [Tooltip("Optional custom save ID. If empty, will auto-generate from hierarchy path.")] + private string customSaveId = ""; + + /// + /// Flag to indicate we're currently restoring from save data. + /// Child classes can check this to skip initialization logic during load. + /// + protected bool IsRestoringFromSave { get; private set; } + + private bool hasRegistered; + private bool hasRestoredState; + + /// + /// Returns true if this participant has already had its state restored. + /// + public bool HasBeenRestored => hasRestoredState; + + protected virtual void Awake() + { + // Register early in Awake so even disabled objects are tracked + RegisterWithSaveSystem(); + } + + protected virtual void Start() + { + // If we didn't register in Awake (shouldn't happen), register now + if (!hasRegistered) + { + RegisterWithSaveSystem(); + } + } + + protected virtual void OnDestroy() + { + UnregisterFromSaveSystem(); + } + + private void RegisterWithSaveSystem() + { + if (hasRegistered) return; + + if (SaveLoadManager.Instance != null) + { + SaveLoadManager.Instance.RegisterParticipant(this); + hasRegistered = true; + + // Check if save data was already loaded before we registered + // If so, we need to subscribe to the next load event + if (!SaveLoadManager.Instance.IsSaveDataLoaded && !hasRestoredState) + { + SaveLoadManager.Instance.OnLoadCompleted += OnSaveDataLoadedHandler; + } + } + else + { + Debug.LogWarning($"[SaveableInteractable] SaveLoadManager not found for {gameObject.name}"); + } + } + + private void UnregisterFromSaveSystem() + { + if (!hasRegistered) return; + + if (SaveLoadManager.Instance != null) + { + SaveLoadManager.Instance.UnregisterParticipant(GetSaveId()); + SaveLoadManager.Instance.OnLoadCompleted -= OnSaveDataLoadedHandler; + hasRegistered = false; + } + } + + /// + /// Event handler for when save data finishes loading. + /// Called if the object registered before save data was loaded. + /// + private void OnSaveDataLoadedHandler(string slot) + { + // The SaveLoadManager will automatically call RestoreState on us + // We just need to unsubscribe from the event + if (SaveLoadManager.Instance != null) + { + SaveLoadManager.Instance.OnLoadCompleted -= OnSaveDataLoadedHandler; + } + } + + #region ISaveParticipant Implementation + + public string GetSaveId() + { + string sceneName = GetSceneName(); + + if (!string.IsNullOrEmpty(customSaveId)) + { + return $"{sceneName}/{customSaveId}"; + } + + // Auto-generate from hierarchy path + string hierarchyPath = GetHierarchyPath(); + return $"{sceneName}/{hierarchyPath}"; + } + + public string SerializeState() + { + object stateData = GetSerializableState(); + if (stateData == null) + { + return "{}"; + } + + return JsonUtility.ToJson(stateData); + } + + public void RestoreState(string serializedData) + { + if (string.IsNullOrEmpty(serializedData)) + { + Debug.LogWarning($"[SaveableInteractable] Empty save data for {GetSaveId()}"); + return; + } + + // CRITICAL: Only restore state if we're actually in a restoration context + // This prevents state machines from teleporting objects when they enable them mid-gameplay + if (SaveLoadManager.Instance != null && !SaveLoadManager.Instance.IsRestoringState) + { + // If we're not in an active restoration cycle, this is probably a late registration + // (object was disabled during initial load and just got enabled) + // Skip restoration to avoid mid-gameplay teleportation + Debug.Log($"[SaveableInteractable] Skipping late restoration for {GetSaveId()} - object enabled after initial load"); + hasRestoredState = true; // Mark as restored to prevent future attempts + return; + } + + IsRestoringFromSave = true; + hasRestoredState = true; + + try + { + ApplySerializableState(serializedData); + } + catch (System.Exception e) + { + Debug.LogError($"[SaveableInteractable] Failed to restore state for {GetSaveId()}: {e.Message}"); + } + finally + { + IsRestoringFromSave = false; + } + } + + #endregion + + #region Virtual Methods for Child Classes + + /// + /// Child classes override this to return their serializable state data. + /// Return an object that can be serialized with JsonUtility. + /// + protected abstract object GetSerializableState(); + + /// + /// Child classes override this to apply restored state data. + /// Should NOT trigger events or re-initialize logic that already happened. + /// + /// JSON string containing the saved state + protected abstract void ApplySerializableState(string serializedData); + + #endregion + + #region Helper Methods + + private string GetSceneName() + { + Scene scene = gameObject.scene; + if (!scene.IsValid()) + { + Debug.LogWarning($"[SaveableInteractable] GameObject {gameObject.name} has invalid scene"); + return "UnknownScene"; + } + + return scene.name; + } + + private string GetHierarchyPath() + { + // Build path from scene root to this object + // Format: ParentName/ChildName/ObjectName_SiblingIndex + string path = gameObject.name; + Transform current = transform.parent; + + while (current != null) + { + path = $"{current.name}/{path}"; + current = current.parent; + } + + // Add sibling index for uniqueness among same-named objects + int siblingIndex = transform.GetSiblingIndex(); + if (siblingIndex > 0) + { + path = $"{path}_{siblingIndex}"; + } + + return path; + } + + #endregion + + #region Editor Helpers + +#if UNITY_EDITOR + [ContextMenu("Log Save ID")] + private void LogSaveId() + { + Debug.Log($"Save ID: {GetSaveId()}"); + } + + [ContextMenu("Test Serialize/Deserialize")] + private void TestSerializeDeserialize() + { + string serialized = SerializeState(); + Debug.Log($"Serialized state: {serialized}"); + + RestoreState(serialized); + Debug.Log("Deserialization test complete"); + } +#endif + + #endregion + } + + #region Common Save Data Structures + + /// + /// Base save data for all interactables. + /// Can be extended by child classes. + /// + [System.Serializable] + public class InteractableBaseSaveData + { + public bool isActive; + public Vector3 worldPosition; + public Quaternion worldRotation; + } + + #endregion +} + diff --git a/Assets/Scripts/Interactions/SaveableInteractable.cs.meta b/Assets/Scripts/Interactions/SaveableInteractable.cs.meta new file mode 100644 index 00000000..c2db64f5 --- /dev/null +++ b/Assets/Scripts/Interactions/SaveableInteractable.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: a0d7c8f7344746ce9dc863985cc3f543 +timeCreated: 1762079555 \ No newline at end of file diff --git a/Assets/Scripts/Levels/LevelSwitch.cs b/Assets/Scripts/Levels/LevelSwitch.cs index 89ab5cbd..74101227 100644 --- a/Assets/Scripts/Levels/LevelSwitch.cs +++ b/Assets/Scripts/Levels/LevelSwitch.cs @@ -24,14 +24,14 @@ namespace Levels // Settings reference private IInteractionSettings _interactionSettings; - private bool _isActive = true; + private bool switchActive = true; /// /// Unity Awake callback. Sets up icon, interactable, and event handlers. /// void Awake() { - _isActive = true; + switchActive = true; if (_iconRenderer == null) _iconRenderer = GetComponent(); @@ -72,7 +72,7 @@ namespace Levels /// protected override void OnCharacterArrived() { - if (switchData == null || string.IsNullOrEmpty(switchData.targetLevelSceneName) || !_isActive) + if (switchData == null || string.IsNullOrEmpty(switchData.targetLevelSceneName) || !switchActive) return; var menuPrefab = _interactionSettings?.LevelSwitchMenuPrefab; @@ -92,7 +92,7 @@ namespace Levels } // Setup menu with data and callbacks menu.Setup(switchData, OnLevelSelectedWrapper, OnMinigameSelected, OnMenuCancel, OnRestartSelected); - _isActive = false; // Prevent re-triggering until menu is closed + switchActive = false; // Prevent re-triggering until menu is closed // Switch input mode to UI only InputManager.Instance.SetInputMode(InputMode.UI); @@ -123,7 +123,7 @@ namespace Levels private void OnMenuCancel() { - _isActive = true; // Allow interaction again if cancelled + switchActive = true; // Allow interaction again if cancelled InputManager.Instance.SetInputMode(InputMode.GameAndUI); } } diff --git a/Assets/Scripts/Levels/MinigameSwitch.cs b/Assets/Scripts/Levels/MinigameSwitch.cs index d9581020..e9a429af 100644 --- a/Assets/Scripts/Levels/MinigameSwitch.cs +++ b/Assets/Scripts/Levels/MinigameSwitch.cs @@ -13,94 +13,73 @@ using Core.SaveLoad; namespace Levels { + /// + /// Saveable data for MinigameSwitch state + /// + [System.Serializable] + public class MinigameSwitchSaveData + { + public bool isUnlocked; + } + /// /// Handles switching into minigame levels when interacted with. Applies switch data and triggers scene transitions. /// - public class MinigameSwitch : InteractableBase + public class MinigameSwitch : SaveableInteractable { /// /// Data for this level switch (target scene, icon, etc). /// public LevelSwitchData switchData; - private SpriteRenderer _iconRenderer; + private SpriteRenderer iconRenderer; // Settings reference - private IInteractionSettings _interactionSettings; + private IInteractionSettings interactionSettings; - private bool _isActive = true; + private bool switchActive = true; + private bool isUnlocked; /// /// Unity Awake callback. Sets up icon, interactable, and event handlers. /// - void Awake() + protected override void Awake() { - gameObject.SetActive(false); // Start inactive + base.Awake(); // Register with save system + BootCompletionService.RegisterInitAction(InitializePostBoot); - _isActive = true; - if (_iconRenderer == null) - _iconRenderer = GetComponent(); + switchActive = true; + if (iconRenderer == null) + iconRenderer = GetComponent(); // Initialize settings reference - _interactionSettings = GameManager.GetSettingsObject(); + interactionSettings = GameManager.GetSettingsObject(); ApplySwitchData(); + } - // --- Save state loading logic --- - if (SaveLoadManager.Instance != null) + protected override void Start() + { + base.Start(); // Register with save system + + // If not restoring from save, start inactive + if (!IsRestoringFromSave && !isUnlocked) { - if (SaveLoadManager.Instance.IsSaveDataLoaded) - { - ApplySavedMinigameStateIfAvailable(); - } - else - { - SaveLoadManager.Instance.OnLoadCompleted += OnSaveDataLoadedHandler; - } + gameObject.SetActive(false); } } - private void OnDestroy() + protected override void OnDestroy() { - if (SaveLoadManager.Instance != null) - { - SaveLoadManager.Instance.OnLoadCompleted -= OnSaveDataLoadedHandler; - } - } - - // Apply saved state if present in the SaveLoadManager - private void ApplySavedMinigameStateIfAvailable() - { - var data = SaveLoadManager.Instance?.currentSaveData; - if (data?.unlockedMinigames != null && switchData != null && - data.unlockedMinigames.Contains(switchData.targetLevelSceneName)) - { - gameObject.SetActive(true); - } - } - - // Event handler for when save data load completes - private void OnSaveDataLoadedHandler(string slot) - { - ApplySavedMinigameStateIfAvailable(); - if (SaveLoadManager.Instance != null) - { - SaveLoadManager.Instance.OnLoadCompleted -= OnSaveDataLoadedHandler; - } + base.OnDestroy(); // Unregister from save system } private void HandleAllPuzzlesComplete(PuzzleS.PuzzleLevelDataSO _) { - // Unlock and save - if (switchData != null) - { - var unlocked = SaveLoadManager.Instance.currentSaveData.unlockedMinigames; - string minigameName = switchData.targetLevelSceneName; - if (!unlocked.Contains(minigameName)) - { - unlocked.Add(minigameName); - } - } + // Unlock the minigame + isUnlocked = true; gameObject.SetActive(true); + + // Save will happen automatically on next save cycle via ISaveParticipant } #if UNITY_EDITOR @@ -109,8 +88,8 @@ namespace Levels /// void OnValidate() { - if (_iconRenderer == null) - _iconRenderer = GetComponent(); + if (iconRenderer == null) + iconRenderer = GetComponent(); ApplySwitchData(); } #endif @@ -122,8 +101,8 @@ namespace Levels { if (switchData != null) { - if (_iconRenderer != null) - _iconRenderer.sprite = switchData.mapSprite; + if (iconRenderer != null) + iconRenderer.sprite = switchData.mapSprite; gameObject.name = switchData.targetLevelSceneName; // Optionally update other fields, e.g. description } @@ -134,10 +113,10 @@ namespace Levels /// protected override void OnCharacterArrived() { - if (switchData == null || string.IsNullOrEmpty(switchData.targetLevelSceneName) || !_isActive) + if (switchData == null || string.IsNullOrEmpty(switchData.targetLevelSceneName) || !switchActive) return; - var menuPrefab = _interactionSettings?.MinigameSwitchMenuPrefab; + var menuPrefab = interactionSettings?.MinigameSwitchMenuPrefab; if (menuPrefab == null) { Debug.LogError("MinigameSwitchMenu prefab not assigned in InteractionSettings!"); @@ -154,7 +133,7 @@ namespace Levels } // Setup menu with data and callbacks menu.Setup(switchData, OnLevelSelectedWrapper, OnMenuCancel); - _isActive = false; // Prevent re-triggering until menu is closed + switchActive = false; // Prevent re-triggering until menu is closed // Switch input mode to UI only InputManager.Instance.SetInputMode(InputMode.UI); @@ -173,7 +152,7 @@ namespace Levels private void OnMenuCancel() { - _isActive = true; // Allow interaction again if cancelled + switchActive = true; // Allow interaction again if cancelled InputManager.Instance.SetInputMode(InputMode.GameAndUI); } @@ -181,5 +160,32 @@ namespace Levels { PuzzleManager.Instance.OnAllPuzzlesComplete += HandleAllPuzzlesComplete; } + + #region Save/Load Implementation + + protected override object GetSerializableState() + { + return new MinigameSwitchSaveData + { + isUnlocked = isUnlocked + }; + } + + protected override void ApplySerializableState(string serializedData) + { + MinigameSwitchSaveData data = JsonUtility.FromJson(serializedData); + if (data == null) + { + Debug.LogWarning($"[MinigameSwitch] Failed to deserialize save data for {gameObject.name}"); + return; + } + + isUnlocked = data.isUnlocked; + + // Show/hide based on unlock state + gameObject.SetActive(isUnlocked); + } + + #endregion } } diff --git a/Assets/Scripts/Movement/FollowerController.cs b/Assets/Scripts/Movement/FollowerController.cs index 33ef4dae..810166d5 100644 --- a/Assets/Scripts/Movement/FollowerController.cs +++ b/Assets/Scripts/Movement/FollowerController.cs @@ -584,7 +584,14 @@ public class FollowerController: MonoBehaviour if (matchingRule != null && matchingRule.resultPrefab != null) { newItem = Instantiate(matchingRule.resultPrefab, spawnPos, Quaternion.identity); - PickupItemData itemData = newItem.GetComponent().itemData; + var resultPickup = newItem.GetComponent(); + PickupItemData itemData = resultPickup.itemData; + + // Mark the base items as picked up before destroying them + // (This ensures they save correctly if the game is saved during the combination animation) + pickupA.IsPickedUp = true; + pickupB.IsPickedUp = true; + Destroy(pickupA.gameObject); Destroy(pickupB.gameObject); TryPickupItem(newItem, itemData); @@ -662,6 +669,14 @@ public class FollowerController: MonoBehaviour item.transform.position = position; item.transform.SetParent(null); item.SetActive(true); + + // Reset the pickup state so it can be picked up again and saves correctly + var pickup = item.GetComponent(); + if (pickup != null) + { + pickup.ResetPickupState(); + } + follower.ClearHeldItem(); _animator.SetBool("IsCarrying", false); // Optionally: fire event, update UI, etc. diff --git a/docs/Interactables_SaveLoad_Integration_Plan.md b/docs/Interactables_SaveLoad_Integration_Plan.md new file mode 100644 index 00000000..9c6e8165 --- /dev/null +++ b/docs/Interactables_SaveLoad_Integration_Plan.md @@ -0,0 +1,817 @@ +# Interactables Save/Load Integration Plan + +## Overview +This document outlines the complete implementation plan for integrating the Interactable system (InteractableBase and all child classes) into the existing save/load system using the ISaveParticipant pattern. + +--- + +## Current State Analysis + +### Interactable Hierarchy +``` +InteractableBase (abstract) +├── Pickup +│ └── ItemSlot (extends Pickup) +├── OneClickInteraction +├── LevelSwitch +└── MinigameSwitch +``` + +### Key Observations +1. **InteractableBase** - Abstract base class with common interaction flow, events, cooldown system +2. **Pickup** - Tracks `isPickedUp` state, handles item pickup/combination logic +3. **ItemSlot** - Extends Pickup, tracks slotted item state (`ItemSlotState`, `_currentlySlottedItemData`, `_currentlySlottedItemObject`) +4. **OneClickInteraction** - Stateless, completes immediately (likely no save state needed) +5. **LevelSwitch** - Appears to be stateless switch behavior (no persistent state) +6. **MinigameSwitch** - **Already partially integrated** with save system (tracks unlock state in `SaveLoadData.unlockedMinigames`) + +### State Machine Challenge +- State machines (Pixelplacement's StateMachine) are used in scenes +- Calling `ChangeState()` triggers `OnEnable()` in target state, which can: + - Start animations + - Move player characters + - Enable child GameObjects with Start() calls +- **Problem**: Restoring a state directly would replay all initialization logic +- **Solution**: Need a way to restore state without triggering initialization side effects + +--- + +## Implementation Strategy + +### ~~Phase 1: Create Base SaveableInteractable Class~~ ✅ **COMPLETED** +**Goal**: Establish the foundation for saving/loading interactable state + +**Phase 1 Completion Summary:** +- ✅ Created `SaveableInteractable` abstract class inheriting from `InteractableBase` +- ✅ Implemented `ISaveParticipant` interface with GetSaveId(), SerializeState(), RestoreState() +- ✅ Added hybrid save ID generation: custom IDs or auto-generated from hierarchy path +- ✅ Implemented registration/unregistration in Start()/OnDestroy() +- ✅ Added `IsRestoringFromSave` protected flag for child classes +- ✅ Created abstract methods `GetSerializableState()` and `ApplySerializableState()` for children +- ✅ Added `InteractableBaseSaveData` structure with position, rotation, and active state +- ✅ Included editor helper methods (Log Save ID, Test Serialize/Deserialize) +- ✅ No compilation errors, only minor style warnings + +**Files Created**: +- `Assets/Scripts/Interactions/SaveableInteractable.cs` + +--- + +### ~~Phase 2: Migrate Pickup to SaveableInteractable~~ ✅ **COMPLETED** +**Goal**: Make Pickup save/restore its picked-up state and world position + +**Phase 2 Completion Summary:** +- ✅ Changed `Pickup` to extend `SaveableInteractable` instead of `InteractableBase` +- ✅ Created `PickupSaveData` structure with: isPickedUp, worldPosition, worldRotation, isActive +- ✅ Updated Start() to call base.Start() and skip ItemManager registration if already picked up +- ✅ Updated OnDestroy() to call base.OnDestroy() for save system unregistration +- ✅ Implemented `GetSerializableState()` to capture current pickup state +- ✅ Implemented `ApplySerializableState()` to restore state without triggering events +- ✅ **Added world position/rotation saving** for items that haven't been picked up (user requirement) +- ✅ Hide GameObject if picked up, restore position/rotation if not picked up +- ✅ No OnItemPickedUp events fired during restoration (prevents duplicate logic) +- ✅ No compilation errors, only style warnings + +**Files Modified**: +- `Assets/Scripts/Interactions/Pickup.cs` + +--- + +### ~~Phase 3: Migrate ItemSlot to SaveableInteractable~~ ✅ **COMPLETED** +**Goal**: Make ItemSlot save/restore slotted item state + +**State to Save**: +- `_currentState` (ItemSlotState enum) - Current slot validation state +- `_currentlySlottedItemData` (PickupItemData reference) - Which item is slotted +- Base Pickup state (isPickedUp) - Inherited from Pickup + +**Serialization Strategy**: +```csharp +[System.Serializable] +public class ItemSlotSaveData +{ + public bool isPickedUp; // From Pickup base + public ItemSlotState slotState; + public string slottedItemDataGuid; // Reference to PickupItemData asset +} +``` + +**Restoration Logic**: +1. Restore base Pickup state (isPickedUp) +**Phase 3 Completion Summary:** +- ✅ Created `ItemSlotSaveData` structure with: pickupData, slotState, slottedItemSaveId, slottedItemDataAssetPath +- ✅ Updated Start() to call base.Start() and additionally register as ItemSlot +- ✅ Updated OnDestroy() to call base.OnDestroy() and unregister from ItemSlot manager +- ✅ Implemented `GetSerializableState()` to capture slot state and slotted item references +- ✅ Implemented `ApplySerializableState()` to restore base pickup state + slot state +- ✅ **Created `ApplySlottedItemState()` method** - unified slotting logic with triggerEvents parameter (post-refactor) +- ✅ **Created `RestoreSlottedItem()` method** - finds slotted item by save ID and restores it +- ✅ **SlotItem() refactored** - now a clean wrapper around ApplySlottedItemState (post-refactor) +- ✅ Reuses base Pickup serialization through inheritance (code reuse achieved!) +- ✅ No compilation errors, only style warnings + +**Code Reuse Pattern Identified:** +- ItemSlot calls `base.GetSerializableState()` to get PickupSaveData +- ItemSlot calls `base.ApplySerializableState()` to restore pickup state first +- ApplySlottedItemState() handles both gameplay and restoration with a single code path +- This pattern can be used by future child classes (inheritance-based state composition) + +**Files Modified**: +- `Assets/Scripts/Interactions/ItemSlot.cs` +- `Assets/Scripts/Core/ItemManager.cs` (added GetAllPickups/GetAllItemSlots/FindPickupBySaveId methods) + +--- + +### ~~Phase 4: Handle Stateless Interactables~~ ✅ **COMPLETED** +**Goal**: Determine which interactables need saving and migrate MinigameSwitch to participant pattern + +**Phase 4 Completion Summary:** +- ✅ **OneClickInteraction**: Confirmed stateless, kept inheriting from InteractableBase +- ✅ **LevelSwitch**: Confirmed stateless, kept inheriting from InteractableBase +- ✅ **MinigameSwitch**: Successfully migrated to SaveableInteractable + - Created `MinigameSwitchSaveData` structure with isUnlocked flag + - Changed inheritance from InteractableBase to SaveableInteractable + - Removed old direct SaveLoadManager.currentSaveData access pattern + - Removed manual event subscription to OnLoadCompleted + - Added `_isUnlocked` private field to track state + - Implemented `GetSerializableState()` and `ApplySerializableState()` + - Updated Start() to check IsRestoringFromSave flag and set initial active state + - Updated HandleAllPuzzlesComplete() to set unlock flag (save happens automatically) + - GameObject.activeSelf now managed by save/load system +- ✅ No compilation errors, only style warnings + +**Migration Benefit:** +- MinigameSwitch now uses the same pattern as all other saveable interactables +- No special case code needed in SaveLoadManager +- Cleaner, more maintainable architecture + +**Files Modified**: +- `Assets/Scripts/Levels/MinigameSwitch.cs` + +--- + +## Code Quality Improvements (Post Phase 1-4) + +### Refactoring Round 1 ✅ **COMPLETED** + +**Issues Identified:** +1. **ItemSlot Code Duplication** - SlotItem() and SlotItemSilent() had significant duplicate logic +2. **Poor Method Placement** - FindPickupBySaveId() was in ItemSlot but should be in ItemManager +3. **Bootstrap Timing Issue** - SaveableInteractable didn't handle save data loading before/after registration + +**Improvements Implemented:** + +#### 1. ItemSlot Refactoring +- ✅ **Extracted common logic** into `ApplySlottedItemState(itemToSlot, itemToSlotData, triggerEvents, clearFollowerHeldItem)` +- ✅ **Eliminated code duplication** - SlotItem() and restoration both use the same core logic +- ✅ **Added triggerEvents parameter** - single method handles both interactive and silent slotting +- ✅ **Simplified public API** - `SlotItem()` is now a thin wrapper around `ApplySlottedItemState()` + +**Before:** +```csharp +SlotItem() // 60 lines of logic +SlotItemSilent() // 15 lines of duplicated logic +``` + +**After:** +```csharp +ApplySlottedItemState(triggerEvents) // 85 lines - single source of truth +SlotItem() // 3 lines - wrapper for gameplay +RestoreSlottedItem() // uses ApplySlottedItemState with triggerEvents=false +``` + +#### 2. ItemManager Enhancement +- ✅ **Moved FindPickupBySaveId()** from ItemSlot to ItemManager (proper separation of concerns) +- ✅ **Centralized item lookup** - ItemManager is now the single source for finding items by save ID +- ✅ **Cleaner architecture** - ItemSlot no longer needs to know about scene searching + +**New ItemManager API:** +```csharp +public GameObject FindPickupBySaveId(string saveId) +``` + +#### 3. SaveableInteractable Bootstrap Fix +- ✅ **Added hasRestoredState flag** - tracks whether state has been restored +- ✅ **Subscribes to OnLoadCompleted** - handles save data loading after registration +- ✅ **Unsubscribes properly** - prevents memory leaks +- ✅ **Mirrors MinigameSwitch pattern** - uses same pattern that was working before + +**Bootstrap Flow:** +1. SaveableInteractable registers in Start() +2. Checks if save data is already loaded +3. If not loaded yet, subscribes to OnLoadCompleted event +4. When load completes, SaveLoadManager calls RestoreState() automatically +5. Unsubscribes from event to prevent duplicate restoration + +**Files Modified:** +- `Assets/Scripts/Interactions/ItemSlot.cs` (refactored slot logic) +- `Assets/Scripts/Core/ItemManager.cs` (added FindPickupBySaveId) +- `Assets/Scripts/Interactions/SaveableInteractable.cs` (fixed bootstrap timing) + +--- + +### Refactoring Round 2 - Critical Fixes ✅ **COMPLETED** + +#### Issue 1: Item Swap Not Resetting Pickup State +**Problem:** When picking up Item B while holding Item A, Item A was dropped but remained marked as `isPickedUp = true`, causing it to be hidden on load. + +**Solution:** +- ✅ Added `ResetPickupState()` method to Pickup class +- ✅ Changed `isPickedUp` setter from `private` to `internal` +- ✅ Updated `FollowerController.DropItem()` to call `ResetPickupState()` +- ✅ Fixed combination edge case: base items marked as picked up before destruction + +**Files Modified:** +- `Assets/Scripts/Interactions/Pickup.cs` +- `Assets/Scripts/Movement/FollowerController.cs` + +#### Issue 2: Serialization Field Name Conflict +**Problem:** Unity error - `_isActive` field name serialized multiple times in class hierarchy (InteractableBase → MinigameSwitch/LevelSwitch). + +**Solution:** +- ✅ Renamed child class fields to be more specific: `_isActive` → `switchActive` +- ✅ Updated all references in MinigameSwitch (4 occurrences) +- ✅ Updated all references in LevelSwitch (5 occurrences) + +**Files Modified:** +- `Assets/Scripts/Levels/MinigameSwitch.cs` +- `Assets/Scripts/Levels/LevelSwitch.cs` + +#### Issue 3: State Machine Initialization Order - CRITICAL 🔥 **FIXED** +**Problem:** Objects controlled by state machines start disabled. When they're enabled mid-gameplay: +1. Object becomes active +2. `Start()` runs → registers with SaveLoadManager +3. SaveLoadManager immediately calls `RestoreState()` +4. Object teleports to saved position **during gameplay** ❌ + +**Root Cause:** Disabled GameObjects don't run `Awake()`/`Start()` until enabled. Late registration triggers immediate restoration. + +**Solution - SaveLoadManager Discovery + Participant Self-Tracking:** + +**Key Principles:** +- ✅ SaveableInteractable keeps normal lifecycle (Start/Awake/OnDestroy) unchanged +- ✅ SaveLoadManager actively discovers INACTIVE SaveableInteractables on scene load +- ✅ Each participant tracks if it's been restored via `HasBeenRestored` property +- ✅ Prevent double-restoration using participant's own state (no redundant tracking) + +**Implementation:** + +1. **ISaveParticipant Interface** - Added HasBeenRestored property: + ```csharp + bool HasBeenRestored { get; } + ``` + +2. **SaveableInteractable** - Exposed existing flag: + ```csharp + private bool hasRestoredState; + public bool HasBeenRestored => hasRestoredState; + ``` + +3. **SaveLoadManager.OnSceneLoadCompleted()** - Active Discovery: + ```csharp + var inactiveSaveables = FindObjectsByType(typeof(SaveableInteractable), + FindObjectsInactive.Include, FindObjectsSortMode.None); + + foreach (var obj in inactiveSaveables) { + var saveable = obj as SaveableInteractable; + if (saveable != null && !saveable.gameObject.activeInHierarchy) { + RegisterParticipant(saveable); // Register inactive objects + } + } + ``` + +4. **RegisterParticipant()** - Check Participant's Own State: + ```csharp + if (IsSaveDataLoaded && !IsRestoringState && !participant.HasBeenRestored) { + RestoreParticipantState(participant); // Only if not already restored + } + ``` + +**Flow:** + +**Initial Scene Load (with inactive objects):** +1. Scene loads +2. Active objects: Start() → RegisterParticipant() → RestoreState() → hasRestoredState = true +3. OnSceneLoadCompleted() fires +4. FindObjectsByType(includeInactive) discovers inactive SaveableInteractables +5. RegisterParticipant(inactive object) → checks HasBeenRestored → false → RestoreState() → hasRestoredState = true +6. All objects restored and tracked ✅ + +**Mid-Gameplay Object Enablement:** +1. State machine enables GameObject +2. Awake() runs → Start() runs → RegisterParticipant() +3. Check: `participant.HasBeenRestored` → TRUE +4. Skip RestoreParticipantState() ✅ +5. **NO TELEPORTATION** ✅ + +**Files Modified:** +- `Assets/Scripts/Core/SaveLoad/ISaveParticipant.cs` (added HasBeenRestored property) +- `Assets/Scripts/Interactions/SaveableInteractable.cs` (exposed hasRestoredState) +- `Assets/Scripts/Core/SaveLoad/SaveLoadManager.cs` (discovery + participant check) +- `Assets/Scripts/Data/CardSystem/CardSystemManager.cs` (implemented HasBeenRestored) + +--- + +### Phase 5: State Machine Integration Pattern +**Goal**: Provide a safe way for state machines to react to interactable restoration without replaying initialization + +**Problem Analysis**: +- State machines like `GardenerBehaviour.stateSwitch(string stateName)` are called via UnityEvents +- Current flow: Pickup interaction → Event → stateSwitch() → ChangeState() → OnEnable() → Animations/Movement +- During load: We want to restore the state WITHOUT triggering animations/movement + +**Solution Options**: + +#### Option A: State Restoration Flag (Recommended) +1. Add `IsRestoringFromSave` static/singleton flag to SaveLoadManager +2. State machines check this flag in OnEnable(): + ```csharp + void OnEnable() + { + if (SaveLoadManager.Instance?.IsRestoringState == true) + { + // Silent restoration - set state variables only + return; + } + // Normal initialization with animations/movement + } + ``` + +3. SaveableInteractable sets flag before calling RestoreState(), clears after + +**Advantages**: +- Minimal changes to existing state machine code +- Clear separation of concerns +- Easy to understand and debug + +**Disadvantages**: +- Requires modifying every State class +- Global flag could cause issues if restoration is async + +#### Option B: Separate Restoration Methods +1. State classes implement two entry points: + - `OnEnable()` - Normal initialization with side effects + - `RestoreState()` - Silent state restoration without side effects + +2. Add a `StateRestorationHelper` that can restore states silently: + ```csharp + public static class StateRestorationHelper + { + public static void RestoreStateSilently(StateMachine sm, string stateName) + { + // Manually set active state without calling OnEnable + } + } + ``` + +**Advantages**: +- More explicit, less global state +- States control their own restoration logic + +**Disadvantages**: +- More code duplication +- Harder to implement with existing StateMachine plugin + +#### Option C: Save State Machine State Separately +1. Don't tie state machine state to interactable state +2. Create separate `SaveableStateMachine` component +3. State machines save their own current state +4. On load, restore state machine state independently + +**Advantages**: +- Clean separation of concerns +- State machines are self-contained + +**Disadvantages**: +- More complex save data structure +- Need to coordinate restoration order + +**Recommendation**: Use **Option A** with a restoration flag. It's the least invasive and most compatible with the existing Pixelplacement StateMachine plugin. + +**Implementation Plan**: +1. Add `IsRestoringState` flag to SaveLoadManager (already exists!) +2. Create helper component `SaveableState` that wraps State classes: + ```csharp + public class SaveableState : State + { + protected virtual void OnEnableBase() + { + if (SaveLoadManager.Instance?.IsRestoringState == true) + { + OnRestoreState(); + return; + } + OnNormalEnable(); + } + + protected virtual void OnNormalEnable() { } + protected virtual void OnRestoreState() { } + } + ``` + +3. Migrate existing states to inherit from SaveableState +4. Move initialization logic from OnEnable to OnNormalEnable +5. Add minimal restoration logic to OnRestoreState + +**Files to Create**: +- `Assets/Scripts/StateMachines/SaveableState.cs` + +**Files to Modify** (examples): +- `Assets/Scripts/StateMachines/Quarry/AnneLise/TakePhotoState.cs` +- Any other State classes that react to interactable events + +--- + +### Phase 6: Unique ID Generation Strategy +**Goal**: Ensure every saveable interactable has a persistent, unique ID across sessions + +**Challenge**: +- Scene instance IDs change between sessions +- Need deterministic IDs that survive scene reloads +- Manual GUID assignment is error-prone + +**Solution: Hybrid Approach** + +#### For Prefab Instances: +- Use prefab path + scene path + sibling index +- Example: `"Quarry/PickupApple_0"` (first apple in scene) +- Stable as long as prefab hierarchy doesn't change + +#### For Unique Scene Objects: +- Add `[SerializeField] private string customSaveId` +- Editor tool validates uniqueness within scene +- Example: `"Quarry/UniquePickup_GoldenApple"` + +#### Implementation: +```csharp +public abstract class SaveableInteractable : InteractableBase, ISaveParticipant +{ + [SerializeField] private string customSaveId = ""; + + public string GetSaveId() + { + if (!string.IsNullOrEmpty(customSaveId)) + { + return $"{GetSceneName()}/{customSaveId}"; + } + + // Auto-generate from hierarchy + string path = GetHierarchyPath(); + return $"{GetSceneName()}/{path}"; + } + + private string GetHierarchyPath() + { + // Build path from scene root to this object + // Include sibling index for uniqueness + } +} +``` + +**Editor Tool**: +- Create validation window that scans scene for duplicate IDs +- Auto-generate IDs for objects missing them +- Warn if hierarchy changes break ID stability + +**Files to Create**: +- `Assets/Editor/SaveIdValidator.cs` + +--- + +### Phase 7: ItemManager Integration +**Goal**: Handle cases where slotted items need to be spawned/found during restoration + +**Current ItemManager Functionality**: +- Registers/unregisters Pickups and ItemSlots +- No explicit save/load support + +**Required Additions**: +1. **Item Lookup by Save ID**: + ```csharp + public Pickup GetPickupBySaveId(string saveId) + ``` + +2. **Item Reference Serialization**: + ```csharp + [System.Serializable] + public class ItemReference + { + public string saveId; // For scene pickups + public string prefabPath; // For prefab items + public string itemDataGuid; // For PickupItemData + } + ``` + +3. **Item Spawning for Restoration**: + ```csharp + public GameObject SpawnItemForLoad(ItemReference itemRef) + { + // Check if item exists in scene first + // If not, instantiate from prefab + // Apply item data + // Return GameObject + } + ``` + +**ItemSlot Restoration Flow**: +1. ItemSlot restores state, finds it had a slotted item +2. Calls `ItemManager.GetPickupBySaveId(slottedItemSaveId)` +3. If found: Use existing GameObject +4. If not found: Call `ItemManager.SpawnItemForLoad(itemReference)` +5. Call `SlotItem()` with restored GameObject + +**Edge Cases**: +- Item was picked up and slotted elsewhere: Use save ID to track +- Item was combined before slotting: May not exist anymore (store flag) +- Item was from player inventory: Need FollowerController integration + +**Files to Modify**: +- `Assets/Scripts/Core/ItemManager.cs` + +--- + +### Phase 8: FollowerController Integration (Optional) +**Goal**: Save/restore items currently held by the follower character + +**State to Save**: +- Currently held item reference +- Held item position/rotation + +**Decision**: +- **Phase 1**: Skip this, assume player starts levels with empty inventory +- **Future Enhancement**: Add when inventory persistence is needed + +**Reasoning**: +- Most levels are self-contained +- Overworld persistence can be added later +- Reduces initial complexity + +--- + +## Implementation Order & Dependencies + +### Phase 1: Foundation (No Dependencies) ✅ +- Create SaveableInteractable base class +- Implement ISaveParticipant +- Add save ID generation +- Test with dummy data + +**Deliverable**: SaveableInteractable.cs compiles and can register/unregister + +--- + +### Phase 2: Pickup Migration (Depends on Phase 1) +- Modify Pickup to extend SaveableInteractable +- Implement PickupSaveData serialization +- Test pickup state save/load +- Verify picked-up items stay hidden after load + +**Deliverable**: Can save/load a scene with pickups, picked-up items remain picked up + +--- + +### Phase 3: ItemSlot Migration (Depends on Phases 1, 2, 7) +- Modify ItemSlot to extend SaveableInteractable +- Implement ItemSlotSaveData serialization +- Integrate with ItemManager for item references +- Test slotted items restore correctly + +**Deliverable**: Can save/load a scene with item slots, slotted items remain slotted + +--- + +### Phase 4: Stateless Interactables (Depends on Phase 1) +- Migrate MinigameSwitch to SaveableInteractable +- Remove direct SaveLoadData access +- Test minigame unlock persistence + +**Deliverable**: MinigameSwitch uses participant pattern instead of direct access + +--- + +### Phase 5: State Machine Integration (Independent) +- Create SaveableState base class +- Migrate example states (TakePhotoState, etc.) +- Test state restoration without side effects +- Document pattern for future states + +**Deliverable**: State machines can restore without replaying animations + +--- + +### Phase 6: Save ID System (Depends on Phase 1) +- Implement GetSaveId() with hierarchy path +- Add custom save ID field +- Create editor validation tool +- Test ID stability across sessions + +**Deliverable**: All saveable interactables have stable, unique IDs + +--- + +### Phase 7: ItemManager Integration (Independent, but needed for Phase 3) +- Add item lookup methods to ItemManager +- Implement ItemReference serialization +- Add item spawning for restoration +- Test with ItemSlot restoration + +**Deliverable**: ItemManager can find/spawn items by reference during load + +--- + +### Phase 8: Testing & Polish (Depends on all previous phases) +- Test full save/load cycle in each level +- Test edge cases (combined items, missing prefabs, etc.) +- Add error handling and logging +- Update documentation + +**Deliverable**: Robust save/load system for all interactables + +--- + +## Data Structures + +### SaveableInteractable Base State +```csharp +[System.Serializable] +public class InteractableBaseSaveData +{ + public bool isActive; // GameObject.activeSelf + public float remainingCooldown; // If cooldown system is used +} +``` + +### Pickup State +```csharp +[System.Serializable] +public class PickupSaveData +{ + public InteractableBaseSaveData baseData; + public bool isPickedUp; +} +``` + +### ItemSlot State +```csharp +[System.Serializable] +public class ItemSlotSaveData +{ + public PickupSaveData pickupData; // Inherited state + public ItemSlotState slotState; + public ItemReference slottedItem; // Null if empty +} + +[System.Serializable] +public class ItemReference +{ + public string saveId; // For scene objects + public string prefabPath; // For prefab spawning + public string itemDataGuid; // PickupItemData reference +} +``` + +### MinigameSwitch State +```csharp +[System.Serializable] +public class MinigameSwitchSaveData +{ + public InteractableBaseSaveData baseData; + public bool isUnlocked; +} +``` + +--- + +## Save/Load Flow + +### Save Flow +1. SaveLoadManager iterates all registered participants +2. Calls `GetSaveId()` on each SaveableInteractable +3. Calls `SerializeState()` on each +4. SaveableInteractable: + - Calls virtual `GetSerializableState()` (child override) + - Serializes to JSON + - Returns string +5. SaveLoadManager stores in `participantStates` dictionary +6. Writes entire SaveLoadData to disk + +### Load Flow +1. SaveLoadManager reads SaveLoadData from disk +2. Sets `IsRestoringState = true` +3. Scene loads (interactables register during Start()) +4. On registration, SaveLoadManager checks for existing state +5. If state exists, immediately calls `RestoreState(serializedData)` +6. SaveableInteractable: + - Deserializes JSON + - Calls virtual `ApplySerializableState(stateData)` (child override) + - Child applies state WITHOUT triggering events/initialization +7. SaveLoadManager sets `IsRestoringState = false` + +### State Machine Flow (During Load) +1. Interactable restores, determines it should trigger state "Scared" +2. Instead of firing UnityEvent, directly calls StateMachine.ChangeState("Scared") +3. StateMachine activates "Scared" state GameObject +4. "Scared" State's OnEnable() checks `SaveLoadManager.IsRestoringState` +5. If true: Sets internal variables only (no animations/movement) +6. If false: Normal initialization + +--- + +## Error Handling + +### Missing Item References +- **Problem**: ItemSlot has slotted item, but item no longer exists +- **Solution**: Log warning, set slot to empty state + +### Duplicate Save IDs +- **Problem**: Two interactables generate same save ID +- **Solution**: Editor validation tool catches this, manual fix required + +### Corrupted Save Data +- **Problem**: JSON deserialization fails +- **Solution**: Log error, use default state, continue loading + +### State Machine Mismatch +- **Problem**: Saved state references state that no longer exists +- **Solution**: Log warning, set to initial state + +--- + +## Testing Strategy + +### Unit Tests +1. SaveableInteractable registration/unregistration +2. Save ID generation uniqueness +3. Serialization/deserialization round-trip + +### Integration Tests +1. Save scene with various interactable states +2. Load scene and verify all states restored +3. Test item slot with slotted items +4. Test state machine restoration +5. Test edge cases (missing items, corrupted data) + +### Playthrough Tests +1. Play through each level +2. Save at various points +3. Load and verify game state is correct +4. Test all puzzle solutions still work + +--- + +## Migration Checklist + +- [ ] Phase 1: Create SaveableInteractable base class +- [ ] Phase 2: Migrate Pickup to SaveableInteractable +- [ ] Phase 3: Migrate ItemSlot to SaveableInteractable +- [ ] Phase 4: Migrate MinigameSwitch to SaveableInteractable +- [ ] Phase 5: Create SaveableState pattern for state machines +- [ ] Phase 6: Implement save ID validation system +- [ ] Phase 7: Extend ItemManager with lookup/spawn functionality +- [ ] Phase 8: Full integration testing and polish + +--- + +## Future Enhancements + +1. **Inventory Persistence**: Save FollowerController held items +2. **Dynamic Object Spawning**: Handle runtime-spawned interactables +3. **State Machine Auto-Save**: Automatically save StateMachine current state +4. **Incremental Saves**: Save only changed interactables (delta saves) +5. **Cloud Sync**: Sync save data across devices +6. **Save Slots**: Multiple save files per player + +--- + +## Notes & Considerations + +### Why Not Auto-Discover Interactables? +- Performance: Scanning scenes is expensive +- Determinism: Registration order affects save ID generation +- Control: Objects control their own lifecycle + +### Why Scene-Based Save IDs? +- Isolation: Each scene's interactables are independent +- Clarity: Easy to debug (see which scene an ID belongs to) +- Flexibility: Can reload individual scenes without affecting others + +### Why Not Save Everything? +- Stateless interactables (OneClickInteraction, LevelSwitch) don't need persistence +- Reduces save file size +- Faster save/load times +- Less complexity + +### State Machine Plugin Limitations +- Pixelplacement's StateMachine doesn't support silent state changes +- OnEnable is always called when state activates +- Need wrapper pattern (SaveableState) to intercept +- Alternative: Fork plugin and add restoration mode + +--- + +## Document Version +- **Version**: 1.0 +- **Date**: November 2, 2025 +- **Author**: GitHub Copilot +- **Status**: Ready for Implementation +