Repository navigation
perf: keep derived-type scans until an assembly loads - #355
Conversation
GetDerivedClasses<T> scanned every eligible assembly on each call, and custom NPC readiness calls it once per custom NPC. Results per base type are now kept per ordinary assembly until another assembly loads. Dynamic assemblies can gain types without a load event, so they stay in the plan and are re-scanned (and their eligibility re-checked) on every call. Each call returns a new list. 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 |
There was a problem hiding this comment.
Reviewed at ca4da6f.
No additional implementation defect found in the derived-type cache. The follow-up test verifies an assembly loaded after cache warm-up is discovered without manual invalidation, complementing the existing late-type-in-an-existing-dynamic-assembly test. Calls still return independent lists; dynamic assemblies are rescanned and assembly-load races do not publish stale cache entries.
Local validation with #353 as the f11 compilation dependency: 779 Mono / 753 IL2CPP tests, zero warnings/errors. Integration: 792 / 766 tests, matching three-load NPC reports against #353 alone, plus passing Mono two-peer gameplay.
|
Keep This is a small maintainability suggestion, separate from runtime validation and loading performance. |
|
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. |
Refs #351
Summary
ReflectionUtils.GetDerivedClasses<T>()repeatedly scans eligible assemblies while checking custom NPC readiness. Cache ordinary assembly results by base type untilAppDomain.AssemblyLoadinvalidates them. Dynamic assemblies are rescanned and their eligibility is rechecked on every call, because they can gain types without another load event. Calls still return independent mutable lists in assembly order; a scan overlapping a load is not cached.The maintainer follow-up adds a regression test that warms the cache before creating a new dynamic assembly, emits a derived type, and verifies discovery without manual invalidation. This complements the existing test for emitting a type into an already-loaded dynamic assembly.
Compatibility
Source and binary: no public/protected API changes. Behavior: retain derived-type eligibility, assembly order, and a fresh list per call; late assemblies and late dynamic types remain discoverable. Persistence/networking: no stable ID, save format, payload, or authority changes. Mono and IL2CPP share the cache implementation; NPC spawning, finalization, and portrait timing are unchanged.
Validation
SteamCMD updated local game copies to 0.4.7f11 (alternate-beta Mono build 25770882; beta IL2CPP build 25770926). Fresh IL2CPP interop wrappers were generated for the updated game. The current PR branch includes the merged beta packaging fix (#353) and builds with zero warnings/errors: 779/779 MonoMelon tests and 753/753 Il2CppMelon tests. The #353 + #354 + #355 integration passes 792/792 Mono and 766/766 IL2CPP tests.
Live integration: four automatic physical NPC roles, three copied-save load/quit-to-menu cycles on each runtime. Active state, movement defaults, voice assignment, renderer counts, and applicable contacts/map entries were checked on every load.
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.
These runs validate compatibility and cache contracts. They do not establish a loading-time improvement for a large mod pack. Disposable game/probe artifacts 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:
1bdfb8b9f5c975ef893c2b3b306fb066e2ae063c. No CI workflows were changed.Local validation on this head: 779/779 MonoMelon and 753/753 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.