chore: deprecate Bukkit commands

[Strokkur424]

The command docs nowadays only recommends Paper's Brigadier command API. Alongside with users in the Discord's #paper-dev channel typically being pointed towards Brigadier instead of Bukkit commands as well. However, many users may still, unaware of the new system, default to the decade-old system. The deprecations in this PR aim to aid plugin developers to be made aware of the no longer relevant Bukkit command system and point towards the recommended alternative through Javadoc comments.

I chose the since parameter on the @Deprecated to be "26.3" because of the near zero chance of this being merged before 26.3 properly releases.

[NonSwag]

Even though I would love to see those classes be nuked, I think marking them as Obsolete first might be a better idea.

[Strokkur424]

Obsolete doesn't create any IDE warnings though. The idea here is to make people aware there's a better system via deprecation warnings.

[NonSwag]

Obsolete doesn't create any IDE warnings though

For me it does… but I also changed my inspection profiles a lot from the defaults so not really sure if that is the default behavior

[Malfrador]

Don't really see much reason for deprecating this.

BasicCommand is a subpar replacement at best, and the built-in API still lacks important features such as easy support for unquoted arguments with non-ASCII characters. As long as brigadier does not support that properly, including on the client, I am very much against deprecating org.bukkit commands.

Pointing people towards third-party projects is not a solution. Something as essential and basic as in-game commands should be part of the API, in a manner that is easy to use and understand. Pure brigadier is not that, even with good docs existing. In fact the sheer length of docs required for understanding Brigadier outside of BasicCommand shows that it's not a good replacement.

As opposed to e.g. Fabric the point of Bukkit and Paper has always been to provide a large set of API methods. And that absolutely includes a good, easy to use command API without needing to rely on external projects.
If anything, an externally library could be included into the API like it was done with adventure.

Additionally, deprecating one of the most used classes in the API with over a decade of plugins, code and tutorials behind it should not just happen randomly on whim.
Only if the maintenance burden of keeping it working in it's current state gets too high it should be deprecated.
I don't see this being the case with commands currently.

Obsolete does seem a lot more fitting. Especially because it does not create IDE warnings by default.

[Strokkur424]

BasicCommand is a subpar replacement at best

If that is the case, what exactly is missing in the current BasicCommand implementation that you think is better in the legacy Bukkit commands system?

[YouHaveTrouble]

BasicCommand is a subpar replacement at best

In what way is it subpar? It can do everything bukkit command interface can, it just provides more context for executor apart from just the sender and makes it compatible with /execute command thanks to that.

That old api no longer represents how commands even work after 1.13. Saying that just reducing entire command api to a single command node with greedy string arg is somehow acceptable is kinda wild to me, given how much more is going on with commands nowadays

[Malfrador]

BasicCommands can not be registered/re-registered at runtime as far as I'm aware.

Last time I tried it out:
Syntax highlighting does not happen beyond the first argument.
Tab completion for subcommands after the first space does not work.
Greedy string tab completion is broken, suggestions for it only ever get added at the end, e.g. if you have /warp The E and press tab, you get /warp The The End.
The latter is a client brigadier issue as far as I'm aware.
This also apparently is not fully fixed, additional spaces get added in some cases: #11211

The org.bukkit commands do not suffer from this.

A greedy string argument is the only one that supports non-ASCII characters properly without quoting. e.g. if you want something like /f show Königreich, it's either Bukkit commands or a greedy string.

[Strokkur424]

Beyond the fact that Bukkit commands do not have any syntax highlighting in general, exactly the same as BasicCommand, runtime command registering/unregistering has always been on the edge of unsupported. Bukkit commands have also never had support for requirements, which technically allow you to control in great detail, what players can or can't see commands, which with Bukkit commands has always only been possible through permissions, which for this use case are flawed.

Regarding the tab completion, I am not sure what you are talking about. Both BasicCommand and Bukkit commands use greedy string. There is genuinely no difference to the client. So if there's a change in behavior, that's a faulty implementation either by you or by Paper (in case of BasicCommand's tab completion, maybe?) which should simply be fixed.

[NonSwag]

I have never used BasicCommand nor do I know the implementation specifics of it, but if those issues are real, they are most certainly unintentional bugs and should be reported formally.

The latter is a client brigadier issue as far as I'm aware.

If anything, that is a server issue in how the suggestion provider is overridden.

[Malfrador]

If those issues are fixed and BasicCommand becomes the easy replacement it was intended to be two years ago, sure. But the deprecation should wait until then.

[Mickey42302]

Wouldn't removing these classes break a lot of plugins? Even if they are considered obsolete, compatibility is still essential.

[Strokkur424]

It is not, and never has been, a goal to remove these classes.
This PR is deprecating them, which produces a warning in the IDE when you use them. Nothing more.