Fix issue with commands being registered on dedicated servers after a resource reload. (#822)

* Add a small datapack to test if custom commands work with command functions

* Fix issue with commands being registered on dedicated servers after a /reload

Also this adds testmods for command functions to verify they are registered properly.

* Clarify a comment

* Use the variable we allocate lol

fabric-command-api-v1/src/main/java/net/fabricmc/fabric/mixin/command/MixinCommandManager.java

@@ -19,6 +19,7 @@ package net.fabricmc.fabric.mixin.command;
 import com.mojang.brigadier.AmbiguityConsumer;
 import com.mojang.brigadier.CommandDispatcher;
 import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Unique;
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.Redirect;
 
@@ -29,16 +30,31 @@ import net.fabricmc.fabric.api.command.v1.CommandRegistrationCallback;
 
 @Mixin(CommandManager.class)
 public abstract class MixinCommandManager {
+	@Unique
+	private static boolean fabric_isFirstRun = true;
+
 	/**
 	 * @reason Add commands before ambiguities are calculated.
 	 */
 	@Redirect(at = @At(value = "INVOKE", target = "Lcom/mojang/brigadier/CommandDispatcher;findAmbiguities(Lcom/mojang/brigadier/AmbiguityConsumer;)V"), method = "<init>")
-	private void fabric_addCommands(CommandDispatcher<ServerCommandSource> dispatcher, AmbiguityConsumer<ServerCommandSource> ambiguityConsumer, CommandManager.RegistrationEnvironment arg) {
-		if (arg != CommandManager.RegistrationEnvironment.DEDICATED) {
-			CommandRegistrationCallback.EVENT.invoker().register(dispatcher, false);
+	private void fabric_addCommands(CommandDispatcher<ServerCommandSource> dispatcher, AmbiguityConsumer<ServerCommandSource> ambiguityConsumer, CommandManager.RegistrationEnvironment registrationEnvironment) {
+		if (fabric_isFirstRun) {
+			// Mods have not initialized yet on a dedicated server. These will be registered later though.
+			if (registrationEnvironment != CommandManager.RegistrationEnvironment.DEDICATED) {
+				CommandRegistrationCallback.EVENT.invoker().register(dispatcher, false);
+
+				// This should only be called on integrated server. On dedicated, we test this later due to mod init.
+				dispatcher.findAmbiguities(ambiguityConsumer);
+			}
+		} else {
+			// This will occur only if "/reload" is called or this is being registered on an integrated server.
+			CommandRegistrationCallback.EVENT.invoker().register(dispatcher, registrationEnvironment == CommandManager.RegistrationEnvironment.DEDICATED);
+
+			// Mimic vanilla logic by calling findAmbiguities.
+			dispatcher.findAmbiguities(ambiguityConsumer);
 		}
 
-		// Now mimic vanilla logic by calling findAmbiguities.
-		dispatcher.findAmbiguities(ambiguityConsumer);
+		fabric_isFirstRun = false;
+		// Ambiguities will be called later if on a dedicated server and it is the first run
 	}
 }

fabric-command-api-v1/src/main/java/net/fabricmc/fabric/mixin/command/MixinMinecraftDedicatedServer.java

@@ -16,21 +16,36 @@
 
 package net.fabricmc.fabric.mixin.command;
 
+import com.mojang.brigadier.CommandDispatcher;
+import org.apache.logging.log4j.Logger;
+import org.spongepowered.asm.mixin.Final;
 import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
 import org.spongepowered.asm.mixin.injection.At;
 import org.spongepowered.asm.mixin.injection.Inject;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
 
+import net.minecraft.server.command.ServerCommandSource;
 import net.minecraft.server.dedicated.MinecraftDedicatedServer;
 
 import net.fabricmc.fabric.api.command.v1.CommandRegistrationCallback;
 
 @Mixin(MinecraftDedicatedServer.class)
 public abstract class MixinMinecraftDedicatedServer {
+	@Shadow
+	@Final
+	private static Logger LOGGER;
+
 	@Inject(method = "setupServer", at = @At("HEAD"))
 	private void setupServer(CallbackInfoReturnable<Boolean> info) {
-		CommandRegistrationCallback.EVENT.invoker().register(((MinecraftDedicatedServer) (Object) this).getCommandManager().getDispatcher(), true);
+		MinecraftDedicatedServer server = ((MinecraftDedicatedServer) (Object) this);
+		CommandDispatcher<ServerCommandSource> dispatcher = server.getCommandManager().getDispatcher();
 
-		//Possibly call findAmbiguities here
+		CommandRegistrationCallback.EVENT.invoker().register(dispatcher, true);
+
+		// Now find ambiguities after commands have loaded.
+		dispatcher.findAmbiguities((parent, child, sibling, collection) -> {
+			LOGGER.warn("Ambiguity between arguments {} and {} with inputs: {}", dispatcher.getPath(child), dispatcher.getPath(sibling), collection);
+		});
 	}
 }

fabric-command-api-v1/src/testmod/resources/data/fabric_command_api_v1_testmod/functions/load.mcfunction

@@ -0,0 +1,2 @@
+# Test the common command to make sure functions can invoke these
+fabric_common_test_command

fabric-command-api-v1/src/testmod/resources/data/fabric_command_api_v1_testmod/functions/tick.mcfunction

@@ -0,0 +1,2 @@
+# Test the common command to make sure functions can invoke these
+fabric_common_test_command

fabric-command-api-v1/src/testmod/resources/data/minecraft/tags/functions/load.json

@@ -0,0 +1,6 @@
+{
+  "replace": false,
+  "values":[
+    "fabric_command_api_v1_testmod:load"
+  ]
+}

fabric-command-api-v1/src/testmod/resources/data/minecraft/tags/functions/tick.json

@@ -0,0 +1,6 @@
+{
+  "replace": false,
+  "values":[
+    "fabric_command_api_v1_testmod:tick"
+  ]
+}