From 7fee72467ccce91204d42ee2e6f8d09c02f3cbd7 Mon Sep 17 00:00:00 2001 From: r-melvin <40235254+r-melvin@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:09:29 +0100 Subject: [PATCH 1/2] fix: NPC discovery scans mods built against any version of S1API 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 --- .../Internal/Utils/ReflectionUtilsTests.cs | 52 +++++++++++++++++++ S1API/Entities/NPC.cs | 2 + S1API/Internal/Utils/ReflectionUtils.cs | 46 +++++++++++++++- 3 files changed, 98 insertions(+), 2 deletions(-) diff --git a/S1API.Tests/Internal/Utils/ReflectionUtilsTests.cs b/S1API.Tests/Internal/Utils/ReflectionUtilsTests.cs index 3dfc4a81..da1e067d 100644 --- a/S1API.Tests/Internal/Utils/ReflectionUtilsTests.cs +++ b/S1API.Tests/Internal/Utils/ReflectionUtilsTests.cs @@ -121,6 +121,58 @@ public void DerivedTypeScanDoesNotFollowSameNameAssembliesWithDifferentIdentitie AppDomain.CurrentDomain.GetAssemblies())); } + [Theory] + [InlineData("S1API, Version=3.0.1.0, Culture=neutral, PublicKeyToken=null")] + [InlineData("S1API, Version=2.9.2.0, Culture=neutral, PublicKeyToken=null")] + [InlineData("S1API, Version=3.2.1.0, Culture=neutral, PublicKeyToken=null")] + [InlineData("s1api, Version=4.0.0.0, Culture=neutral, PublicKeyToken=null")] + public void AReferenceToAnyVersionOfTheBaseAssemblyBindsToIt(string reference) + { + // A mod built against an older S1API runs against the loaded one: the runtime binds by name and key token. + var loaded = new AssemblyName("S1API, Version=3.2.1.0, Culture=neutral, PublicKeyToken=null"); + + Assert.True(ReflectionUtils.ReferenceBindsToDefinition(new AssemblyName(reference), loaded)); + } + + [Fact] + public void AReferenceToADifferentAssemblyNameDoesNotBind() + { + var loaded = new AssemblyName("S1API, Version=3.2.1.0, Culture=neutral, PublicKeyToken=null"); + + Assert.False(ReflectionUtils.ReferenceBindsToDefinition( + new AssemblyName("S1APILoader, Version=3.2.1.0, Culture=neutral, PublicKeyToken=null"), + loaded)); + } + + [Fact] + public void AReferenceWithADifferentPublicKeyTokenDoesNotBind() + { + var signed = new AssemblyName("Example, Version=1.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089"); + var unsigned = new AssemblyName("Example, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null"); + + Assert.False(ReflectionUtils.ReferenceBindsToDefinition(signed, unsigned)); + Assert.False(ReflectionUtils.ReferenceBindsToDefinition(unsigned, signed)); + Assert.True(ReflectionUtils.ReferenceBindsToDefinition(signed, signed)); + } + + [Fact] + public void ALoadedAssemblyOfADifferentVersionIsFollowedOnlyWhenItIsTheOnlyOneOfThatName() + { + var reference = new AssemblyName("Library, Version=1.5.0.0, Culture=neutral, PublicKeyToken=null"); + var newer = new AssemblyName("Library, Version=1.6.0.0, Culture=neutral, PublicKeyToken=null"); + + Assert.True(ReflectionUtils.ShouldFollowLoadedReference(newer, reference, loadedAssembliesWithThatName: 1)); + Assert.False(ReflectionUtils.ShouldFollowLoadedReference(newer, reference, loadedAssembliesWithThatName: 2)); + } + + [Fact] + public void ALoadedAssemblyWithTheExactIdentityIsAlwaysFollowed() + { + var reference = new AssemblyName("Library, Version=1.5.0.0, Culture=neutral, PublicKeyToken=null"); + + Assert.True(ReflectionUtils.ShouldFollowLoadedReference(reference, reference, loadedAssembliesWithThatName: 2)); + } + private static AssemblyBuilder CreateDynamicAssembly(string name, Version version) { var assemblyName = new AssemblyName(name) diff --git a/S1API/Entities/NPC.cs b/S1API/Entities/NPC.cs index bb6befdd..599a2140 100644 --- a/S1API/Entities/NPC.cs +++ b/S1API/Entities/NPC.cs @@ -1151,6 +1151,8 @@ private static GameObject GetOrCreatePerNpcPrefab(System.Type npcType, NPC? owne TypeToPrefab[npcType] = prefabNO.gameObject; MarkPrefabsConfigured(); + // Failures are logged; logging success too tells "never found" apart from "found but not spawned". + Logger.Msg($"[S1API] Registered NPC prefab '{prefabName}' for {npcType.Name}."); return prefabNO.gameObject; } } diff --git a/S1API/Internal/Utils/ReflectionUtils.cs b/S1API/Internal/Utils/ReflectionUtils.cs index 853db58b..a2ec62db 100644 --- a/S1API/Internal/Utils/ReflectionUtils.cs +++ b/S1API/Internal/Utils/ReflectionUtils.cs @@ -137,7 +137,7 @@ private static bool ReferencesAssemblyTransitively( foreach (AssemblyName referencedAssembly in referencedAssemblies) { - if (AssemblyIdentityMatches(referencedAssembly, baseAssemblyName)) + if (ReferenceBindsToDefinition(referencedAssembly, baseAssemblyName)) return true; string referencedName = referencedAssembly.Name ?? string.Empty; @@ -147,7 +147,10 @@ private static bool ReferencesAssemblyTransitively( foreach (Assembly loadedReference in loadedReferences) { - if (!AssemblyIdentityMatches(loadedReference.GetName(), referencedAssembly)) + if (!ShouldFollowLoadedReference( + loadedReference.GetName(), + referencedAssembly, + loadedReferences.Length)) continue; if (ReferencesAssemblyTransitively( @@ -164,6 +167,45 @@ private static bool ReferencesAssemblyTransitively( return false; } + /// + /// INTERNAL: Does this reference bind to that assembly? The simple name and public key token must match; the + /// version need not, because the runtime binds a reference to whichever assembly of that name is loaded. A mod + /// built against an older S1API still runs against the loaded one, so it must still be scanned for the types + /// that derive from it. + /// + internal static bool ReferenceBindsToDefinition( + AssemblyName referenceAssemblyName, + AssemblyName definitionAssemblyName) + { + if (!string.Equals( + referenceAssemblyName.Name, + definitionAssemblyName.Name, + StringComparison.OrdinalIgnoreCase)) + return false; + + byte[] referenceToken = referenceAssemblyName.GetPublicKeyToken() ?? Array.Empty(); + byte[] definitionToken = definitionAssemblyName.GetPublicKeyToken() ?? Array.Empty(); + return referenceToken.SequenceEqual(definitionToken); + } + + /// + /// INTERNAL: Should the scan follow a reference into this loaded assembly? An exact identity always; a different + /// version only when it is the one assembly of that name loaded, since then the reference can only bind to it. + /// With several same-named assemblies loaded it is unknown which one the reference means, so only an exact + /// match is followed. + /// + internal static bool ShouldFollowLoadedReference( + AssemblyName loadedAssemblyName, + AssemblyName referenceAssemblyName, + int loadedAssembliesWithThatName) + { + if (AssemblyIdentityMatches(loadedAssemblyName, referenceAssemblyName)) + return true; + + return loadedAssembliesWithThatName == 1 + && ReferenceBindsToDefinition(referenceAssemblyName, loadedAssemblyName); + } + private static bool AssemblyIdentityMatches( AssemblyName referenceAssemblyName, AssemblyName definitionAssemblyName) => From adee163fb8d654a5da1a9e6c5b6f7f7841dd6a4f Mon Sep 17 00:00:00 2001 From: ifBars Date: Thu, 1 Oct 2026 21:20:22 -0700 Subject: [PATCH 2/2] chore: remove NPC registration debug logging --- S1API.Tests/Internal/Utils/ReflectionUtilsTests.cs | 1 - S1API/Entities/NPC.cs | 2 -- 2 files changed, 3 deletions(-) diff --git a/S1API.Tests/Internal/Utils/ReflectionUtilsTests.cs b/S1API.Tests/Internal/Utils/ReflectionUtilsTests.cs index da1e067d..0d8afed6 100644 --- a/S1API.Tests/Internal/Utils/ReflectionUtilsTests.cs +++ b/S1API.Tests/Internal/Utils/ReflectionUtilsTests.cs @@ -128,7 +128,6 @@ public void DerivedTypeScanDoesNotFollowSameNameAssembliesWithDifferentIdentitie [InlineData("s1api, Version=4.0.0.0, Culture=neutral, PublicKeyToken=null")] public void AReferenceToAnyVersionOfTheBaseAssemblyBindsToIt(string reference) { - // A mod built against an older S1API runs against the loaded one: the runtime binds by name and key token. var loaded = new AssemblyName("S1API, Version=3.2.1.0, Culture=neutral, PublicKeyToken=null"); Assert.True(ReflectionUtils.ReferenceBindsToDefinition(new AssemblyName(reference), loaded)); diff --git a/S1API/Entities/NPC.cs b/S1API/Entities/NPC.cs index 599a2140..bb6befdd 100644 --- a/S1API/Entities/NPC.cs +++ b/S1API/Entities/NPC.cs @@ -1151,8 +1151,6 @@ private static GameObject GetOrCreatePerNpcPrefab(System.Type npcType, NPC? owne TypeToPrefab[npcType] = prefabNO.gameObject; MarkPrefabsConfigured(); - // Failures are logged; logging success too tells "never found" apart from "found but not spawned". - Logger.Msg($"[S1API] Registered NPC prefab '{prefabName}' for {npcType.Name}."); return prefabNO.gameObject; } }