Add PlayerConfigurationEvent and PlayerEnteredConfigurationEvent (Pap…

…erMC#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.

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

@@ -7,21 +7,23 @@
 
 package com.velocitypowered.api.event.player.configuration;
 
-import com.velocitypowered.api.network.ProtocolState;
+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 with version 1.20.2 or higher enters the configuration phase.
- * <p>From this moment on, until the {@link PlayerFinishedConfigurationEvent} is executed,
- * the {@linkplain Player#getProtocolState()} method is guaranteed
- * to return {@link ProtocolState#CONFIGURATION}.</p>
+ * 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.
+ * <p>Velocity will wait for this event before asking the client to enter configuration state.
+ * However due to backend server being unable to keep the connection alive during state changes,
+ * Velocity will only wait for a maximum of 5 seconds.</p>
  *
- * @param player The player that has entered the configuration phase.
- * @param server The server that will now (re-)configure the player.
+ * @param player The player who is about to enter configuration state.
+ * @param server The server that wants to reconfigure the player.
  * @since 3.3.0
  * @sinceMinecraft 1.20.2
  */
+@AwaitingEvent
 public record PlayerEnterConfigurationEvent(@NotNull Player player, ServerConnection server) {
 }

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

@@ -0,0 +1,27 @@
+/*
+ * 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.network.ProtocolState;
+import com.velocitypowered.api.proxy.Player;
+import com.velocitypowered.api.proxy.ServerConnection;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * This event is executed when a player has entered the configuration state.
+ * <p>From this moment on, until the {@link PlayerFinishedConfigurationEvent} is executed,
+ * the {@linkplain Player#getProtocolState()} method is guaranteed
+ * to return {@link ProtocolState#CONFIGURATION}.</p>
+ *
+ * @param player The player who has entered the configuration state.
+ * @param server The server that will now (re-)configure the player.
+ * @since 3.3.0
+ * @sinceMinecraft 1.20.2
+ */
+public record PlayerEnteredConfigurationEvent(@NotNull Player player, ServerConnection server) {
+}

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

@@ -13,11 +13,14 @@
 import org.jetbrains.annotations.NotNull;
 
 /**
- * This event is executed when the player is about to finish the Configuration state.
- * <p>Velocity will wait for this event to finish the configuration phase on the client.</p>
+ * 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.
+ * However due to backend server being unable to keep the connection alive during state changes,
+ * Velocity will only wait for a maximum of 5 seconds. If you need to hold a player in configuration
+ * state, use the {@link PlayerConfigurationEvent}.</p>
  *
- * @param player The player who is about to complete the configuration phase.
- * @param server The server that is currently (re-)configuring the player.
+ * @param player The player who is about to finish the configuration phase.
+ * @param server The server that has (re-)configured the player.
  * @since 3.3.0
  * @sinceMinecraft 1.20.2
  */

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

@@ -13,11 +13,11 @@
 import org.jetbrains.annotations.NotNull;
 
 /**
- * Event executed when a player of version 1.20.2 or higher finishes the Configuration state.
+ * This event is executed when a player has finished the configuration state.
  * <p>From this moment on, the {@link Player#getProtocolState()} method
  * will return {@link ProtocolState#PLAY}.</p>
  *
- * @param player The player who has completed the Configuration state
+ * @param player The player who has finished the configuration state.
  * @param server The server that has (re-)configured the player.
  * @since 3.3.0
  * @sinceMinecraft 1.20.2

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

@@ -19,6 +19,7 @@
 
 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 boolean handle(SetCompressionPacket packet) {
 
   @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,19 +155,20 @@ public boolean handle(ServerLoginSuccessPacket packet) {
     // 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());
       }
       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));
       }
     }
 

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

@@ -178,14 +178,14 @@ public boolean handle(LoginAcknowledgedPacket packet) {
       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 @@ private void completeLoginProtocolPhaseAndInitialize(ConnectedPlayer player) {
         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 @@ private void completeLoginProtocolPhaseAndInitialize(ConnectedPlayer player) {
         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) -> {

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;
   }
 
@@ -106,8 +102,7 @@ public boolean handle(ResourcePackResponsePacket packet) {
 
   @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 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
@@ -209,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()));
   }
 
   /**
@@ -248,11 +244,17 @@ public CompletableFuture<Void> handleBackendFinishUpdate(VelocityServerConnectio
       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;
   }

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

@@ -27,7 +27,7 @@
 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.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;
   }
 
@@ -408,7 +397,7 @@ public boolean handle(FinishedUpdatePacket packet) {
     // 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(() -> {