PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.34k stars 2.2k forks source link

Data packs using function macros are now broken by plugins that overwrite vanilla commands #10994

Open GrantGryczan opened 4 days ago

GrantGryczan commented 4 days ago

Expected behavior

In Spigot servers without Paper, plugins that overwrite vanilla commands don't break data packs because those commands only apply to console, chat, and command blocks. Commands in data pack functions should always run vanilla commands only, which data packs rely on in order to function.

Paper doesn't let these plugins overwrite commands in normal data pack functions, so it's expected the same applies to commands in function macros, as is the case in Spigot.

Observed/Actual behavior

In the (currently) latest Paper version for MC 1.20.6, plugin commands are now used in data pack macro commands, breaking data packs that rely on vanilla commands working in macros if any plugins that overwrite these commands are installed.

EssentialsX for example, one of the most popular plugins, overwrites many vanilla commands. If a data pack macro tries to run /kill, /item, /give, /xp, /clear, /tp, /return, /time, etc. on a Paper server with EssentialsX installed, it won't work as expected. Instead, it'll call EssentialsX's version of the command.

Steps/models to reproduce

  1. Put this minimal reproduction data pack into your world's datapacks folder. (This adds a single function with one macro command that simply runs its input as a command.)
  2. Enter /minecraft:reload, or /reload if testing in vanilla. (Side note: If not for #10995, I'd just tell you to run /execute run reload since that used to work in both Paper and vanilla.)
  3. Enter /function test:macro {command: 'version'}.
  4. In Spigot and vanilla, this outputs an error because the version command doesn't exist in vanilla. In Paper, this outputs the Paper version.

You can further test this by replacing version in step 4 with any other command. If you have EssentialsX installed, for example, entering /function test:macro {command: 'give @s diamond'} fails in Paper too, but works in Spigot and vanilla.

Plugin and Datapack List

Just the minimal reproduction data pack linked in the reproduction steps!

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

I suspect this is caused by the same underlying change as #10995.

Machine-Maker commented 4 days ago

The solution to this is to somehow filter out plugin commands that were registered in the JavaPlugin lifecycle event and prevent them from being executed when a macro is parsed at runtime. We still want commands registered by plugins in the boostrapper to be executable by macros (and datapack command functions in general), but since that system is still new and experimental, we can make it very clear that registering commands there that override vanilla commands can break expected datapack functionality.

Ryltarr commented 4 days ago

Surely this should be a config option so that individual server operators can choose to enable to disable it? Datapacks are built for an entirely vanilla environment most of the time, and allowing plugins to introduce changes to the command behavior seems like a recipe for disaster.

Or, if not a server operator config option, maybe an overload of the vanilla command override registry that allows the plugin author to choose whether they want to expose their new commands to macros or not; and have the overload be an optional secondary true/false arg that defaults to the vanilla behavior for macros?

Machine-Maker commented 4 days ago

Plugin commands registered in JavaPlugin only work in macros at the moment, NOT in plain text command functions. This is the bug, not that they don’t work at all. It’s not possible to have commands registered in JavaPlugin work in all command functions because JavaPlugin loading happens too late.

For the Bootstrapper, that happens before everything, so commands registered there can work.