Merge branch 'master' into 1.13-tab-complete

api/src/main/java/com/velocitypowered/api/proxy/server/ServerPing.java

@@ -11,7 +11,6 @@ import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
 
-import com.velocitypowered.api.network.ProtocolVersion;
 import net.kyori.text.Component;
 import org.checkerframework.checker.nullness.qual.Nullable;
 

proxy/src/main/java/com/velocitypowered/proxy/Velocity.java

@@ -9,24 +9,27 @@ public class Velocity {
   private static final Logger logger = LogManager.getLogger(Velocity.class);
 
   static {
-    // We use BufferedImage for favicons, and on macOS this puts the Java application in the dock. How inconvenient.
+    // We use BufferedImage for favicons, and on macOS this puts the Java application in the dock.
-    // Force AWT to work with its head chopped off.
+    // How inconvenient. Force AWT to work with its head chopped off.
     System.setProperty("java.awt.headless", "true");
   }
 
+  /**
+   * Main method that the JVM will call when {@code java -jar velocity.jar} is executed.
+   * @param args the arguments to the proxy
+   */
   public static void main(String... args) {
-    long startTime = System.currentTimeMillis();
-
     final ProxyOptions options = new ProxyOptions(args);
-
     if (options.isHelp()) {
       return;
     }
 
+    long startTime = System.currentTimeMillis();
+
     VelocityServer server = new VelocityServer(options);
     server.start();
-
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> server.shutdown(false),
-    Runtime.getRuntime().addShutdownHook(new Thread(() -> server.shutdown(false), "Shutdown thread"));
+        "Shutdown thread"));
 
     double bootTime = (System.currentTimeMillis() - startTime) / 1000d;
     logger.info("Done ({}s)!", new DecimalFormat("#.##").format(bootTime));

proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java

