Skip to content

Commit

Permalink
Deprecate anonymous command registrations
Browse files Browse the repository at this point in the history
  • Loading branch information
astei committed Sep 16, 2024
1 parent 4f227ba commit 2016d14
Show file tree
Hide file tree
Showing 15 changed files with 338 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ public interface CommandManager {
* @throws IllegalArgumentException if one of the given aliases is already registered, or
* the given command does not implement a registrable {@link Command} subinterface
* @see Command for a list of registrable Command subinterfaces
* @deprecated use {@link #register(CommandMeta, Command)} instead with a plugin specified
*/
@Deprecated
default void register(String alias, Command command, String... otherAliases) {
register(metaBuilder(alias).aliases(otherAliases).build(), command);
}
Expand All @@ -55,7 +57,9 @@ default void register(String alias, Command command, String... otherAliases) {
*
* @param command the command to register
* @throws IllegalArgumentException if the node alias is already registered
* @deprecated use {@link #register(CommandMeta, Command)} instead with a plugin specified
*/
@Deprecated
void register(BrigadierCommand command);

/**
Expand Down
55 changes: 49 additions & 6 deletions proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@
import com.google.common.collect.ImmutableList;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.velocitypowered.api.command.BrigadierCommand;
import com.velocitypowered.api.event.proxy.ProxyInitializeEvent;
import com.velocitypowered.api.event.proxy.ProxyReloadEvent;
import com.velocitypowered.api.event.proxy.ProxyShutdownEvent;
import com.velocitypowered.api.network.ProtocolVersion;
import com.velocitypowered.api.plugin.PluginContainer;
import com.velocitypowered.api.plugin.PluginDescription;
import com.velocitypowered.api.plugin.PluginManager;
import com.velocitypowered.api.proxy.Player;
import com.velocitypowered.api.proxy.ProxyServer;
Expand All @@ -52,6 +54,9 @@
import com.velocitypowered.proxy.event.VelocityEventManager;
import com.velocitypowered.proxy.network.ConnectionManager;
import com.velocitypowered.proxy.plugin.VelocityPluginManager;
import com.velocitypowered.proxy.plugin.loader.VelocityPluginContainer;
import com.velocitypowered.proxy.plugin.loader.VelocityPluginDescription;
import com.velocitypowered.proxy.plugin.virtual.VelocityVirtualPlugin;
import com.velocitypowered.proxy.protocol.ProtocolUtils;
import com.velocitypowered.proxy.protocol.util.FaviconSerializer;
import com.velocitypowered.proxy.protocol.util.GameProfileSerializer;
Expand All @@ -77,6 +82,7 @@
import java.security.KeyPair;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -111,6 +117,8 @@
*/
public class VelocityServer implements ProxyServer, ForwardingAudience {

public static final String VELOCITY_URL = "https://velocitypowered.com";

private static final Logger logger = LogManager.getLogger(VelocityServer.class);
public static final Gson GENERAL_GSON = new GsonBuilder()
.registerTypeHierarchyAdapter(Favicon.class, FaviconSerializer.INSTANCE)
Expand Down Expand Up @@ -163,7 +171,7 @@ public class VelocityServer implements ProxyServer, ForwardingAudience {
VelocityServer(final ProxyOptions options) {
pluginManager = new VelocityPluginManager(this);
eventManager = new VelocityEventManager(pluginManager);
commandManager = new VelocityCommandManager(eventManager);
commandManager = new VelocityCommandManager(eventManager, pluginManager);
scheduler = new VelocityScheduler(pluginManager);
console = new VelocityConsole(this);
cm = new ConnectionManager(this);
Expand Down Expand Up @@ -200,6 +208,16 @@ public ProxyVersion getVersion() {
return new ProxyVersion(implName, implVendor, implVersion);
}

private VelocityPluginContainer createVirtualPlugin() {
ProxyVersion version = getVersion();
PluginDescription description = new VelocityPluginDescription(
"velocity", version.getName(), version.getVersion(), "The Velocity proxy",
VELOCITY_URL, ImmutableList.of(version.getVendor()), Collections.emptyList(), null);
VelocityPluginContainer container = new VelocityPluginContainer(description);
container.setInstance(VelocityVirtualPlugin.INSTANCE);
return container;
}

@Override
public VelocityCommandManager getCommandManager() {
return commandManager;
Expand All @@ -214,6 +232,7 @@ void awaitProxyShutdown() {
void start() {
logger.info("Booting up {} {}...", getVersion().getName(), getVersion().getVersion());
console.setupStreams();
pluginManager.registerPlugin(this.createVirtualPlugin());

registerTranslations();

Expand All @@ -222,11 +241,35 @@ void start() {
cm.logChannelInformation();

// Initialize commands first
commandManager.register(VelocityCommand.create(this));
commandManager.register(CallbackCommand.create());
commandManager.register(ServerCommand.create(this));
commandManager.register("shutdown", ShutdownCommand.command(this),
"end", "stop");
final BrigadierCommand velocityParentCommand = VelocityCommand.create(this);
commandManager.register(
commandManager.metaBuilder(velocityParentCommand)
.plugin(VelocityVirtualPlugin.INSTANCE)
.build(),
velocityParentCommand
);
final BrigadierCommand callbackCommand = CallbackCommand.create();
commandManager.register(
commandManager.metaBuilder(callbackCommand)
.plugin(VelocityVirtualPlugin.INSTANCE)
.build(),
velocityParentCommand
);
final BrigadierCommand serverCommand = ServerCommand.create(this);
commandManager.register(
commandManager.metaBuilder(serverCommand)
.plugin(VelocityVirtualPlugin.INSTANCE)
.build(),
serverCommand
);
final BrigadierCommand shutdownCommand = ShutdownCommand.command(this);
commandManager.register(
commandManager.metaBuilder(shutdownCommand)
.plugin(VelocityVirtualPlugin.INSTANCE)
.aliases("end", "stop")
.build(),
shutdownCommand
);
new GlistCommand(this).register();
new SendCommand(this).register();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.util.concurrent.MoreExecutors;
import com.mojang.brigadier.CommandDispatcher;
import com.mojang.brigadier.Message;
import com.mojang.brigadier.ParseResults;
Expand All @@ -37,20 +38,23 @@
import com.velocitypowered.api.command.VelocityBrigadierMessage;
import com.velocitypowered.api.event.command.CommandExecuteEvent;
import com.velocitypowered.api.event.command.PostCommandInvocationEvent;
import com.velocitypowered.api.plugin.PluginManager;
import com.velocitypowered.proxy.command.brigadier.VelocityBrigadierCommandWrapper;
import com.velocitypowered.proxy.command.registrar.BrigadierCommandRegistrar;
import com.velocitypowered.proxy.command.registrar.CommandRegistrar;
import com.velocitypowered.proxy.command.registrar.RawCommandRegistrar;
import com.velocitypowered.proxy.command.registrar.SimpleCommandRegistrar;
import com.velocitypowered.proxy.event.VelocityEventManager;
import com.velocitypowered.proxy.plugin.virtual.VelocityVirtualPlugin;
import io.netty.util.concurrent.FastThreadLocalThread;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.Executor;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.stream.Collectors;
Expand All @@ -73,14 +77,16 @@ public class VelocityCommandManager implements CommandManager {
private final SuggestionsProvider<CommandSource> suggestionsProvider;
private final CommandGraphInjector<CommandSource> injector;
private final Map<String, CommandMeta> commandMetas;
private final ExecutorService asyncExecutor;
private final PluginManager pluginManager;

/**
* Constructs a command manager.
*
* @param eventManager the event manager
*/
public VelocityCommandManager(final VelocityEventManager eventManager) {
public VelocityCommandManager(final VelocityEventManager eventManager,
PluginManager pluginManager) {
this.pluginManager = pluginManager;
this.lock = new ReentrantReadWriteLock();
this.dispatcher = new CommandDispatcher<>();
this.eventManager = Preconditions.checkNotNull(eventManager);
Expand All @@ -92,7 +98,6 @@ public VelocityCommandManager(final VelocityEventManager eventManager) {
this.suggestionsProvider = new SuggestionsProvider<>(this.dispatcher, this.lock.readLock());
this.injector = new CommandGraphInjector<>(this.dispatcher, this.lock.readLock());
this.commandMetas = new ConcurrentHashMap<>();
this.asyncExecutor = ForkJoinPool.commonPool(); // TODO: remove entirely
}

public void setAnnounceProxyCommands(boolean announceProxyCommands) {
Expand Down Expand Up @@ -222,16 +227,13 @@ public CompletableFuture<CommandExecuteEvent> callCommandEvent(final CommandSour
return eventManager.fire(new CommandExecuteEvent(source, cmdLine));
}

private boolean executeImmediately0(final CommandSource source, final String cmdLine) {
private boolean executeImmediately0(final CommandSource source, final ParseResults<CommandSource> parsed) {
Preconditions.checkNotNull(source, "source");
Preconditions.checkNotNull(cmdLine, "cmdLine");

final String normalizedInput = VelocityCommands.normalizeInput(cmdLine, true);
CommandResult result = CommandResult.EXCEPTION;
try {
// The parse can fail if the requirement predicates throw
final ParseResults<CommandSource> parse = this.parse(normalizedInput, source);
boolean executed = dispatcher.execute(parse) != BrigadierCommand.FORWARD;
boolean executed = dispatcher.execute(parsed) != BrigadierCommand.FORWARD;
result = executed ? CommandResult.EXECUTED : CommandResult.FORWARDED;
return executed;
} catch (final CommandSyntaxException e) {
Expand All @@ -253,9 +255,9 @@ private boolean executeImmediately0(final CommandSource source, final String cmd
}
} catch (final Throwable e) {
// Ugly, ugly swallowing of everything Throwable, because plugins are naughty.
throw new RuntimeException("Unable to invoke command " + cmdLine + " for " + source, e);
throw new RuntimeException("Unable to invoke command " + parsed.getReader().getString() + "for " + source, e);
} finally {
eventManager.fireAndForget(new PostCommandInvocationEvent(source, cmdLine, result));
eventManager.fireAndForget(new PostCommandInvocationEvent(source, parsed.getReader().getString(), result));
}
}

Expand All @@ -264,13 +266,17 @@ public CompletableFuture<Boolean> executeAsync(final CommandSource source, final
Preconditions.checkNotNull(source, "source");
Preconditions.checkNotNull(cmdLine, "cmdLine");

return callCommandEvent(source, cmdLine).thenApplyAsync(event -> {
return callCommandEvent(source, cmdLine).thenComposeAsync(event -> {
CommandExecuteEvent.CommandResult commandResult = event.getResult();
if (commandResult.isForwardToServer() || !commandResult.isAllowed()) {
return false;
return CompletableFuture.completedFuture(false);
}
return executeImmediately0(source, commandResult.getCommand().orElse(event.getCommand()));
}, asyncExecutor);
final ParseResults<CommandSource> parsed = this.parse(
commandResult.getCommand().orElse(cmdLine), source);
return CompletableFuture.supplyAsync(
() -> executeImmediately0(source, parsed), this.getAsyncExecutor(parsed)
);
}, figureAsyncExecutorForParsing());
}

@Override
Expand All @@ -279,7 +285,13 @@ public CompletableFuture<Boolean> executeImmediatelyAsync(
Preconditions.checkNotNull(source, "source");
Preconditions.checkNotNull(cmdLine, "cmdLine");

return CompletableFuture.supplyAsync(() -> executeImmediately0(source, cmdLine), asyncExecutor);
return CompletableFuture.supplyAsync(
() -> this.parse(cmdLine, source), figureAsyncExecutorForParsing()
).thenCompose(
parsed -> CompletableFuture.supplyAsync(
() -> executeImmediately0(source, parsed), this.getAsyncExecutor(parsed)
)
);
}

/**
Expand Down Expand Up @@ -327,9 +339,10 @@ public CompletableFuture<Suggestions> offerBrigadierSuggestions(
* @return the parse results
*/
private ParseResults<CommandSource> parse(final String input, final CommandSource source) {
final String normalizedInput = VelocityCommands.normalizeInput(input, true);
lock.readLock().lock();
try {
return dispatcher.parse(input, source);
return dispatcher.parse(normalizedInput, source);
} finally {
lock.readLock().unlock();
}
Expand Down Expand Up @@ -373,4 +386,25 @@ RootCommandNode<CommandSource> getRoot() {
public CommandGraphInjector<CommandSource> getInjector() {
return injector;
}

private Executor getAsyncExecutor(ParseResults<CommandSource> parse) {
Object registrant;
if (parse.getContext().getCommand() instanceof VelocityBrigadierCommandWrapper vbcw) {
registrant = vbcw.registrant() == null ? VelocityVirtualPlugin.INSTANCE : vbcw.registrant();
} else {
registrant = VelocityVirtualPlugin.INSTANCE;
}
return pluginManager.ensurePluginContainer(registrant).getExecutorService();
}

private Executor figureAsyncExecutorForParsing() {
final Thread thread = Thread.currentThread();
if (thread instanceof FastThreadLocalThread) {
// we *never* want to block the Netty event loop, so use the async executor
return pluginManager.ensurePluginContainer(VelocityVirtualPlugin.INSTANCE).getExecutorService();
} else {
// it's some other thread that isn't a Netty event loop thread. direct execution it is!
return MoreExecutors.directExecutor();
}
}
}
Loading

0 comments on commit 2016d14

Please sign in to comment.