Make the configuration less misleading with respect to command conflicts
A248 opened this issue ยท 1 comments
Type of bug
Other unexpected behaviour
/ess dump all
output
N/A
Error log (if applicable)
N/A
Bug description
I was uncertain whether to file this issue as a bug or a feature request. Because it is unexpected behavior, of a sort, rather than some superfluous feature, I decided to file a bug.
Essentials is a large plugin with hordes of features, including dozens of commands. Unsurprisingly, this means Essentials' numerous commands sometimes conflict with other plugins that provide similar features. Many times, server owners operate Essentials alongside these more precise plugins when they desire finer granularity in a certain area. Command conflicts are therefore common.
Essentials, seemingly recognizing this, has long offered a feature that attempts to grant commands to the other plugin in the case of a conflict.
The above line in the configuration is the subject of this issue. It is misleading, however, for several reasons. Ultimately, this feature causes so much annoyance, that it should either be removed, or properly documented for its myriad failings.
Firstly, the conflict resolution feature is unreliable: it requires other plugins to implement PluginIdentifiableCommand
. In the case of dynamic command registration via the command map, implementing PluginIdentifiableCommand
is not a strict necessity. It is not enforced by Bukkit, only mentioned in the javadoc as relating to index generation. One might argue that plugin commands ought to implement this interface; however, in reality, the specification is vague, and in practice, plugins have no requirement to do this.
Secondly, it is inaccurate, because tab completion is not properly redirected in all situations. Only commands extending PluginCommand
support tab completion redirection. This incompleteness means that commands appear to work: execution succeeds, while tab completion is missing. The result, for users and developers, is confusion and debugging.
Lastly, it is imprecise. Even if everything seemingly goes well, the command is nevertheless registered according to Bukkit under the plugin Essentials. This results in improper usage messages (#3904), curious stacktraces, and incorrect API information. An exception in another plugin can be reported as occuring while executing an Essentials command because only Exception
is caught here. Even were all exceptions caught, the presence of Essentials in the resulting stacktrace would only raise eyes. Besides funny-looking exceptions, Essentials' command redirection disorients uses of the API that require identifying command ownership, such as administrative utility and help plugins.
Because of the latter two reasons, it is very understandable for authors of plugins with conflicting commands to not want Essentials to redirect commands. Consider the support burden perspective. Users might claim tab completion does not work, causing authors to dig through their own code while frustrated at the lack of reproducibility. Guiding users in solving command conflicts is difficult enough without having to explain that Essentials -- the Essentials they know as a popular, reliable plugin -- was responsible for breaking another plugin's tab completion. If I knew implementing PluginIdentifiableCommand
would generate such problems, I probably would avoid implementing it. Wouldn't you?
Steps to reproduce
Read the configuration. Debug and test with other plugins that provide conflicting commands.
Expected behaviour
To remedy the quagmire, the Essentials plugin should do one of the following:
- Remove this broken feature
- Stop misleading users; update the configuration comment to detail all the caveats of relying on it
- Properly support disabling commands via #3809
Actual behaviour
Annoyance at Essentials for providing a feature so detrimental to users and unfriendly to other plugins in the ecosystem.
I cannot comment on the large paragraph you wrote, as that's out of my knowledgebase, but I can offer you this. EssentialsX does already provide a way to completely disable commands. In the config, you can find disabled-commands
, right below the overridden-commands
that you pulled that quoted line from - adding commands that EssentialsX registers to this, will prevent it from registering them. (meaning EssentialsX no longer handles the command)
I will say that the wording on the comment is indeed slightly misleading, but that has not been updated in 11 years.
I am going to mark this as working as intended, and will look into getting that comment fixed.