YiC200333 / XConomy

An economy plugin that supports data synchronization between multiple servers
GNU General Public License v3.0
98 stars 34 forks source link

Fix economy and eco commands not being registered #13

Closed adamtomi closed 4 years ago

YiC200333 commented 4 years ago

Do you set eco-command to true in the config.yml? I fixed this problem in the 2.7.24 version. Does this problem still exist?

adamtomi commented 4 years ago

Ouch, I completely missed that... :( But now that this PR is open, I'll take the opportunity and test it. :)

adamtomi commented 4 years ago

Okay, I removed the aliases from plugin.yml and changed how eco-commands setting works. If set to true XConomy will register those commands even if EssentialsX is not present. Imho this make migrating from Ess economy less of a hassle since you don't have to dig through every single config that might use the Ess style economy command :D

YiC200333 commented 4 years ago

I think if Essentials plugin is not installed, there is no requirement for additional registration /economy command

adamtomi commented 4 years ago

Just tested this, and if Essentials is not installed, /economy command is not registered. There are multiple ways to register a command, the most common is putting them in plugin.yml. On startup, Bukkit creates you a PluginCommand instance for these commands. Example:

commands:
  money:
    description: Just some description
    aliases:
    - economy
    - eco

In the above case Bukkit will generate a PluginCommand for you with the name money and aliases economy, eco. You can access this PluginCommand by calling JavaPlugin#getCommand. So you'd registering your command would look something like this:

@Override
public void onEnable() {
   PluginCommand command = getCommand("money");
   if (command != null) {
      command.setExecutor(new MyCommandExecutor());
   }
}

Now MyCommandExecutor will be responsible for the execution of money, economy and eco commands (because money is the name of the command, and the others are the aliases). The problem is, that you don't have economy and eco commands in your plugin.yml since it'd conflict with Essentials and you'd have to use xconomy:economy and xconomy:eco which is far from ideal. However, this also means, that the case "economy": and case "eco": part of your Commands class will never get executed because those commands "do not belong that executor".

Just to prove my point (Fresh 1.16 server with only Vault and XConomy):

[09:53:15 INFO]: [Vault] Checking for Updates ...
[09:53:18 INFO]: [XConomy] Is the latest version
> pl
[09:53:19 INFO]: Plugins (2): Vault, XConomy*
[09:53:19 WARN]: [Vault] Stable Version: 1.7.3 is out! You are still running version: 1.7.2
[09:53:19 WARN]: [Vault] Update at: https://dev.bukkit.org/projects/vault
> eco
[09:53:22 INFO]: Unknown command. Type "/help" for help.

As you can see it simply doesn't work (and yes, I have eco-commands true in my configuration).

The results are different with the version I compiled from my latest commit:

> pl
[09:56:46 INFO]: Plugins (2): Vault, XConomy*
> eco
[09:56:47 INFO]: [XConomy]This command cannot be used by the console

And if I disable eco-commands:

> pl
[10:01:33 INFO]: Plugins (2): Vault, XConomy*
> eco
[10:01:34 INFO]: Unknown command. Type "/help" for help.
YiC200333 commented 4 years ago

I know these things, I originally intended to override Essentials economic commands, so I did not intend to register /economy command on servers without Essentials installed.

Thank you for your feedback :D

adamtomi commented 4 years ago

I see. Thanks for merging this PR and I'm here to help if you need something :D