Regression: `/execute` no longer runs vanilla commands

[GrantGryczan]

Expected behavior

In Spigot servers without Paper, and in older Paper versions, /execute runs vanilla commands only. Vanilla commands that are overwritten by Paper or by plugins shouldn't break any of the functionality of commands under /execute.

Observed/Actual behavior

In the (currently) latest version of Paper, /execute can run the overwritten non-vanilla versions of vanilla commands too.

Steps/models to reproduce

1. Enter /execute run reload in chat or (without a slash) in the console.
2. In Spigot and vanilla, this reloads data packs. In Paper, this displays a message warning against reloading plugins.

Plugin and Datapack List

N/A

Paper version

This server is running Paper version 1.20.6-147-ver/1.20.6@e41d44f (2024-06-17T19:24:35Z) (Implementing API version 1.20.6-R0.1-SNAPSHOT) You are running the latest version Previous version: 1.20.6-2233-0d6766e (MC: 1.20.6)

Other

If this change is intentional, it would be fine if it at least prioritized vanilla commands (in the minecraft namespace) over overwritten commands when using /execute, because then nothing would be broken and this change would only add new functionality. But because it breaks vanilla commands too, we can no longer rely on /execute as a way to run vanilla commands that works in both vanilla and Paper.

In the Vanilla Tweaks Discord's data pack help channel, for example, we often tell people to run /execute run reload to reload data packs. But with this change, that no longer works. Now, we instead have to ask the user whether they're in multiplayer and running a Bukkit-based server (giving examples like Spigot and Paper to explain what that means), wait for their response, and then tell them to run /reload if the answer is singleplayer or vanilla multiplayer, or to run /minecraft:reload if the answer is Bukkit-based multiplayer.

I wouldn't be too upset if I had to do that from now on, but I would wonder what the use cases are for /execute working with plugin commands. Usually plugin commands aren't the kind of commands you'd need to manually run under the conditions, context changes, or forks that /execute allows for.

Also worth noting, I've seen servers where /minecraft:reload still somehow runs the Bukkit reload command. No clue why. It's not common, but it's happened once or twice. When this came up in the past, we suggested /execute run reload instead and it worked.

[Machine-Maker]

Ok, so here's my initial thoughts on solutions to this. There are 2 places plugins can register commands, in the bootstrapper and in JavaPlugin. The first is a super new place to register commands, so most plugins aren't using it. What we can do is only have commands registered there override vanilla commands in /execute run and command functions. Then commands registered the other way, that's existed for much longer, won't be able to be run in /execute run. This still provides a way for a plugin to override a vanilla command, which is something that should be supported, but it does it in such a way that the plugin has to explictly know that's what they are doing, instead of just retroactively doing it for all commands in all plugins.

[GrantGryczan]

I think that's a great solution, if I'm understanding correctly. I think whether a command works in data packs being tied to whether it works in /execute, and as an explicit opt-in, makes a lot of sense, more so than having all commands work in /execute but none work in data packs. As long as the purpose of these APIs is made very clear so we don't get lots of plugin devs overwriting vanilla commands using the new API.

[willkroboth]

Could it make sense to solve this (and maybe #10994 as well) using namespaces? I believe with Bukkit's command API, if PluginA and PluginB both register the test command, then whoever registers first gets the /test command, but both implementations are still available using /plugina:test and pluginb:test respectively?

Could that work in a similar way here? The Vanilla server always registers its commands first, so kill and /minecraft:kill for example always run the vanilla implementation. If EssentialsX then comes in and registers kill, /essentialsx:kill will run the plugin implementation, but /kill and /minecraft:kill will still run the vanilla implementation.

[Machine-Maker]

Those namespaces don’t exist on a vanilla server. Namespaced commands are a thing that was added years ago

[jpenilla]

This is working as intended. In the context of functions, vanilla commands are prioritized when the namespace isn't explicit. Command blocks have their behavior controlled by settings in commands.yml (you can add * to the override list to always prioritize vanilla). For players though, plugin commands overriding vanilla ones are intended. Whether the command is nested in /execute doesn't matter, what matters is the context the command is being parsed with (i.e. player, command block, function, or other misc. sources).

I understand this is different than the old/spigot behavior and you may need to adjust your advice depending on server types. However, this change is intentional as it expands functionality and reduces inconsistency in a way that should have been done in the 1.13 update. Now that we are better aligning command behavior with vanilla and other modded platforms, devs will need to be more considerate about whether they want to override vanilla by default (as is the convention on platforms that have been using Brigadier properly since 1.13).