From 142695eb00f51aa4fad4539a369abcc2747b2536 Mon Sep 17 00:00:00 2001 From: Bjarne Koll Date: Tue, 18 Feb 2025 00:09:54 +0100 Subject: [PATCH] Default minecraft alias to redirect (#12146) While the running server will still be using the recently introduced copy-mechanic for vanilla command namespacing, the data converter logic relies on the fact that namespaced aliases were redirects as well. To not break the converted, the commands type now takes a modern flag only set by the running server. --- .../minecraft/commands/Commands.java.patch | 42 +++++++++++++++---- .../ReloadableServerResources.java.patch | 6 ++- .../DFUCommandArgumentUpgraderCompatTest.java | 17 ++++++++ .../MinecraftCommandPermissionsTest.java | 2 +- 4 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 paper-server/src/test/java/io/papermc/paper/command/brigadier/DFUCommandArgumentUpgraderCompatTest.java diff --git a/paper-server/patches/sources/net/minecraft/commands/Commands.java.patch b/paper-server/patches/sources/net/minecraft/commands/Commands.java.patch index dff5340ad3..119e3eb764 100644 --- a/paper-server/patches/sources/net/minecraft/commands/Commands.java.patch +++ b/paper-server/patches/sources/net/minecraft/commands/Commands.java.patch @@ -1,6 +1,18 @@ --- a/net/minecraft/commands/Commands.java +++ b/net/minecraft/commands/Commands.java -@@ -251,6 +_,24 @@ +@@ -150,6 +_,11 @@ + private final CommandDispatcher dispatcher = new CommandDispatcher<>(); + + public Commands(Commands.CommandSelection selection, CommandBuildContext context) { ++ // Paper start - Brigadier API - modern minecraft overloads that do not use redirects but are copies instead ++ this(selection, context, false); ++ } ++ public Commands(Commands.CommandSelection selection, CommandBuildContext context, final boolean modern) { ++ // Paper end - Brigadier API - modern minecraft overloads that do not use redirects but are copies instead + AdvancementCommands.register(this.dispatcher); + AttributeCommand.register(this.dispatcher, context); + ExecuteCommand.register(this.dispatcher, context); +@@ -251,6 +_,40 @@ PublishCommand.register(this.dispatcher); } @@ -14,12 +26,28 @@ + // Paper start - Brigadier Command API + // Create legacy minecraft namespace commands + for (final CommandNode node : new java.util.ArrayList<>(this.dispatcher.getRoot().getChildren())) { -+ this.dispatcher.getRoot().addChild( -+ io.papermc.paper.command.brigadier.PaperBrigadier.copyLiteral( -+ "minecraft:" + node.getName(), -+ (com.mojang.brigadier.tree.LiteralCommandNode) node -+ ) -+ ); ++ if (modern) { ++ // Modern behaviour that simply creates a full copy of the commands node. ++ // Avoids plenty of issues around registering redirects *to* these nodes from the API ++ this.dispatcher.getRoot().addChild( ++ io.papermc.paper.command.brigadier.PaperBrigadier.copyLiteral( ++ "minecraft:" + node.getName(), ++ (com.mojang.brigadier.tree.LiteralCommandNode) node ++ ) ++ ); ++ continue; ++ } ++ ++ // Legacy behaviour of creating a flattened redirecting node. ++ // Used by CommandArgumentUpgrader ++ CommandNode flattenedAliasTarget = node; ++ while (flattenedAliasTarget.getRedirect() != null) flattenedAliasTarget = flattenedAliasTarget.getRedirect(); ++ ++ this.dispatcher.register( ++ com.mojang.brigadier.builder.LiteralArgumentBuilder.literal("minecraft:" + node.getName()) ++ .executes(flattenedAliasTarget.getCommand()) ++ .requires(flattenedAliasTarget.getRequirement()) ++ .redirect(flattenedAliasTarget)); + } + // Paper end - Brigadier Command API this.dispatcher.setConsumer(ExecutionCommandSource.resultConsumer()); diff --git a/paper-server/patches/sources/net/minecraft/server/ReloadableServerResources.java.patch b/paper-server/patches/sources/net/minecraft/server/ReloadableServerResources.java.patch index dd754adca3..caa71bc362 100644 --- a/paper-server/patches/sources/net/minecraft/server/ReloadableServerResources.java.patch +++ b/paper-server/patches/sources/net/minecraft/server/ReloadableServerResources.java.patch @@ -1,9 +1,11 @@ --- a/net/minecraft/server/ReloadableServerResources.java +++ b/net/minecraft/server/ReloadableServerResources.java -@@ -39,6 +_,7 @@ +@@ -38,7 +_,8 @@ + this.fullRegistryHolder = new ReloadableServerRegistries.Holder(registryAccess.compositeAccess()); this.postponedTags = postponedTags; this.recipes = new RecipeManager(registries); - this.commands = new Commands(commandSelection, CommandBuildContext.simple(registries, enabledFeatures)); +- this.commands = new Commands(commandSelection, CommandBuildContext.simple(registries, enabledFeatures)); ++ this.commands = new Commands(commandSelection, CommandBuildContext.simple(registries, enabledFeatures), true); // Paper - Brigadier Command API - use modern alias registration + io.papermc.paper.command.brigadier.PaperCommands.INSTANCE.setDispatcher(this.commands, CommandBuildContext.simple(registries, enabledFeatures)); // Paper - Brigadier Command API this.advancements = new ServerAdvancementManager(registries); this.functionLibrary = new ServerFunctionLibrary(functionCompilationLevel, this.commands.getDispatcher()); diff --git a/paper-server/src/test/java/io/papermc/paper/command/brigadier/DFUCommandArgumentUpgraderCompatTest.java b/paper-server/src/test/java/io/papermc/paper/command/brigadier/DFUCommandArgumentUpgraderCompatTest.java new file mode 100644 index 0000000000..0d32d772e5 --- /dev/null +++ b/paper-server/src/test/java/io/papermc/paper/command/brigadier/DFUCommandArgumentUpgraderCompatTest.java @@ -0,0 +1,17 @@ +package io.papermc.paper.command.brigadier; + +import ca.spottedleaf.dataconverter.util.CommandArgumentUpgrader; +import org.bukkit.support.environment.Normal; +import org.junit.jupiter.api.Test; + +@Normal +public class DFUCommandArgumentUpgraderCompatTest { + + @Test + public void testCompatibilityWithCommandArgumentUpgrader() { + // The Command argument upgrader has some specific assumptions about the tree, specifically around + // Attempt to construct it and to that degree its tree of commands. + CommandArgumentUpgrader.upgrader_1_20_4_to_1_20_5(999); + } + +} diff --git a/paper-server/src/test/java/io/papermc/paper/permissions/MinecraftCommandPermissionsTest.java b/paper-server/src/test/java/io/papermc/paper/permissions/MinecraftCommandPermissionsTest.java index 75ed5050f7..fe08e446e8 100644 --- a/paper-server/src/test/java/io/papermc/paper/permissions/MinecraftCommandPermissionsTest.java +++ b/paper-server/src/test/java/io/papermc/paper/permissions/MinecraftCommandPermissionsTest.java @@ -40,7 +40,7 @@ public class MinecraftCommandPermissionsTest { CraftDefaultPermissions.registerCorePermissions(); Set perms = collectMinecraftCommandPerms(); - Commands commands = new Commands(Commands.CommandSelection.DEDICATED, CommandBuildContext.simple(RegistryHelper.getRegistry(), FeatureFlags.VANILLA_SET)); + Commands commands = new Commands(Commands.CommandSelection.DEDICATED, CommandBuildContext.simple(RegistryHelper.getRegistry(), FeatureFlags.VANILLA_SET), true); RootCommandNode root = commands.getDispatcher().getRoot(); Set missing = new LinkedHashSet<>(); Set foundPerms = new HashSet<>();