Marking plugins as enabled after onEnable executes#14073
Conversation
Strokkur424
left a comment
There was a problem hiding this comment.
Why the massive diff? From what this does, it seems to me that simply replacing if (isEnable) with if (enabled) and moving the isEnable = enabled; assignment after the if/else would've sufficed.
Anyways, I don't know about potential implications of what this might cause to behave differently, I leave this up to someone with more knowledge in the plugin loading system.
|
I also changed some of the logic to remove some nesting. Basically just if (x) {
// do x
}to if (!x) {
return;
}
// do xYou are right though, just that would've been fine. |
|
Wouldn't this break a lot of api checks like scheduling tasks in onEnable? Also I agree that tons of plugins definitely already depends too much on this behaviour, it's too old to be touched |
|
I could totally run through and find all those checks. That kind of thing should probably use the more specific |
|
The big issue is this would break any plugin relying on this behavior in their onEnable. |
|
I don't really understand the usecase here. How is your plugin "being enabled twice"? That should not happen during normal startup, duplicate plugins are simply not loaded. |
|
I feel like the proper solution is for your plugin to have this flag that you track and check it instead of isEnable. Even if you increase this pr from a 50 lines change to a 50000 lines change to adapt the rest of the api, you won't be able to adapt the plugins. And if you're fine with |
|
Alright, if it's gonna cause trouble, I'll just close this then. |
I was using
this.isEnabled()to check if my plugin was being enabled twice. This caused the init to get messed up because all plugins are marked as enabled before onEnable is executed. I changed it to updateenabledafter the plugin is enabled/disabled so that people can check this flag in their onEnable functions.