Added PlayerConfigurationEvent

api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerConfigurationEvent.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2024 Velocity Contributors
+ *
+ * The Velocity API is licensed under the terms of the MIT License. For more details,
+ * reference the LICENSE file in the api top-level directory.
+ */
+
+package com.velocitypowered.api.event.player.configuration;
+
+import com.velocitypowered.api.event.annotation.AwaitingEvent;
+import com.velocitypowered.api.proxy.Player;
+import com.velocitypowered.api.proxy.ServerConnection;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * This event is executed when a player entered the configuration state and can be configured by Velocity.
+ * <p>Velocity will wait for this event before continuing/ending the configuration state.</p>
+ *
+ * @param player The player who can be configured.
+ * @param server The server that is currently configuring the player.
+ * @since 3.3.0
+ * @sinceMinecraft 1.20.2
+ */
+@AwaitingEvent
+public record PlayerConfigurationEvent(@NotNull Player player, ServerConnection server) {
+}

api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerEnterConfigurationEvent.java

@@ -14,10 +14,7 @@
 
 /**
  * This event is executed when a player is about to enter the configuration state.
- * It is <b>not</b> called for the initial configuration of a player after login,
- * because no backed server connection has been established yet.
- * In that case, as soon as a server connection is established and has entered configuration
- * state, the {@link PlayerEnteredConfigurationEvent} is fired.
+ * It is <b>not</b> called for the initial configuration of a player after login.
  * <p>Velocity will wait for this event before asking the client to enter configuration state.</p>
  *
  * @param player The player who is about to enter configuration state.

api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerFinishConfigurationEvent.java

@@ -14,7 +14,10 @@
 
 /**
  * This event is executed when a player is about to finish the configuration state.
- * <p>Velocity will wait for this event before asking the client to finish the configuration state.</p>
+ * <p>Velocity will wait for this event before asking the client to finish the configuration state.
+ * However due to backend server being unable to keep the connection alive for more than 15 seconds,
+ * Velocity will only wait for a maximum of 5 seconds. If you need to hold a player in configuration
+ * state it is recommended to use the {@link PlayerConfigurationEvent}.</p>
  *
  * @param player The player who is about to finish the configuration phase.
  * @param server The server that has (re-)configuring the player.

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

@@ -149,7 +149,7 @@ public boolean handle(StartUpdatePacket packet) {
     smc.setAutoReading(false);
     // Even when not auto reading messages are still decoded. Decode them with the correct state
     smc.getChannel().pipeline().get(MinecraftDecoder.class).setState(StateRegistry.CONFIG);
-    serverConn.getPlayer().switchToConfigState(false);
+    serverConn.getPlayer().switchToConfigState();
     return true;
   }
 

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

@@ -19,6 +19,7 @@
 
 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.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 void activated() {
 
   @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;
   }
 
@@ -140,12 +136,14 @@ public boolean handle(PingIdentifyPacket packet) {
 
   @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
@@ -208,26 +206,25 @@ public void disconnected() {
 
   @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()));
   }
 
   /**
@@ -247,11 +244,17 @@ public CompletableFuture<Void> handleBackendFinishUpdate(VelocityServerConnectio
       smc.write(brandPacket);
     }
 
-    server.getEventManager().fire(new PlayerFinishConfigurationEvent(player, serverConn)).thenAcceptAsync(event -> {
-      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());
+    callConfigurationEvent().thenRun(() -> {
+      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());
+    }).exceptionally(ex -> {
+      logger.error("Error finishing configuration state:", ex);
+      return null;
+    });
 
     return configSwitchFuture;
   }

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

@@ -86,7 +86,6 @@
 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 void deactivated() {
 
   @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;
   }
 
@@ -541,7 +530,7 @@ public CompletableFuture<Void> doSwitch() {
       player.getTabList().clearAllSilent();
     }
 
-    player.switchToConfigState(true);
+    player.switchToConfigState();
 
     return configSwitchFuture;
   }

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

@@ -111,6 +111,7 @@
 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 void disconnect0(Component reason, boolean duringLogin) {
     return connectionInFlight;
   }
 
+  public VelocityServerConnection getConnectionInFlightOrConnectedServer() {
+    return connectionInFlight != null ? connectionInFlight : connectedServer;
+  }
+
   public void resetInFlightConnection() {
     connectionInFlight = null;
   }
@@ -1239,11 +1244,37 @@ public void sendKeepAlive() {
     }
   }
 
+  /**
+   * 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(boolean inFlight) {
-    server.getEventManager().fire(new PlayerEnterConfigurationEvent(this, inFlight ? connectionInFlight : connectedServer)).thenRunAsync(() -> {
+  public void switchToConfigState() {
+    server.getEventManager().fire(new PlayerEnterConfigurationEvent(this, getConnectionInFlightOrConnectedServer())).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