EngineHub / CommandBook

General and administrative commands
https://enginehub.org/commandbook/
GNU Lesser General Public License v3.0
145 stars 105 forks source link

Playing Nice with Bukkit's New Command Handling #241

Closed LadyCailinBot closed 4 years ago

LadyCailinBot commented 10 years ago

CMDBOOK-2361 - Reported by Swords761

I have been having some issues trying to get CommandBook to work with Bukkit's new command handling that was formally introduced in the latest beta build (1.7.2-R0.3). Specifically, it looks as though the /tp command is never registered anymore by CommandBook (/commandbook:tp returns "unknown command"), and trying to make aliases for CommandBook commands is just not working (ie. adding an alias for /clear to be tied with /commandbook:clear doesn't work, although doing the command in-game is fine). Is this a problem on my side? What am I doing wrong or need to make sure of in configuration in order to get it to work? Or is this something that'll require code changes on CommandBook?

LadyCailinBot commented 10 years ago

Comment by sk89q

Are there errors in the log?

WorldEdit and friends use similar methods (but I'm not sure the same code) so they should theoretically break together.

LadyCailinBot commented 10 years ago

Comment by Swords761

Nope, no errors- unless you count the warning that Bukkit delivers upon trying to register the alias: [Server thread/WARN]: Could not register alias clear because it contains commands that do not exist: commandbook:clear $1- [Server thread/WARN]: Could not register alias tp because it contains commands that do not exist: commandbook:tp $1-

I followed the steps as outlined here: wiki.bukkit.org/Commands.yml In-game, /commandbook:clear works, but /commandbook:tp does not. Yet they're both unable to get an alias registered to them.

Also, I should mention I'm running Spigot #1319... I hope that isn't what's causing this issue.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

I can confirm that commit https://github.com/Bukkit/CraftBukkit-Bleeding/commit/bad64516f7f871670a7f43cbd52ee6a6656b0d84 breaks some commands.

LadyCailinBot commented 10 years ago

Comment by Puremin0rez

Can also Confirm. Ban and Kick are sadly not working anymore - which is a huge, huge problem.

LadyCailinBot commented 10 years ago

Comment by tmad40blue

Thought this was just me - glad to see it's an issue multiple people are having. I talked to zml2008 in IRC yesterday and he couldn't put a finger on it either.

LadyCailinBot commented 10 years ago

Comment by tmad40blue

Info from Bukkit on the new command handling:

https://forums.bukkit.org/threads/craftbukkit-1-7-2-r0-3-is-now-available.232215/

http://evilseph.com/post/75608917830/bukkit-supporting-minecraft-commands

LadyCailinBot commented 10 years ago

Comment by Богдан.Шкляренко

yeap, need to fix :c

LadyCailinBot commented 10 years ago

Comment by Puremin0rez

... Any progress?

LadyCailinBot commented 10 years ago

Comment by Богдан.Шкляренко

Post the info about dev. stage if u can, plz

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

I'm investigating the issue at the moment. No promises.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

This appears to be affecting only commands which have aliases that are the same as the default command. Hence, the "teleport" command registering "tp"

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

I believe this issue originates [here|https://github.com/Bukkit/Bukkit/commit/d7e0197e9d096b6ce2e359c260a33665fb930eef#diff-ce6e0e9d338bfef56a7c5947bae2406aR132]. It appears the problem is that "tp" is being registered as an alias instead of as "tp" which means it get thrown out since vanilla has a tp command already.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Here's a hot fix that fixes the TP command built on top of the code-freeze branch (the parchment one has the Util refactor and could be buggy).

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

@Puremin0rez Do you have low priority command registration? If so disable that, and kick and ban should work just fine.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Once [t00thpick1's bleeding commit|https://github.com/Bukkit/Bukkit-Bleeding/commit/0dce5e2b7213b54a3109b9cfcf1dae501344d6c2] is merged the hot fix should no longer be necessary.

LadyCailinBot commented 10 years ago

Comment by Puremin0rez

@Dark_Arc Ahh yes! That worked! Thanks a lot :)

LadyCailinBot commented 10 years ago

Comment by Богдан.Шкляренко

I must use commandbook.jar (352 KB)for now?

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

For the moment yes.

LadyCailinBot commented 10 years ago

Comment by Swords761

The hotfix didn't work for me, but I'm also willing to just wait for Bukkit's commit for the problem to be fixed.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

@Swords761 It only fixes the teleport command, if you have low priority command registration, it won't work for ban and kick -- provided that's what didn't work. You didn't give many details :P

LadyCailinBot commented 10 years ago

Comment by Swords761

Sorry about that- I'm not using ban/kick with CommandBook; my interest is in the /tp, /clear, and /time commands. And none of them register to Commandbook by default, even with this hotfix. That might be because my configuration currently has low priority commands enabled, but the commands.yml file from Bukkit still doesn't like making aliases for commandbook commands (ie. trying to make /tp = /commandbook:tp still won't work). I can also try disabling that later and seeing if it works.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Ya, disable it and they will work with the hotfix.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Updated to include latest fixes.

LadyCailinBot commented 10 years ago

Comment by Swords761

Looks like everything works with low priority commands disabled; thanks for the bugfix!

LadyCailinBot commented 10 years ago

Comment by RustyDagger

Using the latest fix /s Appears to not exist. Its a real shame as my whole player base is used to using it with /call. For now I have told them to use /bring instead and at least that works. Re teaching a player base commands is never an easy task.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

It's /grab, and /g now. However, if you look into bukkit's new aliasing system, you can make /s to be an alias for bring fairly easily! :)

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

You should be able to fix this issue in particular without the hotfix as well. Simply add this to commands.yml:

tp:
- commandbook:teleport $1-

To add /s back:

s:
- commandbook:bring $1-
LadyCailinBot commented 10 years ago

Comment by RustyDagger

Just as a side note dark_arc after entering the s: alias into my commands.yml I got this error in console and I have to agree with it about the command not existing. Maybe it was broken or changed somewhere along the line the config.yml does also point to What you said working too.

[MC]: [16:29:39 WARN]: Could not register alias s because it contains commands that do not exist: /commandb ook:bring $1-

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Do you have low priority command registration enabled? That might break it -- and should probably just be removed since it's only causing problems now it seems.

LadyCailinBot commented 10 years ago

Comment by RustyDagger

I Had the fix working if that's what you are asking I set that the right way days ago to allow the fix to work. But it seems odd even as OP i cant run the /commandbook:bring or any thing for that matter.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Interesting, what do you mean "anything"?

LadyCailinBot commented 10 years ago

Comment by RustyDagger

After a reboot and some more tinkering /commandbook:bring Name Now works from ingame. its odd that commands.yml is still giving me that error on start up

[MC]: [16:29:39 WARN]: Could not register alias s because it contains commands that do not exist: /commandb ook:bring $1-

and by anything I meant any other command such as clear or item. but iv resolved that now small derp on my part.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Remove the /'s my bad.

LadyCailinBot commented 10 years ago

Comment by Богдан.Шкляренко

Any Progress?

LadyCailinBot commented 10 years ago

Comment by Swords761

My understanding is that you need the above download for 1.7.2 and it'll work fine, and that this bug was fixed in a Bukkit commit that is definitely built into 1.7.5. So essentially, this bug is fixed and the ticket could be closed.

LadyCailinBot commented 10 years ago

Comment by Богдан.Шкляренко

o_O Bug is not fixed. Im using latest build from builds.enginehub.org with latest Spigot. And nothing working, only /commandbook:tp or /commandbook:weather. I cant put just /tp, no, it wouldnt work :c

LadyCailinBot commented 10 years ago

Comment by Swords761

And you added this to your commands.yml file?

- commandbook:teleport $1-
LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Use that command.yml change, that's the best way to do it. It's currently not fixed in the lastest builds of Bukkit, though it's still on their end.

LadyCailinBot commented 10 years ago

Comment by Богдан.Шкляренко

U can just add autoedit of commands.yml to Commandbook on server start C:

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

I'm pretty sure that breaks bukkit rules, and it's not worth doing. If they don't fix it soon, I'll just apply the fix from the 1.7.2 jar into master.

LadyCailinBot commented 10 years ago

Comment by wizjany

Commit seems to be merged, https://github.com/Bukkit/Bukkit/commit/fa88ff4138b45de116c4c66c3900e5c93653eca0 Does this issue still occur or can it be closed?

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Nice catch I should be able to do some commandbook stuff tomorrow, I'll take a look then.

LadyCailinBot commented 10 years ago

Comment by Dark_Arc

Confirmed fixed.