Improve command registration timing on Velocity/BungeeCord (#5013)

GeyserMC · Sep 1, 2024 · 1ab3f1f · 1ab3f1f
1 parent 8935b34
commit 1ab3f1f
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 28 deletions.
diff --git a/.../bungeecord/src/main/java/org/geysermc/geyser/platform/bungeecord/GeyserBungeePlugin.java b/.../bungeecord/src/main/java/org/geysermc/geyser/platform/bungeecord/GeyserBungeePlugin.java
@@ -111,6 +111,21 @@ public void onEnable() {
         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 void onGeyserEnable() {
             }
             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/...neoforge/src/main/java/org/geysermc/geyser/platform/neoforge/GeyserNeoForgeBootstrap.java b/...neoforge/src/main/java/org/geysermc/geyser/platform/neoforge/GeyserNeoForgeBootstrap.java
@@ -82,6 +82,7 @@ public GeyserNeoForgeBootstrap(ModContainer container) {
         );
         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
@@ -181,7 +181,7 @@ public void onEnable() {
             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/...ap/velocity/src/main/java/org/geysermc/geyser/platform/velocity/GeyserVelocityPlugin.java b/...ap/velocity/src/main/java/org/geysermc/geyser/platform/velocity/GeyserVelocityPlugin.java
@@ -109,6 +109,22 @@ public void onGeyserInitialize() {
 
         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 void onGeyserEnable() {
             }
             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();