fix: keep a custom NPC's NetworkObject when it is created before the network starts

[r-melvin]

Summary

NPC.InstantiateTemplateInstance removed a new custom NPC's NetworkObject whenever this peer was not the server, which includes the time before the network has started. The Big Pimpin 1.0.11 constructs its four escort NPCs from OnSceneWasLoaded, which on the second load of a session runs before the server is up. Those NPCs had no NetworkObject, so their network behaviours bound to the parent's (@Managers/@NPCs, already spawned). Spawning them then failed:

[WARNING] [S1API] Failed to spawn pending NPC 'Evangeline': System.NullReferenceException   (twice per escort)
[NPC] 'bp_escort_midnight_pres' finalized ... (active in scene: False).

Mods that check npc.NetworkObject.IsSpawned also took them for spawned NPCs. Siesta did, and hid them before their spawn (see #339).

The change. Only a client-only peer removes the NetworkObject, since the server spawns the NPC to it. Before the network starts, it is kept, or added if missing. The server path is unchanged.

Compatibility

• Public/protected API: none changed.
• Existing defaults and behavior: NPCs created on a client-only peer or on the server are handled as before; one created before the network starts keeps its NetworkObject.
• Stable IDs, saves, and network payloads: none touched.

Validation

Mono

dotnet build S1API.sln -c MonoMelon --no-restore -p:AutomateLocalDeployment=false: 0 errors, 0 warnings. dotnet test ... -c MonoMelon: 743 passed (740 on beta plus 3 new: before the network starts, on the server, and on a client-only peer).

IL2CPP

dotnet build S1API.sln -c Il2CppMelon --no-restore -p:AutomateLocalDeployment=false: 0 errors, 0 warnings. dotnet test ... -c Il2CppMelon: 723 passed (720 plus 3 new).

Runtime evidence

How it was tested. Automated runs on 0.4.7f7 load a save, teleport to each custom NPC, record whether its model is shown and its network state, return to the menu and load again. They used a combined build of #332 to #339.

• Same mods on both runtimes: IL2CPP and Mono "Alternate", with BigWillyMod, The Big Pimpin, Drug Expansion, S1MAPI and SteamNetworkLib. IL2CPP ran without Polyfill, on the game's own interop assembly.
• IL2CPP with about 60 mods: including Polyfill, Siesta and S1UMF.
• Saves: an early save and a late one where The Big Pimpin's intro is played through.

I also played the IL2CPP case by hand.

• NetworkObject: on the second load, each escort's npc.NetworkObject is its own, and unspawned until S1API spawns it, on both runtimes. Before this change it was @Managers/@NPCs.
• Spawning: with fix: keep a custom NPC's avatar on until it is spawned #339 as well, all four escorts are finalized ... (active in scene: True) on both loads, on both runtimes, both saves, and with about 60 mods. There are no spawn failures or refusals.

Documentation

None needed.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 4ff37113-cbdd-4778-ac38-268a05b4b7e3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[ifBars]

Thanks for investigating this. I'd like to establish a reproduction through S1API's supported NPC lifecycle before merging this change.

The custom NPC guidance explicitly says to let S1API own instancing and never manually construct custom NPCs with new. Prefab defaults belong in ConfigurePrefab, with runtime setup in OnCreated. S1API also owns prefab registration and retry timing.

Looking through Big Pimpin's 1.0.11 IL2CPP assembly, its escort path differs from that:

• OnSceneWasLoaded("Main") calls EscortNPCRegistry.ResetForSceneReload().
• That clears the registry's lists, then EnsureSlots() directly constructs all four EscortMidnight...NPC subclasses with new, without first obtaining the S1API-owned instances or checking peer authority.
• Its readiness check only confirms that NPCManager.GetNPC(id), the GameObject and Transform exist. S1API registers NPCs during construction, so that does not establish that they have finished spawning.

The escorts do use ConfigurePrefab and OnCreated for some setup. The separate manual construction sequence is the part that bypasses the documented lifecycle. These observations are from static inspection of the IL2CPP mod, not an independent Mono gameplay reproduction.

Distinguishing an inactive network from a confirmed client-only peer may still be a valid S1API correction. But the current reproduction doesn't establish that the normal automatic lifecycle needs it, and retaining the NetworkObject alone doesn't queue an early instance for spawning: RegisterPendingNetworkSpawn still returns when a NetworkManager exists but isn't a server.

Could you provide a minimal NPC using the documented automatic instancing pattern that reproduces the ownership problem without Big Pimpin's construction/display code or #339? Please cover initial load and menu/reload, host and joining client, and Mono/IL2CPP separately. If an instance can be created while the peer's role is unknown, we also need to establish what happens when it subsequently becomes a client.

I'm leaving this open so we can discuss a supported reproduction or a better approach. I don't want S1API to take on another mod's construction/timing workaround without identifying a framework contract that actually fails.

[r-melvin]

Thanks for the detailed review. I ran the reproduction you asked for, and it does not reproduce through the documented lifecycle, so I'm closing this.

What I ran

• S1API: unmodified beta at 6b9ebe7 (3.2.1-beta.7), none of my PRs.
• One test mod with a single NPC written as the Physical NPC skeleton: parameterless constructor, WithIdentity/WithSpawnPosition/WithRelationshipDefaults/WithSchedule in ConfigurePrefab, Appearance.Build() and Schedule.Enable() in OnCreated. No new, no manual construction, no visibility calls.
• No Big Pimpin, no fix: keep a custom NPC's avatar on until it is spawned #339, no Polyfill. Only S1API, its loader, the test NPC and a probe that records the NPC's state.
• Game 0.4.7f7, MelonLoader 0.7.3. IL2CPP on the game's own interop assembly, and Mono ("alternate") separately.
• Each run loads a save, quits to the menu and loads it again in the same session, which is the case where Big Pimpin failed.

What happened (host)

	First load	Load after returning to the menu
IL2CPP	Instanced by S1API under @Managers/@NPCs, its own NetworkObject (npc.NetworkObject is the instance's), spawned about 6 s later, active and visible	Same; about 6 s from instance to spawned
Mono	Same	Same

No spawn failures, no refusals, and no ConsumeProductBehaviour exception on either runtime.

Not covered: a joining client. I don't have a second account or machine, so I can't test it, and I won't claim anything about that case.

Conclusion. The documented lifecycle doesn't need this change. You're right about the rest too:

• RegisterPendingNetworkSpawn returns early on a peer that has a NetworkManager but isn't the server, so keeping the NetworkObject alone doesn't queue a spawn. In my combined build the escorts did spawn, but I didn't trace which path did it, so I won't claim more than that.
• I checked your reading of Big Pimpin 1.0.11 against its decompiled IL2CPP assembly, and it matches. OnSceneWasLoaded("Main") calls EscortNPCRegistry.ResetForSceneReload(), which clears the lists and runs EnsureSlots(), constructing all four escort types with new. Only afterwards does it call EscortLocationDisplayController.HideAll().

I'll fix this in Big Pimpin's own code path instead, in S1UMF, the compatibility mod I keep for other mods. Its escorts will use the instances S1API already owns, and it will release the old display leases before replacing them. I'll also raise it with The Big Pimpin's authors.