Move fabric command registration earlier

This prevents situation where Plan disables due to an error but reload command is not available Affects issues: - Fixed #3392
plan-player-analytics · Jan 21, 2024 · 1fdd328 · 1fdd328
1 parent 9e08794
commit 1fdd328
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 25 deletions.
diff --git a/Plan/fabric/src/main/java/net/playeranalytics/plan/PlanFabric.java b/Plan/fabric/src/main/java/net/playeranalytics/plan/PlanFabric.java
@@ -32,7 +32,7 @@
 import net.fabricmc.fabric.api.event.lifecycle.v1.ServerLifecycleEvents;
 import net.fabricmc.loader.api.FabricLoader;
 import net.minecraft.server.dedicated.MinecraftDedicatedServer;
-import net.playeranalytics.plan.commands.CommandManager;
+import net.playeranalytics.plan.commands.FabricCommandManager;
 import net.playeranalytics.plan.identification.properties.FabricServerProperties;
 import net.playeranalytics.plugin.FabricPlatformLayer;
 import net.playeranalytics.plugin.PlatformAbstractionLayer;
@@ -55,7 +55,7 @@
 public class PlanFabric implements PlanPlugin, DedicatedServerModInitializer {
 
     private MinecraftDedicatedServer server;
-    private CommandManager commandManager;
+    private FabricCommandManager fabricCommandManager;
 
     private PlanSystem system;
     private Locale locale;
@@ -83,7 +83,7 @@ public PlanSystem getSystem() {
 
     @Override
     public void registerCommand(Subcommand command) {
-        commandManager.registerRoot(command, runnableFactory);
+        fabricCommandManager.registerRoot(command, runnableFactory);
     }
 
     @Override
@@ -100,6 +100,7 @@ public void onEnable() {
             errorLogger = component.errorLogger();
             serverShutdownSave = component.serverShutdownSave();
             locale = system.getLocaleSystem().getLocale();
+            registerCommand(component.planCommand().build());
             system.enable();
 
             pluginLogger.info(locale.getString(PluginLang.ENABLED));
@@ -118,7 +119,6 @@ public void onEnable() {
             pluginLogger.error("This error should be reported at https://github.com/plan-player-analytics/Plan/issues");
             onDisable();
         }
-        registerCommand(component.planCommand().build());
         if (system != null) {
             system.getProcessing().submitNonCritical(() -> system.getListenerSystem().callEnableEvent(this));
         }
@@ -167,7 +167,7 @@ public void onInitializeServer() {
             onEnable();
         });
 
-        CommandRegistrationCallback.EVENT.register((dispatcher, registryAccess, environment) -> commandManager = new CommandManager(dispatcher, this, errorLogger));
+        CommandRegistrationCallback.EVENT.register((dispatcher, registryAccess, environment) -> fabricCommandManager = new FabricCommandManager(dispatcher, this, errorLogger));
 
         ServerLifecycleEvents.SERVER_STOPPING.register(server -> onDisable());
     }

diff --git a/...alytics/plan/commands/CommandManager.java → ...s/plan/commands/FabricCommandManager.java b/...alytics/plan/commands/CommandManager.java → ...s/plan/commands/FabricCommandManager.java
@@ -32,22 +32,23 @@
 import com.mojang.brigadier.suggestion.SuggestionsBuilder;
 import me.lucko.fabric.api.permissions.v0.Permissions;
 import net.minecraft.command.CommandSource;
+import net.minecraft.server.command.CommandManager;
 import net.minecraft.server.command.ServerCommandSource;
 import net.minecraft.text.Text;
 import net.playeranalytics.plan.PlanFabric;
 import net.playeranalytics.plugin.scheduling.RunnableFactory;
 
 import java.util.concurrent.CompletableFuture;
 
-public class CommandManager {
+public class FabricCommandManager {
 
     private final CommandDispatcher<ServerCommandSource> dispatcher;
     private RunnableFactory runnableFactory;
     private LiteralArgumentBuilder<ServerCommandSource> root;
     private final PlanFabric plugin;
     private final ErrorLogger errorLogger;
 
-    public CommandManager(CommandDispatcher<ServerCommandSource> dispatcher, PlanFabric plugin, ErrorLogger errorLogger) {
+    public FabricCommandManager(CommandDispatcher<ServerCommandSource> dispatcher, PlanFabric plugin, ErrorLogger errorLogger) {
         this.dispatcher = dispatcher;
         this.plugin = plugin;
         this.errorLogger = errorLogger;
@@ -135,20 +136,18 @@ public void registerChild(Subcommand subcommand, ArgumentBuilder<ServerCommandSo
     }
 
     private LiteralArgumentBuilder<ServerCommandSource> buildCommand(Subcommand subcommand, String alias) {
-        RequiredArgumentBuilder<ServerCommandSource, String> arguments = RequiredArgumentBuilder.argument("arguments", StringArgumentType.greedyString());
-        arguments.suggests((context, builder) -> arguments(subcommand, context, builder));
-        arguments.executes(ctx -> execute(ctx, subcommand));
-        LiteralArgumentBuilder<ServerCommandSource> literal = LiteralArgumentBuilder.literal(alias);
-        literal.executes(ctx -> execute(ctx, subcommand));
-        literal.requires(src -> {
-            for (String permission : subcommand.getRequiredPermissions()) {
-                if (!checkPermission(src, permission)) return false;
-            }
-            return true;
-        });
-
-        literal.then(arguments);
-        return literal;
+        RequiredArgumentBuilder<ServerCommandSource, String> arguments = CommandManager.argument("arguments", StringArgumentType.greedyString());
+        arguments = arguments.suggests((context, builder) -> arguments(subcommand, context, builder))
+                .executes(ctx -> execute(ctx, subcommand));
+        LiteralArgumentBuilder<ServerCommandSource> builder = CommandManager.literal(alias);
+        return builder
+                .executes(ctx -> execute(ctx, subcommand))
+                .requires(src -> {
+                    for (String permission : subcommand.getRequiredPermissions()) {
+                        if (!checkPermission(src, permission)) return false;
+                    }
+                    return true;
+                }).then(arguments);
     }
 
 }
diff --git a/...a/net/playeranalytics/plan/gathering/listeners/events/mixin/ServerCommandSourceMixin.java b/...a/net/playeranalytics/plan/gathering/listeners/events/mixin/ServerCommandSourceMixin.java
@@ -22,7 +22,7 @@
 import net.minecraft.server.command.ServerCommandSource;
 import net.minecraft.server.network.ServerPlayerEntity;
 import net.minecraft.text.Text;
-import net.playeranalytics.plan.commands.CommandManager;
+import net.playeranalytics.plan.commands.FabricCommandManager;
 import net.playeranalytics.plan.commands.use.FabricMessageBuilder;
 import org.jetbrains.annotations.Nullable;
 import org.spongepowered.asm.mixin.Mixin;
@@ -64,7 +64,7 @@ public Optional<String> getPlayerName() {
 
     @Override
     public boolean hasPermission(String permission) {
-        return CommandManager.checkPermission((ServerCommandSource) (Object) this, permission);
+        return FabricCommandManager.checkPermission((ServerCommandSource) (Object) this, permission);
     }
 
     @Override

diff --git a/.../src/main/java/net/playeranalytics/plan/gathering/listeners/fabric/FabricAFKListener.java b/.../src/main/java/net/playeranalytics/plan/gathering/listeners/fabric/FabricAFKListener.java
@@ -24,7 +24,7 @@
 import net.fabricmc.fabric.api.message.v1.ServerMessageEvents;
 import net.fabricmc.fabric.api.networking.v1.ServerPlayConnectionEvents;
 import net.minecraft.server.network.ServerPlayerEntity;
-import net.playeranalytics.plan.commands.CommandManager;
+import net.playeranalytics.plan.commands.FabricCommandManager;
 import net.playeranalytics.plan.gathering.listeners.FabricListener;
 import net.playeranalytics.plan.gathering.listeners.events.PlanFabricEvents;
 
@@ -83,7 +83,7 @@ private void event(ServerPlayerEntity player) {
     }
 
     private boolean checkPermission(ServerPlayerEntity player, String permission) {
-        if (CommandManager.isPermissionsApiAvailable()) {
+        if (FabricCommandManager.isPermissionsApiAvailable()) {
             return Permissions.check(player, permission);
         } else {
             return false;