Do not track plugin channels registered per-player on the proxy (#591)

We don't need to track this information since Velocity uses the JoinGame packet, which is about as good of a server rejoin mechanism we're likely to get in vanilla Minecraft.

proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java

@@ -198,13 +198,8 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler {
       return true;
     }
 
-    // We need to specially handle REGISTER and UNREGISTER packets. Later on, we'll write them to
-    // the client.
-    if (PluginMessageUtil.isRegister(packet)) {
-      serverConn.getPlayer().getKnownChannels().addAll(PluginMessageUtil.getChannels(packet));
-      return false;
-    } else if (PluginMessageUtil.isUnregister(packet)) {
-      serverConn.getPlayer().getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet));
+    // Register and unregister packets are simply forwarded to the server as-is.
+    if (PluginMessageUtil.isRegister(packet) || PluginMessageUtil.isUnregister(packet)) {
       return false;
     }
 

proxy/src/main/java/com/velocitypowered/proxy/connection/backend/TransitionSessionHandler.java

@@ -36,7 +36,6 @@ import com.velocitypowered.proxy.protocol.packet.Disconnect;
 import com.velocitypowered.proxy.protocol.packet.JoinGame;
 import com.velocitypowered.proxy.protocol.packet.KeepAlive;
 import com.velocitypowered.proxy.protocol.packet.PluginMessage;
-import com.velocitypowered.proxy.protocol.util.PluginMessageUtil;
 import java.io.IOException;
 import java.util.concurrent.CompletableFuture;
 import org.apache.logging.log4j.LogManager;
@@ -180,12 +179,6 @@ public class TransitionSessionHandler implements MinecraftSessionHandler {
       return true;
     }
 
-    if (PluginMessageUtil.isRegister(packet)) {
-      serverConn.getPlayer().getKnownChannels().addAll(PluginMessageUtil.getChannels(packet));
-    } else if (PluginMessageUtil.isUnregister(packet)) {
-      serverConn.getPlayer().getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet));
-    }
-
     // We always need to handle plugin messages, for Forge compatibility.
     if (serverConn.getPhase().handle(serverConn, serverConn.getPlayer(), packet)) {
       // Handled, but check the server connection phase.

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

@@ -156,7 +156,6 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
     if (!channels.isEmpty()) {
       PluginMessage register = constructChannelsPacket(player.getProtocolVersion(), channels);
       player.getConnection().write(register);
-      player.getKnownChannels().addAll(channels);
     }
   }
 
@@ -295,7 +294,6 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
             packet.getChannel());
       } else if (PluginMessageUtil.isRegister(packet)) {
         List<String> channels = PluginMessageUtil.getChannels(packet);
-        player.getKnownChannels().addAll(channels);
         List<ChannelIdentifier> channelIdentifiers = new ArrayList<>();
         for (String channel : channels) {
           try {
@@ -309,7 +307,6 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
                 new PlayerChannelRegisterEvent(player, ImmutableList.copyOf(channelIdentifiers)));
         backendConn.write(packet.retain());
       } else if (PluginMessageUtil.isUnregister(packet)) {
-        player.getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet));
         backendConn.write(packet.retain());
       } else if (PluginMessageUtil.isMcBrand(packet)) {
         String brand = PluginMessageUtil.readBrandMessage(packet.content());
@@ -485,12 +482,6 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
     }
     serverBossBars.clear();
 
-    // Tell the server about this client's plugin message channels.
-    ProtocolVersion serverVersion = serverMc.getProtocolVersion();
-    if (!player.getKnownChannels().isEmpty()) {
-      serverMc.delayedWrite(constructChannelsPacket(serverVersion, player.getKnownChannels()));
-    }
-
     // If we had plugin messages queued during login/FML handshake, send them now.
     PluginMessage pm;
     while ((pm = loginPluginMessages.poll()) != null) {

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

@@ -77,12 +77,10 @@ import com.velocitypowered.proxy.tablist.VelocityTabList;
 import com.velocitypowered.proxy.tablist.VelocityTabListLegacy;
 import com.velocitypowered.proxy.util.ClosestLocaleMatcher;
 import com.velocitypowered.proxy.util.DurationUtils;
-import com.velocitypowered.proxy.util.collect.CappedSet;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.buffer.Unpooled;
 import java.net.InetSocketAddress;
 import java.util.ArrayDeque;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
@@ -155,7 +153,6 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player,
   private final InternalTabList tabList;
   private final VelocityServer server;
   private ClientConnectionPhase connectionPhase;
-  private final Collection<String> knownChannels;
   private final CompletableFuture<Void> teardownFuture = new CompletableFuture<>();
   private @MonotonicNonNull List<String> serversToTry = null;
   private @MonotonicNonNull Boolean previousResourceResponse;
@@ -185,7 +182,6 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player,
     this.virtualHost = virtualHost;
     this.permissionFunction = PermissionFunction.ALWAYS_UNDEFINED;
     this.connectionPhase = connection.getType().getInitialClientPhase();
-    this.knownChannels = CappedSet.create(MAX_PLUGIN_CHANNELS);
     this.onlineMode = onlineMode;
 
     if (connection.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_19_3) >= 0) {
@@ -1103,15 +1099,6 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player,
     this.connectionPhase = connectionPhase;
   }
 
