SMAPI

SMAPI copied to clipboard

Hey @Pathoschild. This PR implements marking specific assemblies used by mods as "private", which forbids other mods from using them directly, but allows multiple assemblies named the same to co-exist. This is mostly useful for mods consuming the same Nuget packages but at different versions. Before this PR, such mods could potentially fail to load if the older library loaded first or if the newer library had breaking changes. The developers of said mods can now specify the assemblies coming from these Nuget packages under a new PrivateAssemblies field in the manifest.json.

The PR also adds validations for this new manifest key. If a mod does not reference an assembly listed under PrivateAssemblies, a warning will be logged. Also, if a content pack lists any assembly under that field, a warning will be logged too (as this PR only makes sense for SMAPI mods, not content packs). The unreferenced private assembly warning can be disabled for a given assembly by suffixing the name with a ! - this is only really useful if the assembly is not used directly in the mod - so in the rare case of loading the assembly via reflection.

By default, this change is purely additive and opt-in, so there should be no breaking changes. The loading code changed from using Assembly methods to AssemblyLoadContext ones, but it's pretty much the same change my other PR #891 did, which means stuff like mod rewriting and Edit-and-Continue are still working. Types from private assemblies should still be usable via Pintail-powered APIs. Direct references to such types from other mods will throw an exception at runtime, but that's to be expected.

This PR makes my other PR #891 obsolete, so I will be closing it.

Thanks for the pull request!

I tentatively set this to SMAPI 4.1.0 since this'll be backwards-compatible, so I'll take a closer look when I'm working on that release. But my first impressions based on a quick review:

• I definitely prefer this approach overall.
• I try to avoid hidden features like the ! suffix, so I'll probably replace that with a model field. (Or worst case, we can validate that the DLL exists but skip the is-referenced check.)
• AssemblyLoadContext doesn't seem to exist in Xamarin Android. I'm hoping the Android port team will migrate to .NET 6 in Stardew Valley 1.6, in which case that won't be an issue. But we may need to have graceful degradation (or document what changes are needed for the SMAPI-on-Android dev) to make sure the mods can still load on Android.
• I'd want to check the performance impact of the ModAssemblyLoadContext.Load method, since it does a nested loop for each unresolved assembly (foreach context → foreach known assembly); I might want to create an optimized lookup of public assemblies by name instead and avoid LINQ.

This would also be useful for SpaceCore using Miniscript's nuget package without scripting function registration co-existing with the ones Farmtronics does 👀

I made a few changes.

There's two things left I'd want to look into:

• ModAssemblyLoadContext has an exponential loop (foreach mod context .. foreach assembly) for each assembly to find a match (often multiple times per assembly per mod). Can we add some sort of lookup instead, like a shared dictionary of assembly name => context containing it? (But see the next point first.)
• ModAssemblyLoadContext replaces SMAPI's normal assembly loading. Could we instead only use the contexts for private assemblies, so public assemblies would still be in the global context? That would also avoid needing the above loop, since each mod would only use its own context for private assemblies.

Merged into develop for the upcoming SMAPI 4.1.0. Thanks for the help!