feat(bukkit/paper): add root command deletion support (#371)

CHANGELOG.md

@@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Annotations: Annotation string processors ([#353](https://github.com/Incendo/cloud/pull/353))
 - Annotations: `@CommandContainer` annotation processing ([#364](https://github.com/Incendo/cloud/pull/364))
 - Annotations: `@CommandMethod` annotation processing for compile-time validation ([#365](https://github.com/Incendo/cloud/pull/365))
+- Add root command deletion support (core/pircbotx/javacord/jda/bukkit/paper) ([#369](https://github.com/Incendo/cloud/pull/369),
+  [#371](https://github.com/Incendo/cloud/pull/371))
 
 ### Fixed
 - Core: Fix missing caption registration for the regex caption ([#351](https://github.com/Incendo/cloud/pull/351))

cloud-core/src/main/java/cloud/commandframework/CommandManager.java

@@ -439,7 +439,8 @@ public abstract class CommandManager<C> {
         // Mark the command for deletion.
         final CommandTree.Node<@Nullable CommandArgument<C, ?>> node = this.commandTree.getNamedNode(rootCommand);
         if (node == null) {
-            throw new IllegalArgumentException(String.format("No root command named '%s' exists", rootCommand));
+            // If the node doesn't exist, we don't really need to delete it...
+            return;
         }
 
         // The registration handler gets to act before we destruct the command.

cloud-core/src/main/java/cloud/commandframework/CommandTree.java

@@ -963,15 +963,11 @@ public final class CommandTree<C> {
     }
 
     private boolean removeNode(final @NonNull Node<@Nullable CommandArgument<C, ?>> node) {
-        if (node.isLeaf()) {
         if (this.getRootNodes().contains(node)) {
             this.internalTree.removeChild(node);
         } else {
             return node.getParent().removeChild(node);
         }
-        } else {
-            throw new IllegalStateException(String.format("Cannot delete intermediate node '%s'", node));
-        }
 
         return false;
     }

cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/BukkitCommand.java

@@ -37,13 +37,16 @@ import cloud.commandframework.permission.CommandPermission;
 import cloud.commandframework.permission.Permission;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.CompletionException;
 import java.util.logging.Level;
+import org.apiguardian.api.API;
 import org.bukkit.ChatColor;
 import org.bukkit.command.CommandSender;
 import org.bukkit.command.PluginIdentifiableCommand;
 import org.bukkit.plugin.Plugin;
 import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 final class BukkitCommand<C> extends org.bukkit.command.Command implements PluginIdentifiableCommand {
 
@@ -58,6 +61,8 @@ final class BukkitCommand<C> extends org.bukkit.command.Command implements Plugi
     private final BukkitCommandManager<C> manager;
     private final Command<C> cloudCommand;
 
+    private boolean disabled;
+
     BukkitCommand(
             final @NonNull String label,
             final @NonNull List<@NonNull String> aliases,
@@ -77,6 +82,7 @@ final class BukkitCommand<C> extends org.bukkit.command.Command implements Plugi
         if (this.command.getOwningCommand() != null) {
             this.setPermission(this.command.getOwningCommand().getCommandPermission().toString());
         }
+        this.disabled = false;
     }
 
     @Override
@@ -191,22 +197,38 @@ final class BukkitCommand<C> extends org.bukkit.command.Command implements Plugi
     @Override
     public @NonNull String getUsage() {
         return this.manager.getCommandSyntaxFormatter().apply(
-                Collections.singletonList(this.namedNode().getValue()),
+                Collections.singletonList(Objects.requireNonNull(this.namedNode().getValue())),
                 this.namedNode()
         );
     }
 
     @Override
     public boolean testPermissionSilent(final @NonNull CommandSender target) {
-        final CommandPermission permission = (CommandPermission) this.namedNode()
+        final CommandTree.Node<CommandArgument<C, ?>> node = this.namedNode();
+        if (this.disabled || node == null) {
+            return false;
+        }
+
+        final CommandPermission permission = (CommandPermission) node
                 .getNodeMeta()
                 .getOrDefault("permission", Permission.empty());
 
         return this.manager.hasPermission(this.manager.getCommandSenderMapper().apply(target), permission);
     }
 
-    private CommandTree.Node<CommandArgument<C, ?>> namedNode() {
-        return this.manager.getCommandTree().getNamedNode(this.command.getName());
+    @API(status = API.Status.INTERNAL, since = "1.7.0")
+    void disable() {
+        this.disabled = true;
     }
 
+    @Override
+    public boolean isRegistered() {
+        // This allows us to prevent the command from showing
+        // in Bukkit help topics.
+        return !this.disabled;
+    }
+
+    private CommandTree.@Nullable Node<CommandArgument<C, ?>> namedNode() {
+        return this.manager.getCommandTree().getNamedNode(this.command.getName());
+    }
 }

cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/BukkitCommandManager.java

@@ -130,6 +130,7 @@ public class BukkitCommandManager<C> extends CommandManager<C> implements Brigad
 
         /* Register capabilities */
         CloudBukkitCapabilities.CAPABLE.forEach(this::registerCapability);
+        this.registerCapability(CloudCapability.StandardCapabilities.ROOT_COMMAND_DELETION);
 
         /* Register Bukkit Preprocessor */
         this.registerCommandPreProcessor(new BukkitCommandPreprocessor<>(this));

cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/BukkitPluginRegistrationHandler.java

@@ -37,10 +37,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
+import org.apiguardian.api.API;
 import org.bukkit.Bukkit;
 import org.bukkit.command.CommandMap;
 import org.bukkit.command.PluginIdentifiableCommand;
 import org.bukkit.command.SimpleCommandMap;
+import org.bukkit.entity.Player;
 import org.bukkit.help.GenericCommandHelpTopic;
 import org.checkerframework.checker.nullness.qual.NonNull;
 
@@ -53,7 +55,7 @@ public class BukkitPluginRegistrationHandler<C> implements CommandRegistrationHa
     private BukkitCommandManager<C> bukkitCommandManager;
     private CommandMap commandMap;
 
-    BukkitPluginRegistrationHandler() {
+    protected BukkitPluginRegistrationHandler() {
     }
 
     final void initialize(final @NonNull BukkitCommandManager<C> bukkitCommandManager) throws Exception {
@@ -93,7 +95,7 @@ public class BukkitPluginRegistrationHandler<C> implements CommandRegistrationHa
 
         if (this.bukkitCommandManager.getSetting(CommandManager.ManagerSettings.OVERRIDE_EXISTING_COMMANDS)) {
             this.bukkitCommands.remove(label);
-            aliases.forEach(alias -> this.bukkitCommands.remove(alias));
+            aliases.forEach(this.bukkitCommands::remove);
         }
 
         final Set<String> newAliases = new HashSet<>();
@@ -126,6 +128,46 @@ public class BukkitPluginRegistrationHandler<C> implements CommandRegistrationHa
         return true;
     }
 
+    @Override
+    @SuppressWarnings("unchecked")
+    public final void unregisterRootCommand(
+            final @NonNull StaticArgument<?> rootCommand
+    ) {
+        final org.bukkit.command.Command registeredCommand = this.registeredCommands.get(rootCommand);
+        if (registeredCommand == null) {
+            return;
+        }
+        ((BukkitCommand<C>) registeredCommand).disable();
+
+        final List<String> aliases = new ArrayList<>(rootCommand.getAlternativeAliases());
+        final Set<String> registeredAliases = new HashSet<>();
+
+        for (final String alias : aliases) {
+            registeredAliases.add(this.getNamespacedLabel(alias));
+            if (this.bukkitCommandOrAliasExists(alias)) {
+                registeredAliases.add(alias);
+            }
+        }
+
+        if (this.bukkitCommandExists(rootCommand.getName())) {
+            registeredAliases.add(rootCommand.getName());
+        }
+        registeredAliases.add(this.getNamespacedLabel(rootCommand.getName()));
+
+        this.bukkitCommands.remove(rootCommand.getName());
+        this.bukkitCommands.remove(this.getNamespacedLabel(rootCommand.getName()));
+
+        this.recognizedAliases.removeAll(registeredAliases);
+        if (this.bukkitCommandManager.getSplitAliases()) {
+            registeredAliases.forEach(this::unregisterExternal);
+        }
+
+        this.registeredCommands.remove(rootCommand);
+
+        // Once the command has been unregistered, we need to refresh the command list for all online players.
+        Bukkit.getOnlinePlayers().forEach(Player::updateCommands);
+    }
+
     private @NonNull String getNamespacedLabel(final @NonNull String label) {
         return String.format("%s:%s", this.bukkitCommandManager.getOwningPlugin().getName(), label).toLowerCase();
     }
@@ -147,6 +189,10 @@ public class BukkitPluginRegistrationHandler<C> implements CommandRegistrationHa
     ) {
     }
 
+    @API(status = API.Status.STABLE, since = "1.7.0")
+    protected void unregisterExternal(final @NonNull String label) {
+    }
+
     /**
      * Returns true if a command exists in the Bukkit command map, is not an alias, and is not owned by us.
      *

cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/CloudBukkitListener.java

@@ -27,6 +27,7 @@ import org.bukkit.event.EventHandler;
 import org.bukkit.event.EventPriority;
 import org.bukkit.event.Listener;
 import org.bukkit.event.player.PlayerLoginEvent;
+import org.bukkit.event.server.PluginDisableEvent;
 import org.checkerframework.checker.nullness.qual.NonNull;
 
 
@@ -47,4 +48,10 @@ final class CloudBukkitListener<C> implements Listener {
         this.bukkitCommandManager.lockIfBrigadierCapable();
     }
 
+    @EventHandler(priority = EventPriority.HIGHEST)
+    void onPluginDisable(final @NonNull PluginDisableEvent event) {
+        if (event.getPlugin().equals(this.bukkitCommandManager.getOwningPlugin())) {
+            this.bukkitCommandManager.rootCommands().forEach(this.bukkitCommandManager::deleteRootCommand);
+        }
+    }
 }

cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/CloudCommodoreManager.java

@@ -29,7 +29,11 @@ import cloud.commandframework.bukkit.internal.BukkitBackwardsBrigadierSenderMapp
 import cloud.commandframework.context.CommandContext;
 import com.mojang.brigadier.tree.CommandNode;
 import com.mojang.brigadier.tree.LiteralCommandNode;
+import com.mojang.brigadier.tree.RootCommandNode;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.util.Collections;
+import java.util.List;
 import me.lucko.commodore.Commodore;
 import me.lucko.commodore.CommodoreProvider;
 import org.bukkit.Bukkit;
@@ -74,6 +78,11 @@ class CloudCommodoreManager<C> extends BukkitPluginRegistrationHandler<C> {
         this.registerWithCommodore(label, (Command<C>) command);
     }
 
+    @Override
+    protected void unregisterExternal(final @NonNull String label) {
+        this.unregisterWithCommodore(label);
+    }
+
     protected @NonNull CloudBrigadierManager brigadierManager() {
         return this.brigadierManager;
     }
@@ -84,6 +93,11 @@ class CloudCommodoreManager<C> extends BukkitPluginRegistrationHandler<C> {
     ) {
         final LiteralCommandNode<?> literalCommandNode = this.brigadierManager
                 .createLiteralCommandNode(label, command, (o, p) -> {
+                    // We need to check that the command still exists...
+                    if (this.commandManager.getCommandTree().getNamedNode(label) == null) {
+                        return false;
+                    }
+
                     final CommandSender sender = this.commodore.getBukkitSender(o);
                     return this.commandManager.hasPermission(this.commandManager.getCommandSenderMapper().apply(sender), p);
                 }, false, o -> 1);
@@ -95,6 +109,35 @@ class CloudCommodoreManager<C> extends BukkitPluginRegistrationHandler<C> {
         }
     }
 
+    private void unregisterWithCommodore(
+            final @NonNull String label
+    ) {
+        final CommandNode node = this.commodore.getDispatcher().findNode(Collections.singletonList(label));
+        if (node == null) {
+            return;
+        }
+
+        try {
+            final Class<? extends Commodore> commodoreImpl = (Class<? extends Commodore>) Class.forName("me.lucko.commodore.CommodoreImpl");
+
+            final Method removeChild = commodoreImpl.getDeclaredMethod("removeChild", RootCommandNode.class, String.class);
+            removeChild.setAccessible(true);
+
+            removeChild.invoke(
+                    null /* static method */,
+                    this.commodore.getDispatcher().getRoot(),
+                    node.getName()
+            );
+
+            final Field registeredNodes = commodoreImpl.getDeclaredField("registeredNodes");
+            registeredNodes.setAccessible(true);
+
+            ((List<LiteralCommandNode<?>>) registeredNodes.get(this.commodore)).remove(node);
+        } catch (final Exception e) {
+            throw new RuntimeException(String.format("Failed to unregister command '%s' with commodore", label), e);
+        }
+    }
+
     private void mergeChildren(@Nullable final CommandNode<?> existingNode, @Nullable final CommandNode<?> node) {
         for (final CommandNode child : node.getChildren()) {
             final CommandNode<?> existingChild = existingNode.getChild(child.getName());

cloud-minecraft/cloud-paper/src/main/java/cloud/commandframework/paper/PaperBrigadierListener.java

@@ -79,16 +79,24 @@ class PaperBrigadierListener<C> implements Listener {
 
         final CommandTree<C> commandTree = this.paperCommandManager.getCommandTree();
 
-        String label = event.getCommandLabel();
-        if (label.contains(":")) {
-            label = label.split(Pattern.quote(":"))[1];
+        final String label;
+        if (event.getCommandLabel().contains(":")) {
+            label = event.getCommandLabel().split(Pattern.quote(":"))[1];
+        } else {
+            label = event.getCommandLabel();
         }
 
         final CommandTree.Node<CommandArgument<C, ?>> node = commandTree.getNamedNode(label);
         if (node == null) {
             return;
         }
+
         final BiPredicate<BukkitBrigadierCommandSource, CommandPermission> permissionChecker = (s, p) -> {
+            // We need to check that the command still exists...
+            if (commandTree.getNamedNode(label) == null) {
+                return false;
+            }
+
             final C sender = this.paperCommandManager.getCommandSenderMapper().apply(s.getBukkitSender());
             return this.paperCommandManager.hasPermission(sender, p);
         };

examples/example-bukkit/src/main/java/cloud/commandframework/examples/bukkit/ExamplePlugin.java

@@ -35,6 +35,7 @@ import cloud.commandframework.annotations.Confirmation;
 import cloud.commandframework.annotations.Flag;
 import cloud.commandframework.annotations.Regex;
 import cloud.commandframework.annotations.specifier.Greedy;
+import cloud.commandframework.annotations.suggestions.Suggestions;
 import cloud.commandframework.arguments.CommandArgument;
 import cloud.commandframework.arguments.parser.ParserParameters;
 import cloud.commandframework.arguments.parser.StandardParameters;
@@ -51,6 +52,7 @@ import cloud.commandframework.bukkit.parsers.WorldArgument;
 import cloud.commandframework.bukkit.parsers.selector.SingleEntitySelectorArgument;
 import cloud.commandframework.captions.Caption;
 import cloud.commandframework.captions.SimpleCaptionRegistry;
+import cloud.commandframework.context.CommandContext;
 import cloud.commandframework.execution.AsynchronousCommandExecutionCoordinator;
 import cloud.commandframework.execution.CommandExecutionCoordinator;
 import cloud.commandframework.extra.confirmation.CommandConfirmationManager;
@@ -71,6 +73,7 @@ import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -454,7 +457,7 @@ public final class ExamplePlugin extends JavaPlugin {
                 }));
     }
 
-    @CommandMethod("example help [query]")
+    @CommandMethod("example|e|ex help [query]")
     @CommandDescription("Help menu")
     public void commandHelp(
             final @NonNull CommandSender sender,
@@ -528,6 +531,36 @@ public final class ExamplePlugin extends JavaPlugin {
                 .execute(() -> sender.sendMessage("You have been teleported!"));
     }
 
+    @CommandMethod("removeall")
+    public void removeAll(
+            final @NonNull CommandSender sender
+    ) {
+        this.manager.rootCommands().forEach(this.manager::deleteRootCommand);
+        sender.sendMessage("All root commands have been deleted :)");
+    }
+
+    @CommandMethod("removesingle <command>")
+    public void removeSingle(
+            final @NonNull CommandSender sender,
+            final @Argument(value = "command", suggestions = "commands") String command
+    ) {
+        this.manager.deleteRootCommand(command);
+        sender.sendMessage("Deleted the root command :)");
+    }
+
+    @Suggestions("commands")
+    public List<String> commands(
+            final @NonNull CommandContext<CommandSender> context,
+            final @NonNull String input
+    ) {
+        return new ArrayList<>(this.manager.rootCommands());
+    }
+
+    @CommandMethod("disableme")
+    public void disableMe() {
+        this.getServer().getPluginManager().disablePlugin(this);
+    }
+
 
     /**
      * Command must have the given game mode