Give CommandManager a registration state (#148)

* 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 <sauilitired@gmail.com>
This commit is contained in:
zml 2020-11-29 06:29:41 -08:00 committed by Alexander Söderberg
parent 65684d0036
commit 013d2d61f4
11 changed files with 273 additions and 155 deletions

View file

@ -182,9 +182,9 @@ public class BukkitCommandManager<C> extends CommandManager<C> 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<C> extends CommandManager<C> implements Brigad
}
protected final void setSplitAliases(final boolean value) {
this.requireState(RegistrationState.BEFORE_REGISTRATION);
this.splitAliases = value;
}
@ -311,6 +312,7 @@ public class BukkitCommandManager<C> extends CommandManager<C> implements Brigad
* supported by the platform
*/
public void registerBrigadier() throws BrigadierFailureException {
this.requireState(RegistrationState.BEFORE_REGISTRATION);
this.checkBrigadierCompatibility();
try {
final CloudCommodoreManager<C> cloudCommodoreManager = new CloudCommodoreManager<>(this);
@ -369,6 +371,12 @@ public class BukkitCommandManager<C> extends CommandManager<C> 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
*/

View file

@ -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<C> implements Listener {
final class CloudBukkitListener<C> implements Listener {
private final BukkitCommandManager<C> bukkitCommandManager;
CommandSuggestionsListener(final @NonNull BukkitCommandManager<C> bukkitCommandManager) {
CloudBukkitListener(final @NonNull BukkitCommandManager<C> bukkitCommandManager) {
this.bukkitCommandManager = bukkitCommandManager;
}
@ -67,4 +69,13 @@ final class CommandSuggestionsListener<C> 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();
}
}

View file

@ -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<C> extends CommandManager<C> {
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<C> extends CommandManager<C> {
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<C> extends CommandManager<C> {
return this.owningPlugin;
}
static final class CloudListener implements Listener {
static final CloudListener INSTANCE = new CloudListener();
private CloudListener() {
}
}
}

View file

@ -95,6 +95,7 @@ public class PaperCommandManager<C> extends BukkitCommandManager<C> {
*/
@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<C> extends BukkitCommandManager<C> {
* @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.");
}

View file

@ -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<C> extends CommandManager<C> implements Brig
private final ProxyServer proxyServer;
private final Function<CommandSource, C> commandSenderMapper;
private final Function<C, CommandSource> 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<C>, @NonNull CommandExecutionCoordinator<C>> 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<C> extends CommandManager<C> implements Brig
@Inject
@SuppressWarnings("unchecked")
public VelocityCommandManager(
final @Nullable PluginContainer plugin,
final @NonNull ProxyServer proxyServer,
final @NonNull Function<@NonNull CommandTree<C>, @NonNull CommandExecutionCoordinator<C>> commandExecutionCoordinator,
final @NonNull Function<@NonNull CommandSource, @NonNull C> commandSenderMapper,
@ -114,6 +136,10 @@ public class VelocityCommandManager<C> extends CommandManager<C> implements Brig
(context, key) -> ARGUMENT_PARSE_FAILURE_SERVER
);
}
this.proxyServer.getEventManager().register(plugin, ServerPreConnectEvent.class, ev -> {
this.transitionOrThrow(RegistrationState.REGISTERING, RegistrationState.AFTER_REGISTRATION);
});
}
@Override