search

drok / Harmony-CitiesSkylines

Harmony 2.x assembly provider mod for Cities: Skylines
Other
13 stars 3 forks source link

Workaround needed for scanning mods #1

Open drok opened 3 years ago

drok commented 3 years ago

Many mods use PluginInfo.GetInstance() from their OnEnable(), causing all subscribed mods to be instantiated prematurely, even if they are not enabled, as seen in the stack trace below.

This causes some problems with mods that want to self enable, as this mod (auto-enables once after initial subscription).

Normally, disabled mods are instantiated from ContentManagerPanel.Initialize() after all the enabled mods have been instantiated and OnEnabled()

Being instantiated prematurely means a self-enabling, but disabled mod cannot simply "myPluginInfo.isEnabled = true" in their constructor. This could cause a second instance to be constructed via a second call to get_userModInstance() and OnEnabled() from within the premature get_userModInstance(). The final value of userModInstance would be the premature instance (which is not OnEnabled()), and not the second instance (because the premature call returns after the call triggered by isEnabled=true

This would leave the self-enabling mod in an confused state (two instances, one enabled but not `List'-ed, and one listed but not enabled).

 * [Harmony 0.9.0.40-DEBUG] In Mod..ctor() plugin=False
   at HarmonyMod.Mod..ctor() in U:\proj\skylines\CitiesHarmony\HarmonyMod\Source\Mod.cs:line 118
   at System.Reflection.MonoCMethod.InternalInvoke(System.Object , System.Object[] , System.Exception ByRef )
   [---8<---]
   at ColossalFramework.Plugins.PluginManager+PluginInfo.CreateInstance(System.Reflection.ConstructorInfo constructor)
   at ColossalFramework.Plugins.PluginManager+PluginInfo.get_userModInstance()

                                                VVVVVVVVVVVVVVVVVVVV This should be GetPluginsInfo().GetImplementations<T>, which filters enabled mods.
                                                                     and only instantiates those mods that export the T interface
   at ColossalFramework.Plugins.PluginManager+PluginInfo.GetInstances()
   at Transit.Framework.Mod.TransitModBase.get_PluginInfo()
   at Transit.Framework.Mod.TransitModBase.get_AssetPath()
   [---8<---]
   at Transit.Framework.Mod.TransitModBase.LoadModulesIfNeeded()
   at Transit.Framework.Mod.TransitModBase.OnEnabled()
   [---8<---]
   at ColossalFramework.Plugins.PluginManager.AddPlugins(System.Collections.Generic.Dictionary`2 plugins)
   at ColossalFramework.Plugins.PluginManager.LoadPlugins()
   at Starter.Awake()
drok commented 3 years ago

Mods should do this (good example):

    void ILoadingExtension.OnCreate()
    {
        Singleton<PluginManager>.instance.GetImplementations<IMyAPI>()
            .Do((x) => x.DoStuff());
    }

And should NOT do this (bad example):

    void OnEnable()
    {

        Singleton<PluginManager>.instance.GetPluginsInfo()
            .DoIf(
            (x) => (x.userModInstance as IUserMod).Name == "MyDependent",
            (x) => (x.userModInstance as IMyAPI).DoStuff());
    }

Given an internal interface IMyAPI:

    public interface IMyAPI
    {
        void DoStuff();
    };

The good example scans only the enabled mods, and instantiates only those classes that implement the IMyAPI interface.

The bad example scans a partial list of all mods, including disabled mods, and instantiates all IUserMods that have been enumerated up to the time OnEnable() was called. It's likely not what you want; you're not even scanning the full mod list, so you have a load-ordering problem, in addition to instantiating all mods, unjustifiably.