SpongeAPI icon indicating copy to clipboard operation
SpongeAPI copied to clipboard

Plugin dependencies and plugin initialization

Open stephan-gh opened this issue 9 years ago • 21 comments
trafficstars

Plugins can define dependencies on other plugins using a simple string with a list of plugins to load before or after, and whether they're required for the plugin to run. However, currently this does only affect the construction of the plugin (the call to the constructor), not any of the initialization events when almost everything of plugin initialization will take place.

(The only reason it works kind of currently on SpongeForge is that the events are registered in the order of dependencies. This is however basically an implementation detail of the event manager, and as soon as one of the plugins defines a priority on the initialization events it will already get called out of order.)

Therefore the only thing plugins can rely on is that the classes of the dependencies are on the classpath, but not that the plugin was actually already initialized. One issue that results from this is that plugins have no way to tell if their dependencies were successfully initialized either. Sponge will catch and log all exceptions happening in the initialization events, but it is nowhere stored for plugins (nor the users) to check - the only way to check if a plugin was initialized correctly is to check the log files (and basically hope that nothing else breaks, because all other event handlers etc. will be still called, no matter what happened during the initialization phase).

Generally this is just something plugin developers have to keep in mind, there are still the priorities events are called in as well as multiple plugin initialization events plugins can use to handle things which require initialization of the other plugins. However, we don't need a complex dependency system if it does actually just affect whether the classes are already on classpath or not - then a simple list of (optional required) dependencies is enough.

On the other hand, changing the way plugin initialization is done would probably either require major changes in the event manager, or a different way these events are called with that doesn't go through the global event manager directly.

Note: This is just a problem with how Sponge plugins are loaded. Forge handles initialization on a different way, therefore it's not a problem for Forge mods:

  • Afaik initalization events on Forge have no priority plugins can set
  • Forge calls the initialization events per-mod, with mod-specific properties they can get, so it's not an event handled through the normal global event manager
  • The server will crash if one of the mods throws an exception in the initialization events (iirc)

stephan-gh avatar Feb 10 '16 16:02 stephan-gh

Here's a proposal. What if we added an optional initialization method to plugins that could be called in the correct load order and is not an event at all.

Example:

@Initializer
public void onInit() {
}

windy1 avatar Feb 11 '16 19:02 windy1

@windy1 but how would this interact with plugins? would this be done during the construction phase of the plugin? Also one thing to note is that I'm not 100% sure but I do believe GameConstructionEvent is thrown once but it's at the earliest possible start of the entire mod loading process.

gabizou avatar Feb 11 '16 19:02 gabizou

@gabizou

  1. Load classes and construct all plugins
  2. Sort plugins according to dependencies
  3. Call initialization methods. Perhaps the initializer could also return a boolean or a "response" object to report if the initialization was successful and if not, the plugin can be unloaded.

windy1 avatar Feb 11 '16 19:02 windy1

Also another idea is potentially to not only include ordering, but in the @Listener annotation to tell it that it should come before FoxGuard or what have you such that if two event listeners are being called with the same Order, the sorting could still affect where the event is processed by the other listener after the declared listener.

gabizou avatar Feb 11 '16 19:02 gabizou

The order doesn't really make much sense for the initialization event, because the dependencies should be always initialized before the plugin, no matter what order they choose for the event. And if they really need that order, that's pretty much what the multiple initialization events are for.

stephan-gh avatar Feb 11 '16 19:02 stephan-gh

How if a plugin requests to load before or after all plugins?

liach avatar Feb 11 '16 20:02 liach

And if they really need that order, that's pretty much what the multiple initialization events are for.

Yes, I can imagine that there's limited reasoning that different plugins need to be constructed prior to other plugins, but for the sake of argument, there are certain cases where some plugins that are acting as giant libraries (essentially) and upwards of say 10 some-odd plugins exist and they all have to revolve around a singular init event (for the sake of discussion, say the post init event), it'd be wise if those 10 could properly listen to the post init event in their desired order respective of each other.

I don't think at this point that it's a good idea to dictate that a plugin is initialized before other plugins, except in the case where mini plugins are being loaded based on a class path dependency of a library that must be class loaded prior to those mini plugins.

gabizou avatar Feb 11 '16 21:02 gabizou

@gabizou If I understand you correctly, what you suggest is keeping everything as-is, and basically handling required dependency initalization by listening to a later initialization event or setting a late order to the listener?

stephan-gh avatar Feb 11 '16 22:02 stephan-gh

