Remove nodes from PaperCommandRegistration#registeredNodes when commands are unregistered

Avoids reintroducing unregistered commands after `/minecraft:reload`

Author: willkroboth

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-core/src/main/java/dev/jorel/commandapi/CommandRegistrationStrategy.java

@@ -31,9 +31,8 @@ public abstract class CommandRegistrationStrategy<Source> {
 	}
 
 	// Utility methods
-	protected void removeBrigadierCommands(CommandDispatcher<Source> dispatcher, String commandName,
+	protected void removeBrigadierCommands(RootCommandNode<Source> root, String commandName,
 										   boolean unregisterNamespaces, Predicate<CommandNode<Source>> extraCheck) {
-		RootCommandNode<?> root = dispatcher.getRoot();
 		Map<String, CommandNode<Source>> children = (Map<String, CommandNode<Source>>) commandNodeChildren.get(root);
 		Map<String, CommandNode<Source>> literals = (Map<String, CommandNode<Source>>) commandNodeLiterals.get(root);
 		Map<String, CommandNode<Source>> arguments = (Map<String, CommandNode<Source>>) commandNodeArguments.get(root);

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-core/src/main/java/dev/jorel/commandapi/PaperCommandRegistration.java

@@ -4,8 +4,8 @@
 import com.mojang.brigadier.builder.LiteralArgumentBuilder;
 import com.mojang.brigadier.tree.CommandNode;
 import com.mojang.brigadier.tree.LiteralCommandNode;
+import com.mojang.brigadier.tree.RootCommandNode;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Supplier;
 
@@ -18,7 +18,7 @@ public class PaperCommandRegistration<Source> extends CommandRegistrationStrateg
 	private final Supplier<CommandDispatcher<Source>> getBrigadierDispatcher;
 
 	// Store registered commands nodes for eventual reloads
-	private final List<LiteralCommandNode<Source>> registeredNodes = new ArrayList<>();
+	private final RootCommandNode<Source> registeredNodes = new RootCommandNode<>();
 
 	public PaperCommandRegistration(Supplier<CommandDispatcher<Source>> getBrigadierDispatcher) {
 		this.getBrigadierDispatcher = getBrigadierDispatcher;
@@ -45,8 +45,8 @@ public LiteralCommandNode<Source> registerCommandNode(LiteralArgumentBuilder<Sou
 		LiteralCommandNode<Source> namespacedCommandNode = CommandAPIHandler.getInstance().namespaceNode(commandNode, namespace);
 
 		// Add to registered command nodes
-		registeredNodes.add(commandNode);
-		registeredNodes.add(namespacedCommandNode);
+		registeredNodes.addChild(commandNode);
+		registeredNodes.addChild(namespacedCommandNode);
 
 		// Namespace is not empty on Bukkit forks
 		getBrigadierDispatcher.get().getRoot().addChild(namespacedCommandNode);
@@ -57,16 +57,20 @@ public LiteralCommandNode<Source> registerCommandNode(LiteralArgumentBuilder<Sou
 	@Override
 	public void unregister(String commandName, boolean unregisterNamespaces, boolean unregisterBukkit) {
 		// Remove nodes from the  dispatcher
-		removeBrigadierCommands(getBrigadierDispatcher.get(), commandName, unregisterNamespaces, c -> true);
+		removeBrigadierCommands(getBrigadierDispatcher.get().getRoot(), commandName, unregisterNamespaces, c -> true);
+
+		// Don't add nodes back after a reload
+		removeBrigadierCommands(registeredNodes, commandName, unregisterNamespaces, c -> true);
 
 		// Update the dispatcher file
 		CommandAPIHandler.getInstance().writeDispatcherToFile();
 	}
 
 	@Override
 	public void preReloadDataPacks() {
-		for (LiteralCommandNode<Source> commandNode : registeredNodes) {
-			getBrigadierDispatcher.get().getRoot().addChild(commandNode);
+		RootCommandNode<Source> root = getBrigadierDispatcher.get().getRoot();
+		for (CommandNode<Source> commandNode : registeredNodes.getChildren()) {
+			root.addChild(commandNode);
 		}
 	}
 }

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-core/src/main/java/dev/jorel/commandapi/SpigotCommandRegistration.java

@@ -70,16 +70,15 @@ public void runTasksAfterServerStart() {
 
 	private void fixNamespaces() {
 		Map<String, Command> knownCommands = commandMapKnownCommands.get((SimpleCommandMap) commandAPIBukkit.getPaper().getCommandMap());
-		CommandDispatcher<Source> resourcesDispatcher = getResourcesDispatcher.get();
+		RootCommandNode<Source> resourcesRootNode = getResourcesDispatcher.get().getRoot();
 
 		// Remove namespaces
 		for (String command : namespacesToFix) {
 			knownCommands.remove(command);
-			removeBrigadierCommands(resourcesDispatcher, command, false, c -> true);
+			removeBrigadierCommands(resourcesRootNode, command, false, c -> true);
 		}
 
 		// Add back certain minecraft: namespace commands
-		RootCommandNode<Source> resourcesRootNode = resourcesDispatcher.getRoot();
 		RootCommandNode<Source> brigadierRootNode = brigadierDispatcher.getRoot();
 		for (CommandNode<Source> node : minecraftCommandNamespaces.getChildren()) {
 			knownCommands.put(node.getName(), commandAPIBukkit.wrapToVanillaCommandWrapper(node));
@@ -92,7 +91,7 @@ private void fixNamespaces() {
 			brigadierRootNode.addChild(node);
 		}
 		// Clear minecraftCommandNamespaces for dealing with command conflicts after the server is enabled
-		//  See `CommandAPIBukkit#postCommandRegistration`
+		//  See `SpigotCommandRegistration#postCommandRegistration`
 		minecraftCommandNamespaces = new RootCommandNode<>();
 	}
 
@@ -264,7 +263,7 @@ public void unregister(String commandName, boolean unregisterNamespaces, boolean
 			// Remove nodes from the Vanilla dispatcher
 			// This dispatcher doesn't usually have namespaced version of commands (those are created when commands
 			//  are transferred to Bukkit's CommandMap), but if they ask, we'll do it
-			removeBrigadierCommands(brigadierDispatcher, commandName, unregisterNamespaces, c -> true);
+			removeBrigadierCommands(brigadierDispatcher.getRoot(), commandName, unregisterNamespaces, c -> true);
 
 			// Update the dispatcher file
 			CommandAPIHandler.getInstance().writeDispatcherToFile();
@@ -292,7 +291,7 @@ public void unregister(String commandName, boolean unregisterNamespaces, boolean
 			// Remove commands from the resources dispatcher
 			// If we are unregistering a Bukkit command, ONLY unregister BukkitCommandWrappers
 			// If we are unregistering a Vanilla command, DO NOT unregister BukkitCommandWrappers
-			removeBrigadierCommands(getResourcesDispatcher.get(), commandName, unregisterNamespaces,
+			removeBrigadierCommands(getResourcesDispatcher.get().getRoot(), commandName, unregisterNamespaces,
 				c -> !unregisterBukkit ^ commandAPIBukkit.isBukkitCommandWrapper(c));
 		}
 	}