Fix players disconnecting when updating boss bars (#1656)

* Fix 1.20.2+ clients disconnecting when updating boss bars

On 1.20.2, the Minecraft client started clearing boss bars after the login packet, which meant that the ProxyServer#showBossbar API would result in the player getting kicked if the boss bar they were previously shown was updated after switching servers.

Therefore, I have added BossBarManager which drops boss bar packets once the client enters the configure phase to ensure that they do not disconnect, and then re-adds the boss bar once the client enters the login phase.

This ensures that clients do not receive boss bar updates for boss bars that they don't exist and causing them to disconnect. I have also taken care to ensure that this logic only applies on 1.20.2 and up, as it is not necessary for older clients.

---------

Co-authored-by: Adrian Gonzales <adriangonzalesval@gmail.com>

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

@@ -53,15 +53,23 @@ public final class VelocityBossBarImplementation implements BossBar.Listener,
           viewer.getProtocolVersion(),
           viewer.translateMessage(this.bar.name())
       );
-      viewer.getConnection().write(BossBarPacket.createAddPacket(this.id, this.bar, name));
+      viewer.getBossBarManager().writeUpdate(this, BossBarPacket.createAddPacket(this.id, this.bar, name));
       return true;
     }
     return false;
   }
 
+  public void createDirect(final ConnectedPlayer viewer) {
+    final ComponentHolder name = new ComponentHolder(
+        viewer.getProtocolVersion(),
+        viewer.translateMessage(this.bar.name())
+    );
+    viewer.getConnection().write(BossBarPacket.createAddPacket(this.id, this.bar, name));
+  }
+
   public boolean viewerRemove(final ConnectedPlayer viewer) {
     if (this.viewers.remove(viewer)) {
-      viewer.getConnection().write(BossBarPacket.createRemovePacket(this.id, this.bar));
+      viewer.getBossBarManager().remove(this, BossBarPacket.createRemovePacket(this.id, this.bar));
       return true;
     }
     return false;
@@ -84,7 +92,7 @@ public final class VelocityBossBarImplementation implements BossBar.Listener,
           this.bar,
           new ComponentHolder(viewer.getProtocolVersion(), translated)
       );
-      viewer.getConnection().write(packet);
+      viewer.getBossBarManager().writeUpdate(this, packet);
     }
   }
 
@@ -96,7 +104,7 @@ public final class VelocityBossBarImplementation implements BossBar.Listener,
   ) {
     final BossBarPacket packet = BossBarPacket.createUpdateProgressPacket(this.id, this.bar);
     for (final ConnectedPlayer viewer : this.viewers) {
-      viewer.getConnection().write(packet);
+      viewer.getBossBarManager().writeUpdate(this, packet);
     }
   }
 
@@ -108,7 +116,7 @@ public final class VelocityBossBarImplementation implements BossBar.Listener,
   ) {
     final BossBarPacket packet = BossBarPacket.createUpdateStylePacket(this.id, this.bar);
     for (final ConnectedPlayer viewer : this.viewers) {
-      viewer.getConnection().write(packet);
+      viewer.getBossBarManager().writeUpdate(this, packet);
     }
   }
 
@@ -120,7 +128,7 @@ public final class VelocityBossBarImplementation implements BossBar.Listener,
   ) {
     final BossBarPacket packet = BossBarPacket.createUpdateStylePacket(this.id, this.bar);
     for (final ConnectedPlayer viewer : this.viewers) {
-      viewer.getConnection().write(packet);
+      viewer.getBossBarManager().writeUpdate(this, packet);
     }
   }
 
@@ -132,7 +140,7 @@ public final class VelocityBossBarImplementation implements BossBar.Listener,
   ) {
     final BossBarPacket packet = BossBarPacket.createUpdatePropertiesPacket(this.id, this.bar);
     for (final ConnectedPlayer viewer : this.viewers) {
-      viewer.getConnection().write(packet);
+      viewer.getBossBarManager().writeUpdate(this, packet);
     }
   }
 }

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

@@ -177,10 +177,12 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler {
 
   @Override
   public boolean handle(BossBarPacket packet) {
-    if (packet.getAction() == BossBarPacket.ADD) {
-      playerSessionHandler.getServerBossBars().add(packet.getUuid());
-    } else if (packet.getAction() == BossBarPacket.REMOVE) {
-      playerSessionHandler.getServerBossBars().remove(packet.getUuid());
+    if (serverConn.getPlayer().getProtocolVersion().lessThan(ProtocolVersion.MINECRAFT_1_20_2)) {
+      if (packet.getAction() == BossBarPacket.ADD) {
+        playerSessionHandler.getServerBossBars().add(packet.getUuid());
+      } else if (packet.getAction() == BossBarPacket.REMOVE) {
+        playerSessionHandler.getServerBossBars().remove(packet.getUuid());
+      }
     }
     return false; // forward
   }
@@ -516,4 +518,4 @@ public class BackendPlaySessionHandler implements MinecraftSessionHandler {
 
     playerConnection.setAutoReading(writable);
   }
-}
+}

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

@@ -535,9 +535,13 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
 
       // Config state clears everything in the client. No need to clear later.
       spawned = false;
-      serverBossBars.clear();
       player.clearPlayerListHeaderAndFooterSilent();
       player.getTabList().clearAllSilent();
+      if (player.getProtocolVersion().noLessThan(ProtocolVersion.MINECRAFT_1_20_2)) {
+        player.getBossBarManager().dropPackets();
+      } else {
+        serverBossBars.clear();
+      }
     }
 
     player.switchToConfigState();