@Minecrell well, to be honest, it doesn't make sense class loading certain plugin jars after others when they all have to be considered regardless. The only thing that does matter is the order of the event listeners, so if I have some plugin that says it needs to come after FoxGuard's event listener, then so be it (if the listener order is already resolved), otherwise it needs to re-order the event listener accordingly. This idea would have to involve event listeners directly rather than affecting the plugin globally.

In general, I think it is wise to say that the only thing about dependencies is that the "required" plugins to exist should be checked, and if they're not existing, then don't load the specific plugin. Plain and simple.

gabizou avatar Feb 11 '16 23:02 gabizou

The point of the plugin dependencies is to have all these little things handled automatically, so if you define a required dependency you don't need to care about special event ordering, as soon as your plugin is initialized you can be sure of the successful loading of required dependencies. Class loading is also one important part of this, because missing classes at class load time can easily break things either directly or at a later point.

I mean, we could of course allow plugin listeners to run before other specific plugins. However, this would still not solve cases when for example the other listener (the one you depend on) actually fails. In initialization phase this usally means the plugin will never work properly for all following events. Currently Sponge doesn't care about any errors at all and neither plugins nor the user can see anywhere that the plugin is actually currently in a kind of unknown state.

At the end this would often result in a lot of dependencies being defined twice, as required dependency and as order of the event listener which would probably cause bugs sooner or later, especially because the way event ordering works right now it is quite likely that listeners are still called in proper order even though implementation-wise the execution of listeners with the same order is essentially undefined.

stephan-gh avatar Feb 11 '16 23:02 stephan-gh

So, there's some things I'd like to clarify:

  • Plugins shouldn't dictate whether they are class loaded before or after each other. What they should dictate is that one depends on another existing in the class path (which is perfectly accepted as requires:FoxGuard etc).
  • Plugins should be able to dictate that provided there are multiple plugins listening to the same event with the same Order, one would be able to say "In the event this plugin is listening to the same event at this order, I want to come before/after it." Having that said, if no specific declaration is made, nothing different happens than what currently is happening.
  • Plugins should be well aware that they can handle their own soft dependencies however they please, be it through custom events at different phases, or listening at a specific GameStateEvent at a specific order, provided they are understanding what is available at the desired GameState.
  • Plugins should _NEVER_ be allowed to expect that anything is available during the plugin object's initialization through plugin loading. This is just bad coding.
    • If a plugin has a hard dependency of a plugin that is nonexistent in the class path, then it shouldn't be loaded and should likely be noted in the server log.
    • If plugins depend on eachother, then loading continues as normal but a warning should be logged to console.

Notes:

  • If two plugins are listening to the same event at the same Order and say that they both should come before each other, do NOT crash the server, but simply chuck an error message in the log and proceed as normal. There should be no reason a plugin can't handle it's co-dependencies properly in any case.

gabizou avatar Feb 12 '16 03:02 gabizou

And to further discuss, plugins that provide APIs should be supported where something akin to Forge's @API annotation is provided and handled magically where parents are resolved properly.

gabizou avatar Feb 12 '16 03:02 gabizou

Some other stuff to consider:

  • Potential integration with a future Ore package manager. Ore uses the author and name as the unique identifier, while Sponge uses the plugin ID.
  • API dependencies. As in: plugin depends on SpongeAPI, SpongeForge implements SpongeAPI, so plugin implicitly depends on SpongeForge.

JBYoshi avatar Feb 12 '16 03:02 JBYoshi

Mhh. IMO @API dependencies and actual @Plugin dependencies are closely related and should be treated the same. So you specify the requirements like that:

Requirements

  • org.spongepowered.service.Economy
  • org.example.someFeature
  • otherPlugin

Offers

  • org.spongepowered.service.XYZ
  • vault.shop
  • myPlugin (implizit)

ST-DDT avatar Feb 12 '16 13:02 ST-DDT

