Paper icon indicating copy to clipboard operation
Paper copied to clipboard

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

Open GrantGryczan opened this issue 1 year ago • 4 comments

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.

GrantGryczan avatar Jun 27 '24 04:06 GrantGryczan

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.

Machine-Maker avatar Jun 27 '24 08:06 Machine-Maker

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?

Ryltarr avatar Jun 27 '24 12:06 Ryltarr

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.

Machine-Maker avatar Jun 27 '24 14:06 Machine-Maker

Please test the jar linked in #11011, /version being accessible is considered to be working as intended, however, Vanilla's /give should take priority over EssentialsX in function parsing.

jpenilla avatar Jul 01 '24 19:07 jpenilla

@jpenilla I have confirmed that fixes the bug!

GrantGryczan avatar Jul 04 '24 14:07 GrantGryczan

Since Paper bugs don't get patched in older versions, here's a workaround data pack for those running into this issue with the Vanilla Tweaks Graves data pack in Paper for MC 1.20.6 with EssentialsX: graves v3.0.3 (Paper 1.20.6).zip

Replace the Graves data pack with this to use it. It will not work in vanilla or Spigot, only Paper.

GrantGryczan avatar Jul 09 '24 20:07 GrantGryczan

This is why i've never been a fan of Essentials. It breaks all my work, whether datapacks or simply commands using target selectors.

Zero4793 avatar Aug 19 '25 04:08 Zero4793