diff --git a/api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerConfigurationEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerConfigurationEvent.java new file mode 100644 index 0000000000..6e042af1c7 --- /dev/null +++ b/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. + *

Velocity will wait for this event before continuing/ending the configuration state.

+ * + * @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) { +} diff --git a/api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerEnterConfigurationEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerEnterConfigurationEvent.java index 6f36b30f9c..f0148ef5ac 100644 --- a/api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerEnterConfigurationEvent.java +++ b/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 not 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 not called for the initial configuration of a player after login. *

Velocity will wait for this event before asking the client to enter configuration state.

* * @param player The player who is about to enter configuration state. diff --git a/api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerFinishConfigurationEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerFinishConfigurationEvent.java index c551fdb2f1..8bb9830697 100644 --- a/api/src/main/java/com/velocitypowered/api/event/player/configuration/PlayerFinishConfigurationEvent.java +++ b/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. - *

Velocity will wait for this event before asking the client to finish the configuration state.

+ *

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}.

* * @param player The player who is about to finish the configuration phase. * @param server The server that has (re-)configuring the player. diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java index 6735e813f4..14989b1a3c 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/backend/BackendPlaySessionHandler.java +++ b/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; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConfigSessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientConfigSessionHandler.java index 2d3a03d249..008eaaba5b 100644 --- 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 @@ 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 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 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; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java index a679e40dd8..fed61693f4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ClientPlaySessionHandler.java +++ b/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 doSwitch() { player.getTabList().clearAllSilent(); } - player.switchToConfigState(true); + player.switchToConfigState(); return configSwitchFuture; } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/client/ConnectedPlayer.java index 1507ddc318..b855b54c93 100644 --- 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.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