-  /**
-   * Return all the plugin message channels "known" to the client.
-   *
-   * @return the channels
-   */
-  public Collection<String> getKnownChannels() {
-    return knownChannels;
-  }
-
   @Override
   public @Nullable IdentifiedKey getIdentifiedKey() {
     return playerKey;

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

@@ -24,7 +24,6 @@ import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
 import com.velocitypowered.proxy.connection.backend.BungeeCordMessageResponder;
 import com.velocitypowered.proxy.connection.backend.VelocityServerConnection;
 import com.velocitypowered.proxy.protocol.packet.PluginMessage;
-import com.velocitypowered.proxy.protocol.util.PluginMessageUtil;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.buffer.Unpooled;
 import org.apache.logging.log4j.LogManager;
@@ -55,15 +54,7 @@ public class InitialConnectSessionHandler implements MinecraftSessionHandler {
         return true;
       }
 
-      if (PluginMessageUtil.isRegister(packet)) {
-        player.getKnownChannels().addAll(PluginMessageUtil.getChannels(packet));
-        serverConn.ensureConnected().write(packet.retain());
-        return true;
-      } else if (PluginMessageUtil.isUnregister(packet)) {
-        player.getKnownChannels().removeAll(PluginMessageUtil.getChannels(packet));
-        serverConn.ensureConnected().write(packet.retain());
-        return true;
-      } else if (BungeeCordMessageResponder.isBungeeCordMessage(packet)) {
+      if (BungeeCordMessageResponder.isBungeeCordMessage(packet)) {
         return true;
       }
 

proxy/src/main/java/com/velocitypowered/proxy/util/collect/CappedSet.java

@@ -1,70 +0,0 @@
-/*
- * Copyright (C) 2019-2023 Velocity Contributors
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <https://www.gnu.org/licenses/>.
- */
-
-package com.velocitypowered.proxy.util.collect;
-
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ForwardingSet;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
-
-/**
- * An unsynchronized collection that puts an upper bound on the size of the collection.
- */
-public final class CappedSet<T> extends ForwardingSet<T> {
-
-  private final Set<T> delegate;
-  private final int upperSize;
-
-  private CappedSet(Set<T> delegate, int upperSize) {
-    this.delegate = delegate;
-    this.upperSize = upperSize;
-  }
-
-  /**
-   * Creates a capped collection backed by a {@link HashSet}.
-   *
-   * @param maxSize the maximum size of the collection
-   * @param <T>     the type of elements in the collection
-   * @return the new collection
-   */
-  public static <T> Set<T> create(int maxSize) {
-    return new CappedSet<>(new HashSet<>(), maxSize);
-  }
-
-  @Override
-  protected Set<T> delegate() {
-    return delegate;
-  }
-
-  @Override
-  public boolean add(T element) {
-    if (this.delegate.size() >= upperSize) {
-      Preconditions.checkState(this.delegate.contains(element),
-          "collection is too large (%s >= %s)",
-          this.delegate.size(), this.upperSize);
-      return false;
-    }
-    return this.delegate.add(element);
-  }
-
-  @Override
-  public boolean addAll(Collection<? extends T> collection) {
-    return this.standardAddAll(collection);
-  }
-}

proxy/src/test/java/com/velocitypowered/proxy/util/collect/CappedSetTest.java

@@ -1,71 +0,0 @@
-/*
- * Copyright (C) 2019-2021 Velocity Contributors
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <https://www.gnu.org/licenses/>.
- */
-
-package com.velocitypowered.proxy.util.collect;
-
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
-import com.google.common.collect.ImmutableSet;
-import java.util.Collection;
-import java.util.Set;
-import org.junit.jupiter.api.Test;
-
-class CappedSetTest {
-
-  @Test
-  void basicVerification() {
-    Collection<String> coll = CappedSet.create(1);
-    assertTrue(coll.add("coffee"), "did not add single item");
-    assertThrows(IllegalStateException.class, () -> coll.add("tea"),
-        "item was added to collection although it is too full");
-    assertEquals(1, coll.size(), "collection grew in size unexpectedly");
-  }
-
-  @Test
-  void testAddAll() {
-    Set<String> doesFill1 = ImmutableSet.of("coffee", "tea");
-    Set<String> doesFill2 = ImmutableSet.of("chocolate");
-    Set<String> overfill = ImmutableSet.of("Coke", "Pepsi");
-
-    Collection<String> coll = CappedSet.create(3);
-    assertTrue(coll.addAll(doesFill1), "did not add items");
-    assertTrue(coll.addAll(doesFill2), "did not add items");
-    assertThrows(IllegalStateException.class, () -> coll.addAll(overfill),
-        "items added to collection although it is too full");
-    assertEquals(3, coll.size(), "collection grew in size unexpectedly");
-  }
-
-  @Test
-  void handlesSetBehaviorCorrectly() {
-    Set<String> doesFill1 = ImmutableSet.of("coffee", "tea");
-    Set<String> doesFill2 = ImmutableSet.of("coffee", "chocolate");
-    Set<String> overfill = ImmutableSet.of("coffee", "Coke", "Pepsi");
-
-    Collection<String> coll = CappedSet.create(3);
-    assertTrue(coll.addAll(doesFill1), "did not add items");
-    assertTrue(coll.addAll(doesFill2), "did not add items");
-    assertThrows(IllegalStateException.class, () -> coll.addAll(overfill),
-        "items added to collection although it is too full");
-
-    assertFalse(coll.addAll(doesFill1), "added items?!?");
-
-    assertEquals(3, coll.size(), "collection grew in size unexpectedly");
-  }
-}