Don't be ignorant, The correct way to hook.

How? The first piece of code in post 15 is in the core of bukkit. There is no way to enable a plugin more than once using enablePlugin. A I said, go learn the bukkit codebase before ordering people around.

 

It's still incorrect to do so, you're loading it out of sequence.

Hardly, *ordering* just teaching you the correct way to do it as said by Dinnerbone.

 

You have still given 0 proof as to why it should be done your way. The only reason you've given is completely wrong. And there has been no post from any bukkit dev about this, just you ordering people around.
In regards to "order" of loading, there is no order, they use a function to get the directory list that has no specific guaranteed order of the results.
When I'm not replying via phone I'll post more code to prove your points wrong. Until then try to come up with a valid reason for not enabling the plugin mine depends on when I need it.

 

ask the devs then, I'm not "ordering" I'm giving you a better way than to force someones plugin to enable just because you don't feel like coding correctly.

 

Youre the one who has talked to them about it. Ask them to come back you up if you're so sure you're right.
[MERGETIME="1300153883"][/MERGETIME]
I read over your first post yet again, and you mention no code-based reason or proof that there is a "security issue with users data" or that "it could lead to malformed data loading."

Here is the code for PluginManager.enablePlugin:

Code:

    public void enablePlugin(final Plugin plugin) {
        if (!plugin.isEnabled()) {
            plugin.getPluginLoader().enablePlugin(plugin);
        }
    }

And in PluginLoader.enablePlugin we have:

Code:

    public void enablePlugin(final Plugin plugin) {
        if (!(plugin instanceof JavaPlugin)) {
            throw new IllegalArgumentException("Plugin is not associated with this PluginLoader");
        }

        if (!plugin.isEnabled()) {
            JavaPlugin jPlugin = (JavaPlugin)plugin;

            jPlugin.setEnabled(true);
            server.getPluginManager().callEvent(new PluginEvent(Event.Type.PLUGIN_ENABLE, plugin));
        }
    }

And one for more good measure, this is what actually calls "onEnable()" in the actual plugin people code:

Code:

    protected void setEnabled(final boolean enabled) {
        if (isEnabled != enabled) {
            isEnabled = enabled;

            if (isEnabled) {
                onEnable();
            }  else {
                onDisable();
            }
        }
    }

Now if you actually read that code, you can see that a plugin is set to enabled BEFORE the PLUGIN_ENABLE hook is called.
There is no possible way for my plugin calling enablePlugin() by itself, instead of letting the server do it, to cause ANY problems. There is NO way for data to be "malformed" because it is literally impossible to enable a plugin multiple times. If you look at the setEnabled function, even IT checks to make sure the plugin isn't enabled before going ANY further. There are THREE checks before a plugins onEnable() function is called.

As for "security issues": How can my plugin, which is enabling an already loaded plugin, cause ANY sort of security issue? If you don't trust a plugin, don't put it in the "plugins" directory, because there is no list of what plugins to enable on startup, they are ALL enabled. So please, explain to me these security issues you speak of.

Now, from the looks of your last posts, you've given up trying to spread FUD about "security issues" and "malformed data" but I'd still love to see what your response to this is. Because so far, all you've done is lie, and try to get people to do things your way under the premise of "it's better" and "the devs want it done that way" even though my way works perfectly well (~3000 people using one of my plugins with not a single instance of malformed data) and you haven't had a single dev back you up in this post.

 

I've loaded plenty of plugins both ways, and I can say from experience, that forcing a plugin to load is the wrong way to do it.

When the plugin you load inevitably breaks, or if the plugin uses one of several methods in onEnable(), it will cause your plugin to issue a warning. If you listen for it to be enabled, this issue does not occur.

Test your plugin that forces loading with Permissions 2.5.4 and CB 561+ and you'll probably get a BLOCK_PLACED error.

Forcing a plugin to load is a terrible, terrible thing to do and can disable some features of bukkit or a plugin.

 