@@ -576,16 +580,19 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
     }
 
     destination.setEntityId(joinGame.getEntityId()); // used for sound api
-
-    // Remove previous boss bars. These don't get cleared when sending JoinGame, thus the need to
-    // track them.
-    for (UUID serverBossBar : serverBossBars) {
-      BossBarPacket deletePacket = new BossBarPacket();
-      deletePacket.setUuid(serverBossBar);
-      deletePacket.setAction(BossBarPacket.REMOVE);
-      player.getConnection().delayedWrite(deletePacket);
+    if (player.getProtocolVersion().noLessThan(ProtocolVersion.MINECRAFT_1_20_2)) {
+      player.getBossBarManager().sendBossBars();
+    } else {
+      // Remove previous boss bars. These don't get cleared when sending JoinGame (up until 1.20.2),
+      // thus the need to track them.
+      for (UUID serverBossBar : serverBossBars) {
+        BossBarPacket deletePacket = new BossBarPacket();
+        deletePacket.setUuid(serverBossBar);
+        deletePacket.setAction(BossBarPacket.REMOVE);
+        player.getConnection().delayedWrite(deletePacket);
+      }
+      serverBossBars.clear();
     }
-    serverBossBars.clear();
 
     // Tell the server about the proxy's plugin message channels.
     ProtocolVersion serverVersion = serverMc.getProtocolVersion();

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

@@ -62,6 +62,7 @@ import com.velocitypowered.proxy.adventure.VelocityBossBarImplementation;
 import com.velocitypowered.proxy.connection.MinecraftConnection;
 import com.velocitypowered.proxy.connection.MinecraftConnectionAssociation;
 import com.velocitypowered.proxy.connection.backend.VelocityServerConnection;
+import com.velocitypowered.proxy.connection.player.bossbar.BossBarManager;
 import com.velocitypowered.proxy.connection.player.bundle.BundleDelimiterHandler;
 import com.velocitypowered.proxy.connection.player.resourcepack.VelocityResourcePackInfo;
 import com.velocitypowered.proxy.connection.player.resourcepack.handler.ResourcePackHandler;
@@ -201,6 +202,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player,
   private @Nullable ClientSettingsPacket clientSettingsPacket;
   private volatile ChatQueue chatQueue;
   private final ChatBuilderFactory chatBuilderFactory;
+  private final BossBarManager bossBarManager;
 
   ConnectedPlayer(VelocityServer server, GameProfile profile, MinecraftConnection connection,
                   @Nullable InetSocketAddress virtualHost, @Nullable String rawVirtualHost, boolean onlineMode,
@@ -227,6 +229,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player,
     this.chatQueue = new ChatQueue(this);
     this.chatBuilderFactory = new ChatBuilderFactory(this.getProtocolVersion());
     this.resourcePackHandler = ResourcePackHandler.create(this, server);
+    this.bossBarManager = new BossBarManager(this);
   }
 
   /**
@@ -1431,6 +1434,10 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player,
     return handshakeIntent;
   }
 
+  public BossBarManager getBossBarManager() {
+    return bossBarManager;
+  }
+
   private final class ConnectionRequestBuilderImpl implements ConnectionRequestBuilder {
 
     private final RegisteredServer toConnect;

proxy/src/main/java/com/velocitypowered/proxy/connection/player/bossbar/BossBarManager.java

@@ -0,0 +1,79 @@
+/*
+ * 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.connection.player.bossbar;
+
+import com.velocitypowered.proxy.adventure.VelocityBossBarImplementation;
+import com.velocitypowered.proxy.connection.client.ConnectedPlayer;
+import com.velocitypowered.proxy.protocol.packet.BossBarPacket;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Handles dropping and resending boss bar packets on versions 1.20.2 and newer because the client now
+ * deletes all boss bars during the login phase, and sending update packets would cause the client to be disconnected.
+ */
+public final class BossBarManager {
+
+  private final ConnectedPlayer player;
+  private final Set<VelocityBossBarImplementation> bossBars = new HashSet<>();
+
+  private boolean dropPackets = false;
+
+  public BossBarManager(ConnectedPlayer player) {
+    this.player = player;
+  }
+
+  /**
+   * Records the specified boss bar to be re-sent when a player changes server, and sends the update packet
+   * if the client is able to receive it and not be disconnected.
+   */
+  public synchronized void writeUpdate(VelocityBossBarImplementation bar, BossBarPacket packet) {
+    this.bossBars.add(bar);
+    if (!this.dropPackets) {
+      this.player.getConnection().write(packet);
+    }
+  }
+
+  /**
+   * Removes the specified boss bar from the player to ensure it is not re-sent.
+   */
+  public synchronized void remove(VelocityBossBarImplementation bar, BossBarPacket packet) {
+    this.bossBars.remove(bar);
+    if (!this.dropPackets) {
+      this.player.getConnection().write(packet);
+    }
+  }
+
+  /**
+   * Re-creates the boss bars the player can see with any updates that may have occurred in the meantime,
+   * and allows update packets for those boss bars to be sent.
+   */
+  public synchronized void sendBossBars() {
+    for (VelocityBossBarImplementation bossBar : bossBars) {
+      bossBar.createDirect(player);
+    }
+    this.dropPackets = false;
+  }
+
+  /**
+   * Prevents the player from receiving boss bar update packets while logging in to a new server.
+   */
+  public synchronized void dropPackets() {
+    this.dropPackets = true;
+  }
+}