@gabizou I see your point, but that has still several downsides in my opinion:

  • Non-required classpath dependencies are still required, because while some plugins are not required to be on the classpath of another plugin to function properly; in our implementation non-existent classes are cached, therefore the actual plugin containing the optional classes would later fail to load if the other plugin already triggered the class loading before.
  • I don't like the idea of having certain plugin dependencies defined twice or more in the listeners just to run them after the other plugin, this is bad practice IMO, because dependency information (optional or required) should be available for the plugin metadata so it can be displayed appropriately in plugin repositories like Ore.
  • The problem of handling exceptions during the initialization phase is still not solved by this, how would a plugin with a @Listener that runs after another plugin react if something in the other listener actually failed? Also there should be a way for the users and plugins in the API to see if a plugin has successfully passed the initialization phase.
  • I'm kind of worried allowing to define a listener to run before or after another plugin would result in bad coding practices where you would make your listener execute before/after a certain block protection plugin for example, when this should be really handled using one of our many event orders.
  • What you propose as @API annotations sound pretty much like we should somehow implement that using services, so there is always the chance providing this certain API from other plugins.

stephan-gh avatar Feb 14 '16 09:02 stephan-gh

@Minecrell

I don't like the idea of having certain plugin dependencies defined twice or more in the listeners just to run them after the other plugin, this is bad practice IMO, because dependency information (optional or required) should be available for the plugin metadata so it can be displayed appropriately in plugin repositories like Ore.

IMO plugin dependencies have nothing to do with event execution order. If you want to execute all event handlers after some other plugins event handler then we should add a separate annotation for that. @EventOrdering(plugin="fooBar", order="after") this could be annotated at three locations. The plugin main class (all listeners from this plugin), the listener class (only the listeners inside that class), the listener method (only itself). This would also allow the developers to run certain listeners before and some after their dependency's listeners.

Also there should be a way for the users and plugins in the API to see if a plugin has successfully passed the initialization phase.

I agree on that. I would even extend that to the point of marking a plugin as important, (either in the @Plugin or the server config) => if said plugin fails to load the entire server/game does as well. (For example WorldGuard)

I'm kind of worried allowing to define a listener to run before or after another plugin would result in bad coding practices where you would make your listener execute before/after a certain block protection plugin for example, when this should be really handled using one of our many event orders.

I somehow agree on this, but using one of our many event orders is the issue. There aren't enough to do everything in proper order. IMO we should switch to an int based priority.

ST-DDT avatar Feb 14 '16 10:02 ST-DDT

int based priorities will end up with almost meaningless numbers with devs clueless to how they should be ordering them, instead having to just constantly adjust as bugs come up.

ryantheleach avatar Feb 18 '16 09:02 ryantheleach

Before we come to the actual point of plugin initialization, maybe we could decide if we're going to keep the plugin dependencies for the construction / classpath injection in the same way we have it now. No matter what we do with plugin initialization, the plugin construction must always happen ordered, because used classes must be present when loading the plugin.

That would already justify the requirement for "load my plugin after x" dependencies, optional and required. @progwml6 additionally suggested supported the "load my plugin before x" dependencies. I think generally there is sometimes a case where you need this temporary as a workaround, so I'd personally add that, but make it @Deprecated together with a comment stating this should be only used if really necessary.

I'd like to get a solution for the construction part of plugin dependencies soon so I can implement it in SV and can finish the plugin-meta system, plugin initialization order handling can still be discussed later and changed once we have figured this out.

Any opinions against this?

stephan-gh avatar Feb 19 '16 22:02 stephan-gh

@Minecrell

Status update on this.

Zidane avatar Jul 02 '16 13:07 Zidane

@Katrix as you created a duplicate issue, and I've not seen anyone else mention this in the wild, Do we want to try to get this in for API8 or can it hold off?

ryantheleach avatar Aug 03 '18 10:08 ryantheleach

Seeing as API8 will completely throw out the mcmod.info file anyway, if we want to do it, that's probably the best place.

Also, for me, the only times this has been a problem is under the server initialization, where plugin X relies on the fact that plugin Y has been initialized. IMO once the server is loaded, the order should be a good enough. In other words, from my point of view, this is something that should be specific to the game state events.

When I think back on the situation though, I do think there are better ways to address most of the problems that come from this.

  1. More init events. In 1.7.10 and 1.8, A LOT happened in preInit. Today, most of that is handled by separate events like the item/block/entity registration events. As an example, maybe have dedicated events to register commands and services?
  2. Standardization. Where do plugins do X? The answer, it depends on the plugin. We should try to minimize this.
  3. Documentation. A question I see now and then is "When should I do X?" Is the documentation on the docs? Yes. Is it easy to digest? It could probably be easier. As an example, compare Forge's and Sponge's information on the game state events. https://docs.spongepowered.org/stable/en/plugin/lifecycle.html https://mcforge.readthedocs.io/en/latest/conventions/loadstages/

Katrix avatar Aug 03 '18 13:08 Katrix