From 1ab3f1f2e06de06720154f205a2f5bcf8402bf31 Mon Sep 17 00:00:00 2001
From: Konicai <71294714+Konicai@users.noreply.github.com>
Date: Sun, 1 Sep 2024 15:34:53 -0400
Subject: [PATCH] Improve command registration timing on Velocity/BungeeCord
 (#5013)

---
 .../bungeecord/GeyserBungeePlugin.java        | 28 +++++++++--------
 .../neoforge/GeyserNeoForgeBootstrap.java     |  1 +
 .../platform/spigot/GeyserSpigotPlugin.java   |  2 +-
 .../velocity/GeyserVelocityPlugin.java        | 30 ++++++++++---------
 4 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/bootstrap/bungeecord/src/main/java/org/geysermc/geyser/platform/bungeecord/GeyserBungeePlugin.java b/bootstrap/bungeecord/src/main/java/org/geysermc/geyser/platform/bungeecord/GeyserBungeePlugin.java
index e2735c80e..7adfd488f 100644
--- a/bootstrap/bungeecord/src/main/java/org/geysermc/geyser/platform/bungeecord/GeyserBungeePlugin.java
+++ b/bootstrap/bungeecord/src/main/java/org/geysermc/geyser/platform/bungeecord/GeyserBungeePlugin.java
@@ -111,6 +111,21 @@ public class GeyserBungeePlugin extends Plugin implements GeyserBootstrap {
         if (geyser == null) {
             return; // Config did not load properly!
         }
+
+        // After Geyser initialize for parity with other platforms.
+        var sourceConverter = new CommandSourceConverter<>(
+            CommandSender.class,
+            id -> getProxy().getPlayer(id),
+            () -> getProxy().getConsole(),
+            BungeeCommandSource::new
+        );
+        CommandManager<GeyserCommandSource> cloud = new BungeeCommandManager<>(
+            this,
+            ExecutionCoordinator.simpleCoordinator(),
+            sourceConverter
+        );
+        this.commandRegistry = new CommandRegistry(geyser, cloud, false); // applying root permission would be a breaking change because we can't register permission defaults
+
         // Big hack - Bungee does not provide us an event to listen to, so schedule a repeating
         // task that waits for a field to be filled which is set after the plugin enable
         // process is complete
@@ -150,19 +165,6 @@ public class GeyserBungeePlugin extends Plugin implements GeyserBootstrap {
             }
             this.geyserLogger.setDebug(geyserConfig.isDebugMode());
             GeyserConfiguration.checkGeyserConfiguration(geyserConfig, geyserLogger);
-        } else {
-            var sourceConverter = new CommandSourceConverter<>(
-                    CommandSender.class,
-                    id -> getProxy().getPlayer(id),
-                    () -> getProxy().getConsole(),
-                    BungeeCommandSource::new
-            );
-            CommandManager<GeyserCommandSource> cloud = new BungeeCommandManager<>(
-                    this,
-                    ExecutionCoordinator.simpleCoordinator(),
-                    sourceConverter
-            );
-            this.commandRegistry = new CommandRegistry(geyser, cloud, false); // applying root permission would be a breaking change because we can't register permission defaults
         }
 
         // Force-disable query if enabled, or else Geyser won't enable
diff --git a/bootstrap/mod/neoforge/src/main/java/org/geysermc/geyser/platform/neoforge/GeyserNeoForgeBootstrap.java b/bootstrap/mod/neoforge/src/main/java/org/geysermc/geyser/platform/neoforge/GeyserNeoForgeBootstrap.java
index 7d3b9dc5f..ad56eda39 100644
--- a/bootstrap/mod/neoforge/src/main/java/org/geysermc/geyser/platform/neoforge/GeyserNeoForgeBootstrap.java
+++ b/bootstrap/mod/neoforge/src/main/java/org/geysermc/geyser/platform/neoforge/GeyserNeoForgeBootstrap.java
@@ -82,6 +82,7 @@ public class GeyserNeoForgeBootstrap extends GeyserModBootstrap {
         );
         GeyserNeoForgeCommandRegistry registry = new GeyserNeoForgeCommandRegistry(getGeyser(), cloud);
         this.setCommandRegistry(registry);
+        // An auxiliary listener for registering undefined permissions belonging to commands. See javadocs for more info.
         NeoForge.EVENT_BUS.addListener(EventPriority.LOWEST, registry::onPermissionGatherForUndefined);
     }
 
diff --git a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotPlugin.java b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotPlugin.java
index c52927a83..a2d5c992b 100644
--- a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotPlugin.java
+++ b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotPlugin.java
@@ -181,7 +181,7 @@ public class GeyserSpigotPlugin extends JavaPlugin implements GeyserBootstrap {
             return;
         }
 
-        // Create command manager early so we can add Geyser extension commands
+        // Register commands after Geyser initialization, but before the server starts.
         var sourceConverter = new CommandSourceConverter<>(
                 CommandSender.class,
                 Bukkit::getPlayer,
diff --git a/bootstrap/velocity/src/main/java/org/geysermc/geyser/platform/velocity/GeyserVelocityPlugin.java b/bootstrap/velocity/src/main/java/org/geysermc/geyser/platform/velocity/GeyserVelocityPlugin.java
index 413355813..8fa47f569 100644
--- a/bootstrap/velocity/src/main/java/org/geysermc/geyser/platform/velocity/GeyserVelocityPlugin.java
+++ b/bootstrap/velocity/src/main/java/org/geysermc/geyser/platform/velocity/GeyserVelocityPlugin.java
@@ -109,6 +109,22 @@ public class GeyserVelocityPlugin implements GeyserBootstrap {
 
         this.geyser = GeyserImpl.load(PlatformType.VELOCITY, this);
         this.geyserInjector = new GeyserVelocityInjector(proxyServer);
+
+        // We need to register commands here, rather than in onGeyserEnable which is invoked during the appropriate ListenerBoundEvent.
+        // Reason: players can connect after a listener is bound, and a player join locks registration to the cloud CommandManager.
+        var sourceConverter = new CommandSourceConverter<>(
+            CommandSource.class,
+            id -> proxyServer.getPlayer(id).orElse(null),
+            proxyServer::getConsoleCommandSource,
+            VelocityCommandSource::new
+        );
+        CommandManager<GeyserCommandSource> cloud = new VelocityCommandManager<>(
+            container,
+            proxyServer,
+            ExecutionCoordinator.simpleCoordinator(),
+            sourceConverter
+        );
+        this.commandRegistry = new CommandRegistry(geyser, cloud, false); // applying root permission would be a breaking change because we can't register permission defaults
     }
 
     @Override
@@ -123,20 +139,6 @@ public class GeyserVelocityPlugin implements GeyserBootstrap {
             }
             this.geyserLogger.setDebug(geyserConfig.isDebugMode());
             GeyserConfiguration.checkGeyserConfiguration(geyserConfig, geyserLogger);
-        } else {
-            var sourceConverter = new CommandSourceConverter<>(
-                    CommandSource.class,
-                    id -> proxyServer.getPlayer(id).orElse(null),
-                    proxyServer::getConsoleCommandSource,
-                    VelocityCommandSource::new
-            );
-            CommandManager<GeyserCommandSource> cloud = new VelocityCommandManager<>(
-                    container,
-                    proxyServer,
-                    ExecutionCoordinator.simpleCoordinator(),
-                    sourceConverter
-            );
-            this.commandRegistry = new CommandRegistry(geyser, cloud, false); // applying root permission would be a breaking change because we can't register permission defaults
         }
 
         GeyserImpl.start();