Repository navigation
[FEATURE] Loading a save with many custom NPCs repeats scene and assembly scans per NPC (multi-second stalls) #351
Description
What would you like to see?
Loading a save with many custom NPCs (measured with ~48) spends several seconds on the main thread repeating the same global lookups once per NPC, and the loading screen visibly freezes. I'd like S1API to do these lookups once per scene (or once per assembly set) instead of once per NPC. I have fixes prepared locally and would like to agree the approach with you before opening any PR.
Where the time goes (IL2CPP, game 0.4.7f9, S1API 3.2.1-beta.8, per load, Harmony timers around each call site)
| Call site | Repeated work | Cost per load |
|---|---|---|
NPCDataAccess.EnsureMovementDefaults |
Resources.FindObjectsOfTypeAll<MovementPreset>() per NPC data preparation (~100 per load) |
~1.8–1.9 s |
ReflectionUtils.GetDerivedClasses<NPC>() (readiness, contract preparation, type lookups) |
a full assembly/type scan per call | ~190 ms per call; up to ~9 s per load in readiness checks |
NPCVoiceResolver.Resolve |
FindObjectsOfTypeAll<VODatabase>() per NPC |
~0.9 s |
AvatarCompatibility.FindMugshotGenerator |
FindObjectOfType + FindObjectsOfTypeAll per portrait (both usually empty in Main) |
~2.4 s |
NPCAppearance.UpdatePoiIcons, ContactsAppPatches.RefreshContactIcon |
a scene-wide FindObjectsOfType per portrait |
~0.3 s each |
Proposed approach (implemented locally against 3.2.1-beta.8)
- A small internal per-scene object index: built from one scan, cleared from
SceneStateCleaner, rebuilt when an indexed object has been destroyed, and absent keys remembered until invalidated. Movement presets, voice databases, NPC PoIs, contact circles and scene mugshot generators use it. Values (e.g.MovementPreset.GetValue()) are still read live. GetDerivedClasses<T>: the result per base type per ordinary assembly is kept until another assembly loads; dynamic assemblies (every modded game has some) are kept in order and re-scanned on each call; a freshListis returned per call, since some callers sort it.
Why would this help?
Shorter loads and fewer frozen loading screens for anyone with many custom NPCs, without changing the NPC lifecycle.
Results (same save and load sequence, three warm loads; only the S1API build differs; IL2CPP 0.4.7f10, measured 2026-10-07):
- Warm loading window: unmodified 34.3 / 34.8 / 35.6 s → with these changes 19.3 / 19.8 / 19.7 s.
- The longest single main-thread stall: ~11.4 s → ~1.7 s (what remains is native scene loading).
- (On 0.4.7f9 the same comparison gave ~37 s → ~17–20 s.)
Extra Context (Optional)
Correctness, through the supported lifecycle only. A minimal test mod written as skills/schedule-one-custom-npcs documents it (automatic instancing, defaults in ConfigurePrefab, runtime setup in OnCreated): a plain, a customer, a dealer and a supplier NPC, each WithVoice and with distinct WithAppearanceDefaults. Only S1API and that mod installed; a fresh new game, then a first load and two quit-to-menu reloads. Checked per NPC on every load: active, walk speed (movement defaults), voice database, generated mugshot, contact entry, map icon, dealer/supplier components, and the avatar objects actually applied to the NPC in the world (a close-up photo with the player beside it, and a check that every reload applies the same objects as the first load).
| Runtime / game | Unmodified 3.2.1-beta.8 | With the changes |
|---|---|---|
| IL2CPP 0.4.7f10 (MelonLoader 0.7.3) | all correct on load 1; reloads lose the mugshots (separate bug, reporting it separately) | all correct on all three loads, the same objects applied on every load |
| Mono 0.4.7f10 Alternate (MelonLoader 0.7.3) | all correct on all three loads | all correct on all three loads, identical |
An earlier version of this proposal also cached generated portraits for the session (a reload reused them). The appearance check above caught it: on reload it left some NPCs without garments that only custom NPCs wear (e.g. 7 → 5 applied objects). It is withdrawn; the numbers below are without it.
The same checks were also run on IL2CPP 0.4.7f9 and Mono 0.4.7f7 with the same result. The timings above are from 0.4.7f9; on f10 I re-checked correctness, not timings.
Builds and tests: Il2CppMelon and MonoMelon build with 0 warnings; 757/757 (Il2CppMelon) and 783/783 (MonoMelon) tests pass, including new behavioural tests for the index (invalidation on destroy, absent-key caching, scene reset) and the discovery cache (a dynamic assembly gaining a type after caching, a fresh list per call).
Not tested: host plus a joining client. These are local lookups and caches on each peer, but I haven't verified co-op.
A further suggestion, for your view before anyone attempts it: after the above, most remaining loading stalls are inside native loading stages doing per-NPC work (custom NPC construction during contract preparation, pending network spawns ~0.3–0.4 s, finalisation). Processing pending spawns and finalisation in small per-frame batches would keep the loading screen responsive without changing the total, but it touches lifecycle and network ordering.
If you're happy with the approach I can split it into separate PRs (the index and its users; the discovery cache), each linked here.
Activity
ifBars commentedon Oct 7, 2026
Yeah, feel free to open the PRs. Thanks for putting the time into profiling this and testing both runtimes.
My main concern is making sure nothing regresses from the current behavior. Each NPC and the objects it needs should still be found properly, including cases where a lookup returns nothing but the object gets created later during loading. I'd want to make sure that holds across reloads and in co-op too.
LocalLobby might help with testing a host and joining client locally. I can also do our own testing on the PRs before merging.
I'd personally leave the remaining stalls alone for now. With the improvements you've reported, I don't think the remaining time is worth changing spawn/finalisation ordering over. Mugshot generation has also been pretty sensitive to timing in the past. I remember Heimdall and I working on it months ago, and generating the icon too late during loading could leave it as a blank white texture. The recent game updates to mugshots/icon generation might have helped with that, but I'd rather keep these changes focused on the repeated lookups, since that feels like an obvious case of optimization/re-work needed.
r-melvin commentedon Oct 7, 2026
Thanks. The PRs, kept to the repeated lookups:
GetDerivedClasses<T>results kept until another assembly loads; dynamic assemblies are re-scanned on every call.Your "created later" case. I changed the index from what this issue described. It no longer remembers that something was absent: a lookup that finds nothing always scans again, so an object created later in loading is still found. A found object is reused only while it's alive and still under the key it was indexed by (otherwise the index rebuilds). Movement defaults also rescan if no indexed preset yields a value yet. Tests pin each of these, including "lookup finds nothing, object created, next lookup finds it". The derived-type cache can't miss types either: a new assembly clears it, and dynamic assemblies are always scanned.
Mugshots. Left alone, as you suggested. The scene mugshot-generator lookup and the reload portrait-rig change that I'd mentioned are not in these PRs, and no portrait timing changes.
Testing on game 0.4.7f11 (IL2CPP and Mono Alternate, MelonLoader 0.7.3). The setup was only S1API and a small test mod written to
skills/schedule-one-custom-npcs: plain, customer, dealer and supplier NPCs, eachWithVoicewith its own appearance. A fresh new game, then a first load and two quit-to-menu reloads. Each run comparedbeta+ both PRs againstbetaalone (#353 applied to both, sincebetadoesn't compile against the f11 assemblies without it). Per NPC per load, I checked:All 12 reports (4 NPCs × 3 loads) were identical with and without the PRs on both runtimes. On IL2CPP both builds lose the mugshots on reloads (eight "capture returned no complete portrait" errors in each). That's the existing reload problem I mentioned in the issue, and I'll report it separately. Builds have 0 warnings; tests pass on both configurations (counts in each PR).
Not covered yet:
steam_api64.dllreplaced with a Steam emulator, which I didn't want to do on this install. LocalMultiplayer (rebuilt for 0.4.7) won't start a second instance until the first exits under Proton here, and its host spams connection-state logs on 0.4.7. The changes are lookups local to each peer, but I haven't verified them in co-op, so your own testing there would be welcome.ifBars commentedon Oct 8, 2026
Maintainer validation completed for #354 and #355 after updating both SteamCMD beta installs to 0.4.7f11.
#354 now preserves populated-bucket re-keying, late native UI assignments, active/inactive PoI behavior, and null native identity data. Voice lookups remain uncached so a late duplicate database still raises the existing ambiguity error. #355 has an additional test for an assembly loaded after the derived-type cache is warm. The final integration passes 792 Mono / 766 IL2CPP tests with zero build warnings/errors.
Four automatic NPC roles over three save loads produce identical state reports with the integration and #353 alone on both runtimes. Mono host/client gameplay passes with distinct identities, two-member native lobby membership, correct server/client state, cache mutation checks, and custom packaging rendering.
Remaining baseline limitations: IL2CPP reload portraits fail identically in both builds (eight capture errors), and the IL2CPP joining client reproduces Customer.Awake failure plus a duplicate dealer ID with #353 alone as well as with both optimizations. The strict four-NPC co-op gate remains failing. I left these as separate follow-up observations rather than expanding the optimization PRs. A large mod-pack loading-speed benchmark remains unverified.
Both private beta assembly repos and CI caches are now updated from f7 to f11. Fresh standalone #354/#355 CI correctly fails on the existing GenerateIcons packaging references: #353 needs to land first. The PR descriptions and posted reviews record that dependency. This issue and the PRs remain open.
perf(npc): index scene objects instead of scanning for every custom NPC
perf(reflection): keep derived-type scans until an assembly loads
ifBars commentedon Oct 9, 2026
Shipped the agreed lookup changes in 3.2.1-beta.9: #354 indexes movement presets, map PoIs, and contact circles; #355 caches ordinary assembly discovery while rescanning dynamic assemblies and invalidating on assembly load. Voice lookups remain uncached to preserve the existing late-duplicate ambiguity behavior. Mugshot-generator lookup and spawn/finalization ordering remain outside these optimizations.
The final combined candidate passes 792 Mono / 766 IL2CPP contract tests. On game beta 0.4.7f11 with MelonLoader 0.7.3, 32 custom NPC types pass two loads and produce 64 nonblank portraits per runtime. Both Mono and IL2CPP host/client gameplay pass first load and same-process reload/rejoin with separate local identities, four NPC roles, one native component per object, customer data before Awake, and nonblank portraits. The baseline portrait/client initialization bugs were fixed in #358/#359 before this validation.
The release ZIP and DLL versions were downloaded and verified. Real Steam transport and a repeatable maintainer measurement of total loading-time improvement remain unverified. Closing the agreed lookup implementation; the release notes make no numerical speedup claim.