I don't understand why you see the call to the PluginManager as "forcing" another plugin to load. It is merely re-scheduling that action to an earlier point in the arbitrarily (alphabetically) determined sequence of enablePlugin() calls. If the plugin to be enabled throws an error, so be it. It would do so anyways and if your plugin depends on the other plugin, it better catch that error and act accordingly.

I am with Drakia here, I don't see the problem in enabling a plugin through the PluginManager. Calling the plugin's onEnable() method directly, is of course another matter entirely.

 

Fairly standard forced load:

Code:

public void setupPermissions() {
    Plugin test = this.getServer().getPluginManager().getPlugin("Permissions");
    if (APlugin.Permissions == null) {
        if (test != null) {
            this.getServer().getPluginManager().enablePlugin(test);
            APlugin.Permissions = ((Permissions) test).getHandler();
        } else {}//possibly do something in here
    }
}

Assuming setupPermissions is called from within your onEnable method, some functions will now return as if they are run in your plugin.

Case 1, godPowers:

Code:

21:48:41 [SEVERE] null loading General v2.2 (Is it up to date?)
java.lang.NullPointerException
at com.FriedTaco.taco.godPowers.godPowers.onEnable(godPowers.java:121)
at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:118)     
at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:414)
at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:187)
at com.nijikokun.cjcfork.bukkit.General.iListen.setupCmds(iListen.java:116)
at com.nijikokun.cjcfork.bukkit.General.General.onEnable(General.java:122)
at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:118)
at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:414)
at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:187)
at org.bukkit.craftbukkit.CraftServer.loadPlugin(CraftServer.java:83)
at org.bukkit.craftbukkit.CraftServer.loadPlugins(CraftServer.java:61)
at net.minecraft.server.MinecraftServer.e(MinecraftServer.java:204)
at net.minecraft.server.MinecraftServer.a(MinecraftServer.java:191)
at net.minecraft.server.MinecraftServer.d(MinecraftServer.java:131)
at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:246)
at net.minecraft.server.ThreadServerApplication.run(SourceFile:366)

General was force-loading godPowers, which used perfectly legitimate bukkit methods that returned null when godPowers was loaded by General. Each worked perfectly independently of each other, and this problem ceased once I fixed my code.

Case 2, LightVote:

Code:

08:45:34 [SEVERE] PLAYER_COMMAND loading General v2.2 (Is it up to date?) java.lang.NoSuchFieldError: PLAYER_COMMAND
 	at nl.xupwup.LightVote.LightVote.onEnable(LightVote.java:108)
 	at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:118)
 	at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:414)
 	at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:187)
 	at com.nijikokun.cjcfork.bukkit.General.iListen.setupCmds(iListen.java:116)
 	at com.nijikokun.cjcfork.bukkit.General.General.onEnable(General.java:122)
 	at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:118)
 	at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:414)
 	at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:187)
 	at org.bukkit.craftbukkit.CraftServer.loadPlugin(CraftServer.java:83)
 	at org.bukkit.craftbukkit.CraftServer.loadPlugins(CraftServer.java:61)
 	at org.bukkit.craftbukkit.CraftServer.reload(CraftServer.java:205)
 	at org.bukkit.command.SimpleCommandMap$ReloadCommand.execute(SimpleCommandMap.jav:196)
 	at org.bukkit.command.SimpleCommandMap.dispatch(SimpleCommandMap.java:80)
 	at org.bukkit.craftbukkit.CraftServer.dispatchCommand(CraftServer.java:183)
 	at net.minecraft.server.MinecraftServer.b(MinecraftServer.java:381)
 	at net.minecraft.server.MinecraftServer.h(MinecraftServer.java:366)
 	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:272)
 	at net.minecraft.server.ThreadServerApplication.run(SourceFile:366)

A player was using an old version of LightVote that was still using the deprecated PLAYER_COMMAND, an update to LightVote was available, but the player identified the problem as being with General because of the way I loaded it.

Case 3, CButD:

