Only send callback command when needed

proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java

@@ -62,6 +62,7 @@
 import com.velocitypowered.api.util.GameProfile;
 import com.velocitypowered.api.util.ProxyVersion;
 import com.velocitypowered.api.util.ServerLink;
+import com.velocitypowered.proxy.adventure.ClickCallbackManager;
 import com.velocitypowered.proxy.command.VelocityCommandManager;
 import com.velocitypowered.proxy.command.builtin.BuiltinCommandDefinition;
 import com.velocitypowered.proxy.command.builtin.CallbackCommand;
@@ -85,6 +86,7 @@
 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.StateRegistry;
 import com.velocitypowered.proxy.protocol.util.FaviconSerializer;
 import com.velocitypowered.proxy.protocol.util.GameProfileSerializer;
 import com.velocitypowered.proxy.scheduler.VelocityScheduler;
@@ -438,6 +440,20 @@ void start() {
 
     registerCommands();
 
+    // Re-send the available commands to all online players once a click-callback has been registered.
+    // Vanilla Velocity does not register any click-callbacks, only plugins may do so via the Adventure API.
+    // If no plugins are making use of this feature, we can omit the /velocity:callback (ClickCallbackManager#COMMAND_LABEL)
+    // from the available commands, as it only adds clutter to command completion suggestions.
+    // ConnectedPlayer#sendAvailableCommands will include this callback command in the command set if a click-listener
+    // has been registered at least once.
+    ClickCallbackManager.INSTANCE.setOnFirstRegistration(() -> {
+      for (ConnectedPlayer player : getAllPlayers()) {
+        if (player.getConnection().getState() == StateRegistry.PLAY) {
+          player.sendAvailableCommands();
+        }
+      }
+    });
+
     LOGGER.info("Loading localizations...");
     translationRegistryManager.registerTranslations();
 

proxy/src/main/java/com/velocitypowered/proxy/adventure/ClickCallbackManager.java

@@ -22,10 +22,12 @@
 import com.github.benmanes.caffeine.cache.Expiry;
 import com.github.benmanes.caffeine.cache.Scheduler;
 import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import net.kyori.adventure.audience.Audience;
 import net.kyori.adventure.text.event.ClickCallback;
 import org.checkerframework.checker.index.qual.NonNegative;
+import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 import org.jetbrains.annotations.NotNull;
 
 /**
@@ -39,6 +41,10 @@ public final class ClickCallbackManager {
 
   static final String COMMAND = "/" + COMMAND_LABEL + " ";
 
+  private final AtomicBoolean hadRegistrations = new AtomicBoolean(false);
+
+  private volatile @MonotonicNonNull Runnable onFirstRegistration;
+
   private final Cache<UUID, RegisteredCallback> registrations = Caffeine.newBuilder()
       .expireAfter(new Expiry<UUID, RegisteredCallback>() {
         @Override
@@ -68,6 +74,28 @@ public long expireAfterRead(@NotNull UUID key, @NotNull RegisteredCallback value
   private ClickCallbackManager() {
   }
 
+  /**
+   * Sets a listener that is invoked the first time a callback is registered.
+   *
+   * @param listener the listener to invoke on the first registration
+   * @throws IllegalStateException if a listener has already been set
+   */
+  public void setOnFirstRegistration(Runnable listener) {
+    if (this.onFirstRegistration != null) {
+      throw new IllegalStateException("A first-registration listener has already been set");
+    }
+    this.onFirstRegistration = listener;
+  }
+
+  /**
+   * Returns whether any callback has ever been registered.
+   *
+   * @return {@code true} if at least one callback has been registered, {@code false} otherwise
+   */
+  public boolean hasHadRegistrations() {
+    return hadRegistrations.get();
+  }
+
   /**
    * Run a callback.
    *
@@ -97,6 +125,15 @@ public UUID register(ClickCallback<Audience> callback,
     UUID id = UUID.randomUUID();
     RegisteredCallback registration = new RegisteredCallback(options.lifetime(), options.uses(), callback);
     this.registrations.put(id, registration);
+
+    boolean alreadyHadRegistrations = hadRegistrations.getAndSet(true);
+    if (!alreadyHadRegistrations) {
+      Runnable listener = this.onFirstRegistration;
+      if (listener != null) {
+        listener.run();
+      }
+    }
+
     return id;
   }
 }

proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java

@@ -889,9 +889,12 @@ public CompletableFuture<Void> sendAvailableCommands(@Nullable VelocityServerCon
       CommandGraphInjector<CommandSource> injector = server.getCommandManager().getInjector();
       injector.inject(workingNode, this);
 
-      // In 1.21.6 a confirmation prompt was added when executing a command via `run_command` click
-      // action if the command is unknown. To prevent this prompt we have to send the command.
-      if (this.connection.getProtocolVersion().lessThan(ProtocolVersion.MINECRAFT_1_21_6)) {
+      // Omit the click-callback command from the client's command tree unless:
+      // - the client is 1.21.6+ (needs it to suppress the unknown-command confirmation prompt), AND
+      // - at least one callback has been registered since proxy startup (i.e. some plugin is
+      //   using the click-callback feature).
+      if (this.connection.getProtocolVersion().lessThan(ProtocolVersion.MINECRAFT_1_21_6)
+          || !ClickCallbackManager.INSTANCE.hasHadRegistrations()) {
         workingNode.removeChildByName(ClickCallbackManager.COMMAND_LABEL);
       }
     }