`IPlugin::isActive()` is not queried for most plugin types

[AnyOldName3]

The problem

IPlugin::isActive() is not queried for most plugin types (i.e. just IPluginFileMapper and IPluginInstaller) so plugins cannot selectively deactivate themselves. For example:

• My OpenMW export plugin can't hide itself when MO isn't managing Morrowind.
• My FNIS integration plugin(s) can't hide themselves when MO is managing anything FNIS doesn't support.

I'm pretty sure the function was intended specifically for this kind of feature.

Environment

• Mod Organizer Version that exhibits the issue: All
• Last Mod Organizer Verison that did not exhibit the issue (if applicable): N/A
• Desktop OS/version used to run Mod Organizer: N/A

Details

We only call the function in installationmanager.cpp at line 777 for installers and organizercore.cpp at line 2251 for file mappers.

Link to Mod Organizer logs

USVFS

N/A

MO Interface

With the following inserted into a (Python-based) plugin, nothing extra is added to the log and the message box isn't displayed:

def isActive(self):
        qDebug(str(self.__organizer))
        qDebug(str(self.__organizer.managedGame()))
        qDebug(self.__organizer.managedGame().gameName())
        qDebug("isActive")
        QMessageBox.critical(self.__parentWidget, self.__tr("isActive"), self.__tr("isActive"))
        return True

(Ignore the loading of test plugins - this is my development copy of MO) https://gist.github.com/AnyOldName3/8951fb223b0939b551b6c13ce6247676

I think this is the first MO issue I've reported where a log is remotely relevant, so that's exciting. 😃

[AnyOldName3]

Aww, I can't use markdown in the issue title. I retract my smiley emoticon. 😞

[alostsoul341]

I really don't know what's going on. NMM is not making the faster sneak mods work either. So I should retract the post. I just thought it was MO because I have 238 mods in NMM with 178 running plus 21 on MO including Beyond Skyrim: Burma, and they all work great. The only ones not working are the sneak mods. They are; Sneak at run speed. sneak-no speed penalty and METAS-faster running and sneaking. All found on Nexus. But I can't even get them to run by manually installing them. So I guess i'll just do the player.setav speedmult I hate to do that, but I can't get anything to work. Any help would be appreciated. Note, I also have had a problem with setting the time scale with mods that say they do it. I have to manually do that too. But that is just time. Not like all speed. I just want to adjust my sneak speed. And also, I am starting Skyrim with MO, and NMM mods work well with that.

On Thu, Sep 27, 2018 at 3:58 PM AnyOldName3 notifications@github.com wrote:

Aww, I can't use markdown in the issue title. I retract my smiley emoticon. 😞

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ModOrganizer2/modorganizer/issues/540#issuecomment-425268490, or mute the thread https://github.com/notifications/unsubscribe-auth/ApolX5fw8szHGN0vrYTyE_wF7ZvE2v_xks5ufVgYgaJpZM4W9lw8 .

[AnyOldName3]

@alostsoul341 I think you've replied to the wrong issue - you're after https://github.com/ModOrganizer2/modorganizer/issues/539

[AnyOldName3]

This is resolved for IPluginTools by https://github.com/ModOrganizer2/modorganizer/commit/667ef70cc89e52b6f5ae5897d79cefdda3cc0e16

This is the most obvious case solved, but we really should have it fixed for all plugin types as bad things can still happen. For example, if an IPluginDiagnose is active when it doesn't want to be, you could be warned about things that don't matter.

[LostDragonist]

That was basically the last of the low hanging fruit regarding this. Everything else I checked would require some minor-to-moderate changes to interfaces to be effective. For example, if we wanted isActive to be called before issuing a callback, the interface for registering a callback would need to know about the plugin.

The simpler option seems to be for the plugins to call isActive in their own callback when needed.

For IPluginDiagnose, it seems like it was explictly designed for "inactive" plugins to be able to report problems per this comment in iplugin.h: called to test if this plugin is active. inactive plugins can still be configured and report problems but otherwise have no effect. Again, the solution here seems to be for plugins to not report problems if they think they don't matter. Though that one is easier to change than the callbacks.

[LostDragonist]

I guess another tactic would be to modify the plugin definitions to match the reality. Defining isActive in iplugin.h seems overly generic if it's not going to be called for every plugin. It could be better to define it in the individual subclasses instead. With comments that are very explicit about what isActive means for that type of plugin.

[AnyOldName3]

Surely we can just query isActive for each plugin after we enable the game plugin at load time or when switching instances and simply not let things failing that check ever leave the plugin manager.

[LostDragonist]

That doesn't seem too realistic with the current set up. By the time a plugin is loaded, its init has been called. It's likely callbacks have already been registered at that point. All sorts of stuff would have to be reinitialized in order to unload specific plugins and clear callbacks. Then there's no telling where those callbacks came from as they're not specific to the plugin interface.

I guess the first thing to do would be to define what we want isActive to mean. What it currently means is "called to test if this plugin is active. inactive plugins can still be configured and report problems but otherwise have no effect." "Can still be configured" means the plugins need to be initialized and loaded so their settings can be accessed. "Report problems" means IPluginDiagnose needs to be initialized, loaded, and the relevant functions called.

What do you propose the new definition of isActive should be? Once that is defined, we can work out what changes would be needed in MO2 and all the plugins to make that happen.

[AnyOldName3]

I don't think an inactive diagnosis plugin should be able to report active problems as that's just absurd (unless the game plugins need this, as I know they're diagnosis plugins). Settings need to be accessible as some plugins have a setting to disable them. I was going to suggest making the function static and giving it the IOrganizer as an argument so it could be called before init but that's not sufficient as we've got to init at least the game plugin first for plugins that depend on games.

I'm beginning to remember why I didn't fix this already.

[Holt59]

Closing this after the requirements() PR. Better open specific issues if we encounter new problems regarding this.