Brigadier Command Support

[Owen1212055]

Adds the ability for plugins to register their own brigadier commands by using a CommandBuilder.

Summary

• Deprecates CommandMap, moving all logic to a brig-backed map for legacy support. (All custom brig commands are wrapped in a vanilla command wrapper)
• Moves away from simplecommandmap dispatch for command logic, now brig will always be used.
• Exposes some "primitive" vanilla types, with players having the ability to add their own custom arguments using WrapperArgumentType.
• ALL bukkit commands will now be turned into brig nodes, no more syncCommands logic.
• Exposes Server#getCommandDispatcher, which returns a "mirror" of the NMS dispatcher.
  • Command nodes that are passed into this are converted into NMS types then passed into the NMS dispatcher.
  • This conversion prevents "nms object leaking"
• Our intention is not to make a built in command framework. Instead, suggesting that people use popular frameworks instead.

👋 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

[Owen1212055]

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?

[Owen1212055]

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.

[TheFruxz]

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! 👍

[Andre601]

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.

[Owen1212055]

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.

[Owen1212055]

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.

[Owen1212055]

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.

[Owen1212055]

Please feel free to test this branch and ensure that all legacy commands work the same.
So, just load your server and ensure that all commands (with plugins) seem to be working correctly. This includes stuff like namespaced commands, etc.

There is now a jar attached to the PR body.

[Owen1212055]

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.

[A248]

Calling Map.values() on the knownCommands map causes the server to hang. BukkitBrigForwardingMap.values appears to be responsible. Steps to reproduce using the plugin LibertyBans:

1. Setup server, install the latest release of LibertyBans.
2. Run /libertybans restart from the server console
3. View server hang, including thread dump, and subsequent termination by WatchDog.

Nothing hacky is happening here - the plugin is unregistering its commands by calling map.values().remove(command) for each of its command objects. However, this removal is happening during a command execution (though the command executed is not the command unregistered).

[Owen1212055]

Thanks for your report,

In general, now it'll prefer to lazily convert command nodes and properly support mutability.

[A248]

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.

[JorelAli]

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. /execute), but it is also compatible with datapacks, allowing Bukkit command plugins to be used in datapacks.

This PR breaks the CommandAPI's command conversion feature. If I have a plugin SimpleBukkitPlugin with a command mycommand, the CommandAPI would register the command (using the default Minecraft namespace) /minecraft:mycommand. If you have a datapack with a function mynamespace:myfunction which contains mycommand, the CommandAPI would normally allow /function mynamespace:myfunction to execute mycommand with no issues, however this PR instead fails to load the function with this error message:

[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

• Download SimpleBukkitPlugin-1.0.0.jar from here: https://github.com/JorelAli/SimpleBukkitPlugin/releases/tag/1.0.0

• Download CommandAPI-8.7.1.jar from here: https://github.com/JorelAli/CommandAPI/releases/tag/8.7.1

• Create plugins/CommandAPI/config.yml (if it doesn't exist) and populate it with the following:

  verbose-outputs: true
  silent-logs: false
  messages:
    missing-executor-implementation: "This command has no implementations for %s"
  create-dispatcher-json: false
  use-latest-nms-version: false
  plugins-to-convert:
    - SimpleBukkitPlugin:
      - mycommand
  other-commands-to-convert: []
  skip-sender-proxy: []

• Create the following default datapack, where myfunction.mcfunction contains mycommand

  server/
  ├── world/
  │   ├── advancements/
  │   ├── data/
  │   ├── datapacks/
  │   │   └── bukkit/
  │   │       ├── pack.mcmeta
  │   │       └── data/
  │   │           └── mynamespace/
  │   │               └── functions/
  │   │                   └── myfunction.mcfunction
  │   └── ...
  ├── world_nether/
  ├── world_the_end/
  ├── ...
  └── paper.jar
  

• Load paper with the paperclip jar from the first comment.

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 /execute), it would be nice to see some support for Bukkit commands registered via datapacks, especially given that this PR breaks my solution to this problem.

[Owen1212055]

@A248 Please take a look now, I have improved this removal logic and tested it locally.

[Owen1212055]

@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

[bluelhf]

Awesome! 🎉