From 070631902a78d6046f31ee7edc0c5cc60b28bab5 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 28 Oct 2018 03:32:18 -0400 Subject: [PATCH] Fix some suboptimal behavior in invoking KickedFromServerEvent. Previously, the event would only fire when a player was kicked from the current server they were on. Now, under certain cases, it can be fired even if the player was already connected to a server. To faciliate this, a new result (Notify) was introduced. This result will "do the right thing" if the player is kicked from the current server or is trying to connect to a different server than the one they were on. --- .../event/player/KickedFromServerEvent.java | 60 +++++++++++++++++-- .../connection/client/ConnectedPlayer.java | 50 +++++++++------- 2 files changed, 83 insertions(+), 27 deletions(-) diff --git a/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java b/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java index fcc95c0eb..4811c669f 100644 --- a/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java +++ b/api/src/main/java/com/velocitypowered/api/event/player/KickedFromServerEvent.java @@ -17,16 +17,16 @@ public final class KickedFromServerEvent implements private final Player player; private final RegisteredServer server; private final Component originalReason; - private final boolean duringLogin; + private final boolean duringServerConnect; private ServerKickResult result; public KickedFromServerEvent(Player player, RegisteredServer server, Component originalReason, - boolean duringLogin, Component fancyReason) { + boolean duringServerConnect, Component fancyReason) { this.player = Preconditions.checkNotNull(player, "player"); this.server = Preconditions.checkNotNull(server, "server"); this.originalReason = Preconditions.checkNotNull(originalReason, "originalReason"); - this.duringLogin = duringLogin; - this.result = new DisconnectPlayer(fancyReason); + this.duringServerConnect = duringServerConnect; + this.result = new Notify(fancyReason); } @Override @@ -51,8 +51,25 @@ public final class KickedFromServerEvent implements return originalReason; } + /** + * Returns whether or not the player got kicked while connecting to another server. + * + * @return whether or not the player got kicked + */ + public boolean kickedDuringServerConnect() { + return duringServerConnect; + } + + /** + * Returns whether or not the player got kicked while logging in. + * + * @return whether or not the player got kicked + * @deprecated {@link #kickedDuringServerConnect()} has a better name and reflects the actual + * result + */ + @Deprecated public boolean kickedDuringLogin() { - return duringLogin; + return duringServerConnect; } /** @@ -124,4 +141,37 @@ public final class KickedFromServerEvent implements return new RedirectPlayer(server); } } + + /** + * Notifies the player with the specified message but does nothing else. This is only a valid + * result to use if the player was trying to connect to a different server, otherwise it is + * treated like a {@link DisconnectPlayer} result. + */ + public static final class Notify implements ServerKickResult { + + private final Component message; + + private Notify(Component message) { + this.message = Preconditions.checkNotNull(message, "message"); + } + + @Override + public boolean isAllowed() { + return false; + } + + public Component getMessage() { + return message; + } + + /** + * Notifies the player with the specified message but does nothing else. + * + * @param message the server to send the player to + * @return the redirect result + */ + public static Notify create(Component message) { + return new Notify(message); + } + } } 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 91c9d3c53..2c823a00f 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 @@ -4,6 +4,9 @@ import com.google.common.base.Preconditions; import com.google.gson.JsonObject; import com.velocitypowered.api.event.connection.DisconnectEvent; import com.velocitypowered.api.event.player.KickedFromServerEvent; +import com.velocitypowered.api.event.player.KickedFromServerEvent.DisconnectPlayer; +import com.velocitypowered.api.event.player.KickedFromServerEvent.Notify; +import com.velocitypowered.api.event.player.KickedFromServerEvent.RedirectPlayer; import com.velocitypowered.api.event.player.PlayerModInfoEvent; import com.velocitypowered.api.event.player.PlayerSettingsChangedEvent; import com.velocitypowered.api.event.player.ServerPreConnectEvent; @@ -336,9 +339,9 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { private void handleConnectionException(RegisteredServer rs, @Nullable Component kickReason, Component friendlyReason) { - boolean alreadyConnected = - connectedServer != null && connectedServer.getServerInfo().equals(rs.getServerInfo()); + // There can't be any connection in flight now. connectionInFlight = null; + if (connectedServer == null) { // The player isn't yet connected to a server. Optional nextServer = getNextServerToTry(); @@ -347,30 +350,33 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player { } else { connection.closeWith(Disconnect.create(friendlyReason)); } - } else if (connectedServer.getServerInfo().equals(rs.getServerInfo())) { + } else if (kickReason != null) { // Already connected to the server being disconnected from. - if (kickReason != null) { - server.getEventManager().fire( - new KickedFromServerEvent(this, rs, kickReason, !alreadyConnected, friendlyReason)) - .thenAcceptAsync(event -> { - if (event.getResult() instanceof KickedFromServerEvent.DisconnectPlayer) { - KickedFromServerEvent.DisconnectPlayer res = (KickedFromServerEvent.DisconnectPlayer) event - .getResult(); - connection.closeWith(Disconnect.create(res.getReason())); - } else if (event.getResult() instanceof KickedFromServerEvent.RedirectPlayer) { - KickedFromServerEvent.RedirectPlayer res = (KickedFromServerEvent.RedirectPlayer) event - .getResult(); - createConnectionRequest(res.getServer()).fireAndForget(); + KickedFromServerEvent originalEvent = new KickedFromServerEvent(this, rs, kickReason, + !connectedServer.getServer().equals(rs), friendlyReason); + + server.getEventManager().fire(originalEvent) + .thenAcceptAsync(event -> { + if (event.getResult() instanceof DisconnectPlayer) { + DisconnectPlayer res = (DisconnectPlayer) event.getResult(); + connection.closeWith(Disconnect.create(res.getReason())); + } else if (event.getResult() instanceof RedirectPlayer) { + RedirectPlayer res = (RedirectPlayer) event.getResult(); + createConnectionRequest(res.getServer()).fireAndForget(); + } else if (event.getResult() instanceof Notify) { + Notify res = (Notify) event.getResult(); + if (event.kickedDuringServerConnect()) { + sendMessage(res.getMessage()); } else { - // In case someone gets creative, assume we want to disconnect the player. - connection.closeWith(Disconnect.create(friendlyReason)); + connection.closeWith(Disconnect.create(res.getMessage())); } - }, connection.eventLoop()); - } else { - connection.closeWith(Disconnect.create(friendlyReason)); - } + } else { + // In case someone gets creative, assume we want to disconnect the player. + connection.closeWith(Disconnect.create(friendlyReason)); + } + }, connection.eventLoop()); } else { - connection.write(Chat.createClientbound(friendlyReason)); + connection.closeWith(Disconnect.create(friendlyReason)); } }