From 75b9323ee2797e999681f847bdaf46f3970bc854 Mon Sep 17 00:00:00 2001 From: Andrew Wong <42793301+md5sha256@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:46:22 +1100 Subject: [PATCH 1/3] feat(modules): split ModuleLifecycleManager.start into load and initializeLoaded A host can now load module jars, act on the loaded modules (for example, let them declare commands during the server's registration window), and initialize them later. start() keeps its behaviour as load() followed by initializeLoaded(). stop() closes modules that were loaded but never initialized, without calling shutdown. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011DvdDwRoC7qHZ7yKUv1ufw --- README.md | 4 +- .../modules/ModuleLifecycleManager.java | 61 ++++++++++++++-- .../modules/ModuleLifecycleManagerTest.java | 69 +++++++++++++++++++ 3 files changed, 126 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 798bc40..1fd4ff0 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,9 @@ by a `ModuleManifest` and loaded at runtime by its own `URLClassLoader`. (configurate-serializable). - `ModuleLoader` — reads module jars from a directory and instantiates their entry classes. - `ModuleLifecycleManager` — owns the runtime lifecycle: `start()`, `stop()`, - `reloadAsync(moduleName)`, `getActiveModules()`. + `reloadAsync(moduleName)`, `getActiveModules()`. `start()` is `load()` then `initializeLoaded()`; + call the two separately when the host must do something with the loaded modules first, such as + letting them declare commands while the server still accepts registrations. > **Failed start:** if `initialize` throws, the manager closes the module's class loader but does > **not** call `shutdown`. A module must do its fallible work before registering anything, or undo diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java index c30d325..323993d 100644 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java @@ -38,6 +38,12 @@ public class ModuleLifecycleManager { */ private final Map> activeModules = new LinkedHashMap<>(); + /** + * Modules returned by {@link #load()} and not yet passed to {@link #initializeLoaded()}, or + * {@code null} when nothing is waiting. + */ + private List> loadedModules; + /** * Creates a manager that expects modules targeting the host plugin's own class name and logs * through the plugin's logger. @@ -59,17 +65,52 @@ public ModuleLifecycleManager( } /** - * Loads every module from disk and initializes it against the host plugin. + * Loads every module from disk and initializes it against the host plugin. Equivalent to + * {@link #load()} followed by {@link #initializeLoaded()}. * - * @throws IllegalStateException if modules are already active; call {@link #stop()} first + * @throws IllegalStateException if modules are already loaded or active; call {@link #stop()} first * @throws IOException if the module directory cannot be scanned */ public void start() throws IOException { - if (!this.activeModules.isEmpty()) { - throw new IllegalStateException("Modules already started; call stop() or reload() first"); + load(); + initializeLoaded(); + } + + /** + * Loads every module from disk without initializing any of them. The host can then inspect the + * returned modules, for example to let them declare commands while the server still accepts + * registrations, before calling {@link #initializeLoaded()}. + * + *

A loaded module's classes are live but {@link PluginModule#initialize} has not run, so the + * host must not expect any module state yet. + * + * @return the loaded modules, in load order + * @throws IllegalStateException if modules are already loaded or active; call {@link #stop()} first + * @throws IOException if the module directory cannot be scanned + */ + public @NotNull List> load() throws IOException { + if (this.loadedModules != null || !this.activeModules.isEmpty()) { + throw new IllegalStateException("Modules already loaded; call stop() first"); + } + List> loaded = List.copyOf( + this.moduleLoader.loadModulesFromDisk(this.expectedPluginClass, this.logger)); + this.loadedModules = loaded; + return loaded; + } + + /** + * Initializes the modules returned by the last {@link #load()}. A module whose + * {@code initialize} throws is unloaded and is not active afterwards; see + * {@link PluginModule#initialize} for its cleanup contract. + * + * @throws IllegalStateException if nothing is loaded + */ + public void initializeLoaded() { + List> loaded = this.loadedModules; + if (loaded == null) { + throw new IllegalStateException("No modules loaded; call load() first"); } - List> loaded = this.moduleLoader.loadModulesFromDisk(this.expectedPluginClass, - this.logger); + this.loadedModules = null; for (LoadedModule module : loaded) { initialize(module); } @@ -77,9 +118,15 @@ public void start() throws IOException { /** * Shuts down every active module in reverse initialization order and releases their class - * loaders. Safe to call when nothing is active. + * loaders. Modules that were loaded but never initialized are closed without + * {@link PluginModule#shutdown}. Safe to call when nothing is loaded or active. */ public void stop() { + List> pending = this.loadedModules; + if (pending != null) { + this.loadedModules = null; + pending.forEach(this::closeQuietly); + } List> ordered = new ArrayList<>(this.activeModules.values()); for (int i = ordered.size() - 1; i >= 0; i--) { shutdown(ordered.get(i)); diff --git a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManagerTest.java b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManagerTest.java index 4ea58ec..7f58dbc 100644 --- a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManagerTest.java +++ b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManagerTest.java @@ -267,6 +267,75 @@ void reloadNullFutureBecomesCompleted() throws Exception { assertFalse(future.isCompletedExceptionally()); } + @Test + @DisplayName("load() loads modules without initializing them") + void loadDoesNotInitialize() throws Exception { + FakeModule a = new FakeModule("a", new ArrayList<>()); + stubLoader(List.of(loaded("a", false, a))); + + ModuleLifecycleManager manager = manager(); + List> loaded = manager.load(); + + assertFalse(a.initialized); + assertEquals(1, loaded.size()); + assertTrue(manager.getActiveModules().isEmpty()); + } + + @Test + @DisplayName("initializeLoaded() initializes what load() loaded and activates it") + void initializeLoadedActivates() throws Exception { + FakeModule a = new FakeModule("a", new ArrayList<>()); + stubLoader(List.of(loaded("a", false, a))); + + ModuleLifecycleManager manager = manager(); + manager.load(); + manager.initializeLoaded(); + + assertTrue(a.initialized); + assertEquals(java.util.Set.of("a"), manager.getActiveModules().keySet()); + } + + @Test + @DisplayName("initializeLoaded() without load() is rejected") + void initializeLoadedWithoutLoadThrows() { + assertThrows(IllegalStateException.class, manager()::initializeLoaded); + } + + @Test + @DisplayName("load() twice without stop() is rejected") + void loadTwiceThrows() throws Exception { + stubLoader(List.of(loaded("a", false, new FakeModule("a", new ArrayList<>())))); + ModuleLifecycleManager manager = manager(); + manager.load(); + + assertThrows(IllegalStateException.class, manager::load); + } + + @Test + @DisplayName("stop() after load() closes loaded modules without shutting them down") + void stopAfterLoadClosesWithoutShutdown() throws Exception { + List shutdowns = new ArrayList<>(); + boolean[] closed = new boolean[1]; + ModuleManifest manifest = new ModuleManifest("a", "Entry", "author", HOST, false); + URLClassLoader classLoader = new URLClassLoader(new URL[0]) { + @Override + public void close() throws IOException { + closed[0] = true; + super.close(); + } + }; + stubLoader(List.of(new LoadedModule<>(manifest, new FakeModule("a", shutdowns), Path.of("a"), classLoader))); + + ModuleLifecycleManager manager = manager(); + manager.load(); + manager.stop(); + + assertTrue(closed[0]); + assertTrue(shutdowns.isEmpty()); + // Releasing the loaded modules allows a fresh load. + manager.load(); + } + @Test @DisplayName("getActiveModules() returns an immutable snapshot") void activeModulesIsImmutableSnapshot() throws Exception { From 8ec3a2f5a6880d5c6f9103cdc23ad42bb6e7e8bf Mon Sep 17 00:00:00 2001 From: Andrew Wong <42793301+md5sha256@users.noreply.github.com> Date: Tue, 27 Oct 2026 11:59:37 +1100 Subject: [PATCH 2/3] feat(modules): let modules declare guarded Brigadier commands Paper only accepts commands during LifecycleEvents.COMMANDS, which fires before modules initialize, and cannot unregister them. A module that implements CommandDeclaringModule now declares its commands through ModuleLifecycleManager#declareCommands, between load() and initializeLoaded(), using a ModuleCommandRoot (subcommands under the host's root) and a ModuleCommandRegistrar (top-level commands). CommandGuard rebuilds each tree so executors and suggestion providers run module code only while the module is active; otherwise the host's ModuleUnavailableHandler replies. A failed start therefore leaves no command to clean up. Redirects are rejected because they would bypass the guard. Declared trees are kept and replayed when the commands event fires again on /minecraft:reload. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011DvdDwRoC7qHZ7yKUv1ufw --- AGENTS.md | 6 + README.md | 19 +- .../modules/CommandDeclaringModule.java | 29 ++ .../modules/CommandGuard.java | 73 +++++ .../modules/ModuleCommandRegistrar.java | 32 ++ .../modules/ModuleCommandRoot.java | 15 + .../modules/ModuleLifecycleManager.java | 101 +++++- .../modules/ModuleUnavailableHandler.java | 15 + .../modules/PluginModule.java | 6 +- .../modules/ModuleCommandDeclarationTest.java | 287 ++++++++++++++++++ 10 files changed, 578 insertions(+), 5 deletions(-) create mode 100644 src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandDeclaringModule.java create mode 100644 src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandGuard.java create mode 100644 src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java create mode 100644 src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java create mode 100644 src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleUnavailableHandler.java create mode 100644 src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java diff --git a/AGENTS.md b/AGENTS.md index 406270e..1d98c9d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,6 +59,12 @@ outcome that cannot occur. class loader, so a module must clean up its own partial registrations before throwing (documented on `PluginModule#initialize`). `SimplePluginModule` tracks listeners but unregisters nothing by itself. Changing this is a contract change for every module, not a bug fix. +- **Module commands are declared, never unregistered.** Paper only accepts commands during its + lifecycle commands event, before modules initialize, and its API cannot remove them. A + `CommandDeclaringModule` declares through `ModuleLifecycleManager#declareCommands` between `load()` + and `initializeLoaded()`; `CommandGuard` rebuilds each tree so executors and suggestions run only + while the module is active, and the host's `ModuleUnavailableHandler` replies otherwise. Redirects + are rejected because they would bypass the guard. A module never cleans up its commands. - **`ModuleLifecycleManager` is not thread-safe.** Its state must be touched from one thread (the server main thread on Paper). - **`SchemaMigrator` is abstract and dialect-agnostic.** It provides no SQL of its own — subclasses diff --git a/README.md b/README.md index 1fd4ff0..ed19006 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,16 @@ by a `ModuleManifest` and loaded at runtime by its own `URLClassLoader`. `reloadAsync(moduleName)`, `getActiveModules()`. `start()` is `load()` then `initializeLoaded()`; call the two separately when the host must do something with the loaded modules first, such as letting them declare commands while the server still accepts registrations. +- `CommandDeclaringModule` — implemented by a module that adds Brigadier commands. It declares them + through a `ModuleCommandRoot` (subcommands under the host's root) and a `ModuleCommandRegistrar` + (top-level commands), both of which guard every tree. + +> **Commands:** Paper only accepts commands during `LifecycleEvents.COMMANDS`, before modules +> initialize, and cannot unregister them. Call `declareCommands(commands, root, unavailable)` from that +> event handler, every time it fires, between `load()` and `initializeLoaded()`. A module command runs +> module code only while its module is active; when the module failed to initialize or has stopped, +> the host's `ModuleUnavailableHandler` replies instead. Commands therefore need no cleanup on a +> failed start. Module commands must not use redirects. > **Failed start:** if `initialize` throws, the manager closes the module's class loader but does > **not** call `shutdown`. A module must do its fallible work before registering anything, or undo @@ -71,7 +81,14 @@ Path moduleDir = getDataFolder().toPath().resolve("modules"); ModuleLoader loader = new ModuleLoader(moduleDir); var lifecycle = new ModuleLifecycleManager<>(this, loader, MyPlugin.class.getName(), getLogger()); -lifecycle.start(); // load + initialize all modules (call from the main thread) +lifecycle.load(); // load module jars, no initialize yet +getLifecycleManager().registerEventHandler(LifecycleEvents.COMMANDS, event -> { + LiteralArgumentBuilder root = Commands.literal("myplugin"); + lifecycle.declareCommands(event.registrar(), root, + (source, module) -> source.getSender().sendMessage("That feature is unavailable.")); + event.registrar().register(root.build()); +}); +lifecycle.initializeLoaded(); // initialize (call from the main thread) // ... lifecycle.reloadAsync("economy"); // refresh one reloadable module's config // ... diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandDeclaringModule.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandDeclaringModule.java new file mode 100644 index 0000000..0c9de57 --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandDeclaringModule.java @@ -0,0 +1,29 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import org.jetbrains.annotations.NotNull; + +/** + * Implemented by a {@link PluginModule} that contributes Brigadier commands. + * + *

Paper only accepts commands during its lifecycle commands event, which fires before modules + * initialize. The {@link ModuleLifecycleManager} therefore asks each loaded module to declare its + * commands from {@link ModuleLifecycleManager#declareCommands}, between {@code load()} and + * {@code initializeLoaded()}. Every command declared here is guarded: while the module is not + * active (it failed to initialize, or has been shut down) the command does not run module code and + * the host's unavailable handler replies instead. Commands are never unregistered. + */ +public interface CommandDeclaringModule { + + /** + * Declares this module's commands. Called before {@link PluginModule#initialize}, so it must + * only build command trees: no IO, listeners or tasks, and no state that {@code initialize} + * sets up. Executors and suggestion providers may use that state, because they only run while + * the module is active. Requirements ({@code requires}) run regardless and must not depend on it. + * + *

If this method throws, nothing it declared is registered and the module is unloaded. + * + * @param root subcommands under the host's root command + * @param registrar top-level commands + */ + void declareCommands(@NotNull ModuleCommandRoot root, @NotNull ModuleCommandRegistrar registrar); +} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandGuard.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandGuard.java new file mode 100644 index 0000000..9c0632f --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandGuard.java @@ -0,0 +1,73 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import com.mojang.brigadier.Command; +import com.mojang.brigadier.builder.ArgumentBuilder; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.mojang.brigadier.suggestion.SuggestionProvider; +import com.mojang.brigadier.tree.ArgumentCommandNode; +import com.mojang.brigadier.tree.CommandNode; +import com.mojang.brigadier.tree.LiteralCommandNode; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import org.jetbrains.annotations.NotNull; + +import java.util.function.BooleanSupplier; + +/** + * Rebuilds a module's command tree so that every executor and suggestion provider first checks + * whether the module is active. Brigadier nodes are immutable once built, so the guard is applied + * by rebuilding each node from {@link CommandNode#createBuilder()}. + */ +final class CommandGuard { + + private CommandGuard() { + } + + // Rebuilding a literal yields a literal, and an argument node's builder is a RequiredArgumentBuilder. + @SuppressWarnings("unchecked") + static @NotNull LiteralCommandNode guard( + @NotNull LiteralArgumentBuilder command, + @NotNull String moduleName, + @NotNull BooleanSupplier active, + @NotNull ModuleUnavailableHandler unavailable + ) { + return (LiteralCommandNode) rebuild(command.build(), moduleName, active, unavailable); + } + + @SuppressWarnings("unchecked") + private static @NotNull CommandNode rebuild( + @NotNull CommandNode node, + @NotNull String moduleName, + @NotNull BooleanSupplier active, + @NotNull ModuleUnavailableHandler unavailable + ) { + // A redirect points at a node outside this rebuild, which would bypass the guard. + if (node.getRedirect() != null) { + throw new IllegalArgumentException("Module '" + moduleName + "' declared a redirect at '" + + node.getName() + "'; module commands must not use redirects"); + } + ArgumentBuilder builder = node.createBuilder(); + Command command = node.getCommand(); + if (command != null) { + builder.executes(ctx -> { + if (active.getAsBoolean()) { + return command.run(ctx); + } + unavailable.handle(ctx.getSource(), moduleName); + return 0; + }); + } + if (node instanceof ArgumentCommandNode argumentNode + && argumentNode.getCustomSuggestions() != null) { + SuggestionProvider suggestions = argumentNode.getCustomSuggestions(); + RequiredArgumentBuilder argument = (RequiredArgumentBuilder) builder; + argument.suggests((ctx, suggestionsBuilder) -> active.getAsBoolean() + ? suggestions.getSuggestions(ctx, suggestionsBuilder) + : suggestionsBuilder.buildFuture()); + } + for (CommandNode child : node.getChildren()) { + builder.then(rebuild(child, moduleName, active, unavailable)); + } + return builder.build(); + } +} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java new file mode 100644 index 0000000..c5dd141 --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java @@ -0,0 +1,32 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; +import java.util.List; + +/** + * Registers top-level commands for a {@link CommandDeclaringModule}. Each tree is guarded by the + * module's availability before it reaches Paper's registrar, so a module cannot register an + * unguarded command. + */ +public interface ModuleCommandRegistrar { + + /** + * Registers a top-level command. The tree must not contain redirects. + * + * @param command the command tree + * @param description the help description, or {@code null} + * @param aliases extra labels + */ + void register(@NotNull LiteralArgumentBuilder command, + @Nullable String description, @NotNull Collection aliases); + + default void register(@NotNull LiteralArgumentBuilder command, + @Nullable String description) { + register(command, description, List.of()); + } +} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java new file mode 100644 index 0000000..8121c84 --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java @@ -0,0 +1,15 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import org.jetbrains.annotations.NotNull; + +/** The host's root command, as offered to a {@link CommandDeclaringModule}. */ +public interface ModuleCommandRoot { + + /** + * Adds a subcommand under the host's root command. The tree is guarded by the module's + * availability; it must not contain redirects. + */ + void then(@NotNull LiteralArgumentBuilder subcommand); +} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java index 323993d..ad9b252 100644 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java @@ -1,15 +1,26 @@ package com.minecraftcitiesnetwork.pluginInfrastructure.modules; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.tree.LiteralCommandNode; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import io.papermc.paper.command.brigadier.Commands; import org.bukkit.plugin.Plugin; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.BooleanSupplier; import java.util.logging.Level; import java.util.logging.Logger; @@ -44,6 +55,22 @@ public class ModuleLifecycleManager { */ private List> loadedModules; + /** + * Names of the active modules, read by command guards. Commands can run off the main thread, + * so this is concurrent while the rest of the manager is not. + */ + private final Set activeNames = ConcurrentHashMap.newKeySet(); + + /** + * Guarded command trees keyed by module name. Kept after a module stops so that a later + * commands event (Paper re-fires it on {@code /minecraft:reload}) registers the same commands, + * which then reply as unavailable. + */ + private final Map declaredCommands = new LinkedHashMap<>(); + + /** Modules from the current {@link #load()} that have already declared their commands. */ + private final Set declaredThisLoad = new HashSet<>(); + /** * Creates a manager that expects modules targeting the host plugin's own class name and logs * through the plugin's logger. @@ -92,10 +119,11 @@ public void start() throws IOException { if (this.loadedModules != null || !this.activeModules.isEmpty()) { throw new IllegalStateException("Modules already loaded; call stop() first"); } - List> loaded = List.copyOf( + List> loaded = new ArrayList<>( this.moduleLoader.loadModulesFromDisk(this.expectedPluginClass, this.logger)); this.loadedModules = loaded; - return loaded; + this.declaredThisLoad.clear(); + return List.copyOf(loaded); } /** @@ -116,6 +144,60 @@ public void initializeLoaded() { } } + /** + * Registers module commands. Call it from Paper's {@code LifecycleEvents.COMMANDS} handler, + * after {@link #load()} and before {@link #initializeLoaded()}, every time the event fires. + * + *

Each loaded {@link CommandDeclaringModule} that has not declared yet is asked to declare. + * A module whose {@code declareCommands} throws is logged, unloaded and not initialized; nothing + * it declared is registered. Then every module's guarded commands are added: subcommands to + * {@code root}, top-level commands to {@code commands}. A guarded command runs the module's code + * only while the module is active; otherwise {@code unavailable} replies. + * + * @param commands Paper's registrar from the commands event + * @param root the host's root command builder; the host registers it after this call + * @param unavailable replies when a command's module is not active + */ + public void declareCommands( + @NotNull Commands commands, + @NotNull LiteralArgumentBuilder root, + @NotNull ModuleUnavailableHandler unavailable + ) { + List> pending = this.loadedModules; + if (pending != null) { + Iterator> modules = pending.iterator(); + while (modules.hasNext()) { + LoadedModule loaded = modules.next(); + String name = loaded.manifest().moduleName(); + // A second copy with the same name is dropped by initializeLoaded and declares nothing. + if (!(loaded.module() instanceof CommandDeclaringModule declaring) || !this.declaredThisLoad.add(name)) { + continue; + } + BooleanSupplier active = () -> this.activeNames.contains(name); + DeclaredCommands declared = new DeclaredCommands(new ArrayList<>(), new ArrayList<>()); + try { + declaring.declareCommands( + sub -> declared.subcommands().add(CommandGuard.guard(sub, name, active, unavailable)), + (command, description, aliases) -> declared.topLevel().add(new TopLevelCommand( + CommandGuard.guard(command, name, active, unavailable), description, + List.copyOf(aliases)))); + } catch (RuntimeException e) { + this.logger.log(Level.SEVERE, "Failed to declare commands for module " + name + "; unloading", e); + modules.remove(); + closeQuietly(loaded); + continue; + } + this.declaredCommands.put(name, declared); + } + } + for (DeclaredCommands declared : this.declaredCommands.values()) { + declared.subcommands().forEach(root::then); + for (TopLevelCommand command : declared.topLevel()) { + commands.register(command.node(), command.description(), command.aliases()); + } + } + } + /** * Shuts down every active module in reverse initialization order and releases their class * loaders. Modules that were loaded but never initialized are closed without @@ -179,6 +261,7 @@ private void initialize(@NotNull LoadedModule loaded) { try { loaded.module().initialize(this.plugin, loaded.dataFolder()); this.activeModules.put(name, loaded); + this.activeNames.add(name); this.logger.info("Initialized module " + name); } catch (ModuleInitializationException | RuntimeException e) { this.logger.log(Level.SEVERE, "Failed to initialize module " + name + "; unloading", e); @@ -188,6 +271,7 @@ private void initialize(@NotNull LoadedModule loaded) { private void shutdown(@NotNull LoadedModule loaded) { String name = loaded.manifest().moduleName(); + this.activeNames.remove(name); try { loaded.module().shutdown(this.plugin); this.logger.info("Shut down module " + name); @@ -206,4 +290,17 @@ private void closeQuietly(@NotNull LoadedModule loaded) { "Failed to close class loader for module " + loaded.manifest().moduleName(), e); } } + + private record DeclaredCommands( + @NotNull List> subcommands, + @NotNull List topLevel + ) { + } + + private record TopLevelCommand( + @NotNull LiteralCommandNode node, + @Nullable String description, + @NotNull Collection aliases + ) { + } } diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleUnavailableHandler.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleUnavailableHandler.java new file mode 100644 index 0000000..a34cbc9 --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleUnavailableHandler.java @@ -0,0 +1,15 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import io.papermc.paper.command.brigadier.CommandSourceStack; +import org.jetbrains.annotations.NotNull; + +/** Replies to a sender who ran a module command while that module is not active. */ +@FunctionalInterface +public interface ModuleUnavailableHandler { + + /** + * @param source who ran the command + * @param moduleName the {@link ModuleManifest#moduleName()} of the inactive module + */ + void handle(@NotNull CommandSourceStack source, @NotNull String moduleName); +} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java index 9510487..f173236 100644 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java @@ -9,8 +9,8 @@ public interface PluginModule { /** - * Starts this module: reads its configuration and registers its listeners, tasks and commands - * with the host. + * Starts this module: reads its configuration and registers its listeners and tasks with the + * host. Commands are not registered here; see {@link CommandDeclaringModule}. * *

Failure is the module's to clean up. If this method throws, the * {@link ModuleLifecycleManager} logs the failure and closes the module's class loader, but it @@ -18,6 +18,8 @@ public interface PluginModule { * a listener keeps firing with classes from a closed class loader, a task keeps running. An * implementation must therefore either do all fallible work (reading config, resolving * services) before registering anything, or undo what it registered before rethrowing. + * Commands declared through {@link CommandDeclaringModule} are the exception: they stay + * registered and reply as unavailable, so they need no cleanup. * * @param plugin the host plugin * @param dataFolder this module's own data folder diff --git a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java new file mode 100644 index 0000000..1bab3f4 --- /dev/null +++ b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java @@ -0,0 +1,287 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import com.mojang.brigadier.CommandDispatcher; +import com.mojang.brigadier.arguments.StringArgumentType; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.mojang.brigadier.suggestion.Suggestions; +import com.mojang.brigadier.tree.LiteralCommandNode; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import io.papermc.paper.command.brigadier.Commands; +import org.bukkit.plugin.Plugin; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.function.BiConsumer; +import java.util.logging.Logger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +class ModuleCommandDeclarationTest { + + private static final String HOST = "HostPlugin"; + + private Plugin plugin; + private ModuleLoader loader; + private Commands commands; + private CommandSourceStack source; + private final List unavailableFor = new ArrayList<>(); + private final ModuleUnavailableHandler unavailable = (src, name) -> this.unavailableFor.add(name); + + /** A module that declares commands through a callback and can fail to initialize. */ + private static final class CommandModule implements PluginModule, CommandDeclaringModule { + private final BiConsumer declare; + private RuntimeException initFailure; + int declareCalls; + boolean initialized; + + CommandModule(BiConsumer declare) { + this.declare = declare; + } + + @Override + public void declareCommands(@NotNull ModuleCommandRoot root, @NotNull ModuleCommandRegistrar registrar) { + this.declareCalls++; + this.declare.accept(root, registrar); + } + + @Override + public void initialize(@NotNull Plugin plugin, @NotNull Path dataFolder) { + if (this.initFailure != null) { + throw this.initFailure; + } + this.initialized = true; + } + + @Override + public void shutdown(@NotNull Plugin plugin) { + } + } + + @BeforeEach + void setUp() { + this.plugin = mock(Plugin.class); + this.loader = mock(ModuleLoader.class); + this.commands = mock(Commands.class); + this.source = mock(CommandSourceStack.class); + } + + private ModuleLifecycleManager manager(CommandModule... modules) throws Exception { + List> loaded = new ArrayList<>(); + for (int i = 0; i < modules.length; i++) { + String name = "m" + i; + ModuleManifest manifest = new ModuleManifest(name, "Entry", "author", HOST, false); + loaded.add(new LoadedModule<>(manifest, modules[i], Path.of(name), new URLClassLoader(new URL[0]))); + } + doReturn(loaded).when(this.loader).loadModulesFromDisk(anyString(), any(Logger.class)); + return new ModuleLifecycleManager<>(this.plugin, this.loader, HOST, Logger.getLogger("mcd-test")); + } + + private static LiteralArgumentBuilder ping(List ran) { + return LiteralArgumentBuilder.literal("ping").executes(ctx -> { + ran.add("ping"); + return 1; + }); + } + + /** Runs one commands event and returns a dispatcher holding the host root as "host". */ + private CommandDispatcher declare(ModuleLifecycleManager manager) { + LiteralArgumentBuilder root = LiteralArgumentBuilder.literal("host"); + manager.declareCommands(this.commands, root, this.unavailable); + CommandDispatcher dispatcher = new CommandDispatcher<>(); + dispatcher.getRoot().addChild(root.build()); + return dispatcher; + } + + @Test + @DisplayName("a subcommand runs module code once the module is active") + void subcommandRunsWhenActive() throws Exception { + List ran = new ArrayList<>(); + ModuleLifecycleManager manager = manager(new CommandModule((root, reg) -> root.then(ping(ran)))); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + + assertEquals(1, dispatcher.execute("host ping", this.source)); + assertEquals(List.of("ping"), ran); + assertTrue(this.unavailableFor.isEmpty()); + } + + @Test + @DisplayName("commands are declared before initialize and reply unavailable until then") + void unavailableBeforeInitialize() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> root.then(ping(ran))); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + + assertFalse(module.initialized); + assertEquals(0, dispatcher.execute("host ping", this.source)); + assertTrue(ran.isEmpty()); + assertEquals(List.of("m0"), this.unavailableFor); + } + + @Test + @DisplayName("a module that fails to initialize keeps its commands, which reply unavailable") + void failedInitializeIsUnavailable() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> root.then(ping(ran))); + module.initFailure = new RuntimeException("boom"); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + + assertEquals(0, dispatcher.execute("host ping", this.source)); + assertTrue(ran.isEmpty()); + assertEquals(List.of("m0"), this.unavailableFor); + } + + @Test + @DisplayName("commands reply unavailable after stop()") + void unavailableAfterStop() throws Exception { + List ran = new ArrayList<>(); + ModuleLifecycleManager manager = manager(new CommandModule((root, reg) -> root.then(ping(ran)))); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + manager.stop(); + + assertEquals(0, dispatcher.execute("host ping", this.source)); + assertTrue(ran.isEmpty()); + } + + @Test + @DisplayName("nested executors are guarded too") + void nestedExecutorsGuarded() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> root.then( + LiteralArgumentBuilder.literal("home").then( + RequiredArgumentBuilder.argument("name", StringArgumentType.word()) + .executes(ctx -> { + ran.add(StringArgumentType.getString(ctx, "name")); + return 1; + })))); + module.initFailure = new RuntimeException("boom"); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + + assertEquals(0, dispatcher.execute("host home base", this.source)); + assertTrue(ran.isEmpty()); + } + + @Test + @DisplayName("suggestions come from the module only while it is active") + void suggestionsGuarded() throws Exception { + CommandModule module = new CommandModule((root, reg) -> root.then( + LiteralArgumentBuilder.literal("home").then( + RequiredArgumentBuilder.argument("name", StringArgumentType.word()) + .suggests((ctx, builder) -> builder.suggest("base").buildFuture()) + .executes(ctx -> 1)))); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + + Suggestions before = dispatcher.getCompletionSuggestions(dispatcher.parse("host home ", this.source)).get(); + manager.initializeLoaded(); + Suggestions after = dispatcher.getCompletionSuggestions(dispatcher.parse("host home ", this.source)).get(); + + assertTrue(before.isEmpty()); + assertEquals("base", after.getList().getFirst().getText()); + } + + @Test + @DisplayName("top-level commands go to Paper's registrar guarded") + @SuppressWarnings("unchecked") + void topLevelRegisteredGuarded() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> reg.register(ping(ran), "Pings", List.of("p"))); + module.initFailure = new RuntimeException("boom"); + ModuleLifecycleManager manager = manager(module); + manager.load(); + declare(manager); + manager.initializeLoaded(); + + ArgumentCaptor> node = ArgumentCaptor.forClass(LiteralCommandNode.class); + verify(this.commands).register(node.capture(), eq("Pings"), eq(List.of("p"))); + CommandDispatcher dispatcher = new CommandDispatcher<>(); + dispatcher.getRoot().addChild(node.getValue()); + + assertEquals(0, dispatcher.execute("ping", this.source)); + assertTrue(ran.isEmpty()); + } + + @Test + @DisplayName("a module whose declareCommands throws registers nothing and is not initialized") + void declareFailureUnloads() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> { + root.then(ping(ran)); + throw new IllegalStateException("bad declaration"); + }); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + + assertFalse(module.initialized); + assertTrue(manager.getActiveModules().isEmpty()); + assertTrue(dispatcher.getRoot().getChild("host").getChildren().isEmpty()); + } + + @Test + @DisplayName("a redirect in a module command is rejected as a failed declaration") + void redirectRejected() throws Exception { + CommandModule module = new CommandModule((root, reg) -> { + LiteralCommandNode target = LiteralArgumentBuilder.literal("t").build(); + root.then(LiteralArgumentBuilder.literal("alias").redirect(target)); + }); + ModuleLifecycleManager manager = manager(module); + manager.load(); + declare(manager); + manager.initializeLoaded(); + + assertFalse(module.initialized); + } + + @Test + @DisplayName("a later commands event re-registers the same commands without asking the module again") + @SuppressWarnings("unchecked") + void laterEventReplays() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> { + root.then(ping(ran)); + reg.register(ping(ran), null); + }); + ModuleLifecycleManager manager = manager(module); + manager.load(); + declare(manager); + manager.initializeLoaded(); + CommandDispatcher second = declare(manager); + + assertEquals(1, module.declareCalls); + assertEquals(1, second.execute("host ping", this.source)); + verify(this.commands, times(2)).register(any(LiteralCommandNode.class), eq(null), any(Collection.class)); + } +} From d691f9fcd10d17cdd1468dc045e5b69cdfbde974 Mon Sep 17 00:00:00 2001 From: Andrew Wong <42793301+md5sha256@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:01:33 +1100 Subject: [PATCH 3/3] refactor(modules): make declareCommands a default method on PluginModule Every module can declare commands by overriding PluginModule#declareCommands, which does nothing by default. Removes the separate CommandDeclaringModule interface. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011DvdDwRoC7qHZ7yKUv1ufw --- AGENTS.md | 11 +++---- README.md | 6 ++-- .../modules/CommandDeclaringModule.java | 29 ------------------- .../modules/ModuleCommandRegistrar.java | 2 +- .../modules/ModuleCommandRoot.java | 2 +- .../modules/ModuleLifecycleManager.java | 6 ++-- .../modules/PluginModule.java | 25 ++++++++++++++-- .../modules/ModuleCommandDeclarationTest.java | 2 +- 8 files changed, 38 insertions(+), 45 deletions(-) delete mode 100644 src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandDeclaringModule.java diff --git a/AGENTS.md b/AGENTS.md index 1d98c9d..9938de3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,11 +60,12 @@ outcome that cannot occur. on `PluginModule#initialize`). `SimplePluginModule` tracks listeners but unregisters nothing by itself. Changing this is a contract change for every module, not a bug fix. - **Module commands are declared, never unregistered.** Paper only accepts commands during its - lifecycle commands event, before modules initialize, and its API cannot remove them. A - `CommandDeclaringModule` declares through `ModuleLifecycleManager#declareCommands` between `load()` - and `initializeLoaded()`; `CommandGuard` rebuilds each tree so executors and suggestions run only - while the module is active, and the host's `ModuleUnavailableHandler` replies otherwise. Redirects - are rejected because they would bypass the guard. A module never cleans up its commands. + lifecycle commands event, before modules initialize, and its API cannot remove them. A module + overrides `PluginModule#declareCommands` (a default no-op), which + `ModuleLifecycleManager#declareCommands` calls between `load()` and `initializeLoaded()`. + `CommandGuard` rebuilds each tree so executors and suggestions run only while the module is + active, and the host's `ModuleUnavailableHandler` replies otherwise. Redirects are rejected + because they would bypass the guard. A module never cleans up its commands. - **`ModuleLifecycleManager` is not thread-safe.** Its state must be touched from one thread (the server main thread on Paper). - **`SchemaMigrator` is abstract and dialect-agnostic.** It provides no SQL of its own — subclasses diff --git a/README.md b/README.md index ed19006..b316a99 100644 --- a/README.md +++ b/README.md @@ -52,9 +52,9 @@ by a `ModuleManifest` and loaded at runtime by its own `URLClassLoader`. `reloadAsync(moduleName)`, `getActiveModules()`. `start()` is `load()` then `initializeLoaded()`; call the two separately when the host must do something with the loaded modules first, such as letting them declare commands while the server still accepts registrations. -- `CommandDeclaringModule` — implemented by a module that adds Brigadier commands. It declares them - through a `ModuleCommandRoot` (subcommands under the host's root) and a `ModuleCommandRegistrar` - (top-level commands), both of which guard every tree. +- `PluginModule#declareCommands(ModuleCommandRoot, ModuleCommandRegistrar)` — a default no-op that a + module overrides to add Brigadier commands: subcommands under the host's root, or top-level + commands. Both parameters guard every tree. > **Commands:** Paper only accepts commands during `LifecycleEvents.COMMANDS`, before modules > initialize, and cannot unregister them. Call `declareCommands(commands, root, unavailable)` from that diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandDeclaringModule.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandDeclaringModule.java deleted file mode 100644 index 0c9de57..0000000 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandDeclaringModule.java +++ /dev/null @@ -1,29 +0,0 @@ -package com.minecraftcitiesnetwork.pluginInfrastructure.modules; - -import org.jetbrains.annotations.NotNull; - -/** - * Implemented by a {@link PluginModule} that contributes Brigadier commands. - * - *

Paper only accepts commands during its lifecycle commands event, which fires before modules - * initialize. The {@link ModuleLifecycleManager} therefore asks each loaded module to declare its - * commands from {@link ModuleLifecycleManager#declareCommands}, between {@code load()} and - * {@code initializeLoaded()}. Every command declared here is guarded: while the module is not - * active (it failed to initialize, or has been shut down) the command does not run module code and - * the host's unavailable handler replies instead. Commands are never unregistered. - */ -public interface CommandDeclaringModule { - - /** - * Declares this module's commands. Called before {@link PluginModule#initialize}, so it must - * only build command trees: no IO, listeners or tasks, and no state that {@code initialize} - * sets up. Executors and suggestion providers may use that state, because they only run while - * the module is active. Requirements ({@code requires}) run regardless and must not depend on it. - * - *

If this method throws, nothing it declared is registered and the module is unloaded. - * - * @param root subcommands under the host's root command - * @param registrar top-level commands - */ - void declareCommands(@NotNull ModuleCommandRoot root, @NotNull ModuleCommandRegistrar registrar); -} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java index c5dd141..1e85111 100644 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java @@ -9,7 +9,7 @@ import java.util.List; /** - * Registers top-level commands for a {@link CommandDeclaringModule}. Each tree is guarded by the + * Registers top-level commands for {@link PluginModule#declareCommands}. Each tree is guarded by the * module's availability before it reaches Paper's registrar, so a module cannot register an * unguarded command. */ diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java index 8121c84..b746f21 100644 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java @@ -4,7 +4,7 @@ import io.papermc.paper.command.brigadier.CommandSourceStack; import org.jetbrains.annotations.NotNull; -/** The host's root command, as offered to a {@link CommandDeclaringModule}. */ +/** The host's root command, as offered to {@link PluginModule#declareCommands}. */ public interface ModuleCommandRoot { /** diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java index ad9b252..37ba204 100644 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java @@ -148,7 +148,7 @@ public void initializeLoaded() { * Registers module commands. Call it from Paper's {@code LifecycleEvents.COMMANDS} handler, * after {@link #load()} and before {@link #initializeLoaded()}, every time the event fires. * - *

Each loaded {@link CommandDeclaringModule} that has not declared yet is asked to declare. + *

Each loaded module that has not declared yet is asked to, via {@link PluginModule#declareCommands}. * A module whose {@code declareCommands} throws is logged, unloaded and not initialized; nothing * it declared is registered. Then every module's guarded commands are added: subcommands to * {@code root}, top-level commands to {@code commands}. A guarded command runs the module's code @@ -170,13 +170,13 @@ public void declareCommands( LoadedModule loaded = modules.next(); String name = loaded.manifest().moduleName(); // A second copy with the same name is dropped by initializeLoaded and declares nothing. - if (!(loaded.module() instanceof CommandDeclaringModule declaring) || !this.declaredThisLoad.add(name)) { + if (!this.declaredThisLoad.add(name)) { continue; } BooleanSupplier active = () -> this.activeNames.contains(name); DeclaredCommands declared = new DeclaredCommands(new ArrayList<>(), new ArrayList<>()); try { - declaring.declareCommands( + loaded.module().declareCommands( sub -> declared.subcommands().add(CommandGuard.guard(sub, name, active, unavailable)), (command, description, aliases) -> declared.topLevel().add(new TopLevelCommand( CommandGuard.guard(command, name, active, unavailable), description, diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java index f173236..6bba5ce 100644 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java @@ -10,7 +10,7 @@ public interface PluginModule { /** * Starts this module: reads its configuration and registers its listeners and tasks with the - * host. Commands are not registered here; see {@link CommandDeclaringModule}. + * host. Commands are not registered here; see {@link #declareCommands}. * *

Failure is the module's to clean up. If this method throws, the * {@link ModuleLifecycleManager} logs the failure and closes the module's class loader, but it @@ -18,7 +18,7 @@ public interface PluginModule { * a listener keeps firing with classes from a closed class loader, a task keeps running. An * implementation must therefore either do all fallible work (reading config, resolving * services) before registering anything, or undo what it registered before rethrowing. - * Commands declared through {@link CommandDeclaringModule} are the exception: they stay + * Commands declared through {@link #declareCommands} are the exception: they stay * registered and reply as unavailable, so they need no cleanup. * * @param plugin the host plugin @@ -28,6 +28,27 @@ public interface PluginModule { */ void initialize(@NotNull T plugin, @NotNull Path dataFolder) throws ModuleInitializationException; + /** + * Declares this module's Brigadier commands. Override it to add commands; the default declares + * none. + * + *

Paper only accepts commands during its lifecycle commands event, which fires before modules + * initialize, so the {@link ModuleLifecycleManager} calls this from + * {@link ModuleLifecycleManager#declareCommands}, between {@code load()} and + * {@code initializeLoaded()}, at most once per load. It must only build command trees: no IO, + * listeners or tasks, and no state that {@link #initialize} sets up. Executors and suggestion + * providers may use that state, because they only run while the module is active; otherwise + * the host's unavailable handler replies. Requirements ({@code requires}) run regardless and must + * not depend on it. Commands are never unregistered, so they need no cleanup. + * + *

If this method throws, nothing it declared is registered and the module is unloaded. + * + * @param root subcommands under the host's root command + * @param registrar top-level commands + */ + default void declareCommands(@NotNull ModuleCommandRoot root, @NotNull ModuleCommandRegistrar registrar) { + } + /** * Stops this module and releases everything {@link #initialize} registered. * diff --git a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java index 1bab3f4..d3b0025 100644 --- a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java +++ b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java @@ -47,7 +47,7 @@ class ModuleCommandDeclarationTest { private final ModuleUnavailableHandler unavailable = (src, name) -> this.unavailableFor.add(name); /** A module that declares commands through a callback and can fail to initialize. */ - private static final class CommandModule implements PluginModule, CommandDeclaringModule { + private static final class CommandModule implements PluginModule { private final BiConsumer declare; private RuntimeException initFailure; int declareCalls;