Register custom flags in onEnable instead of onLoad
LadyCailinBot opened this issue ยท 2 comments
WORLDGUARD-3788 - Reported by AlvinB
So I'm working with the WorldGuard API to register my own custom flag, which is working just fine. There is however one thing I'm slightly annoyed at, and it's the fact that you have to register your flags before WorldGuard (and your own plugin) is enabled. This is becomes problematic quite quickly, because
Softdepend becomes essentially useless, because you can't use the plugin manager to check that WorldGuard is enabled (you have to work around this by using Class.forName() and catching the ClassNotFoundException)
Pretty much everything in the JavaPlugin class is null when onLoad is called, so you can't do any logging using getLogger() (such as alerting the server owner/admin of conflicts), or in my case, I have a translation system which uses resources from the plugins jar file, which becomes a pain to get when the plugin isn't enabled, not to mention,
Any actual logging that you manage to do will most likely get drowned in the spam of loading worlds, it's much easier to spot for server owners if it's in the onEnable
I understand that it's entirely possible to work around all these problems, but it becomes a pain in the rear to do so, and the rest of the WorldGuard API is so smooth and seamless, so this was quite the sore thumb. I think if plugins were allowed to register flags in the onEnable, it'd make for a much more seamless integration with WorldGuard.
Now, don't get me wrong, I understand that there is a reason for it to be this way because of information that has to be loaded from disk, but I figured, hey, you guys are pretty smart developers, so I thought I might give this suggestion a shot. I totally understand if this is impractical to do.
Comment by wizjany
-
What the heck? the plugin manager is already populated before it even begins calling onLoad. Doing
server.getPluginManager().getPlugin("WorldGuard")
works perfectly fine in onLoad. No reason to depend on reflection hacks. -
Once again completely wrong. Plugin gets initialized by the class constructor, which sets the server object, the metadata, and the logger.
getLogger()
works fine in onLoad, which is called after every plugin gets constructed (this means you can even access another plugin's logger before IT gets loaded). Getting resources is even easier, since it works as soon as the jar is classloaded, even before onLoad is called. (although it's considered bad practice to do things before load in the bukkit plugin lifecycle) -
Entirely subjective. From what I've seen, servers with lots of plugins have everything drowned out regardless of world loading. Even if everything was logged onEnable, what's to say other plugins aren't also logging things then. If this is somehow a huge issue for you (are you ''expecting'' flag conflicts? maybe you should change your flag names), devise a smarter method like storing messages and run them in a scheduled task a tick after startup, or holding them for when the admin logs in.
-
You didn't number this point, but yes regions have to be loaded from the disk/database, and if this isn't done before WorldGuard is done enabling, it has to be done sometime in the future when players could already be on the server doing things. That means the world could be unprotected while WorldGuard is still waiting to load its regions.
Comment by AlvinB
Oh, I was attempting to use PluginManager#isPluginEnabled (it is what the bukkit tutorials and wiki recommend). I didn't think of doing a nullcheck on PluginManager#getPlugin.
My mistake, the Logging NullPointerExceptions I was getting must have been something else.
My point here is that server owners ''expect'' any errors/information being printed when they enable, since it's what most plugins do. Not many plugins even use the onLoad method for a whole lot. But I guess it may be of personal preference.
So while my arguments weren't very well researched, and as it turns out could effortlessely be worked around, I'd still say that letting the flag integration happen when enabling would be more smooth. But seeing your point number 4, that seems like a valid reason not to do so. I was thinking there might have been an easier solution and that you guys would not have thought of the potential issues that might occur when executing code in the onLoad method. But alas, I was wrong.
Cheers!