Another round of improvements to tab complete. Fix fallback servers.

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

@@ -57,7 +57,7 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
   private final Set<String> knownChannels = new HashSet<>();
   private final Queue<PluginMessage> loginPluginMessages = new ArrayDeque<>();
   private final VelocityServer server;
-  private @Nullable TabCompleteRequest outstandingTabComplete;
+  private @Nullable TabCompleteRequest legacyCommandTabComplete;
 
   public ClientPlaySessionHandler(VelocityServer server, ConnectedPlayer player) {
     this.player = player;
@@ -143,7 +143,63 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
       return false;
     }
 
-    // See if this is a proxy command.
+    if (player.getProtocolVersion().compareTo(MINECRAFT_1_13) >= 0) {
+      return handleTabCompleteModern(packet);
+    } else {
+      return handleTabCompleteLegacy(packet);
+    }
+  }
+
+  private boolean handleTabCompleteModern(TabCompleteRequest packet) {
+    // In 1.13+, we need to do additional work for the richer suggestions available.
+    String command = packet.getCommand().substring(1);
+    int spacePos = command.indexOf(' ');
+    if (spacePos == -1) {
+      return false;
+    }
+
+    String commandLabel = command.substring(0, spacePos);
+    if (!server.getCommandManager().hasCommand(commandLabel)) {
+      return false;
+    }
+
+
+    List<String> suggestions = server.getCommandManager().offerSuggestions(player, command);
+    if (suggestions.isEmpty()) {
+      return false;
+    }
+
+    List<Offer> offers = new ArrayList<>();
+    int longestLength = 0;
+    for (String suggestion : suggestions) {
+      offers.add(new Offer(suggestion));
+      if (suggestion.length() > longestLength) {
+        longestLength = suggestion.length();
+      }
+    }
+
+    TabCompleteResponse resp = new TabCompleteResponse();
+    resp.setTransactionId(packet.getTransactionId());
+
+    int startPos = packet.getCommand().lastIndexOf(' ') + 1;
+    int length;
+    if (startPos == 0) {
+      startPos = packet.getCommand().length() + 1;
+      length = longestLength;
+    } else {
+      length = packet.getCommand().substring(startPos).indexOf(' ') + 1;
+    }
+
+    resp.setStart(startPos);
+    resp.setLength(length);
+    resp.getOffers().addAll(offers);
+
+    player.getMinecraftConnection().write(resp);
+    return true;
+  }
+
+  private boolean handleTabCompleteLegacy(TabCompleteRequest packet) {
+    // Let us check for a possible proxy command.
     String command = packet.getCommand().substring(1);
     int spacePos = command.indexOf(' ');
     if (spacePos >= 0) {
@@ -151,18 +207,11 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
       if (server.getCommandManager().hasCommand(commandLabel)) {
         List<String> suggestions = server.getCommandManager().offerSuggestions(player, command);
         if (!suggestions.isEmpty()) {
-          int longestLength = 0;
           List<Offer> offers = new ArrayList<>();
           for (String suggestion : suggestions) {
-            offers.add(new Offer(suggestion, null));
+            offers.add(new Offer(suggestion));
-            if (suggestion.length() > longestLength) {
-              longestLength = suggestion.length();
-            }
           }
           TabCompleteResponse resp = new TabCompleteResponse();
-          resp.setTransactionId(packet.getTransactionId());
-          resp.setStart(command.lastIndexOf(' ') + 2);
-          resp.setLength(longestLength);
           resp.getOffers().addAll(offers);
 
           player.getMinecraftConnection().write(resp);
@@ -171,13 +220,10 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
       }
     }
 
-    boolean is113 = player.getProtocolVersion().compareTo(MINECRAFT_1_13) >= 0;
+    // Outstanding tab completes are recorded for use with 1.12 clients and below to provide
-    if (!is113) {
+    // tab list completion support for command names. In 1.13, Brigadier handles everything for
-      // Outstanding tab completes are recorded for use with 1.12 clients and below to provide
+    // us.
-      // tab list completion support for command names. In 1.13, Brigadier handles everything for
+    legacyCommandTabComplete = packet;
-      // us.
-      outstandingTabComplete = packet;
-    }
 
     return false;
   }
@@ -398,23 +444,20 @@ public class ClientPlaySessionHandler implements MinecraftSessionHandler {
    * @param response the tab complete response from the backend
    */
   public void handleTabCompleteResponse(TabCompleteResponse response) {
-    if (outstandingTabComplete != null) {
+    if (legacyCommandTabComplete != null) {
-      if (!outstandingTabComplete.isAssumeCommand()
+      String command = legacyCommandTabComplete.getCommand().substring(1);
-          && outstandingTabComplete.getCommand().startsWith("/")) {
+      try {
-        String command = outstandingTabComplete.getCommand().substring(1);
+        List<String> offers = server.getCommandManager().offerSuggestions(player, command);
-        try {
+        for (String offer : offers) {
-          List<String> offers = server.getCommandManager().offerSuggestions(player, command);
+          response.getOffers().add(new Offer(offer, null));
-          for (String offer : offers) {
-            response.getOffers().add(new Offer(offer, null));
-          }
-          response.getOffers().sort(null);
-        } catch (Exception e) {
-          logger.error("Unable to provide tab list completions for {} for command '{}'",
-              player.getUsername(),
-              command, e);
         }
-        outstandingTabComplete = null;
+        response.getOffers().sort(null);
+      } catch (Exception e) {
+        logger.error("Unable to provide tab list completions for {} for command '{}'",
+            player.getUsername(),
+            command, e);
       }
+      legacyCommandTabComplete = null;
     }
 
     player.getMinecraftConnection().write(response);

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

@@ -357,13 +357,13 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player {
 
   private void handleConnectionException(RegisteredServer rs, @Nullable Component kickReason,
       Component friendlyReason) {
-    // There can't be any connection in flight now.
-    connectionInFlight = null;
-
     if (connectedServer == null) {
       // The player isn't yet connected to a server.
-      Optional<RegisteredServer> nextServer = getNextServerToTry();
+      Optional<RegisteredServer> nextServer = getNextServerToTry(rs);
       if (nextServer.isPresent()) {
+        // There can't be any connection in flight now.
+        connectionInFlight = null;
+
         createConnectionRequest(nextServer.get()).fireAndForget();
       } else {
         disconnect(friendlyReason);
@@ -372,7 +372,7 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player {
       boolean kickedFromCurrent = connectedServer.getServer().equals(rs);
       ServerKickResult result;
       if (kickedFromCurrent) {
-        Optional<RegisteredServer> next = getNextServerToTry();
+        Optional<RegisteredServer> next = getNextServerToTry(rs);
         result = next.<ServerKickResult>map(RedirectPlayer::create)
             .orElseGet(() -> DisconnectPlayer.create(friendlyReason));
       } else {
@@ -392,6 +392,9 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player {
             DisconnectPlayer res = (DisconnectPlayer) event.getResult();
             disconnect(res.getReason());
           } else if (event.getResult() instanceof RedirectPlayer) {
+            // There can't be any connection in flight now.
+            connectionInFlight = null;
+
             RedirectPlayer res = (RedirectPlayer) event.getResult();
             createConnectionRequest(res.getServer())
                 .connectWithIndication()
@@ -419,9 +422,22 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player {
   /**
    * Finds another server to attempt to log into, if we were unexpectedly disconnected from the
    * server.
+   *
    * @return the next server to try
    */
   public Optional<RegisteredServer> getNextServerToTry() {
+    return this.getNextServerToTry(null);
+  }
+
+  /**
+   * Finds another server to attempt to log into, if we were unexpectedly disconnected from the
+   * server.
+   *
+   * @param current the "current" server that the player is on, useful as an override
+   *
+   * @return the next server to try
+   */
+  private Optional<RegisteredServer> getNextServerToTry(@Nullable RegisteredServer current) {
     if (serversToTry == null) {
       String virtualHostStr = getVirtualHost().map(InetSocketAddress::getHostString).orElse("");
       serversToTry = server.getConfiguration().getForcedHosts().getOrDefault(virtualHostStr,
@@ -432,17 +448,24 @@ public class ConnectedPlayer implements MinecraftConnectionAssociation, Player {
       serversToTry = server.getConfiguration().getAttemptConnectionOrder();
     }
 
-    for (; tryIndex < serversToTry.size(); tryIndex++) {
+    for (int i = tryIndex; i < serversToTry.size(); i++) {
-      String toTryName = serversToTry.get(tryIndex);
+      String toTryName = serversToTry.get(i);
-      if (connectedServer != null && toTryName.equals(connectedServer.getServerInfo().getName())) {
+      if ((connectedServer != null && hasSameName(connectedServer.getServer(), toTryName))
+          || (connectionInFlight != null && hasSameName(connectionInFlight.getServer(), toTryName))
+          || (current != null && hasSameName(current, toTryName))) {
         continue;
       }
 
+      tryIndex = i;
       return server.getServer(toTryName);
     }
     return Optional.empty();
   }
 
+  private static boolean hasSameName(RegisteredServer server, String name) {
+    return server.getServerInfo().getName().equalsIgnoreCase(name);
+  }
+
   /**
    * Sets the player's new connected server and clears the in-flight connection.
    *

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/TabCompleteRequest.java

@@ -38,7 +38,7 @@ public class TabCompleteRequest implements MinecraftPacket {
     this.assumeCommand = assumeCommand;
   }
 
-  public boolean isHasPosition() {
+  public boolean hasPosition() {
     return hasPosition;
   }
 

proxy/src/main/java/com/velocitypowered/proxy/protocol/packet/TabCompleteResponse.java

@@ -112,6 +112,10 @@ public class TabCompleteResponse implements MinecraftPacket {
     @Nullable
     private final Component tooltip;
 
+    public Offer(String text) {
+      this(text, null);
+    }
+
     public Offer(String text,
         @Nullable Component tooltip) {
       this.text = text;