From 8860d65513d43cd9e546bf4cfa162cca7a8a29c7 Mon Sep 17 00:00:00 2001 From: r-melvin <40235254+r-melvin@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:09:29 +0100 Subject: [PATCH] fix: repair and contain a custom NPC's product-behaviour ownership failure 0.4.7's ConsumeProductBehaviour.OnStartServer dereferences beh.Npc, both set only in Awake. An S1API NPC's behaviour whose Awake ran before it was parented keeps nulls, and the NullReferenceException aborts FishNet's scene setup, so the save never finishes loading. A prefix repairs the references as Awake would; if there is still no NPC, a finalizer contains that one exception and logs it (at most 20 times). S1API NPCs only. A test fails if a game member the patch relies on goes missing. Co-Authored-By: Claude Opus 5.5 --- .../NPCs/NPCBehaviourOwnershipTests.cs | 13 ++ S1API/Internal/Patches/NPCPatches.cs | 115 ++++++++++++++++++ 2 files changed, 128 insertions(+) create mode 100644 S1API.Tests/NPCs/NPCBehaviourOwnershipTests.cs diff --git a/S1API.Tests/NPCs/NPCBehaviourOwnershipTests.cs b/S1API.Tests/NPCs/NPCBehaviourOwnershipTests.cs new file mode 100644 index 00000000..c50a5bca --- /dev/null +++ b/S1API.Tests/NPCs/NPCBehaviourOwnershipTests.cs @@ -0,0 +1,13 @@ +using S1API.Internal.Patches; + +namespace S1API.Tests.NPCs; + +public sealed class NPCBehaviourOwnershipTests +{ + [Fact] + public void GameMembersTheBehaviourOwnershipRepairReliesOnExist() + { + // If the game renames one of these, the repair stops working and a custom NPC can stop a save from loading. + Assert.Empty(NPCPatches.FindMissingBehaviourOwnershipMembers()); + } +} diff --git a/S1API/Internal/Patches/NPCPatches.cs b/S1API/Internal/Patches/NPCPatches.cs index d480e800..b62692ec 100644 --- a/S1API/Internal/Patches/NPCPatches.cs +++ b/S1API/Internal/Patches/NPCPatches.cs @@ -315,6 +315,121 @@ private static void Prefix(S1NPCs.NPCMovement __instance, ref Vector3 pos) return ReflectionUtils.TryGetFieldOrProperty(action, "npc") as S1NPCs.NPC; } + /// + /// Names the game members the behaviour-ownership repair relies on that are missing, so a game update + /// that renames one is a clear message and not a silent load failure. + /// + internal static System.Collections.Generic.IReadOnlyList FindMissingBehaviourOwnershipMembers() + { + const BindingFlags flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; + var missing = new System.Collections.Generic.List(); + if (typeof(S1NPCsBehaviour.ConsumeProductBehaviour).GetMethod("OnStartServer", flags) == null) + missing.Add("ConsumeProductBehaviour.OnStartServer"); + if (typeof(S1NPCsBehaviour.Behaviour).GetProperty("beh", flags) == null) + missing.Add("Behaviour.beh"); + if (typeof(S1NPCsBehaviour.NPCBehaviour).GetProperty("Npc", flags) == null) + missing.Add("NPCBehaviour.Npc"); +#if MONOMELON + // Mono writes the auto-property backing fields, since the setters are not public. + if (ConsumeProductBehaviourOwnershipPatch.BehaviourOwnerField == null) + missing.Add("Behaviour.k__BackingField"); + if (ConsumeProductBehaviourOwnershipPatch.NpcOwnerField == null) + missing.Add("NPCBehaviour.k__BackingField"); +#endif + return missing; + } + + /// + /// A custom NPC's product behaviour must not abort loading because it cannot find its NPC. + /// + /// + /// 0.4.7's OnStartServer is base.OnStartServer(); Npc.OnNPCDeinitialized += ..., where Npc is + /// beh.Npc. Both references are set only in Awake, so an instance whose Awake ran before it was + /// parented keeps nulls, and the exception aborts FishNet's scene setup: the save never finishes loading. + /// The prefix repairs them as Awake would have. If there is still no NPC, the finalizer contains that one + /// exception (only the deinitialize subscription is lost). S1API's own NPCs only. + /// + [HarmonyPatch] + private static class ConsumeProductBehaviourOwnershipPatch + { + private const int MaxContainedLogs = 20; + private static int _contained; +#if MONOMELON + internal static readonly FieldInfo? BehaviourOwnerField = + AccessTools.Field(typeof(S1NPCsBehaviour.Behaviour), "k__BackingField"); + internal static readonly FieldInfo? NpcOwnerField = + AccessTools.Field(typeof(S1NPCsBehaviour.NPCBehaviour), "k__BackingField"); +#endif + + private static MethodBase? TargetMethod() => + AccessTools.Method(typeof(S1NPCsBehaviour.ConsumeProductBehaviour), "OnStartServer"); + + [HarmonyPrefix] + private static void Prefix(S1NPCsBehaviour.ConsumeProductBehaviour __instance) + { + try + { + if (__instance == null || !IsS1ApiCustomNpcComponent(__instance)) + return; + + S1NPCsBehaviour.NPCBehaviour? owner = __instance.beh; + if (owner == null) + { + owner = __instance.GetComponentInParent(true); + if (owner == null) + return; +#if IL2CPPMELON + __instance.beh = owner; +#else + BehaviourOwnerField?.SetValue(__instance, owner); +#endif + } + + if (owner.Npc != null) + return; + + S1NPCs.NPC? npc = owner.GetComponentInParent(true); + if (npc == null) + return; +#if IL2CPPMELON + owner.Npc = npc; +#else + NpcOwnerField?.SetValue(owner, npc); +#endif + } + catch (Exception ex) + { + Logger.Warning($"[NPC] Could not repair behaviour ownership: {ex.Message}"); + } + } + + [HarmonyFinalizer] + private static Exception? Finalizer(Exception? __exception, S1NPCsBehaviour.ConsumeProductBehaviour __instance) + { + if (__exception == null) + return null; + + try + { + // Only the failure the prefix could not repair: an S1API NPC's behaviour still without its NPC. + if (__instance == null || !IsS1ApiCustomNpcComponent(__instance) || __instance.beh?.Npc != null) + return __exception; + } + catch + { + return __exception; + } + + if (_contained++ < MaxContainedLogs) + { + Logger.Warning( + $"[NPC] Contained an OnStartServer failure on '{__instance.gameObject.name}' (no NPC found); loading continues: {__exception.Message}"); + } + + return null; + } + } + [HarmonyPatch] private static class NpcBehaviourSummonLogicPatch {