diff --git a/S1API.Tests/Entities/NPCInventoryPersistencePolicyTests.cs b/S1API.Tests/Entities/NPCInventoryPersistencePolicyTests.cs new file mode 100644 index 00000000..cd7c6111 --- /dev/null +++ b/S1API.Tests/Entities/NPCInventoryPersistencePolicyTests.cs @@ -0,0 +1,115 @@ +using S1API.Internal.Patches; +using S1API.Entities; + +namespace S1API.Tests.Entities; + +public sealed class NPCInventoryPersistencePolicyTests +{ + [Fact] + public void SavedInventoryRestoresAfterSlotInitialization() + { + var calls = new List(); + + NPCPatches.RestoreInventoryAfterInitialization( + () => calls.Add("initialize"), + () => calls.Add("restore")); + + Assert.Equal(new[] { "initialize", "restore" }, calls); + } + + [Fact] + public void AlreadyAwakeInventoryStillRestoresExactlyOnce() + { + bool awakeCompleted = true; + int initializationCount = 0; + int restoreCount = 0; + + NPCPatches.RestoreInventoryAfterInitialization( + () => + { + Assert.True(awakeCompleted); + initializationCount++; + }, + () => + { + Assert.True(awakeCompleted); + restoreCount++; + }); + + Assert.Equal(1, initializationCount); + Assert.Equal(1, restoreCount); + } + + [Fact] + public void FailedInitializationDoesNotAttemptRestore() + { + bool restoreAttempted = false; + + Assert.Throws(() => + NPCPatches.RestoreInventoryAfterInitialization( + () => throw new InvalidOperationException("initialization failed"), + () => restoreAttempted = true)); + + Assert.False(restoreAttempted); + } + + [Fact] + public void CurrentNpcDataSlotCountIsUsedWhenLegacyMemberIsMissing() + { + int result = NPCInventory.ResolveTargetSlotCount( + legacySlotCount: null, + npcDataSlotCount: 5, + fallback: 0, + isCustomNpc: true); + + Assert.Equal(5, result); + } + + [Fact] + public void LegacySlotCountRemainsPreferredForOlderGameVersions() + { + int result = NPCInventory.ResolveTargetSlotCount( + legacySlotCount: 6, + npcDataSlotCount: 5, + fallback: 0, + isCustomNpc: true); + + Assert.Equal(6, result); + } + + [Fact] + public void ExistingCollectionCountIsFinalFallback() + { + int result = NPCInventory.ResolveTargetSlotCount( + legacySlotCount: -1, + npcDataSlotCount: null, + fallback: 4, + isCustomNpc: true); + + Assert.Equal(4, result); + } + + [Fact] + public void CustomNpcUsesVanillaFiveSlotDefaultWhenNativeCountsAreZero() + { + int result = NPCInventory.ResolveTargetSlotCount( + legacySlotCount: null, + npcDataSlotCount: 0, + fallback: 0, + isCustomNpc: true); + + Assert.Equal(5, result); + } + + [Fact] + public void BaseNpcCanRetainAnIntentionallyEmptyInventory() + { + int result = NPCInventory.ResolveTargetSlotCount( + legacySlotCount: null, + npcDataSlotCount: 0, + fallback: 0, + isCustomNpc: false); + + Assert.Equal(0, result); + } +} diff --git a/S1API/Entities/NPCInventory.cs b/S1API/Entities/NPCInventory.cs index 1ad52263..aae74d7f 100644 --- a/S1API/Entities/NPCInventory.cs +++ b/S1API/Entities/NPCInventory.cs @@ -1,4 +1,5 @@ #if (IL2CPPMELON) +using Il2CppInterop.Runtime; using S1NPCs = Il2CppScheduleOne.NPCs; using S1Items = Il2CppScheduleOne.ItemFramework; using S1Interaction = Il2CppScheduleOne.Interaction; @@ -24,6 +25,7 @@ namespace S1API.Entities /// public sealed class NPCInventory { + private const int DefaultCustomNpcSlotCount = 5; private static readonly Logging.Log Logger = new Logging.Log("NPCInventory"); private readonly NPC NPC; @@ -164,10 +166,10 @@ public void EnsureInitialized() // ignored } }); - slot.onItemDataChanged = (Il2CppSystem.Action)Il2CppSystem.Delegate.Combine( - slot.onItemDataChanged, - (Il2CppSystem.Action)handler - ); + slot.onItemDataChanged = Il2CppSystem.Delegate.Combine( + slot.onItemDataChanged, + (Il2CppSystem.Action)handler) + .Cast(); #else slot.onItemDataChanged = (Action)Delegate.Combine( slot.onItemDataChanged, @@ -249,12 +251,42 @@ public void EnsureInitialized() try { inv.NetworkInitializeIfDisabled(); } catch (Exception ex) { Logger.Warning($"[NPCInventory] EnsureInitialized: NetworkInitializeIfDisabled threw for '{npcId}': {ex.Message}"); } } - private static int GetSlotCount(S1NPCs.NPCInventory inv, int fallback) + private int GetSlotCount(S1NPCs.NPCInventory inv, int fallback) + { + var legacyValue = ReflectionUtils.TryGetFieldOrProperty(inv, "SlotCount"); + int? npcDataSlotCount = null; + + try + { + var npcData = NPC?.S1NPC?.NPCData; + if (npcData?.Inventory != null) + npcDataSlotCount = npcData.Inventory.InventorySlotCount; + } + catch + { + // Fall back to the current collection when native data is unavailable. + } + + return ResolveTargetSlotCount( + legacyValue is int legacySlotCount ? legacySlotCount : null, + npcDataSlotCount, + fallback, + NPC?.IsCustomNPC == true); + } + + internal static int ResolveTargetSlotCount( + int? legacySlotCount, + int? npcDataSlotCount, + int fallback, + bool isCustomNpc) { - var value = ReflectionUtils.TryGetFieldOrProperty(inv, "SlotCount"); - return value is int slotCount && slotCount >= 0 - ? slotCount - : fallback; + if (legacySlotCount > 0) + return legacySlotCount.Value; + if (npcDataSlotCount > 0) + return npcDataSlotCount.Value; + if (fallback > 0) + return fallback; + return isCustomNpc ? DefaultCustomNpcSlotCount : 0; } private static void TryInvokeContentsChanged(S1NPCs.NPCInventory inv) diff --git a/S1API/Internal/Patches/NPCPatches.cs b/S1API/Internal/Patches/NPCPatches.cs index 76d6efa4..f272e61d 100644 --- a/S1API/Internal/Patches/NPCPatches.cs +++ b/S1API/Internal/Patches/NPCPatches.cs @@ -80,10 +80,6 @@ internal class NPCPatches private static readonly System.Collections.Generic.Dictionary _savedCurrentAddiction = new System.Collections.Generic.Dictionary(); - // Pending inventory loads for custom dealers - stored until NPCInventory.Awake creates slots - private static readonly System.Collections.Generic.Dictionary _pendingInventoryLoads - = new System.Collections.Generic.Dictionary(); - private static object? GetInventoryMember(S1NPCs.NPCInventory inventory, string memberName) { return ReflectionUtils.TryGetFieldOrProperty(inventory, memberName); @@ -495,7 +491,14 @@ internal static void ResetState() { _savedCurrentAddiction.Clear(); _loadingDealers.Clear(); - _pendingInventoryLoads.Clear(); + } + + internal static void RestoreInventoryAfterInitialization( + Action ensureInitialized, + Action restoreInventory) + { + ensureInitialized(); + restoreInventory(); } private static void LogCustomNpcInstantiationException(Type? type, string context, Exception? ex) @@ -1065,13 +1068,6 @@ private static void NPCInventory_Awake_Postfix(S1NPCs.NPCInventory __instance) { var wrapperInventory = new NPCInventory(apiNpc); wrapperInventory.EnsureInitialized(); - - // Load pending inventory after slots are initialized - if (baseNpc != null && _pendingInventoryLoads.TryGetValue(baseNpc.ID, out var pendingItemSet)) - { - pendingItemSet.LoadTo(__instance.ItemSlots); - _pendingInventoryLoads.Remove(baseNpc.ID); - } } catch (Exception ex) { @@ -1405,8 +1401,8 @@ private static void NPC_GetSaveData(S1NPCs.NPC __instance, ref S1Datas.DynamicSa /// /// Temporary patch while S1API NPCs are not networked - /// Handle NPCLoader.Load for custom S1API NPCs to avoid inventory hydration which uses networking. - /// Replicates core parts of the original loader except Inventory and Health (Health already guarded). + /// Handles NPCLoader.Load for custom S1API NPCs without native networked inventory hydration. + /// Restores saved inventory after the final slot collection is initialized. /// [HarmonyPatch(typeof(S1Loaders.NPCLoader), nameof(S1Loaders.NPCLoader.Load))] [HarmonyPrefix] @@ -1454,6 +1450,7 @@ private static bool NPCLoader_Load_Prefix(S1Datas.DynamicSaveData saveData) { return true; // run original for base NPCs } + var customNpc = apiNpc; // Custom S1API NPC: perform safe subset of loading and skip original try @@ -1573,7 +1570,19 @@ private static bool NPCLoader_Load_Prefix(S1Datas.DynamicSaveData saveData) { if (S1Datas.ItemSet.TryDeserialize(inventoryData, out var itemSet)) { - itemSet.LoadTo(s1BaseNpc.Inventory.ItemSlots); + RestoreInventoryAfterInitialization( + customNpc.Inventory.EnsureInitialized, + () => + { + var inventory = s1BaseNpc.GetComponent(); + if (inventory?.ItemSlots == null) + { + throw new InvalidOperationException( + $"Inventory slots were not initialized for custom NPC '{baseData.ID}'."); + } + + itemSet.LoadTo(inventory.ItemSlots); + }); } else { @@ -1586,7 +1595,6 @@ private static bool NPCLoader_Load_Prefix(S1Datas.DynamicSaveData saveData) $"NPCLoader_Load_Prefix: Exception loading Inventory data for '{baseData.ID}': {ex.Message}"); } } - } catch (Exception ex) { @@ -2728,16 +2736,6 @@ private static bool Dealer_Load_Prefix(S1Economy.Dealer __instance, S1Datas.Dyna } } - if (isCustomNPC) - { - if (dynamicData.TryGetData("Inventory", out var inventoryData)) - { - if (S1Datas.ItemSet.TryDeserialize(inventoryData, out var itemSet)) - _pendingInventoryLoads[__instance.ID] = itemSet; - else - Logger.Warning($"Failed to deserialize inventory data for custom NPC dealer {__instance.ID}"); - } - } } catch (Exception ex) {