From 3b6b73f216bd851c9ba16c5feffab41f2852ffe0 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 24 Jun 2021 10:10:34 -0400 Subject: [PATCH] Make announce-player-commands not suggest aliases Fixes #533 --- proxy/build.gradle | 22 ++++++++++---- .../velocitypowered/proxy/VelocityServer.java | 3 ++ .../proxy/command/SuggestionsProvider.java | 16 ++++++++++ .../proxy/command/VelocityCommandManager.java | 4 +++ .../protocol/packet/AvailableCommands.java | 1 - .../proxy/command/CommandTestSuite.java | 12 ++++++++ .../command/SuggestionsProviderTests.java | 29 +++++++++++++++++++ 7 files changed, 81 insertions(+), 6 deletions(-) diff --git a/proxy/build.gradle b/proxy/build.gradle index 1e0b3635f..fc5be0265 100644 --- a/proxy/build.gradle +++ b/proxy/build.gradle @@ -64,7 +64,7 @@ dependencies { runtimeOnly 'org.jline:jline-terminal-jansi:3.16.0' // Needed for JLine runtimeOnly 'com.lmax:disruptor:3.4.2' // Async loggers - implementation 'it.unimi.dsi:fastutil:8.4.1' + implementation 'it.unimi.dsi:fastutil-core:8.5.4' implementation(platform("net.kyori:adventure-bom:${adventureVersion}")) implementation("net.kyori:adventure-nbt") @@ -84,6 +84,7 @@ dependencies { testImplementation "org.junit.jupiter:junit-jupiter-api:${junitVersion}" testImplementation "org.junit.jupiter:junit-jupiter-engine:${junitVersion}" + testImplementation "org.mockito:mockito-core:3.+" } test { @@ -91,25 +92,35 @@ test { } shadowJar { + // Exclude all the collection types we don't intend to use exclude 'it/unimi/dsi/fastutil/booleans/**' exclude 'it/unimi/dsi/fastutil/bytes/**' exclude 'it/unimi/dsi/fastutil/chars/**' exclude 'it/unimi/dsi/fastutil/doubles/**' exclude 'it/unimi/dsi/fastutil/floats/**' + exclude 'it/unimi/dsi/fastutil/longs/**' + exclude 'it/unimi/dsi/fastutil/shorts/**' + + // Exclude the fastutil IO utilities - we don't use them. + exclude 'it/unimi/dsi/fastutil/io/**' + + // Exclude most of the int types - Object2IntMap have a values() method that returns an IntCollection exclude 'it/unimi/dsi/fastutil/ints/*Int2*' exclude 'it/unimi/dsi/fastutil/ints/IntAVL*' exclude 'it/unimi/dsi/fastutil/ints/IntArray*' - exclude 'it/unimi/dsi/fastutil/ints/IntBi*' + exclude 'it/unimi/dsi/fastutil/ints/*IntBi*' + exclude 'it/unimi/dsi/fastutil/ints/Int*Pair' exclude 'it/unimi/dsi/fastutil/ints/IntLinked*' exclude 'it/unimi/dsi/fastutil/ints/IntList*' + exclude 'it/unimi/dsi/fastutil/ints/IntHeap*' exclude 'it/unimi/dsi/fastutil/ints/IntOpen*' exclude 'it/unimi/dsi/fastutil/ints/IntRB*' exclude 'it/unimi/dsi/fastutil/ints/IntSet*' exclude 'it/unimi/dsi/fastutil/ints/IntSorted*' exclude 'it/unimi/dsi/fastutil/ints/*Priority*' exclude 'it/unimi/dsi/fastutil/ints/*BigList*' - exclude 'it/unimi/dsi/fastutil/io/**' - exclude 'it/unimi/dsi/fastutil/longs/**' + + // Try to exclude everything BUT Object2Int{LinkedOpen,Open,CustomOpen}HashMap exclude 'it/unimi/dsi/fastutil/objects/*ObjectArray*' exclude 'it/unimi/dsi/fastutil/objects/*ObjectAVL*' exclude 'it/unimi/dsi/fastutil/objects/*Object*Big*' @@ -127,7 +138,8 @@ shadowJar { exclude 'it/unimi/dsi/fastutil/objects/*Object2Short*' exclude 'it/unimi/dsi/fastutil/objects/*ObjectRB*' exclude 'it/unimi/dsi/fastutil/objects/*Reference*' - exclude 'it/unimi/dsi/fastutil/shorts/**' + + // Exclude Checker Framework annotations exclude 'org/checkerframework/checker/**' relocate 'org.bstats', 'com.velocitypowered.proxy.bstats' diff --git a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java index 34b7d966a..7c644bb15 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java @@ -247,6 +247,8 @@ public class VelocityServer implements ProxyServer, ForwardingAudience { LogManager.shutdown(); System.exit(1); } + + commandManager.setAnnounceProxyCommands(configuration.isAnnounceProxyCommands()); } catch (Exception e) { logger.error("Unable to read/load/save your velocity.toml. The server will shut down.", e); LogManager.shutdown(); @@ -384,6 +386,7 @@ public class VelocityServer implements ProxyServer, ForwardingAudience { newConfiguration.getQueryPort()); } + commandManager.setAnnounceProxyCommands(newConfiguration.isAnnounceProxyCommands()); ipAttemptLimiter = Ratelimiters.createWithMilliseconds(newConfiguration.getLoginRatelimit()); this.configuration = newConfiguration; eventManager.fireAndForget(new ProxyReloadEvent()); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java index 24ef2430a..0639e7cb0 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/SuggestionsProvider.java @@ -33,6 +33,7 @@ import com.spotify.futures.CompletableFutures; import com.velocitypowered.api.command.Command; import com.velocitypowered.api.command.CommandMeta; import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.proxy.Player; import com.velocitypowered.proxy.command.brigadier.VelocityArgumentCommandNode; import java.util.ArrayList; import java.util.Collection; @@ -62,10 +63,12 @@ final class SuggestionsProvider { private final @GuardedBy("lock") CommandDispatcher dispatcher; private final Lock lock; + private boolean announceProxyCommands; SuggestionsProvider(final CommandDispatcher dispatcher, final Lock lock) { this.dispatcher = Preconditions.checkNotNull(dispatcher, "dispatcher"); this.lock = Preconditions.checkNotNull(lock, "lock"); + this.announceProxyCommands = true; } /** @@ -149,6 +152,10 @@ final class SuggestionsProvider { // TODO Is this actually faster? It may incur an allocation final String input = reader.getRead().toLowerCase(Locale.ENGLISH); + if (source instanceof Player && !this.announceProxyCommands) { + return new SuggestionsBuilder(input, 0).buildFuture(); + } + final Collection> aliases = contextSoFar.getRootNode().getChildren(); @SuppressWarnings("unchecked") final CompletableFuture[] futures = new CompletableFuture[aliases.size()]; @@ -368,4 +375,13 @@ final class SuggestionsProvider { return Suggestions.merge(fullInput, suggestions); }); } + + /** + * Sets a flag indicating whether or not alias suggestions shall be returned to the user. + * + * @param announceProxyCommands whether alias suggestions can be returned + */ + public void setAnnounceProxyCommands(boolean announceProxyCommands) { + this.announceProxyCommands = announceProxyCommands; + } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java index b2951590b..0fc0ea8b8 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommandManager.java @@ -77,6 +77,10 @@ public class VelocityCommandManager implements CommandManager { this.injector = new CommandGraphInjector<>(this.dispatcher, this.lock.readLock()); } + public void setAnnounceProxyCommands(boolean announceProxyCommands) { + this.suggestionsProvider.setAnnounceProxyCommands(announceProxyCommands); + } + @Override public CommandMeta.Builder metaBuilder(final String alias) { Preconditions.checkNotNull(alias, "alias"); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java index b40d13f00..50424f5ee 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/AvailableCommands.java @@ -43,7 +43,6 @@ import com.velocitypowered.proxy.protocol.packet.brigadier.ArgumentPropertyRegis import com.velocitypowered.proxy.util.collect.IdentityHashStrategy; import io.netty.buffer.ByteBuf; import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenCustomHashMap; -import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenHashMap; import it.unimi.dsi.fastutil.objects.Object2IntMap; import java.util.ArrayDeque; import java.util.Arrays; diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java index 62ee06a8c..45c479747 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/CommandTestSuite.java @@ -20,8 +20,13 @@ package com.velocitypowered.proxy.command; 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.Mockito.mock; +import static org.mockito.Mockito.when; import com.velocitypowered.api.command.CommandSource; +import com.velocitypowered.api.permission.Tristate; +import com.velocitypowered.api.proxy.Player; import com.velocitypowered.proxy.event.MockEventManager; import com.velocitypowered.proxy.event.VelocityEventManager; import java.util.Arrays; @@ -68,4 +73,11 @@ abstract class CommandTestSuite { final var actual = manager.offerSuggestions(source, input).join(); assertEquals(Arrays.asList(expectedSuggestions), actual); } + + final void assertPlayerSuggestions(final String input, final String... expectedSuggestions) { + final var player = mock(Player.class); + when(player.getPermissionValue(any())).thenReturn(Tristate.UNDEFINED); + final var actual = manager.offerSuggestions(player, input).join(); + assertEquals(Arrays.asList(expectedSuggestions), actual); + } } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java index 3a93c2b4c..5e768d4fb 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/command/SuggestionsProviderTests.java @@ -45,6 +45,17 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("", "bar", "baz", "foo"); // in alphabetical order } + @Test + void willNotSuggestAliasesIfNotAnnouncingForPlayer() { + manager.setAnnounceProxyCommands(false); + manager.register(manager.metaBuilder("foo").build(), NoSuggestionsCommand.INSTANCE); + manager.register(manager.metaBuilder("bar").build(), NoSuggestionsCommand.INSTANCE); + manager.register(manager.metaBuilder("baz").build(), NoSuggestionsCommand.INSTANCE); + + assertPlayerSuggestions(""); // for a fake player + assertSuggestions("", "bar", "baz", "foo"); // for non-players + } + @Test void testDoesNotSuggestForLeadingWhitespace() { final var meta = manager.metaBuilder("hello").build(); @@ -210,6 +221,24 @@ public class SuggestionsProviderTests extends CommandTestSuite { assertSuggestions("hello ", "suggestion"); } + // Hints and argument suggestions should still be sent even when aliases are not being suggested. + @Test + void testSuggestWillSuggestArgumentsEvenWhenAliasesAreNot() { + final var hint = RequiredArgumentBuilder + .argument("hint", word()) + .suggests((context, builder) -> builder.suggest("suggestion").buildFuture()) + .build(); + final var meta = manager.metaBuilder("hello") + .hint(hint) + .build(); + manager.setAnnounceProxyCommands(false); + manager.register(meta, NoSuggestionsCommand.INSTANCE); + + assertSuggestions("hello ", "suggestion"); + assertPlayerSuggestions("hello ", "suggestion"); + } + + @Test void testDoesNotSuggestHintIfHintSuggestionProviderFutureCompletesExceptionally() { final var hint = RequiredArgumentBuilder