Add PlayerConfigurationEvent and PlayerEnteredConfigurationEvent (#1371)

* Configuring the player (i.e. sending resource packs) should now be done in the new PlayerConfigurationEvent. * The new PlayerEnteredConfigurationEvent is called when a player acknowledged the switch to configuration state. * The PlayerEnterConfigurationEvent is no longer called twice. It is now called when the backed wants to reconfigure the player. * The PlayerFinishConfigurationEvent should no longer be used to configure the player (i.e. sending resource packs). This is because since 1.20.5 the backend server can't send keep alive packets between switching state anymore and the connection will thus time out.
2024-07-12 11:16:42 +02:00
parent e0f74a8493
commit 6073f698e2
10 changed files with 173 additions and 95 deletions
--- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/LoginSessionHandler.java
@@ -19,6 +19,7 @@ package com.velocitypowered.proxy.connection.backend;

 import com.velocitypowered.api.event.player.CookieRequestEvent;
 import com.velocitypowered.api.event.player.ServerLoginPluginMessageEvent;
+import com.velocitypowered.api.event.player.configuration.PlayerEnteredConfigurationEvent;
 import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.api.proxy.messages.MinecraftChannelIdentifier;
 import com.velocitypowered.proxy.VelocityServer;
@@ -142,10 +143,8 @@ public class LoginSessionHandler implements MinecraftSessionHandler {

  @Override
  public boolean handle(ServerLoginSuccessPacket packet) {
-    if (server.getConfiguration().getPlayerInfoForwardingMode() == PlayerInfoForwarding.MODERN
-        && !informationForwarded) {
-      resultFuture.complete(ConnectionRequestResults.forDisconnect(MODERN_IP_FORWARDING_FAILURE,
-          serverConn.getServer()));
+    if (server.getConfiguration().getPlayerInfoForwardingMode() == PlayerInfoForwarding.MODERN && !informationForwarded) {
+      resultFuture.complete(ConnectionRequestResults.forDisconnect(MODERN_IP_FORWARDING_FAILURE, serverConn.getServer()));
      serverConn.disconnect();
      return true;
    }
@@ -156,12 +155,10 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
    // Move into the PLAY phase.
    MinecraftConnection smc = serverConn.ensureConnected();
    if (smc.getProtocolVersion().lessThan(ProtocolVersion.MINECRAFT_1_20_2)) {
-      smc.setActiveSessionHandler(StateRegistry.PLAY,
-          new TransitionSessionHandler(server, serverConn, resultFuture));
+      smc.setActiveSessionHandler(StateRegistry.PLAY, new TransitionSessionHandler(server, serverConn, resultFuture));
    } else {
      smc.write(new LoginAcknowledgedPacket());
-      smc.setActiveSessionHandler(StateRegistry.CONFIG,
-          new ConfigSessionHandler(server, serverConn, resultFuture));
+      smc.setActiveSessionHandler(StateRegistry.CONFIG, new ConfigSessionHandler(server, serverConn, resultFuture));
      ConnectedPlayer player = serverConn.getPlayer();
      if (player.getClientSettingsPacket() != null) {
        smc.write(player.getClientSettingsPacket());
@@ -169,6 +166,9 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
      if (player.getConnection().getActiveSessionHandler() instanceof ClientPlaySessionHandler clientPlaySessionHandler) {
        smc.setAutoReading(false);
        clientPlaySessionHandler.doSwitch().thenAcceptAsync((unused) -> smc.setAutoReading(true), smc.eventLoop());
+      } else {
+        // Initial login - the player is already in configuration state.
+        server.getEventManager().fireAndForget(new PlayerEnteredConfigurationEvent(player, serverConn));
      }
    }

--- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/AuthSessionHandler.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/AuthSessionHandler.java
@@ -178,14 +178,14 @@ public class AuthSessionHandler implements MinecraftSessionHandler {
      inbound.disconnect(Component.translatable("multiplayer.disconnect.invalid_player_data"));
    } else {
      loginState = State.ACKNOWLEDGED;
-      mcConnection.setActiveSessionHandler(StateRegistry.CONFIG,
-          new ClientConfigSessionHandler(server, connectedPlayer));
+      mcConnection.setActiveSessionHandler(StateRegistry.CONFIG, new ClientConfigSessionHandler(server, connectedPlayer));

-      server.getEventManager().fire(new PostLoginEvent(connectedPlayer))
-          .thenCompose((ignored) -> connectToInitialServer(connectedPlayer)).exceptionally((ex) -> {
-            logger.error("Exception while connecting {} to initial server", connectedPlayer, ex);
-            return null;
-          });
+      server.getEventManager().fire(new PostLoginEvent(connectedPlayer)).thenCompose(ignored -> {
+        return connectToInitialServer(connectedPlayer);
+      }).exceptionally((ex) -> {
+        logger.error("Exception while connecting {} to initial server", connectedPlayer, ex);
+        return null;
+      });
    }
    return true;
  }
@@ -224,8 +224,7 @@ public class AuthSessionHandler implements MinecraftSessionHandler {
        player.disconnect0(reason.get(), true);
      } else {
        if (!server.registerConnection(player)) {
-          player.disconnect0(Component.translatable("velocity.error.already-connected-proxy"),
-              true);
+          player.disconnect0(Component.translatable("velocity.error.already-connected-proxy"), true);
          return;
        }

@@ -238,13 +237,13 @@ public class AuthSessionHandler implements MinecraftSessionHandler {
        loginState = State.SUCCESS_SENT;
        if (inbound.getProtocolVersion().lessThan(ProtocolVersion.MINECRAFT_1_20_2)) {
          loginState = State.ACKNOWLEDGED;
-          mcConnection.setActiveSessionHandler(StateRegistry.PLAY,
-              new InitialConnectSessionHandler(player, server));
-          server.getEventManager().fire(new PostLoginEvent(player))
-              .thenCompose((ignored) -> connectToInitialServer(player)).exceptionally((ex) -> {
-                logger.error("Exception while connecting {} to initial server", player, ex);
-                return null;
-              });
+          mcConnection.setActiveSessionHandler(StateRegistry.PLAY, new InitialConnectSessionHandler(player, server));
+          server.getEventManager().fire(new PostLoginEvent(player)).thenCompose((ignored) -> {
+            return connectToInitialServer(player);
+          }).exceptionally((ex) -> {
+            logger.error("Exception while connecting {} to initial server", player, ex);
+            return null;
+          });
        }
      }
    }, mcConnection.eventLoop()).exceptionally((ex) -> {
--- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConfigSessionHandler.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConfigSessionHandler.java
@@ -19,6 +19,7 @@ package com.velocitypowered.proxy.connection.client;

 import com.velocitypowered.api.event.player.CookieReceiveEvent;
 import com.velocitypowered.api.event.player.PlayerClientBrandEvent;
+import com.velocitypowered.api.event.player.configuration.PlayerConfigurationEvent;
 import com.velocitypowered.api.event.player.configuration.PlayerFinishConfigurationEvent;
 import com.velocitypowered.api.event.player.configuration.PlayerFinishedConfigurationEvent;
 import com.velocitypowered.proxy.VelocityServer;
@@ -48,8 +49,6 @@ import net.kyori.adventure.text.Component;
 import net.kyori.adventure.text.format.NamedTextColor;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;

 /**
 * Handles the client config stage.
@@ -61,6 +60,7 @@ public class ClientConfigSessionHandler implements MinecraftSessionHandler {
  private final ConnectedPlayer player;
  private String brandChannel = null;

+  private CompletableFuture<?> configurationFuture;
  private CompletableFuture<Void> configSwitchFuture;

  /**
@@ -81,11 +81,7 @@ public class ClientConfigSessionHandler implements MinecraftSessionHandler {

  @Override
  public boolean handle(final KeepAlivePacket packet) {
-    final VelocityServerConnection serverConnection = player.getConnectedServer();
-    if (!this.sendKeepAliveToBackend(serverConnection, packet)) {
-      final VelocityServerConnection connectionInFlight = player.getConnectionInFlight();
-      this.sendKeepAliveToBackend(connectionInFlight, packet);
-    }
+    player.forwardKeepAlive(packet);
    return true;
  }

@@ -106,8 +102,7 @@ public class ClientConfigSessionHandler implements MinecraftSessionHandler {

  @Override
  public boolean handle(FinishedUpdatePacket packet) {
-    player.getConnection()
-        .setActiveSessionHandler(StateRegistry.PLAY, new ClientPlaySessionHandler(server, player));
+    player.getConnection().setActiveSessionHandler(StateRegistry.PLAY, new ClientPlaySessionHandler(server, player));

    configSwitchFuture.complete(null);
    return true;
@@ -141,12 +136,14 @@ public class ClientConfigSessionHandler implements MinecraftSessionHandler {

  @Override
  public boolean handle(KnownPacksPacket packet) {
-    if (player.getConnectionInFlight() != null) {
-      player.getConnectionInFlight().ensureConnected().write(packet);
-      return true;
-    }
+    callConfigurationEvent().thenRun(() -> {
+      player.getConnectionInFlightOrConnectedServer().ensureConnected().write(packet);
+    }).exceptionally(ex -> {
+      logger.error("Error forwarding known packs response to backend:", ex);
+      return null;
+    });

-    return false;
+    return true;
  }

  @Override
@@ -209,26 +206,25 @@ public class ClientConfigSessionHandler implements MinecraftSessionHandler {

  @Override
  public void exception(Throwable throwable) {
-    player.disconnect(
-        Component.translatable("velocity.error.player-connection-error", NamedTextColor.RED));
+    player.disconnect(Component.translatable("velocity.error.player-connection-error", NamedTextColor.RED));
  }

-  private boolean sendKeepAliveToBackend(
-          final @Nullable VelocityServerConnection serverConnection,
-          final @NotNull KeepAlivePacket packet
-  ) {
-    if (serverConnection != null) {
-      final Long sentTime = serverConnection.getPendingPings().remove(packet.getRandomId());
-      if (sentTime != null) {
-        final MinecraftConnection smc = serverConnection.getConnection();
-        if (smc != null) {
-          player.setPing(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - sentTime));
-          smc.write(packet);
-          return true;
-        }
-      }
+  /**
+   * Calls the {@link PlayerConfigurationEvent}.
+   * For 1.20.5+ backends this is done when the client responds to
+   * the known packs request. The response is delayed until the event
+   * has been called.
+   * For 1.20.2-1.20.4 servers this is done when the client acknowledges
+   * the end of the configuration.
+   * This is handled differently because for 1.20.5+ servers can't keep
+   * their connection alive between states and older servers don't have
+   * the known packs transaction.
+   */
+  private CompletableFuture<?> callConfigurationEvent() {
+    if (configurationFuture != null) {
+      return configurationFuture;
    }
-    return false;
+    return configurationFuture = server.getEventManager().fire(new PlayerConfigurationEvent(player, player.getConnectionInFlightOrConnectedServer()));
  }

  /**
@@ -248,11 +244,17 @@ public class ClientConfigSessionHandler implements MinecraftSessionHandler {
      smc.write(brandPacket);
    }

-    server.getEventManager().fire(new PlayerFinishConfigurationEvent(player, serverConn)).thenAcceptAsync(event -> {
+    callConfigurationEvent().thenCompose(v -> {
+      return server.getEventManager().fire(new PlayerFinishConfigurationEvent(player, serverConn))
+          .completeOnTimeout(null, 5, TimeUnit.SECONDS);
+    }).thenRunAsync(() -> {
      player.getConnection().write(FinishedUpdatePacket.INSTANCE);
      player.getConnection().getChannel().pipeline().get(MinecraftEncoder.class).setState(StateRegistry.PLAY);
      server.getEventManager().fireAndForget(new PlayerFinishedConfigurationEvent(player, serverConn));
-    }, player.getConnection().eventLoop());
+    }, player.getConnection().eventLoop()).exceptionally(ex -> {
+      logger.error("Error finishing configuration state:", ex);
+      return null;
+    });

    return configSwitchFuture;
  }
--- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java
@@ -27,7 +27,7 @@ import com.velocitypowered.api.event.player.CookieReceiveEvent;
 import com.velocitypowered.api.event.player.PlayerChannelRegisterEvent;
 import com.velocitypowered.api.event.player.PlayerClientBrandEvent;
 import com.velocitypowered.api.event.player.TabCompleteEvent;
-import com.velocitypowered.api.event.player.configuration.PlayerEnterConfigurationEvent;
+import com.velocitypowered.api.event.player.configuration.PlayerEnteredConfigurationEvent;
 import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.api.proxy.messages.ChannelIdentifier;
 import com.velocitypowered.api.proxy.messages.LegacyChannelIdentifier;
@@ -86,7 +86,6 @@ import java.util.Queue;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentLinkedQueue;
-import java.util.concurrent.TimeUnit;
 import net.kyori.adventure.key.Key;
 import net.kyori.adventure.text.Component;
 import net.kyori.adventure.text.format.NamedTextColor;
@@ -178,17 +177,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {

  @Override
  public boolean handle(KeepAlivePacket packet) {
-    final VelocityServerConnection serverConnection = player.getConnectedServer();
-    if (serverConnection != null) {
-      final Long sentTime = serverConnection.getPendingPings().remove(packet.getRandomId());
-      if (sentTime != null) {
-        final MinecraftConnection smc = serverConnection.getConnection();
-        if (smc != null) {
-          player.setPing(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - sentTime));
-          smc.write(packet);
-        }
-      }
-    }
+    player.forwardKeepAlive(packet);
    return true;
  }

@@ -408,7 +397,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
    // Complete client switch
    player.getConnection().setActiveSessionHandler(StateRegistry.CONFIG);
    VelocityServerConnection serverConnection = player.getConnectedServer();
-    server.getEventManager().fireAndForget(new PlayerEnterConfigurationEvent(player, serverConnection));
+    server.getEventManager().fireAndForget(new PlayerEnteredConfigurationEvent(player, serverConnection));
    if (serverConnection != null) {
      MinecraftConnection smc = serverConnection.ensureConnected();
      CompletableFuture.runAsync(() -> {
--- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java
+++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java
@@ -111,6 +111,7 @@ import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.TimeUnit;
 import net.kyori.adventure.audience.MessageType;
 import net.kyori.adventure.bossbar.BossBar;
 import net.kyori.adventure.identity.Identity;
@@ -634,6 +635,10 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player,
    return connectionInFlight;
  }

+  public VelocityServerConnection getConnectionInFlightOrConnectedServer() {
+    return connectionInFlight != null ? connectionInFlight : connectedServer;
+  }
+
  public void resetInFlightConnection() {
    connectionInFlight = null;
  }
@@ -1239,21 +1244,46 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player,
    }
  }

+  /**
+   * Forwards the keep alive packet to the backend server it belongs to.
+   * This is either the connection in flight or the connected server.
+   */
+  public boolean forwardKeepAlive(final KeepAlivePacket packet) {
+    if (!this.sendKeepAliveToBackend(connectedServer, packet)) {
+      return this.sendKeepAliveToBackend(connectionInFlight, packet);
+    }
+    return false;
+  }
+
+  private boolean sendKeepAliveToBackend(final @Nullable VelocityServerConnection serverConnection, final @NotNull KeepAlivePacket packet) {
+    if (serverConnection != null) {
+      final Long sentTime = serverConnection.getPendingPings().remove(packet.getRandomId());
+      if (sentTime != null) {
+        final MinecraftConnection smc = serverConnection.getConnection();
+        if (smc != null) {
+          setPing(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - sentTime));
+          smc.write(packet);
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
  /**
   * Switches the connection to the client into config state.
   */
  public void switchToConfigState() {
-    CompletableFuture.runAsync(() -> {
-      connection.write(StartUpdatePacket.INSTANCE);
-      connection.getChannel().pipeline()
-              .get(MinecraftEncoder.class).setState(StateRegistry.CONFIG);
-      // Make sure we don't send any play packets to the player after update start
-      connection.addPlayPacketQueueHandler();
-      server.getEventManager().fireAndForget(new PlayerEnterConfigurationEvent(this, connectionInFlight));
-    }, connection.eventLoop()).exceptionally((ex) -> {
-      logger.error("Error switching player connection to config state", ex);
-      return null;
-    });
+    server.getEventManager().fire(new PlayerEnterConfigurationEvent(this, getConnectionInFlightOrConnectedServer()))
+        .completeOnTimeout(null, 5, TimeUnit.SECONDS).thenRunAsync(() -> {
+          connection.write(StartUpdatePacket.INSTANCE);
+          connection.getChannel().pipeline().get(MinecraftEncoder.class).setState(StateRegistry.CONFIG);
+          // Make sure we don't send any play packets to the player after update start
+          connection.addPlayPacketQueueHandler();
+        }, connection.eventLoop()).exceptionally((ex) -> {
+          logger.error("Error switching player connection to config state", ex);
+          return null;
+        });
  }

  /**