@@ -144,7 +144,7 @@ public class VelocityServer implements ProxyServer {
 
   @EnsuresNonNull({"serverKeyPair", "servers", "pluginManager", "eventManager", "scheduler",
       "console", "cm", "configuration"})
-  public void start() {
+  void start() {
     logger.info("Booting up {} {}...", getVersion().getName(), getVersion().getVersion());
 
     serverKeyPair = EncryptionUtils.createRsaKeyPair(1024);
@@ -160,12 +160,12 @@ public class VelocityServer implements ProxyServer {
       Path configPath = Paths.get("velocity.toml");
       configuration = VelocityConfiguration.read(configPath);
 
-      AnnotatedConfig
+      // Resave config to add new values
-              .saveConfig(configuration.dumpConfig(), configPath); // Resave config to add new values
+      AnnotatedConfig.saveConfig(configuration.dumpConfig(), configPath);
 
       if (!configuration.validate()) {
-        logger.error(
+        logger.error("Your configuration is invalid. Velocity will not start up until the errors "
-            "Your configuration is invalid. Velocity will refuse to start up until the errors are resolved.");
+            + "are resolved.");
         LogManager.shutdown();
         System.exit(1);
       }

proxy/src/main/java/com/velocitypowered/proxy/command/VelocityCommand.java

@@ -231,7 +231,6 @@ public class VelocityCommand implements Command {
     }
 
     private TextComponent componentForPlugin(PluginDescription description) {
-      TextComponent pluginSelf = TextComponent.of(description.getId(), TextColor.GRAY);
       String pluginInfo = description.getName().orElse(description.getId())
           + description.getVersion().map(v -> " " + v).orElse("");
 
@@ -256,7 +255,8 @@ public class VelocityCommand implements Command {
         hoverText.append(TextComponent.of(pdesc));
       });
 
-      return pluginSelf.hoverEvent(new HoverEvent(Action.SHOW_TEXT, hoverText.build()));
+      return TextComponent.of(description.getId(), TextColor.GRAY)
+          .hoverEvent(new HoverEvent(Action.SHOW_TEXT, hoverText.build()));
     }
 
     @Override

proxy/src/main/java/com/velocitypowered/proxy/config/AnnotatedConfig.java

@@ -141,8 +141,7 @@ public abstract class AnnotatedConfig {
           @SuppressWarnings("unchecked")
           Map<String, ?> map = (Map<String, ?>) value;
           for (Entry<String, ?> entry : map.entrySet()) {
-            lines.add(
+            lines.add(escapeKeyIfNeeded(entry.getKey()) + " = " + serialize(entry.getValue()));
-                escapeKeyIfNeeded(entry.getKey()) + " = " + serialize(entry.getValue())); // Save map data
           }
           lines.add(""); // Add empty line
           continue;
@@ -165,7 +164,7 @@ public abstract class AnnotatedConfig {
   }
 
   /**
-   * Serializes <pre>value</pre> so it could be parsed by TOML specification
+   * Serializes <pre>value</pre> so it can be parsed as a TOML value.
    *
    * @param value object to serialize
    * @return Serialized object

proxy/src/main/java/com/velocitypowered/proxy/config/VelocityConfiguration.java

@@ -17,6 +17,7 @@ import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Random;
@@ -32,7 +33,8 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
   @ConfigKey("config-version")
   private final String configVersion = "1.0";
 
-  @Comment("What port should the proxy be bound to? By default, we'll bind to all addresses on port 25577.")
+  @Comment("What port should the proxy be bound to? By default, we'll bind to all addresses on"
+      + " port 25577.")
   private String bind = "0.0.0.0:25577";
 
   @Comment({"What should be the MOTD? This gets displayed when the player adds your server to",
@@ -53,12 +55,12 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
   @Comment({
       "Should we forward IP addresses and other data to backend servers?",
       "Available options:",
-      "- \"none\":   No forwarding will be done. All players will appear to be connecting from the proxy",
+      "- \"none\":   No forwarding will be done. All players will appear to be connecting from the",
-      "            and will have offline-mode UUIDs.",
+      "            proxy and will have offline-mode UUIDs.",
-      "- \"legacy\": Forward player IPs and UUIDs in BungeeCord-compatible fashion. Use this if you run",
+      "- \"legacy\": Forward player IPs and UUIDs in a BungeeCord-compatible format. Use this if",
-      "            servers using Minecraft 1.12 or lower.",
+      "            you run servers using Minecraft 1.12 or lower.",
-      "- \"modern\": Forward player IPs and UUIDs as part of the login process using Velocity's native",
+      "- \"modern\": Forward player IPs and UUIDs as part of the login process using Velocity's ",
-      "            forwarding. Only applicable for Minecraft 1.13 or higher."
+      "            native forwarding. Only applicable for Minecraft 1.13 or higher."
   })
   @ConfigKey("player-info-forwarding-mode")
   private PlayerInfoForwarding playerInfoForwardingMode = PlayerInfoForwarding.NONE;
@@ -68,7 +70,8 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
   @ConfigKey("forwarding-secret")
   private byte[] forwardingSecret = generateRandomString(12).getBytes(StandardCharsets.UTF_8);
 
-  @Comment("Announce whether or not your server supports Forge/FML. If you run a modded server, we suggest turning this on.")
+  @Comment({"Announce whether or not your server supports Forge. If you run a modded server, we",
+      "suggest turning this on."})
   @ConfigKey("announce-forge")
   private boolean announceForge = false;
 
@@ -90,7 +93,7 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
   @Ignore
   private @Nullable Favicon favicon;
 
-  public VelocityConfiguration(Servers servers, ForcedHosts forcedHosts, Advanced advanced,
+  private VelocityConfiguration(Servers servers, ForcedHosts forcedHosts, Advanced advanced,
       Query query) {
     this.servers = servers;
     this.forcedHosts = forcedHosts;
@@ -114,6 +117,10 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
     this.query = query;
   }
 
+  /**
+   * Attempts to validate the configuration.
+   * @return {@code true} if the configuration is sound, {@code false} if not
+   */
   public boolean validate() {
     boolean valid = true;
     Logger logger = AnnotatedConfig.getLogger();
@@ -136,8 +143,8 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
 
     switch (playerInfoForwardingMode) {
       case NONE:
-        logger.warn(
+        logger.warn("Player info forwarding is disabled! All players will appear to be connecting "
-            "Player info forwarding is disabled! All players will appear to be connecting from the proxy and will have offline-mode UUIDs.");
+            + "from the proxy and will have offline-mode UUIDs.");
         break;
       case MODERN:
         if (forwardingSecret == null || forwardingSecret.length == 0) {
@@ -145,6 +152,8 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
           valid = false;
         }
         break;
+      default:
+        break;
     }
 
     if (servers.getServers().isEmpty()) {
@@ -199,16 +208,16 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
       logger.error("Invalid compression level {}", advanced.compressionLevel);
       valid = false;
     } else if (advanced.compressionLevel == 0) {
-      logger.warn(
+      logger.warn("ALL packets going through the proxy will be uncompressed. This will increase "
-          "ALL packets going through the proxy will be uncompressed. This will increase bandwidth usage.");
+          + "bandwidth usage.");
     }
 
     if (advanced.compressionThreshold < -1) {
       logger.error("Invalid compression threshold {}", advanced.compressionLevel);
       valid = false;
     } else if (advanced.compressionThreshold == 0) {
-      logger.warn(
+      logger.warn("ALL packets going through the proxy will be compressed. This will compromise "
-          "ALL packets going through the proxy will be compressed. This will compromise throughput and increase CPU usage!");
+          + "throughput and increase CPU usage!");
     }
 
     if (advanced.loginRatelimit < 0) {
@@ -366,14 +375,16 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
     byte[] forwardingSecret = toml.getString("forwarding-secret", "5up3r53cr3t")
         .getBytes(StandardCharsets.UTF_8);
 
+    String forwardingModeName = toml.getString("player-info-forwarding-mode", "MODERN")
+        .toUpperCase(Locale.US);
+
     VelocityConfiguration configuration = new VelocityConfiguration(
         toml.getString("bind", "0.0.0.0:25577"),
         toml.getString("motd", "&3A Velocity Server"),
         toml.getLong("show-max-players", 500L).intValue(),
         toml.getBoolean("online-mode", true),
         toml.getBoolean("announce-forge", false),
-        PlayerInfoForwarding
+        PlayerInfoForwarding.valueOf(forwardingModeName),
-            .valueOf(toml.getString("player-info-forwarding-mode", "MODERN").toUpperCase()),
         forwardingSecret,
         servers,
         forcedHosts,
@@ -385,20 +396,15 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
   }
 
   private static void upgradeConfig(VelocityConfiguration configuration, Toml toml) {
-    switch (toml.getString("config-version", configuration.configVersion)) {
+    // Will be implemented once there has been a backwards-incompatible change in the config file
-      case "1.0":
+    // format.
-        //TODO: Upgrade a 1.0 config to a new version. Maybe add a recursive support in future.
-        break;
-      default:
-        break;
-    }
   }
 
-  private static String generateRandomString(int lenght) {
+  private static String generateRandomString(int length) {
     String chars = "AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz1234567890";
     StringBuilder builder = new StringBuilder();
     Random rnd = new Random();
-    for (int i = 0; i < lenght; i++) {
+    for (int i = 0; i < length; i++) {
       builder.append(chars.charAt(rnd.nextInt(chars.length())));
     }
     return builder.toString();
@@ -489,8 +495,8 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
         Map<String, List<String>> forcedHosts = new HashMap<>();
         for (Map.Entry<String, Object> entry : toml.entrySet()) {
           if (entry.getValue() instanceof String) {
-            forcedHosts
+            forcedHosts.put(unescapeKeyIfNeeded(entry.getKey()), ImmutableList.of(
-                .put(unescapeKeyIfNeeded(entry.getKey()), ImmutableList.of((String) entry.getValue()));
+                (String) entry.getValue()));
           } else if (entry.getValue() instanceof List) {
             forcedHosts.put(unescapeKeyIfNeeded(entry.getKey()),
                 ImmutableList.copyOf((List<String>) entry.getValue()));
@@ -532,14 +538,14 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
     @ConfigKey("compression-threshold")
     private int compressionThreshold = 1024;
 
-    @Comment({"How much compression should be done (from 0-9). The default is -1, which uses zlib's",
+    @Comment({"How much compression should be done (from 0-9). The default is -1, which uses the",
         "default level of 6."})
     @ConfigKey("compression-level")
     private int compressionLevel = -1;
 
     @Comment({
-        "How fast (in miliseconds) are clients allowed to connect after the last connection? Default: 3000",
+        "How fast (in milliseconds) are clients allowed to connect after the last connection? By",
-        "Disable by setting to 0"
+        "default, this is three seconds. Disable this by setting this to 0."
     })
     @ConfigKey("login-ratelimit")
     private int loginRatelimit = 3000;
@@ -610,11 +616,11 @@ public class VelocityConfiguration extends AnnotatedConfig implements ProxyConfi
 
   private static class Query {
 
-    @Comment("Whether to enable responding to GameSpy 4 query responses or not")
+    @Comment("Whether to enable responding to GameSpy 4 query responses or not.")
     @ConfigKey("enabled")
     private boolean queryEnabled = false;
 
-    @Comment("If query responding is enabled, on what port should query response listener listen on?")
+    @Comment("If query is enabled, on what port should the query protocol listen on?")
     @ConfigKey("port")
     private int queryPort = 25577;
 

proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java

@@ -59,6 +59,11 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
   private final VelocityServer server;
   private ConnectionType connectionType = ConnectionTypes.UNDETERMINED;
 
+  /**
+   * Initializes a new {@link MinecraftConnection} instance.
+   * @param channel the channel on the connection
+   * @param server the Velocity instance
+   */
   public MinecraftConnection(Channel channel, VelocityServer server) {
     this.channel = channel;
     this.remoteAddress = channel.remoteAddress();
@@ -151,30 +156,48 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
     return channel.eventLoop();
   }
 
+  /**
+   * Writes and immediately flushes a message to the connection.
+   * @param msg the message to write
+   */
   public void write(Object msg) {
     if (channel.isActive()) {
       channel.writeAndFlush(msg, channel.voidPromise());
     }
   }
 
+  /**
+   * Writes, but does not flush, a message to the connection.
+   * @param msg the message to write
+   */
   public void delayedWrite(Object msg) {
     if (channel.isActive()) {
       channel.write(msg, channel.voidPromise());
     }
   }
 
+  /**
+   * Flushes the connection.
+   */
   public void flush() {
     if (channel.isActive()) {
       channel.flush();
     }
   }
 
+  /**
+   * Closes the connection after writing the {@code msg}.
+   * @param msg the message to write
+   */
   public void closeWith(Object msg) {
     if (channel.isActive()) {
       channel.writeAndFlush(msg).addListener(ChannelFutureListener.CLOSE);
     }
   }
 
+  /**
+   * Immediately closes the connection.
+   */
   public void close() {
     if (channel.isActive()) {
       channel.close();
@@ -197,6 +220,10 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
     return state;
   }
 
+  /**
+   * Changes the state of the Minecraft connection.
+   * @param state the new state
+   */
   public void setState(StateRegistry state) {
     this.state = state;
     this.channel.pipeline().get(MinecraftEncoder.class).setState(state);
@@ -207,6 +234,10 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
     return protocolVersion;
   }
 
+  /**
+   * Sets the new protocol version for the connection.
+   * @param protocolVersion the protocol version to use
+   */
   public void setProtocolVersion(ProtocolVersion protocolVersion) {
     this.protocolVersion = protocolVersion;
     this.nextProtocolVersion = protocolVersion;
@@ -225,6 +256,10 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
     return sessionHandler;
   }
 
+  /**
+   * Sets the session handler for this connection.
+   * @param sessionHandler the handler to use
+   */
   public void setSessionHandler(MinecraftSessionHandler sessionHandler) {
     if (this.sessionHandler != null) {
       this.sessionHandler.deactivated();
@@ -237,6 +272,11 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
     Preconditions.checkState(!isClosed(), "Connection is closed.");
   }
 
+  /**
+   * Sets the compression threshold on the connection. You are responsible for sending
+   * {@link com.velocitypowered.proxy.protocol.packet.SetCompression} beforehand.
+   * @param threshold the compression threshold to use
+   */
   public void setCompressionThreshold(int threshold) {
     ensureOpen();
 
@@ -255,6 +295,11 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
     channel.pipeline().addBefore(MINECRAFT_ENCODER, COMPRESSION_ENCODER, encoder);
   }
 
+  /**
+   * Enables encryption on the connection.
+   * @param secret the secret key negotiated between the client and the server
+   * @throws GeneralSecurityException if encryption can't be enabled
+   */
   public void enableEncryption(byte[] secret) throws GeneralSecurityException {
     ensureOpen();
 
@@ -287,7 +332,7 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
   }
 
   /**
-   * Gets the detected {@link ConnectionType}
+   * Gets the detected {@link ConnectionType}.
    * @return The {@link ConnectionType}
    */
   public ConnectionType getType() {
@@ -295,7 +340,7 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter {
   }
 
   /**
-   * Sets the detected {@link ConnectionType}
+   * Sets the detected {@link ConnectionType}.
    * @param connectionType The {@link ConnectionType}
    */
   public void setType(ConnectionType connectionType) {

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

@@ -26,7 +26,7 @@ public interface BackendConnectionPhase {
   }
 
   /**
-   * Indicates whether the connection is considered complete
+   * Indicates whether the connection is considered complete.
    * @return true if so
    */
   default boolean consideredComplete() {

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

@@ -1,6 +1,7 @@
 package com.velocitypowered.proxy.connection.backend;
 
 import static com.velocitypowered.proxy.VelocityServer.GSON;
+import static com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants.HANDSHAKE_HOSTNAME_TOKEN;
 import static com.velocitypowered.proxy.network.Connections.FRAME_DECODER;
 import static com.velocitypowered.proxy.network.Connections.FRAME_ENCODER;
 import static com.velocitypowered.proxy.network.Connections.HANDLER;
@@ -10,19 +11,19 @@ import static com.velocitypowered.proxy.network.Connections.READ_TIMEOUT;
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.VerifyException;
+import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.api.proxy.ConnectionRequestBuilder;
 import com.velocitypowered.api.proxy.ServerConnection;
 import com.velocitypowered.api.proxy.messages.ChannelIdentifier;
 import com.velocitypowered.api.proxy.server.ServerInfo;
-import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.proxy.VelocityServer;
 import com.velocitypowered.proxy.config.PlayerInfoForwarding;
 import com.velocitypowered.proxy.connection.ConnectionTypes;
 import com.velocitypowered.proxy.connection.MinecraftConnection;
 import com.velocitypowered.proxy.connection.MinecraftConnectionAssociation;
 import com.velocitypowered.proxy.connection.client.ConnectedPlayer;
-import com.velocitypowered.proxy.protocol.ProtocolUtils;
 import com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants;
+import com.velocitypowered.proxy.protocol.ProtocolUtils;
 import com.velocitypowered.proxy.protocol.StateRegistry;
 import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder;
 import com.velocitypowered.proxy.protocol.netty.MinecraftEncoder;
@@ -75,22 +76,15 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation,
                     new MinecraftDecoder(ProtocolUtils.Direction.CLIENTBOUND))
                 .addLast(MINECRAFT_ENCODER,
                     new MinecraftEncoder(ProtocolUtils.Direction.SERVERBOUND));
-
-            MinecraftConnection mc = new MinecraftConnection(ch, server);
-            mc.setState(StateRegistry.HANDSHAKE);
-            mc.setAssociation(VelocityServerConnection.this);
-            ch.pipeline().addLast(HANDLER, mc);
           }
         })
         .connect(registeredServer.getServerInfo().getAddress())
         .addListener((ChannelFutureListener) future -> {
           if (future.isSuccess()) {
-            connection = future.channel().pipeline().get(MinecraftConnection.class);
+            connection = new MinecraftConnection(future.channel(), server);
-
+            connection.setState(StateRegistry.HANDSHAKE);
-            // This is guaranteed not to be null, but Checker Framework is whining about it anyway
+            connection.setAssociation(VelocityServerConnection.this);
-            if (connection == null) {
+            future.channel().pipeline().addLast(HANDLER, connection);
-              throw new VerifyException("MinecraftConnection not injected into pipeline");
-            }
 
             // Kick off the connection process
             connection.setSessionHandler(
@@ -133,21 +127,21 @@ public class VelocityServerConnection implements MinecraftConnectionAssociation,
 
     PlayerInfoForwarding forwardingMode = server.getConfiguration().getPlayerInfoForwardingMode();
 
-    // Initiate a handshake.
+    // Initiate the handshake.
+    ProtocolVersion protocolVersion = proxyPlayer.getConnection().getNextProtocolVersion();
     Handshake handshake = new Handshake();
     handshake.setNextStatus(StateRegistry.LOGIN_ID);
-    handshake.setProtocolVersion(proxyPlayer.getConnection().getNextProtocolVersion());
+    handshake.setProtocolVersion(protocolVersion);
     if (forwardingMode == PlayerInfoForwarding.LEGACY) {
       handshake.setServerAddress(createLegacyForwardingAddress());
     } else if (proxyPlayer.getConnection().getType() == ConnectionTypes.LEGACY_FORGE) {
-      handshake.setServerAddress(handshake.getServerAddress() + LegacyForgeConstants.HANDSHAKE_HOSTNAME_TOKEN);
+      handshake.setServerAddress(handshake.getServerAddress() + HANDSHAKE_HOSTNAME_TOKEN);
     } else {
       handshake.setServerAddress(registeredServer.getServerInfo().getAddress().getHostString());
     }
     handshake.setPort(registeredServer.getServerInfo().getAddress().getPort());
     mc.write(handshake);
 
-    ProtocolVersion protocolVersion = proxyPlayer.getConnection().getNextProtocolVersion();
     mc.setProtocolVersion(protocolVersion);
     mc.setState(StateRegistry.LOGIN);
     mc.write(new ServerLogin(proxyPlayer.getUsername()));

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

@@ -4,8 +4,8 @@ import static com.velocitypowered.api.network.ProtocolVersion.MINECRAFT_1_13;
 
 import com.velocitypowered.api.event.connection.PluginMessageEvent;
 import com.velocitypowered.api.event.player.PlayerChatEvent;
-import com.velocitypowered.api.proxy.messages.ChannelIdentifier;
 import com.velocitypowered.api.network.ProtocolVersion;
+import com.velocitypowered.api.proxy.messages.ChannelIdentifier;
 import com.velocitypowered.proxy.VelocityServer;
 import com.velocitypowered.proxy.connection.MinecraftConnection;
 import com.velocitypowered.proxy.connection.MinecraftSessionHandler;
@@ -24,7 +24,6 @@ import com.velocitypowered.proxy.protocol.packet.TabCompleteResponse;
 import com.velocitypowered.proxy.protocol.packet.TabCompleteResponse.Offer;
 import com.velocitypowered.proxy.protocol.packet.TitlePacket;
 import com.velocitypowered.proxy.protocol.util.PluginMessageUtil;
-import com.velocitypowered.proxy.util.ThrowableUtils;
 import io.netty.buffer.ByteBuf;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
@@ -201,18 +200,15 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
         knownChannels.removeAll(channels);
         backendConn.write(packet);
       } else if (PluginMessageUtil.isMcBrand(packet)) {
-        PluginMessage rewritten = PluginMessageUtil.rewriteMinecraftBrand(packet, server.getVersion());
+        backendConn.write(PluginMessageUtil.rewriteMinecraftBrand(packet, server.getVersion()));
-        backendConn.write(rewritten);
       } else if (!player.getPhase().handle(player, this, packet)) {
-
+        if (!player.getPhase().consideredComplete() || !serverConn.getPhase()
-        if (!player.getPhase().consideredComplete()
+            .consideredComplete()) {
-            || !serverConn.getPhase().consideredComplete()) {
+          // The client is trying to send messages too early. This is primarily caused by mods, but
-
+          // it's further aggravated by Velocity. To work around these issues, we will queue any
-            // The client is trying to send messages too early. This is primarily caused by mods, but
+          // non-FML handshake messages to be sent once the FML handshake has completed or the
-            // it's further aggravated by Velocity. To work around these issues, we will queue any
+          // JoinGame packet has been received by the proxy, whichever comes first.
-            // non-FML handshake messages to be sent once the FML handshake has completed or the JoinGame
+          loginPluginMessages.add(packet);
-            // packet has been received by the proxy, whichever comes first.
-            loginPluginMessages.add(packet);
         } else {
           ChannelIdentifier id = server.getChannelRegistrar().getFromId(packet.getChannel());
           if (id == null) {
@@ -265,11 +261,8 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
 
   @Override
   public void exception(Throwable throwable) {
-    player.disconnect(TextComponent.builder()
+    player.disconnect(TextComponent.of("Your connection has encountered an error. Try again later.",
-        .content("An exception occurred in your connection: ")
+        TextColor.RED));
-        .color(TextColor.RED)
-        .append(TextComponent.of(ThrowableUtils.briefDescription(throwable), TextColor.WHITE))
-        .build());
   }
 
   @Override

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

@@ -10,6 +10,7 @@ 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;
+import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.api.permission.PermissionFunction;
 import com.velocitypowered.api.permission.PermissionProvider;
 import com.velocitypowered.api.permission.Tristate;
@@ -22,7 +23,6 @@ import com.velocitypowered.api.proxy.server.RegisteredServer;
 import com.velocitypowered.api.util.GameProfile;
 import com.velocitypowered.api.util.MessagePosition;
 import com.velocitypowered.api.util.ModInfo;
-import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.api.util.title.TextTitle;
 import com.velocitypowered.api.util.title.Title;
 import com.velocitypowered.api.util.title.Titles;
@@ -41,7 +41,6 @@ import com.velocitypowered.proxy.protocol.packet.PluginMessage;
 import com.velocitypowered.proxy.protocol.packet.TitlePacket;
 import com.velocitypowered.proxy.server.VelocityRegisteredServer;
 import com.velocitypowered.proxy.tablist.VelocityTabList;
-import com.velocitypowered.proxy.util.ThrowableUtils;
 import java.net.InetSocketAddress;
 import java.util.Collections;
 import java.util.List;
@@ -383,8 +382,8 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player {
   public Optional<RegisteredServer> getNextServerToTry() {
     if (serversToTry == null) {
       String virtualHostStr = getVirtualHost().map(InetSocketAddress::getHostString).orElse("");
-      serversToTry = server.getConfiguration().getForcedHosts()
+      serversToTry = server.getConfiguration().getForcedHosts().getOrDefault(virtualHostStr,
-          .getOrDefault(virtualHostStr, Collections.emptyList());
+          Collections.emptyList());
     }
 
     if (serversToTry.isEmpty()) {

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

@@ -128,7 +128,7 @@ public class HandshakeSessionHandler implements MinecraftSessionHandler {
   private ConnectionType checkForForge(Handshake handshake) {
     // Determine if we're using Forge (1.8 to 1.12, may not be the case in 1.13).
     if (handshake.getServerAddress().endsWith(LegacyForgeConstants.HANDSHAKE_HOSTNAME_TOKEN)
-        && handshake.getProtocolVersion().getProtocol() < ProtocolVersion.MINECRAFT_1_13.getProtocol()) {
+        && handshake.getProtocolVersion().compareTo(ProtocolVersion.MINECRAFT_1_13) < 0) {
       return ConnectionTypes.LEGACY_FORGE;
     } else {
       // For later: See if we can determine Forge 1.13+ here, else this will need to be UNDETERMINED

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

@@ -4,6 +4,8 @@ import static com.velocitypowered.api.network.ProtocolVersion.MINECRAFT_1_13;
 import static com.velocitypowered.proxy.VelocityServer.GSON;
 import static com.velocitypowered.proxy.connection.VelocityConstants.EMPTY_BYTE_ARRAY;
 import static com.velocitypowered.proxy.connection.VelocityConstants.VELOCITY_IP_FORWARDING_CHANNEL;
+import static com.velocitypowered.proxy.util.EncryptionUtils.decryptRsa;
+import static com.velocitypowered.proxy.util.EncryptionUtils.generateServerId;
 
 import com.google.common.base.Preconditions;
 import com.google.common.net.UrlEscapers;
@@ -109,16 +111,13 @@ public class LoginSessionHandler implements MinecraftSessionHandler {
 
     try {
       KeyPair serverKeyPair = server.getServerKeyPair();
-      byte[] decryptedVerifyToken = EncryptionUtils
+      byte[] decryptedVerifyToken = decryptRsa(serverKeyPair, packet.getVerifyToken());
-          .decryptRsa(serverKeyPair, packet.getVerifyToken());
       if (!Arrays.equals(verify, decryptedVerifyToken)) {
         throw new IllegalStateException("Unable to successfully decrypt the verification token.");
       }
 
-      byte[] decryptedSharedSecret = EncryptionUtils
+      byte[] decryptedSharedSecret = decryptRsa(serverKeyPair, packet.getSharedSecret());
-          .decryptRsa(serverKeyPair, packet.getSharedSecret());
+      String serverId = generateServerId(decryptedSharedSecret, serverKeyPair.getPublic());
-      String serverId = EncryptionUtils
-          .generateServerId(decryptedSharedSecret, serverKeyPair.getPublic());
 
       String playerIp = ((InetSocketAddress) inbound.getRemoteAddress()).getHostString();
       String url = String.format(MOJANG_HASJOINED_URL,

proxy/src/main/java/com/velocitypowered/proxy/connection/forge/legacy/LegacyForgeConnectionType.java

@@ -6,7 +6,7 @@ import com.velocitypowered.proxy.connection.ConnectionTypes;
 import com.velocitypowered.proxy.connection.util.ConnectionTypeImpl;
 
 /**
- * Contains extra logic for {@link ConnectionTypes#LEGACY_FORGE}
+ * Contains extra logic for {@link ConnectionTypes#LEGACY_FORGE}.
  */
 public class LegacyForgeConnectionType extends ConnectionTypeImpl {
 
@@ -18,7 +18,8 @@ public class LegacyForgeConnectionType extends ConnectionTypeImpl {
   }
 
   @Override
-  public GameProfile addGameProfileTokensIfRequired(GameProfile original, PlayerInfoForwarding forwardingType) {
+  public GameProfile addGameProfileTokensIfRequired(GameProfile original,
+      PlayerInfoForwarding forwardingType) {
     // We can't forward the FML token to the server when we are running in legacy forwarding mode,
     // since both use the "hostname" field in the handshake. We add a special property to the
     // profile instead, which will be ignored by non-Forge servers and can be intercepted by a

proxy/src/main/java/com/velocitypowered/proxy/connection/forge/legacy/LegacyForgeConstants.java

@@ -48,7 +48,7 @@ public class LegacyForgeConstants {
   static final int REGISTRY_DISCRIMINATOR = 3;
 
   /**
-   * The form of the data for the reset packet
+   * The payload for the reset packet.
    */
   static final byte[] FORGE_LEGACY_HANDSHAKE_RESET_DATA = new byte[]{RESET_DATA_DISCRIMINATOR, 0};
 

proxy/src/main/java/com/velocitypowered/proxy/connection/forge/legacy/LegacyForgeHandshakeBackendPhase.java

@@ -15,7 +15,7 @@ import javax.annotation.Nullable;
 public enum LegacyForgeHandshakeBackendPhase implements BackendConnectionPhase {
 
   /**
-   * Dummy phase for use with {@link BackendConnectionPhases#UNKNOWN}
+   * Dummy phase for use with {@link BackendConnectionPhases#UNKNOWN}.
    */
   NOT_STARTED(LegacyForgeConstants.SERVER_HELLO_DISCRIMINATOR) {
     @Override

proxy/src/main/java/com/velocitypowered/proxy/connection/forge/legacy/LegacyForgeHandshakeClientPhase.java

@@ -11,14 +11,13 @@ import java.util.List;
 import javax.annotation.Nullable;
 
 /**
- * Allows for simple tracking of the phase that the Legacy
+ * Allows for simple tracking of the phase that the Legacy Forge handshake is in.
- * Forge handshake is in
  */
 public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase {
 
   /**
-   * No handshake packets have yet been sent.
+   * No handshake packets have yet been sent. Transition to {@link #HELLO} when the ClientHello
-   * Transition to {@link #HELLO} when the ClientHello is sent.
+   * is sent.
    */
   NOT_STARTED(LegacyForgeConstants.CLIENT_HELLO_DISCRIMINATOR) {
     @Override
@@ -49,8 +48,8 @@ public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase {
   },
 
   /**
-   * Client and Server exchange pleasantries.
+   * Client and Server exchange pleasantries. Transition to {@link #MOD_LIST} when the ModList is
-   * Transition to {@link #MOD_LIST} when the ModList is sent.
+   * sent.
    */
   HELLO(LegacyForgeConstants.MOD_LIST_DISCRIMINATOR) {
     @Override
@@ -202,7 +201,7 @@ public enum LegacyForgeHandshakeClientPhase implements ClientConnectionPhase {
   }
 
   /**
-   * Handles the phase tasks
+   * Handles the phase tasks.
    *
    * @param player The player
    * @param handler The {@link ClientPlaySessionHandler} that is handling

proxy/src/main/java/com/velocitypowered/proxy/connection/forge/legacy/LegacyForgeUtil.java

@@ -1,5 +1,9 @@
 package com.velocitypowered.proxy.connection.forge.legacy;
 
+import static com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL;
+import static com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_RESET_DATA;
+import static com.velocitypowered.proxy.connection.forge.legacy.LegacyForgeConstants.MOD_LIST_DISCRIMINATOR;
+
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.velocitypowered.api.util.ModInfo;
@@ -16,20 +20,16 @@ class LegacyForgeUtil {
   }
 
   /**
-   * Gets the discriminator from the FML|HS packet (the first byte in the data)
+   * Gets the discriminator from the FML|HS packet (the first byte in the data).
    *
    * @param message The message to analyse
    * @return The discriminator
    */
   static byte getHandshakePacketDiscriminator(PluginMessage message) {
-    Preconditions.checkArgument(
+    Preconditions.checkArgument(message.getChannel().equals(FORGE_LEGACY_HANDSHAKE_CHANNEL));
-        message.getChannel().equals(LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL));
+    byte[] data = message.getData();
-    ByteBuf buf = Unpooled.wrappedBuffer(message.getData());
+    Preconditions.checkArgument(data.length >= 1);
-    try {
+    return data[0];
-      return buf.readByte();
-    } finally {
-      buf.release();
-    }
   }
 
   /**
@@ -41,14 +41,14 @@ class LegacyForgeUtil {
   static List<ModInfo.Mod> readModList(PluginMessage message) {
     Preconditions.checkNotNull(message, "message");
     Preconditions
-        .checkArgument(message.getChannel().equals(LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL),
+        .checkArgument(message.getChannel().equals(FORGE_LEGACY_HANDSHAKE_CHANNEL),
             "message is not a FML HS plugin message");
 
     ByteBuf byteBuf = Unpooled.wrappedBuffer(message.getData());
     try {
       byte discriminator = byteBuf.readByte();
 
-      if (discriminator == LegacyForgeConstants.MOD_LIST_DISCRIMINATOR) {
+      if (discriminator == MOD_LIST_DISCRIMINATOR) {
         ImmutableList.Builder<ModInfo.Mod> mods = ImmutableList.builder();
         int modCount = ProtocolUtils.readVarInt(byteBuf);
 
@@ -74,8 +74,8 @@ class LegacyForgeUtil {
    */
   static PluginMessage resetPacket() {
     PluginMessage msg = new PluginMessage();
-    msg.setChannel(LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_CHANNEL);
+    msg.setChannel(FORGE_LEGACY_HANDSHAKE_CHANNEL);
-    msg.setData(LegacyForgeConstants.FORGE_LEGACY_HANDSHAKE_RESET_DATA.clone());
+    msg.setData(FORGE_LEGACY_HANDSHAKE_RESET_DATA.clone());
     return msg;
   }
 }

proxy/src/main/java/com/velocitypowered/proxy/connection/util/ConnectionRequestResults.java

@@ -15,6 +15,11 @@ public class ConnectionRequestResults {
     throw new AssertionError();
   }
 
+  /**
+   * Returns a plain result (one with a status but no reason).
+   * @param status the status to use
+   * @return the result
+   */
   public static ConnectionRequestBuilder.Result plainResult(
       ConnectionRequestBuilder.Status status) {
     return new ConnectionRequestBuilder.Result() {
@@ -35,6 +40,11 @@ public class ConnectionRequestResults {
     return forDisconnect(deserialized);
   }
 
+  /**
+   * Returns a disconnect result with a reason.
+   * @param component the reason for disconnecting from the server
+   * @return the result
+   */
   public static ConnectionRequestBuilder.Result forDisconnect(Component component) {
     return new ConnectionRequestBuilder.Result() {
       @Override

proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityEventManager.java

@@ -45,8 +45,12 @@ public class VelocityEventManager implements EventManager {
   private final PluginManager pluginManager;
 
   public VelocityEventManager(PluginManager pluginManager) {
+    // Expose the event executors to the plugins - required in order for the generated ASM classes
+    // to work.
     PluginClassLoader cl = new PluginClassLoader(new URL[0]);
     cl.addToClassloaders();
+
+    // Initialize the event bus.
     this.bus = new SimpleEventBus<Object>(Object.class) {
       @Override
       protected boolean shouldPost(@NonNull Object event, @NonNull EventSubscriber<?> subscriber) {
@@ -126,8 +130,8 @@ public class VelocityEventManager implements EventManager {
   }
 
   private void unregisterHandler(EventHandler<?> handler) {
-    bus.unregister(s -> s instanceof KyoriToVelocityHandler &&
+    bus.unregister(s -> s instanceof KyoriToVelocityHandler
-        ((KyoriToVelocityHandler<?>) s).handler == handler);
+        && ((KyoriToVelocityHandler<?>) s).handler == handler);
   }
 
   @Override

proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java

@@ -3,8 +3,8 @@ package com.velocitypowered.proxy.protocol;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 
-import com.velocitypowered.api.util.GameProfile;
 import com.velocitypowered.api.network.ProtocolVersion;
+import com.velocitypowered.api.util.GameProfile;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufUtil;
 

proxy/src/main/java/com/velocitypowered/proxy/protocol/util/PluginMessageUtil.java

@@ -84,8 +84,8 @@ public class PluginMessageUtil {
   public static PluginMessage constructChannelsPacket(ProtocolVersion protocolVersion,
                                                       Collection<String> channels) {
     Preconditions.checkNotNull(channels, "channels");
-    String channelName = protocolVersion.compareTo(ProtocolVersion.MINECRAFT_1_13) >= 0 ? REGISTER_CHANNEL
+    String channelName = protocolVersion.compareTo(ProtocolVersion.MINECRAFT_1_13) >= 0
-        : REGISTER_CHANNEL_LEGACY;
+        ? REGISTER_CHANNEL : REGISTER_CHANNEL_LEGACY;
     PluginMessage message = new PluginMessage();
     message.setChannel(channelName);
     message.setData(String.join("\0", channels).getBytes(StandardCharsets.UTF_8));

proxy/src/main/java/com/velocitypowered/proxy/server/PingSessionHandler.java

@@ -1,8 +1,8 @@
 package com.velocitypowered.proxy.server;
 
+import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.api.proxy.server.RegisteredServer;
 import com.velocitypowered.api.proxy.server.ServerPing;
-import com.velocitypowered.api.network.ProtocolVersion;
 import com.velocitypowered.proxy.VelocityServer;
 import com.velocitypowered.proxy.connection.MinecraftConnection;
 import com.velocitypowered.proxy.connection.MinecraftSessionHandler;

proxy/src/main/java/com/velocitypowered/proxy/util/ThrowableUtils.java

@@ -1,11 +0,0 @@
-package com.velocitypowered.proxy.util;
-
-public class ThrowableUtils {
-  private ThrowableUtils() {
-    throw new AssertionError();
-  }
-
-  public static String briefDescription(Throwable throwable) {
-    return throwable.getClass().getSimpleName() + ": " + throwable.getMessage();
-  }
-}

proxy/src/test/java/com/velocitypowered/proxy/plugin/util/PluginDependencyUtilsTest.java

@@ -39,8 +39,8 @@ class PluginDependencyUtilsTest {
   private static final PluginDescription CIRCULAR_DEPENDENCY_2 = testDescription("oval",
       ImmutableList.of(new PluginDependency("circle", "", false)));
 
-  // Note: Kahn's algorithm is non-unique in its return result, although the topological sort will have the correct
+  // Note: Kahn's algorithm is non-unique in its return result, although the topological sort will
-  // order.
+  // have the correct order.
   private static final List<PluginDescription> EXPECTED = ImmutableList.of(
       NEVER_DEPENDED,
       NO_DEPENDENCY_1_EXAMPLE,