-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Brigadier Command Support #8235
Brigadier Command Support #8235
Conversation
Would people rather have the ability to override nodes sent to the client? Or just simple argument types? Because currently, you can use a WrapperArgumentType inorder to send the client a different argument than is on the server. So basically this allows you to make custom argument types whilst still sending it as a string argument to the player. Would it be useful to extend this to nodes in general? |
31e1982
to
a706ff3
Compare
Rebased and tested with plugins like FAWE. If people have any plugins that do hacky things with the command map, if you would be willing to link that would be awesome. I want to make sure bukkit commands still properly work here. Although they seem fine. |
Tested it with my on-demand command registration system, which uses the command map and manipulates + updates it during runtime, and it worked just fine! 👍 |
One thing I wonder here is what "popular" libraries there are that allow brigardier implementation. I can only think of Commodore by Lucko and obviously dealing with brigardier directly... To be honest, I feel like a more native support for Commodore - at least for the file - would benefit the community here, as you otherwise would need to include yet another library for the brigardier support. Tho that's just my opinion on this. |
We want to be able to provide native very low level support. If we decide to add a library natively we might do that in the future, but this is supposed to just allow for custom brig commands to be registered using the api. Any api may be much more in the future, this is bare bones. |
a706ff3
to
059906a
Compare
Initial review is ready. I have added support for signed command arguments. This allows you to retrieve a SignedMessage if a Message argument is provided. |
All command node registration will be done in the vanilla dispatcher. When adding custom nodes, those nodes will be "unwrapped" which converts them into an NMS version which is then registered to the dispatcher. When retrieving nodes from the dispatcher it will check to see if there is a cached "unwrapped" version, where then it will be able to assume that this was created using API. However, if no unwrapped version exists (like for vanilla commands) it will use a "shadow" node which provides restricted access. |
Please feel free to test this branch and ensure that all legacy commands work the same. There is now a jar attached to the PR body. |
Got rid of duplicate command execution logic. Player#chat will now use the same chat logic and will no longer throw exceptions to match vanilla behavior. (Todo, exceptions are thrown rn) Fixed issue with redirect flattening on shadow nodes. Also fixed many issues with tab completion/command execution in general when using redirects (eg: /execute) In general things seem to be running quite smoothly, but PLEASE feel free to test on your server to ensure all commands work properly. |
Calling
Nothing hacky is happening here - the plugin is unregistering its commands by calling |
f286e3e
to
f18d89c
Compare
Thanks for your report, In general, now it'll prefer to lazily convert command nodes and properly support mutability. |
Okay. I now have another issue. Command unregistration remains unsuccessful. The server no longer hangs when a command is unregistered using the method I described above. However, the command remains accessible. I suppose it was never truly unregistered. |
Hello! I'm the creator of the CommandAPI, a Brigadier-based command API for Bukkit/Spigot/Paper that lets users define brigadier commands. The CommandAPI has two main users: developers (people who make plugins) and non-developers (people who don't make plugins - typically server admins). To accommodate non-developers, the CommandAPI provides a "command conversion" feature which lets the CommandAPI register brigadier-compatible commands on behalf of plugins that were written using the standard Bukkit command framework. The CommandAPI's command conversion feature is not only compatible with the built-in commands (e.g. This PR breaks the CommandAPI's command conversion feature. If I have a plugin [14:33:38 ERROR]: Failed to load function mynamespace:myfunction
java.util.concurrent.CompletionException: java.lang.UnsupportedOperationException: Not supported yet.
at java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315) ~[?:?]
at java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320) ~[?:?]
at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1770) ~[?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) ~[?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) ~[?:?]
at java.lang.Thread.run(Thread.java:833) ~[?:?]
Caused by: java.lang.UnsupportedOperationException: Not supported yet.
at net.minecraft.commands.CommandSource$1.getBukkitSender(CommandSource.java:29) ~[?:?]
at net.minecraft.commands.CommandSourceStack.getBukkitSender(CommandSourceStack.java:404) ~[?:?]
at io.papermc.paper.command.brigadier.bukkit.BukkitCommandNode.lambda$new$0(BukkitCommandNode.java:31) ~[paper-1.19.3.jar:git-Paper-"0cf8c8e"]
at com.mojang.brigadier.tree.CommandNode.canUse(CommandNode.java:81) ~[paper-1.19.3.jar:git-Paper-"0cf8c8e"]
at com.mojang.brigadier.CommandDispatcher.parseNodes(CommandDispatcher.java:359) ~[paper-1.19.3.jar:?]
at com.mojang.brigadier.CommandDispatcher.parse(CommandDispatcher.java:349) ~[paper-1.19.3.jar:?]
at net.minecraft.commands.CommandFunction.fromLines(CommandFunction.java:61) ~[?:?]
at net.minecraft.server.ServerFunctionLibrary.lambda$reload$2(ServerFunctionLibrary.java:80) ~[?:?]
at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1768) ~[?:?]
... 3 more Steps to reproduce
While I'm aware that the CommandAPI's command conversion feature is fairly hacky (ideally, it shouldn't exist and commands should work naturally, as they sort of do with this PR with |
@A248 Please take a look now, I have improved this removal logic and tested it locally. |
@JorelAli This should be fixed now. Although there is still an error at the beginning server log, I am now able to run the command. https://nightly.link/PaperMC/Paper/actions/artifacts/495487438.zip |
8e41123
to
0d8c390
Compare
Awesome! 🎉 |
…tegy` Just a rough example implementation `NMS#getResourceDispatcher` replaced by `NMS#createCommandRegistrationStrategy`, which creates a `SpigotCommandRegistration` implementation using the resources dispatcher Alternate fix for #554. If we're on Paper-1.20.6-65, a `PaperCommandRegistration` implementation is used instead. This can be used to properly handle the internal changes made by PaperMC/Paper#8235.
…tegy` Just a rough example implementation `NMS#getResourceDispatcher` replaced by `NMS#createCommandRegistrationStrategy`, which creates a `SpigotCommandRegistration` implementation using the resources dispatcher Alternate fix for #554. If we're on Paper-1.20.6-65, a `PaperCommandRegistration` implementation is used instead. This can be used to properly handle the internal changes made by PaperMC/Paper#8235.
…tegy` Just a rough example implementation `NMS#getResourceDispatcher` replaced by `NMS#createCommandRegistrationStrategy`, which creates a `SpigotCommandRegistration` implementation using the resources dispatcher Alternate fix for #554. If we're on Paper-1.20.6-65, a `PaperCommandRegistration` implementation is used instead. This can be used to properly handle the internal changes made by PaperMC/Paper#8235.
Adds the ability for plugins to register their own brigadier commands by using a CommandBuilder.
Summary
WrapperArgumentType
.👋 Hello rich syntax checking!
They show in the help menu and such just like normal plugins! (Description can be set in the command builder)
Indirectly fixes: #6293
#8255
Download the paperclip jar for this pull request: paper-8235.zip