Code:

2011-03-23 17:35:05 [SEVERE] BLOCK_PLACED loading CraftBukkitUpToDate v2.1.3 (Is it up to date?)
java.lang.NoSuchFieldError: BLOCK_PLACED
 at com.nijikokun.bukkit.Permissions.Permissions.onEnable(Permissions.java:153)
 at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:118)
 at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:453)
 at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:217)
 at de.nofear13.craftbukkituptodate.CraftBukkitUpToDateHelper.setupPermissions(CraftBukkitUpToDateHelper.java:522)
 at de.nofear13.craftbukkituptodate.CraftBukkitUpToDate.onEnable(CraftBukkitUpToDate.java:65)
 at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:118)
 at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:453)
 at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:217)
 at org.bukkit.craftbukkit.CraftServer.loadPlugin(CraftServer.java:92)
 at org.bukkit.craftbukkit.CraftServer.loadPlugins(CraftServer.java:70)
 at net.minecraft.server.MinecraftServer.e(MinecraftServer.java:203)
 at net.minecraft.server.MinecraftServer.a(MinecraftServer.java:190)
 at net.minecraft.server.MinecraftServer.d(MinecraftServer.java:130)
 at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:245)
 at net.minecraft.server.ThreadServerApplication.run(SourceFile:366)

The player was using a newer version of CB that no longer supports BLOCK_PLACED with an old version of Permissions that doesn't support BLOCK_PLACE. It got blamed on CButD because of the way Permissions is loaded.

All of these are avoided by listening for plugins to load instead of forcing them too.

 

I am not sure what you mean by "perfectly legitimate bukkit methods" that somehow don't work when the stacktrace before the call to onEnable does not follow what the developer of godPowers expected.

Public methods are public methods and they can be called by anything any time. Plugins should be able to enable and disable other plugins at any time and do so without having to worry about the other plugin blowing up.

The two other examples you list are simply errors that would have happened anyway. What assumptions users make about what caused them is somewhat secondary to this debate, but if your code does enable a plugin and you want to be sure that any errors are reported correctly, then you can always catch them in your code, no?

 

@cjc343 That makes no sense, if they NPEd while being enabled inside General, then they would NPE outside of general, the only difference is the stack trace (Which again, users don't understand, but a plugin dev should be able to).

You calling enablePlugin doesn't magically change what environment they run in, it doesn't change what server variables they have access to, it doesn't change ANYTHING except the random order with which they already load.

The more likely scenario is your plugin, instead of dieing when those plugins did, just continued to load without them. This is one upside to doing it how Niji suggested, and is one reason I'm slowly (Very slowly) switching my plugins over. But it doesn't CAUSE problems like that NPE to occur. Just as Niji was trying to spread FUD by saying that it causes data corruption and security issues, you're trying to spread FUD by saying it causes NPEs to occur.

As sunkid said, if you want to make sure your plugin still loads even if one of its dependencies dies in such a manner, just put a try block around it, and your problem is solved.

 

java.lang.NullPointerException at com.FriedTaco.taco.godPowers.godPowers.onEnable(godPowers.java:121)

It was open source, I took a look, here's line 121:

getCommand("godMode").setExecutor(new godModeCommand(this));

Instead of checking for the godMode command in godPowers, it was checked for in General, breaking godPowers if General was installed because of the way General forced godPowers to load.

Example 1 is not an example of an error being attributed to General, it is General causing an error by forcing another plugin to load.

By catching that error, you may prevent a user from even seeing that another plugin is not working correctly.

I never meant to claim that the 2nd two were a direct result of forcing a plugin to load, but they do cause user confusion about who has an error in their plugin, causing more work for you.

Example 2 did not kill General and Example 3, I don't know, but likely did not kill CButD, but they did cause users to misattribute errors and have a harder time keeping their servers up to date.

There are definitely some cases where force loading another plugin will CAUSE an NPE, I am not trying to claim that this is the case with any of the methods Permissions currently uses.