fix: NPC discovery scans mods built against any version of S1API - #333
Conversation
|
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 configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 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 |
- AccessoryFactory no longer checks the game's members by reflection on every clone. Clone and RegisterInLibrary already report a missing member, and the test keeps the check. - ApplyTexturesToAccessory goes back to private; nothing outside the class uses it. - GetRegisteredAssetForType uses FindCompatibleTypedAsset instead of repeating its loop, and the path prefix is built once per lookup. - The "Registered NPC prefab" log moves to the NPC discovery change (ifBars#333), where it belongs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GetDerivedClasses only scans assemblies that reference the base assembly, and compared full names, so a mod built against another S1API version (BigWillyMod: 3.0.1.0 against 3.2.1.0) was never scanned and its NPCs never registered, without any log line. A reference now binds by simple name and public key token, as the runtime binds it; a transitive hop accepts another version only when one assembly of that name is loaded. A custom NPC prefab that registers is now logged once, so "never found" and "found but not spawned" can be told apart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6d0a766 to
7fee724
Compare
|
I pushed adee163 to remove the added NPC registration success log and a redundant test comment. Discovery logic and failure logging are unchanged. Full builds and suites pass: 748 Mono, 728 IL2CPP. I also updated the description so it no longer presents the removed log as part of the current change. |
|
Tested on 0.4.7f7 Mono and IL2CPP. Three custom NPC classes were discovered and spawned with a test mod whose S1API reference version was set to 3.0.1.0, while the loaded API was 3.2.1.0. This confirms discovery across a reference-version mismatch. Compatibility with older API signatures still depends on the mod. CI passed. The combined build of #332, #333, #334, #336 and #337 also passed all 770 Mono and 744 IL2CPP tests. Merged into beta. |
…eload, and hide only once spawned Its EscortNPCRegistry.EnsureSlots builds the four escorts with `new` from its scene-load callback, beside the ones S1API makes: two of each on a first load, and on a reload S1API refuses to spawn them (hidden before spawn). S1UMF now waits until the save has loaded with the network up, adopts S1API's instances (building its own only if S1API made none), releases the old display leases before rebuilding, and defers a hide until the escort spawns (skipped if it is leased again by then). Applies only with an S1API whose discovery finds Big Pimpin (ifBars/S1API#333): 3.2.1-beta.7 can't spawn them at all, so there it stands down. Tested on IL2CPP, slot 5 late save, full mod set, first load and reload: with current S1API beta, one instance per escort, all four spawned on both loads, no refusals; with 3.2.1-beta.7 it stands down and the mod behaves as before. Requested in ifBars/S1API#338 and #339 instead of an S1API change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
A mod built against an S1API version other than the loaded one never has its custom NPCs found, so they never register, and nothing is logged.
ReflectionUtils.GetDerivedClassesonly scans assemblies that reference the base assembly (#289), and compared full names, which include the version. BigWillyMod 1.0.3 references S1API 3.0.1.0, the loaded one is 3.2.1.0, and the runtime binds it anyway. Of 11 installed mods referencing S1API, only one matched exactly.The change.
DerivedTypeScanDoesNotFollowSameNameAssembliesWithDifferentIdentitiesrequires.Compatibility
internal statichelpers onReflectionUtils.Validation
Mono
dotnet build S1API.sln -c MonoMelon --no-restore -p:AutomateLocalDeployment=false: 0 errors, 0 warnings.dotnet test ... -c MonoMelon: 748 passed (740 onbetaplus 8 new).IL2CPP
dotnet build S1API.sln -c Il2CppMelon --no-restore -p:AutomateLocalDeployment=false: 0 errors, 0 warnings.dotnet test ... -c Il2CppMelon: 728 passed (720 plus 8 new).Contributor-reported runtime evidence (before maintainer cleanup)
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.
I also played the IL2CPP case by hand.
Registered NPC prefablines forS1API_BigWilly,S1API_DiscoDaveyandS1API_BigPimpNPC(The Big Pimpin, built against 3.1.15.0) on both runtimes. Before this change, none of them registered.teleport big_willyreaches Big Willy.Documentation
XML
<summary>on the new internal helpers. No DocFX changes.🤖 Generated with Claude Code
Maintainer follow-up
Removed the added NPC prefab registration success log and a redundant test comment. Discovery behavior and failure logging are unchanged. Full solution builds and contract suites pass: 748 Mono, 728 IL2CPP. The runtime evidence above is from the contributor''s earlier combined build, which included the removed log.