Repository navigation
perf: index scene objects instead of scanning for every custom NPC - #354
Conversation
Movement presets, voice databases, NPC map PoIs and contact circles were each found with a scene or asset scan per custom NPC. They now come from a per-scene index built from one scan. Only found objects are reused, and only while alive and still under the key they were indexed by; a lookup that finds nothing scans again, so an object created later during loading is still found. Indexes clear on scene change. Refs ifBars#351 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
ifBars
left a comment
ifBars
left a comment
There was a problem hiding this comment.
Reviewed at daa4836.
The maintainer follow-ups address demonstrated lookup regressions: re-keying into an existing bucket, late native UI membership, PoI disable/reactivation, uninitialized native NPC identity data, and duplicate voice-database ambiguity. Voice lookups remain uncached; spawning, finalization, and portrait timing are unchanged.
Local validation with #353 as the f11 compilation dependency: 788 Mono / 762 IL2CPP tests, zero warnings/errors. Integration: 792 / 766 tests. Four roles over three save loads produce the same 12 NPC reports as the #353-only control on both runtimes. Mono two-peer gameplay/cache/packaging validation passes.
Remaining gates: fresh f11 CI fails on the existing packaging GenerateIcons references, so #353 must land first. Healthy IL2CPP four-role client validation is still unavailable: both this integration and #353 alone reproduce native Customer.Awake failure and a duplicate dealer ID (five entries). Both also reproduce the same eight reload portrait errors. These baseline problems belong in separate follow-up work, not in this optimization PR. A large mod-pack speed benchmark has not been established.
Leave this PR open and revalidate after the packaging dependency lands.
|
Follow-up validation with fixes #358 (head The initial portrait freeze alone was incomplete: the larger probe exposed a transient hip cached by native Avatar.Awake and restored by Avatar.Update. #358 now corrects that saved position at clone creation; capture timing and serialized ownership remain intact. The optimization combinations changed when this existing lifecycle hazard appeared, so the earlier blank images are not evidence that either optimization directly caused it. The combined final source passes 792 Mono and 766 IL2CPP contract tests. Both fix PRs have green hosted checks and remain open. Fresh IL2CPP co-op reload/rejoin is still inconclusive, real Steam transport is untested, and baseline client Contacts warnings remain. These checks establish compatibility in the tested matrix; they do not establish a repeatable loading-time improvement or an unconditional absence of regressions. |
2 participants
Refs #351
Summary
Custom NPC creation repeatedly scans loaded scene objects for movement defaults, map PoIs, and contact circles. This PR indexes those lookups per scene while reading movement values and UI activation live.
The maintainer follow-up preserves the original lookup contracts:
NPCPoI.SetNPCorRelationCircle.AssignNPCassigns a new object, so a late second object under an existing NPC ID is found.NPC spawning, finalization, appearance capture, and portrait timing are unchanged.
Compatibility
Source and binary: no public/protected API changes; the index and lifecycle notifications are internal. Behavior: preserve lookup order, movement defaults, active-only PoIs, late UI membership, and voice ambiguity checks. Persistence/networking: stable IDs, save formats, payloads, and host authority are unchanged; indexes are local to each peer. Mono and IL2CPP use the same implementation and matching native assignment hooks.
Validation
SteamCMD updated both local beta copies to game 0.4.7f11: alternate-beta Mono build 25770882 and beta IL2CPP build 25770926. Fresh IL2CPP interop wrappers were generated from that installation. Builds used isolated worktrees, explicit native reference paths, and automatic deployment disabled.
The separate PR builds have zero warnings/errors. Five additional scene-index regression tests cover populated-bucket re-keying, formerly unkeyed objects, late membership notifications, unchanged membership reuse, and activation filtering without another scan.
Live integration: four automatic physical NPCs (plain, customer, dealer, supplier), one copied completed save, three load/quit-to-menu cycles per runtime. All 12 NPC reports per runtime preserved active state, movement speed 1.2, Hippie VO, renderer count, and applicable contact/map membership. Native probes passed populated-bucket re-keying, late duplicate circles/PoIs, PoI disable/reactivation, and late duplicate voice rejection.
Mono host + joining client: passed using separate isolated installs and verified GSE identities. Both peers observed the same two-member native lobby. The host started server/client networking; the joining client started client networking only and followed the host's save load. Both passed four-role NPC state, cache mutations, and custom packaging checks.
IL2CPP host passed the same checks. Its joining client reached Main but failed the strict four-NPC gate: native
Customer.Awakethrows and a dealer ID is duplicated, leaving five custom NPC entries. A matching #353-only control reproduces the same exception, duplicate ID, and terminal failure with neither optimization installed. This is an existing baseline limitation; healthy IL2CPP four-role co-op has not been established by this validation.Compared the final integration with #353 alone using the same four-role probe and copied save: all 12 per-NPC state reports match on each runtime. Mono has no portrait-capture errors. IL2CPP has the same eight incomplete-portrait errors on reloads in both builds, with missing reload mugshots; this existing baseline problem remains. Native custom packaging passes in both the #353-only control and the final integration.
This is a behavior and scan-reuse validation with four custom NPCs, not a full mod-pack loading-speed benchmark. Disposable probes, saves, game assemblies, and screenshots are excluded from the commit.
CI repair
Merged current beta into this PR without rewriting its history. This incorporates #353's f11 packaging fix; the merge adds only ProductPackagingContentPatches.cs relative to the previous PR head. Current head:
43154c1edf1b3c01c1651b07c5c6171f792fa310. No CI workflows were changed.Local validation on this head: 788/788 MonoMelon and 762/762 Il2CppMelon tests, zero build warnings/errors, f11 references, automatic deployment disabled. Both hosted workflows pass on this head: IL2CPP build, Mono contract tests, documentation build and coverage, and API compatibility. Deployment is skipped.
The separately reproduced IL2CPP reload-portrait and joining-client failures are tracked in #356 and #357. Existing gameplay evidence above remains applicable; this CI repair adds the same #353 dependency used during that validation.