From 013d2d61f4331ca44bd1896b83b54f0ccbb732fd Mon Sep 17 00:00:00 2001 From: zml Date: Sun, 29 Nov 2020 06:29:41 -0800 Subject: [PATCH] :sparkles: Give CommandManager a registration state (#148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Make CommandManager track its availability for registration This prevents situations where changes to the manager would result in undefined state in other places. * Add unsafe registration capability * Very minor formatting + `@since` tags * Add changes to changelog Co-authored-by: Alexander Söderberg --- CHANGELOG.md | 2 +- .../commandframework/CommandManager.java | 114 +++++++++++++++++- .../LockableCommandManager.java | 53 +------- .../CommandRegistrationStateTest.java | 77 ++++++++++++ .../LockableCommandManagerTest.java | 45 ------- .../TestLockableCommandManager.java | 54 --------- .../bukkit/BukkitCommandManager.java | 12 +- ...Listener.java => CloudBukkitListener.java} | 15 ++- .../cloudburst/CloudburstCommandManager.java | 28 +++++ .../paper/PaperCommandManager.java | 2 + .../velocity/VelocityCommandManager.java | 26 ++++ 11 files changed, 273 insertions(+), 155 deletions(-) create mode 100644 cloud-core/src/test/java/cloud/commandframework/CommandRegistrationStateTest.java delete mode 100644 cloud-core/src/test/java/cloud/commandframework/LockableCommandManagerTest.java delete mode 100644 cloud-core/src/test/java/cloud/commandframework/TestLockableCommandManager.java rename cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/{CommandSuggestionsListener.java => CloudBukkitListener.java} (78%) diff --git a/CHANGELOG.md b/CHANGELOG.md index c58396bb..457e2d5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added TextColorArgument to minecraft-extras - Added LocationArgument to cloud-bukkit - Added ServerArgument to cloud-velocity - - Added LockableCommandManager to cloud-core - Added VelocityCommandPreprocessor to cloud-velocity - Added PlayerArgument to cloud-bungee - Added ServerArgument to cloud-bungee @@ -44,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added BungeeCommandPreprocessor to cloud-bungee - Added named suggestion providers - Added a PircBotX implementation + - Added registration state to command managers ### Changed - Allow for combined presence flags, such that `-a -b -c` is equivalent to `-abc` diff --git a/cloud-core/src/main/java/cloud/commandframework/CommandManager.java b/cloud-core/src/main/java/cloud/commandframework/CommandManager.java index 019315b4..b163d2f8 100644 --- a/cloud-core/src/main/java/cloud/commandframework/CommandManager.java +++ b/cloud-core/src/main/java/cloud/commandframework/CommandManager.java @@ -69,6 +69,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; import java.util.function.Function; @@ -95,6 +96,7 @@ public abstract class CommandManager { private CommandSuggestionProcessor commandSuggestionProcessor = new FilteringCommandSuggestionProcessor<>(); private CommandRegistrationHandler commandRegistrationHandler; private CaptionRegistry captionRegistry; + private final AtomicReference state = new AtomicReference<>(RegistrationState.BEFORE_REGISTRATION); /** * Create a new command manager instance @@ -207,6 +209,11 @@ public abstract class CommandManager { * return {@code this}. */ public @NonNull CommandManager command(final @NonNull Command command) { + if (!(this.transitionIfPossible(RegistrationState.BEFORE_REGISTRATION, RegistrationState.REGISTERING) + || this.isCommandRegistrationAllowed())) { + throw new IllegalStateException("Unable to register commands because the manager is no longer in a registration " + + "state. Your platform may allow unsafe registrations by enabling the appropriate manager setting."); + } this.commandTree.insertCommand(command); this.commands.add(command); return this; @@ -250,6 +257,7 @@ public abstract class CommandManager { } protected final void setCommandRegistrationHandler(final @NonNull CommandRegistrationHandler commandRegistrationHandler) { + this.requireState(RegistrationState.BEFORE_REGISTRATION); this.commandRegistrationHandler = commandRegistrationHandler; } @@ -767,6 +775,71 @@ public abstract class CommandManager { } } + /** + * Transition from the {@code in} state to the {@code out} state, if the manager is not already in that state. + * + * @param in The starting state + * @param out The ending state + * @throws IllegalStateException if the manager is in any state but {@code in} or {@code out} + * @since 1.2.0 + */ + protected final void transitionOrThrow(final @NonNull RegistrationState in, final @NonNull RegistrationState out) { + if (!this.transitionIfPossible(in, out)) { + throw new IllegalStateException("Command manager was in state " + this.state.get() + ", while we were expecting a state " + + "of " + in + " or " + out + "!"); + } + } + + /** + * Transition from the {@code in} state to the {@code out} state, if the manager is not already in that state. + * + * @param in The starting state + * @param out The ending state + * @return {@code true} if the state transition was successful, or the manager was already in the desired state + * @since 1.2.0 + */ + protected final boolean transitionIfPossible(final @NonNull RegistrationState in, final @NonNull RegistrationState out) { + return this.state.compareAndSet(in, out) || this.state.get() == out; + } + + /** + * Require that the commands manager is in a certain state. + * + * @param expected The required state + * @throws IllegalStateException if the manager is not in the expected state + * @since 1.2.0 + */ + protected final void requireState(final @NonNull RegistrationState expected) { + if (this.state.get() != expected) { + throw new IllegalStateException("This operation required the commands manager to be in state " + expected + ", but it " + + "was in " + this.state.get() + " instead!"); + } + } + + /** + * Get the active registration state for this manager. + *

+ * If this state is {@link RegistrationState#AFTER_REGISTRATION}, commands can no longer be registered + * + * @return The current state + * @since 1.2.0 + */ + public final @NonNull RegistrationState getRegistrationState() { + return this.state.get(); + } + + /** + * Check if command registration is allowed. + *

+ * On platforms where unsafe registration is possible, this can be overridden by enabling the + * {@link ManagerSettings#ALLOW_UNSAFE_REGISTRATION} setting. + * + * @return {@code true} if the registration is allowed, else {@code false} + * @since 1.2.0 + */ + public boolean isCommandRegistrationAllowed() { + return this.getSetting(ManagerSettings.ALLOW_UNSAFE_REGISTRATION) || this.state.get() != RegistrationState.AFTER_REGISTRATION; + } /** * Configurable command related settings @@ -785,7 +858,46 @@ public abstract class CommandManager { * Force sending of an empty suggestion (i.e. a singleton list containing an empty string) * when no suggestions are present */ - FORCE_SUGGESTION + FORCE_SUGGESTION, + + /** + * Allow registering commands even when doing so has the potential to produce inconsistent results. + *

+ * For example, if a platform serializes the command tree and sends it to clients, + * this will allow modifying the command tree after it has been sent, as long as these modifications are not blocked by + * the underlying platform + */ + ALLOW_UNSAFE_REGISTRATION + } + + /** + * The point in the registration lifecycle for this commands manager + * + * @since 1.2.0 + */ + public enum RegistrationState { + /** + * The point when no commands have been registered yet. + * + *

At this point, all configuration options can be changed.

+ */ + BEFORE_REGISTRATION, + + /** + * When at least one command has been registered, and more commands have been registered. + * + *

In this state, some options that affect how commands are registered with the platform are frozen. Some platforms + * will remain in this state for their lifetime.

+ */ + REGISTERING, + + /** + * Once registration has been completed. + * + *

At this point, the command manager is effectively immutable. On platforms where command registration happens via + * callback, this state is achieved the first time the manager's callback is executed for registration.

+ */ + AFTER_REGISTRATION } } diff --git a/cloud-core/src/main/java/cloud/commandframework/LockableCommandManager.java b/cloud-core/src/main/java/cloud/commandframework/LockableCommandManager.java index b10aa3d7..a0219d42 100644 --- a/cloud-core/src/main/java/cloud/commandframework/LockableCommandManager.java +++ b/cloud-core/src/main/java/cloud/commandframework/LockableCommandManager.java @@ -40,12 +40,11 @@ import java.util.function.Function; * * @param Command sender type * @since 1.1.0 + * @deprecated Use a normal {@link CommandManager}'s registration state instead */ +@Deprecated public abstract class LockableCommandManager extends CommandManager { - private final Object writeLock = new Object(); - private volatile boolean writeLocked = false; - /** * Create a new command manager instance * @@ -67,57 +66,11 @@ public abstract class LockableCommandManager extends CommandManager { super(commandExecutionCoordinator, commandRegistrationHandler); } - /** - * {@inheritDoc} - *

- * This should only be called when {@link #isCommandRegistrationAllowed()} is {@code true}, - * else {@link IllegalStateException} will be called - * - * @param command Command to register - * @return The command manager instance - */ - @Override - public final @NonNull CommandManager command(final @NonNull Command command) { - synchronized (this.writeLock) { - if (!isCommandRegistrationAllowed()) { - throw new IllegalStateException( - "Command registration is not allowed. The command manager has been locked." - ); - } - return super.command(command); - } - } - - /** - * {@inheritDoc} - *

- * This should only be called when {@link #isCommandRegistrationAllowed()} is {@code true}, - * else {@link IllegalStateException} will be called - * - * @param command Command to register. {@link Command.Builder#build()}} will be invoked. - * @return The command manager instance - */ - @Override - public final @NonNull CommandManager command(final Command.@NonNull Builder command) { - return super.command(command); - } - /** * Lock writing. After this, {@link #isCommandRegistrationAllowed()} will return {@code false} */ protected final void lockWrites() { - synchronized (this.writeLock) { - this.writeLocked = true; - } - } - - /** - * Check if command registration is allowed - * - * @return {@code true} if the registration is allowed, else {@code false} - */ - public final boolean isCommandRegistrationAllowed() { - return !this.writeLocked; + this.transitionOrThrow(RegistrationState.REGISTERING, RegistrationState.AFTER_REGISTRATION); } } diff --git a/cloud-core/src/test/java/cloud/commandframework/CommandRegistrationStateTest.java b/cloud-core/src/test/java/cloud/commandframework/CommandRegistrationStateTest.java new file mode 100644 index 00000000..61398607 --- /dev/null +++ b/cloud-core/src/test/java/cloud/commandframework/CommandRegistrationStateTest.java @@ -0,0 +1,77 @@ +// +// MIT License +// +// Copyright (c) 2020 Alexander Söderberg & Contributors +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. +// + +package cloud.commandframework; + +import cloud.commandframework.internal.CommandRegistrationHandler; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class CommandRegistrationStateTest { + + @Test + void testInitialState() { + final TestCommandManager manager = new TestCommandManager(); + assertEquals(CommandManager.RegistrationState.BEFORE_REGISTRATION, manager.getRegistrationState()); + } + + @Test + void testRegistrationChangesState() { + final TestCommandManager manager = new TestCommandManager(); + manager.command(manager.commandBuilder("test").handler(ctx -> {})); + assertEquals(CommandManager.RegistrationState.REGISTERING, manager.getRegistrationState()); + // And a second registration maintains it + manager.command(manager.commandBuilder("test2").handler(ctx -> {})); + assertEquals(CommandManager.RegistrationState.REGISTERING, manager.getRegistrationState()); + } + + @Test + void testChangingRegistrationHandlerFails() { + final TestCommandManager manager = new TestCommandManager(); + manager.command(manager.commandBuilder("test").handler(ctx -> {})); + assertThrows(IllegalStateException.class, + () -> manager.setCommandRegistrationHandler(CommandRegistrationHandler.nullCommandRegistrationHandler())); + } + + @Test + void testRegistrationFailsInAfterRegistrationState() { + final TestCommandManager manager = new TestCommandManager(); + manager.command(manager.commandBuilder("test").handler(ctx -> {})); + + manager.transitionOrThrow(CommandManager.RegistrationState.REGISTERING, CommandManager.RegistrationState.AFTER_REGISTRATION); + assertThrows(IllegalStateException.class, () -> manager.command(manager.commandBuilder("test2").handler(ctx -> {}))); + } + + @Test + void testAllowUnsafeRegistration() { + final TestCommandManager manager = new TestCommandManager(); + manager.setSetting(CommandManager.ManagerSettings.ALLOW_UNSAFE_REGISTRATION, true); + manager.command(manager.commandBuilder("test").handler(ctx -> {})); + manager.transitionOrThrow(CommandManager.RegistrationState.REGISTERING, CommandManager.RegistrationState.AFTER_REGISTRATION); + manager.command(manager.commandBuilder("unsafe").handler(ctx -> {})); + } + +} diff --git a/cloud-core/src/test/java/cloud/commandframework/LockableCommandManagerTest.java b/cloud-core/src/test/java/cloud/commandframework/LockableCommandManagerTest.java deleted file mode 100644 index ad9496ce..00000000 --- a/cloud-core/src/test/java/cloud/commandframework/LockableCommandManagerTest.java +++ /dev/null @@ -1,45 +0,0 @@ -// -// MIT License -// -// Copyright (c) 2020 Alexander Söderberg & Contributors -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. -// -package cloud.commandframework; - -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; - -public class LockableCommandManagerTest { - - @Test - void testLockableCommandManager() { - final LockableCommandManager manager = new TestLockableCommandManager(); - /* Add a command before locking */ - manager.command(manager.commandBuilder("test1")); - /* Lock */ - manager.lockWrites(); - /* Add a command after locking */ - Assertions.assertThrows( - IllegalStateException.class, - () -> manager.command(manager.commandBuilder("test2")) - ); - } - -} diff --git a/cloud-core/src/test/java/cloud/commandframework/TestLockableCommandManager.java b/cloud-core/src/test/java/cloud/commandframework/TestLockableCommandManager.java deleted file mode 100644 index 6a295230..00000000 --- a/cloud-core/src/test/java/cloud/commandframework/TestLockableCommandManager.java +++ /dev/null @@ -1,54 +0,0 @@ -// -// MIT License -// -// Copyright (c) 2020 Alexander Söderberg & Contributors -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. -// -package cloud.commandframework; - -import cloud.commandframework.execution.CommandExecutionCoordinator; -import cloud.commandframework.internal.CommandRegistrationHandler; -import cloud.commandframework.meta.SimpleCommandMeta; - -public class TestLockableCommandManager extends LockableCommandManager { - - /** - * Construct a new test command manager - */ - public TestLockableCommandManager() { - super( - CommandExecutionCoordinator.simpleCoordinator(), - CommandRegistrationHandler.nullCommandRegistrationHandler() - ); - } - - @Override - public final SimpleCommandMeta createDefaultCommandMeta() { - return SimpleCommandMeta.empty(); - } - - @Override - public final boolean hasPermission(final TestCommandSender sender, final String permission) { - System.out.printf("Testing permission: %s\n", permission); - return !permission.equalsIgnoreCase("no"); - } - -} - diff --git a/cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/BukkitCommandManager.java b/cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/BukkitCommandManager.java index 5e91a475..6fd2f834 100644 --- a/cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/BukkitCommandManager.java +++ b/cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/BukkitCommandManager.java @@ -182,9 +182,9 @@ public class BukkitCommandManager extends CommandManager implements Brigad this.getParserRegistry().registerParserSupplier(TypeToken.get(MultiplePlayerSelector.class), parserParameters -> new MultiplePlayerSelectorArgument.MultiplePlayerSelectorParser<>()); - /* Register suggestion listener */ + /* Register suggestion and state listener */ this.owningPlugin.getServer().getPluginManager().registerEvents( - new CommandSuggestionsListener<>(this), + new CloudBukkitListener<>(this), this.owningPlugin ); @@ -241,6 +241,7 @@ public class BukkitCommandManager extends CommandManager implements Brigad } protected final void setSplitAliases(final boolean value) { + this.requireState(RegistrationState.BEFORE_REGISTRATION); this.splitAliases = value; } @@ -311,6 +312,7 @@ public class BukkitCommandManager extends CommandManager implements Brigad * supported by the platform */ public void registerBrigadier() throws BrigadierFailureException { + this.requireState(RegistrationState.BEFORE_REGISTRATION); this.checkBrigadierCompatibility(); try { final CloudCommodoreManager cloudCommodoreManager = new CloudCommodoreManager<>(this); @@ -369,6 +371,12 @@ public class BukkitCommandManager extends CommandManager implements Brigad return this.backwardsCommandSenderMapper; } + final void lockIfBrigadierCapable() { + if (this.minecraftVersion >= BRIGADIER_MINIMUM_VERSION) { + this.transitionOrThrow(RegistrationState.REGISTERING, RegistrationState.AFTER_REGISTRATION); + } + } + /** * Reasons to explain why Brigadier failed to initialize */ diff --git a/cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/CommandSuggestionsListener.java b/cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/CloudBukkitListener.java similarity index 78% rename from cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/CommandSuggestionsListener.java rename to cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/CloudBukkitListener.java index fe7689a1..5d677110 100644 --- a/cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/CommandSuggestionsListener.java +++ b/cloud-minecraft/cloud-bukkit/src/main/java/cloud/commandframework/bukkit/CloudBukkitListener.java @@ -25,18 +25,20 @@ package cloud.commandframework.bukkit; import org.bukkit.command.CommandSender; import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; import org.bukkit.event.Listener; +import org.bukkit.event.player.PlayerLoginEvent; import org.bukkit.event.server.TabCompleteEvent; import org.checkerframework.checker.nullness.qual.NonNull; import java.util.ArrayList; import java.util.List; -final class CommandSuggestionsListener implements Listener { +final class CloudBukkitListener implements Listener { private final BukkitCommandManager bukkitCommandManager; - CommandSuggestionsListener(final @NonNull BukkitCommandManager bukkitCommandManager) { + CloudBukkitListener(final @NonNull BukkitCommandManager bukkitCommandManager) { this.bukkitCommandManager = bukkitCommandManager; } @@ -67,4 +69,13 @@ final class CommandSuggestionsListener implements Listener { event.setCompletions(suggestions); } + @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) + void onPlayerLogin(final @NonNull PlayerLoginEvent event) { + /* If the server is brigadier-capable, any registration after players + have joined (and been sent a command tree) is unsafe. + Bukkit's PlayerJoinEvent is called just after the command tree is sent, + so we have to perform this state change at PlayerLoginEvent to lock before that happens. */ + this.bukkitCommandManager.lockIfBrigadierCapable(); + } + } diff --git a/cloud-minecraft/cloud-cloudburst/src/main/java/cloud/commandframework/cloudburst/CloudburstCommandManager.java b/cloud-minecraft/cloud-cloudburst/src/main/java/cloud/commandframework/cloudburst/CloudburstCommandManager.java index ee5d071b..1e432d7e 100644 --- a/cloud-minecraft/cloud-cloudburst/src/main/java/cloud/commandframework/cloudburst/CloudburstCommandManager.java +++ b/cloud-minecraft/cloud-cloudburst/src/main/java/cloud/commandframework/cloudburst/CloudburstCommandManager.java @@ -30,6 +30,9 @@ import cloud.commandframework.meta.CommandMeta; import cloud.commandframework.meta.SimpleCommandMeta; import org.checkerframework.checker.nullness.qual.NonNull; import org.cloudburstmc.server.command.CommandSender; +import org.cloudburstmc.server.event.EventPriority; +import org.cloudburstmc.server.event.Listener; +import org.cloudburstmc.server.event.server.RegistriesClosedEvent; import org.cloudburstmc.server.plugin.Plugin; import java.util.function.Function; @@ -67,6 +70,15 @@ public class CloudburstCommandManager extends CommandManager { this.commandSenderMapper = commandSenderMapper; this.backwardsCommandSenderMapper = backwardsCommandSenderMapper; this.owningPlugin = owningPlugin; + + // Prevent commands from being registered when the server would reject them anyways + this.owningPlugin.getServer().getPluginManager().registerEvent( + RegistriesClosedEvent.class, + CloudListener.INSTANCE, + EventPriority.NORMAL, + (listener, event) -> this.lock(), + this.owningPlugin + ); } @Override @@ -77,11 +89,20 @@ public class CloudburstCommandManager extends CommandManager { return this.backwardsCommandSenderMapper.apply(sender).hasPermission(permission); } + final void lock() { + this.transitionOrThrow(RegistrationState.REGISTERING, RegistrationState.AFTER_REGISTRATION); + } + @Override public final @NonNull CommandMeta createDefaultCommandMeta() { return SimpleCommandMeta.builder().build(); } + @Override + public final boolean isCommandRegistrationAllowed() { + return this.getRegistrationState() != RegistrationState.AFTER_REGISTRATION; + } + final @NonNull Function<@NonNull CommandSender, @NonNull C> getCommandSenderMapper() { return this.commandSenderMapper; } @@ -95,4 +116,11 @@ public class CloudburstCommandManager extends CommandManager { return this.owningPlugin; } + static final class CloudListener implements Listener { + static final CloudListener INSTANCE = new CloudListener(); + + private CloudListener() { + } + } + } diff --git a/cloud-minecraft/cloud-paper/src/main/java/cloud/commandframework/paper/PaperCommandManager.java b/cloud-minecraft/cloud-paper/src/main/java/cloud/commandframework/paper/PaperCommandManager.java index bda5c4c3..74797027 100644 --- a/cloud-minecraft/cloud-paper/src/main/java/cloud/commandframework/paper/PaperCommandManager.java +++ b/cloud-minecraft/cloud-paper/src/main/java/cloud/commandframework/paper/PaperCommandManager.java @@ -95,6 +95,7 @@ public class PaperCommandManager extends BukkitCommandManager { */ @Override public void registerBrigadier() throws BrigadierFailureException { + this.requireState(RegistrationState.BEFORE_REGISTRATION); this.checkBrigadierCompatibility(); if (!this.queryCapability(CloudBukkitCapabilities.NATIVE_BRIGADIER)) { super.registerBrigadier(); @@ -133,6 +134,7 @@ public class PaperCommandManager extends BukkitCommandManager { * @see #queryCapability(CloudBukkitCapabilities) Check if the capability is present */ public void registerAsynchronousCompletions() throws IllegalStateException { + this.requireState(RegistrationState.BEFORE_REGISTRATION); if (!this.queryCapability(CloudBukkitCapabilities.ASYNCHRONOUS_COMPLETION)) { throw new IllegalStateException("Failed to register asynchronous command completion listener."); } diff --git a/cloud-minecraft/cloud-velocity/src/main/java/cloud/commandframework/velocity/VelocityCommandManager.java b/cloud-minecraft/cloud-velocity/src/main/java/cloud/commandframework/velocity/VelocityCommandManager.java index 0f2b12ff..a8bb2f06 100644 --- a/cloud-minecraft/cloud-velocity/src/main/java/cloud/commandframework/velocity/VelocityCommandManager.java +++ b/cloud-minecraft/cloud-velocity/src/main/java/cloud/commandframework/velocity/VelocityCommandManager.java @@ -37,11 +37,14 @@ import com.google.inject.Inject; import com.google.inject.Module; import com.google.inject.Singleton; import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.event.player.ServerPreConnectEvent; +import com.velocitypowered.api.plugin.PluginContainer; import com.velocitypowered.api.proxy.Player; import com.velocitypowered.api.proxy.ProxyServer; import com.velocitypowered.api.proxy.server.RegisteredServer; import io.leangen.geantyref.TypeToken; import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.Nullable; import java.util.function.Function; @@ -69,10 +72,28 @@ public class VelocityCommandManager extends CommandManager implements Brig private final ProxyServer proxyServer; private final Function commandSenderMapper; private final Function backwardsCommandSenderMapper; + /** + * Create a new command manager instance. + * + * @param proxyServer ProxyServer instance + * @param commandExecutionCoordinator Coordinator provider + * @param commandSenderMapper Function that maps {@link CommandSource} to the command sender type + * @param backwardsCommandSenderMapper Function that maps the command sender type to {@link CommandSource} + */ + @Deprecated + public VelocityCommandManager( + final @NonNull ProxyServer proxyServer, + final @NonNull Function<@NonNull CommandTree, @NonNull CommandExecutionCoordinator> commandExecutionCoordinator, + final @NonNull Function<@NonNull CommandSource, @NonNull C> commandSenderMapper, + final @NonNull Function<@NonNull C, @NonNull CommandSource> backwardsCommandSenderMapper + ) { + this(null, proxyServer, commandExecutionCoordinator, commandSenderMapper, backwardsCommandSenderMapper); + } /** * Create a new command manager instance * + * @param plugin Container for the owning plugin. Nullable for backwards compatibility * @param proxyServer ProxyServer instance * @param commandExecutionCoordinator Coordinator provider * @param commandSenderMapper Function that maps {@link CommandSource} to the command sender type @@ -81,6 +102,7 @@ public class VelocityCommandManager extends CommandManager implements Brig @Inject @SuppressWarnings("unchecked") public VelocityCommandManager( + final @Nullable PluginContainer plugin, final @NonNull ProxyServer proxyServer, final @NonNull Function<@NonNull CommandTree, @NonNull CommandExecutionCoordinator> commandExecutionCoordinator, final @NonNull Function<@NonNull CommandSource, @NonNull C> commandSenderMapper, @@ -114,6 +136,10 @@ public class VelocityCommandManager extends CommandManager implements Brig (context, key) -> ARGUMENT_PARSE_FAILURE_SERVER ); } + + this.proxyServer.getEventManager().register(plugin, ServerPreConnectEvent.class, ev -> { + this.transitionOrThrow(RegistrationState.REGISTERING, RegistrationState.AFTER_REGISTRATION); + }); } @Override