Unregister Commands on PluginDisableEvent (#22)

* Add CommandManager#unregisterCommand(BaseCommand), CommandManager#unregisterCommands(), and CommandManager#getCommandByAlias(String) + implementations

* Add to knownCommands upon registration

* allSuccess was not the right comparison to check

* Add a built-in listener to BukkitCommandManager to listen for plugin disable, so as to unregister commands automatically.

* Fix unregistration, because Bukkit is weird. Also ensure we iterate all the BaseCommand's Commands.

* Stop abusing AtomicBoolean, use BitSet instead!

* Switch to more standard way of checking allSuccess

* Didn't see the cool shorthand, changed

* Remove pom change

* Remove unused imports

* Unnecessary to set the first index of that array

src/main/java/co/aikar/commands/BukkitCommandManager.java

@@ -27,10 +27,13 @@ import org.bukkit.Bukkit;
 import org.bukkit.Server;
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandMap;
+import org.bukkit.event.EventHandler;
+import org.bukkit.event.Listener;
+import org.bukkit.event.server.PluginDisableEvent;
 import org.bukkit.plugin.Plugin;
 
 import java.lang.reflect.Method;
-import java.util.Map;
+import java.util.*;
 
 @SuppressWarnings("WeakerAccess")
 public class BukkitCommandManager implements CommandManager {
@@ -38,6 +41,7 @@ public class BukkitCommandManager implements CommandManager {
     @SuppressWarnings("WeakerAccess")
     protected final Plugin plugin;
     private final CommandMap commandMap;
+    protected Map<String, BaseCommand> knownCommands;
     protected CommandContexts contexts;
     protected CommandCompletions completions;
 
@@ -54,6 +58,19 @@ public class BukkitCommandManager implements CommandManager {
             ACFUtil.sneaky(e);
         }
         this.commandMap = commandMap;
+        this.knownCommands = new HashMap<>();
+        Bukkit.getPluginManager().registerEvents(
+                new Listener() {
+                    @EventHandler
+                    public void onPluginDisable(PluginDisableEvent event) {
+                        if (!(plugin.getName().equalsIgnoreCase(event.getPlugin().getName()))) {
+                            return;
+                        }
+                        unregisterCommands();
+                    }
+                },
+                plugin
+        );
     }
 
     @Override
@@ -65,6 +82,11 @@ public class BukkitCommandManager implements CommandManager {
         return commandMap;
     }
 
+    @Override
+    public Map<String, BaseCommand> getKnownCommands() {
+        return knownCommands;
+    }
+
     @Override
     public CommandContexts getCommandContexts() {
         if (this.contexts == null) {
@@ -87,11 +109,44 @@ public class BukkitCommandManager implements CommandManager {
         command.onRegister(this);
         boolean allSuccess = true;
         for (Map.Entry<String, Command> entry : command.registeredCommands.entrySet()) {
-            if (!commandMap.register(entry.getKey().toLowerCase(), plugin, entry.getValue())) {
+            if (!(commandMap.register(entry.getKey().toLowerCase(), plugin, entry.getValue()))) {
                 allSuccess = false;
+            } else {
+                knownCommands.put(entry.getKey(), command);
             }
         }
 
         return allSuccess;
     }
+
+    @Override
+    public boolean unregisterCommand(BaseCommand command) {
+        boolean[] allSuccess = {true};
+        command.registeredCommands.entrySet().removeIf(entry -> {
+            boolean removed = entry.getValue().unregister(commandMap);
+            if (removed) {
+                commandMap.getKnownCommands().remove(entry.getKey());
+            } else {
+                allSuccess[0] = false;
+            }
+            return removed;
+        });
+        return allSuccess[0];
+    }
+
+    @Override
+    public boolean unregisterCommands() {
+        boolean allSuccess = true;
+        for (Map.Entry<String, BaseCommand> entry : knownCommands.entrySet()) {
+            if (!(unregisterCommand(entry.getValue()))) {
+                allSuccess = false;
+            }
+        }
+        return allSuccess;
+    }
+
+    @Override
+    public BaseCommand getCommandByAlias(String alias) {
+        return knownCommands.get(alias);
+    }
 }

src/main/java/co/aikar/commands/CommandManager.java

@@ -25,10 +25,18 @@ package co.aikar.commands;
 
 import org.bukkit.plugin.Plugin;
 
+import java.util.Map;
+
 public interface CommandManager {
 
     Plugin getPlugin();
 
+    /**
+     * Gets the commands this CommandManager knows about
+     * @return the commands
+     */
+    Map<String, BaseCommand> getKnownCommands();
+
     /**
      * Gets the command contexts manager
      * @return Command Contexts
@@ -48,4 +56,27 @@ public interface CommandManager {
      * @return boolean
      */
     boolean registerCommand(BaseCommand command);
+
+    /**
+     * Unregisters a command from ACF
+     *
+     * @param command The command to unregister
+     * @return boolean
+     */
+    boolean unregisterCommand(BaseCommand command);
+
+    /**
+     * Unregisters all commands registered via this manager from ACF
+     *
+     * @return boolean
+     */
+    boolean unregisterCommands();
+
+    /**
+     * Matches an alias to its BaseCommand instance
+     *
+     * @param alias The command alias
+     * @return The command
+     */
+    BaseCommand getCommandByAlias(String alias);
 }