Skip to content
This repository has been archived by the owner on Jun 27, 2021. It is now read-only.

Plugin dependencies #37

Open
YouKnowBlom opened this issue Aug 5, 2018 · 8 comments
Open

Plugin dependencies #37

YouKnowBlom opened this issue Aug 5, 2018 · 8 comments
Labels
category: core Something to do with loading plugins or pre-included features category: major Difficult feature to create or bug to fix. category: plugins Related to a specific plugin, or new plugin request. status: delayed Don't have the time/resources to implement now. will be worked on, but not in the near future. type: enhancement New feature or request

Comments

@YouKnowBlom
Copy link
Collaborator

As more and more util and helper plugins are released, the next logical step would be to add a form of dependency system where you could declare other plugins that your plugin rely on.

I'd imagine it might need to modify how plugins are loaded, as the load order will be important (we skyrim now boys)

@jakub-studio
Copy link
Contributor

jakub-studio commented Aug 5, 2018

In addition to this, I would like to see some kind of ED.onPluginsLoaded event implemented so that plugins that create global variables as some sort of APIs would be easier to work with,

nvm.

@rauenzi
Copy link
Collaborator

rauenzi commented Aug 5, 2018

I'm not sure there are really many plugins that need dependencies. Mine do of course, but they automatically check for the dependency and let you know if it exists. To have a dependency system, really the best way would cause some restructures of how plugins work in ED. Ideally there would be a separate config files that states plugin info including dependencies that way dependencies can be checked and ensured even before the plugin itself is required that way it can ensure no errors occur and that plugins can require their dependency at any time (as opposed to during load() which would be necessary if sticking with current structure and adding dependencies.)

That being said, either way of moving forwards would require rewriting the current plugin loader. So it would really be up to you guys and Joe how you would want to move forward (if at all).

@jakuski an event like that would be simple enough. But realistically for most dependency cases shouldn't be needed. For instance if I had a plugin that wanted to use ZeresUtils.js I could simply do require("ZeresUtils") and anything exported would be available to you, e.g. static functions and such would immediately be available without the plugin having had load() fired.

@jakub-studio
Copy link
Contributor

jakub-studio commented Aug 5, 2018

@rauenzi I was thinking more of something like this

Plugin A is lets say, a chat command plugin.
Plugin B is a plugin that shows you all emojis in a guild for example.

Plugin A creates a global variable called ChatCommands and any can plugin can utilise it and make their own chat commands.

However due to the way plugins are required/loaded, Plugin B doesn't know when the global variable ChatCommands will be ready and instead of a while (!ChatCommands) sleep... I think that having a event fire off, (Not really a NodeEventEmitter since I see multiple issues with it when it comes to multiple plugins utilisng it.) will be better for speeds sake.

So instead of Plugin B having to integrate it's own way of UI, it could simply wait for ED's plugins to be loaded and if the chat command global var is not there, throw a error.

If it is there, then it could register a chatcommand to show guild emojis.

Example:

module.exports = new Plugin({
    load () {
        ED.onPluginsLoaded(() => {
            // do something
        })
    }
})

or maybe

module.exports = new Plugin({
    load () {}
    onPluginsLoaded () {
            // do something
    })
})

@rauenzi
Copy link
Collaborator

rauenzi commented Aug 5, 2018

well in ChatCommands you could do this

const myPlugin = new Plugin({});
myPlugin.commands = [];
module.exports = myPlugin;

and in another plugin called Shrug you can do this

module.exports = new Plugin({
    load () {
        const commands = require("ChatCommands").commands;
        commands.push("my new command");
    }
});

that should work fine since it all refers to the same object.

@jakub-studio
Copy link
Contributor

jakub-studio commented Aug 5, 2018

Only issue I see with this is user's renaming the file (which is what the ED.plugins key's are based off.)

@rauenzi
Copy link
Collaborator

rauenzi commented Aug 5, 2018

You can never fully eliminate pebkac errors, that can happen even with a dependency scheme.

@YouKnowBlom YouKnowBlom added the category: core Something to do with loading plugins or pre-included features label Aug 5, 2018
@joe27g joe27g added status: delayed Don't have the time/resources to implement now. will be worked on, but not in the near future. category: plugins Related to a specific plugin, or new plugin request. category: major Difficult feature to create or bug to fix. labels Aug 6, 2018
@joe27g
Copy link
Owner

joe27g commented Aug 6, 2018

I can sort of see a use for this, but have no idea how to implement it at this time. Hence the 'status:delayed'.

@joe27g joe27g added the type: enhancement New feature or request label Aug 6, 2018
@rauenzi
Copy link
Collaborator

rauenzi commented Aug 6, 2018

I could help with implementing this, but like I said above it depends on the structuring you want to go with.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
category: core Something to do with loading plugins or pre-included features category: major Difficult feature to create or bug to fix. category: plugins Related to a specific plugin, or new plugin request. status: delayed Don't have the time/resources to implement now. will be worked on, but not in the near future. type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants