Aliases for commands defined in commands.yml don't properly TAB complete

WiIIiam278 commented 1 year ago

Expected behavior

I'm making this issue since #1733 has long since gone stale, and with the recent changes to paper plugins and brigadier-backed plugin commands, I think it's relevant again.

Since the addition of TAB completion in Minecraft 1.13, the commands.yml aliases system hasn't been updated to properly respect the TAB completion providers of commands.

A side effect of paper plugins being introduced is that the load order of plugins has changed, in part as developers seek to close the loops of circular dependencies. In turn, this means that the order commands are registered has changed between plugins. The official solution to this problem, aside from developers implementing configuration options to outright disable commands, is to define overriding aliases in commands.yml, redirecting the standard command to the namespace-backed fallback version of a command for the plugin you wish to take precedence.

For instance, say two plugins define a /home command: Plugin1 and Plugin2. Plugin2 happens to register its command before Plugin1, but the user would like it so that Plugin1's home command is the one effectively used when the user types /home.

So, the user defines an alias in commands.yml (relevant bukkit docs):

aliases:
  home:
    - "plugin1:home $1-"

This alias tells the server that when /home is executed, to instead execute /plugin1:home with all the arguments passed, starting with the first argument (this system is 1-indexed). Executing the alias with any amount of arguments now correctly executes plugin1's home command.

Observed/Actual behavior

The problem, however, is now when the user attempts to TAB complete when using an alias.

Instead of displaying the TAB suggestions provided by plugin1's home command, the suggestion provider instead always returns the list of every online player as the suggested input for every argument.

The same is true if you define an alias that forwards a different number or range of passed command arguments; the online player list is always returned instead of the correct TAB completion.

Steps/models to reproduce

Install a plugin that provides TAB suggestions for a command
Define an alias for it in commands.yml that passes all arguments to the command
Enter the /installedplugin:command that you defined an alias for; observe TAB suggestions working normally
Observe that when you enter the alias you defined, instead of the target command's TAB suggestions being shown as before, the local player list is instead displayed for every possible argument, for some reason.

Plugin and Datapack List

n/a — Though for testing with a command override, you could try HuskHomes and Essentials, which both provide a /home command, among others.

Paper version

1.19.4-534

Other

I'm not familiar with the (presumably, mostly ancient CraftBukkit) code that handles this system, but I suspect given the fact the alias system supports logic such as argument ranges, an implementation of this system would be fairly complex to pull off, particularly as paper commands don't necessarily handle TAB completion based on the context of how many arguments have been passed—or use something else to handle this, such as Lucko's Commodore library.

The solution I'd like to see is, if a user is TAB completing with an aliased command, that Paper would check if their current input is within the range of the supported aliases, and, if so, grabs the TAB suggestions from the aliases' target command TAB suggestion provider (if there is one); otherwise does not suggest any arguments; and not the list of online players.

Owen1212055 commented 1 year ago

Can you see, does https://github.com/PaperMC/Paper/pull/8235 resolve this issue?

WiIIiam278 commented 1 year ago

Can you see, does #8235 resolve this issue?

Just tested with the linked paperclip in that issue -- the issue persists and there is no change in behaviour to that of what I described here

electronicboy commented 1 year ago

I don't think tab completion had ever been a thing for aliases as they're not a1 to 1 mapping

On Mon, 15 May 2023, 13:09 William, @.***> wrote:

Can you see, does #8235 https://github.com/PaperMC/Paper/pull/8235 resolve this issue?

Just tested with the linked paperclip in that issue -- the issue persists and there is no change in behaviour to that of what I described here

— Reply to this email directly, view it on GitHub https://github.com/PaperMC/Paper/issues/9214#issuecomment-1547736096, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJMAZB3SVGGIDTSS6YGWMDXGIMHJANCNFSM6AAAAAAYCBN3U4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

WiIIiam278 commented 1 year ago

Yeah, that's right -- there's never been an implementation for it since TAB completion was introduced in 1.13, due to the aforementioned ability to map aliases to commands in all kinds of different ways.

I think there should be, though -- this is currently the only way for users to modify the priority of which commands execute outside of developers, and I think that's quite an important customisation option for server owners. Even though the mapping isn't 1-1, I think an implementation where the alias is resolved and the arguments during TAB completion passed to the target command's TAB provider if there is one would be feasible and make sense.

At the very least, the current behaviour is certainly unexpected behaviour; commands shouldn't return the list of online players as the suggestion for every possible argument (imo).

EDIT: I totally forgot, you're right, aliases can run multiple commands :/. Maybe it'd take the TAB provider from the first alias, if only one command is passed? This whole system makes me tear my hair out with how convoluted it is. I wish there were a better way to customise command priorities than this feature, which is the bottom line of what a lot of people use it for.

electronicboy commented 1 year ago

Tab completion existed long before 1.13, just now its much more structural, which makes it much harder to handle this sort of thing given the two vastly different systems.

Would be easier to have a 1:1 system of some form.

molor commented 1 year ago

There was once a PR that could partially solve this issue, but stupid stale bot killed it: https://github.com/PaperMC/Paper/pull/7387

WiIIiam278 commented 1 year ago

There was once a PR that could partially solve this issue, but stupid stale bot killed it: #7387

Damn that stale bot! Yeah, I think this PR is the obvious and simple implementation. But I do agree with the thing raised in the replies -- adding a new field for mapping command priorities would probably be the best way of doing this, as I imagine command priorities are what most people are using this feature for nowerdays. (I'm actually kind of surprised bukkit has quietly had command scripting like this in it's code for this long)

Owen1212055 commented 1 year ago

~~I have pulled the fix in my brig command PR, feel free to test it.~~ Reverted

WiIIiam278 commented 1 year ago

I guess this sort of is a feature request in disguise in a sense that the solution requires new functionality to be added.

I may consider doing a PR to add a "prioritized commands" mapping (though not sure if I should wait for Owen's stuff first). If so, would it be a good idea to put this stuff in paper's global config, or adding a new field in the existing spigot commands.yml? Are the paper maintainers on board with a "priority" command 1:1 mapping option (instead of the hacky fix for the aliases map)?

PaperMC / Paper