Merge pull request #5647 from Baaaaaz/feature/5616-initiative-sort-order-improvements

Enchanced Token Initiative sorting.

Author: kwvanderlinde

src/main/java/net/rptools/maptool/model/InitiativeList.java

@@ -22,6 +22,8 @@
 import java.io.Serializable;
 import java.util.*;
 import java.util.concurrent.ExecutionException;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import javax.swing.Icon;
 import net.rptools.maptool.client.AppPreferences;
@@ -476,59 +478,30 @@ public void update() {
 
   /**
    * Sort the tokens by their initiative state according to the default, descending order. See
-   * {@link #sort(boolean)} for more details on handling of strings and nulls.
+   * {@link #sort(boolean)} for more details.
+   *
+   * @see TokenInitiativeDescComparator
    */
   public void sort() {
     this.sort(false);
   }
 
   /**
-   * Sort the tokens by their initiative state, in either ascending or descending order. If the
-   * initiative state string can be converted into a {@link Double} that is done first. All values
-   * converted to {@link Double}s are always considered bigger than the {@link String} values. The
-   * {@link String} values are considered bigger than any <code>null</code> values.
+   * Sort the tokens by their initiative state using a {@link Comparator}
+   *
+   * @param ascendingOrder
+   * @see TokenInitiativeDescComparator
    */
   public void sort(boolean ascendingOrder) {
     startUnitOfWork();
-    final int DIRECTION = ascendingOrder ? -1 : 1;
     TokenInitiative currentInitiative =
         getTokenInitiative(getCurrent()); // Save the currently selected initiative
-    tokens.sort(
-        (o1, o2) -> {
-
-          // Get a number, string, or null for first parameter
-          Object one = null;
-          if (o1.state != null) {
-            one = o1.state;
-            try {
-              one = Double.valueOf(o1.state);
-            } catch (NumberFormatException e) {
-              // Not a number so ignore
-            } // endtry
-          } // endif
-
-          // Repeat for second param
-          Object two = null;
-          if (o2.state != null) {
-            two = o2.state;
-            try {
-              two = Double.valueOf(o2.state);
-            } catch (NumberFormatException e) {
-              // Not a number so ignore
-            } // endtry
-          } // endif
-
-          // Do the comparison
-          if (Objects.equals(one, two)) return 0;
-          if (one == null) return 1 * DIRECTION; // Null is always the smallest value
-          if (two == null) return -1 * DIRECTION;
-          if (one instanceof Double & two instanceof Double)
-            return ((Double) two).compareTo((Double) one) * DIRECTION;
-          if (one instanceof String & two instanceof String)
-            return ((String) two).compareTo((String) one) * DIRECTION;
-          if (one instanceof Double) return -1 * DIRECTION; // Integers are bigger than strings
-          return 1 * DIRECTION;
-        });
+
+    Collections.sort(tokens, new TokenInitiativeDescComparator());
+    if (ascendingOrder) {
+      Collections.reverse(tokens);
+    }
+
     getPCS().firePropertyChange(TOKENS_PROP, null, tokens);
     setCurrent(indexOf(currentInitiative)); // Restore current initiative
     finishUnitOfWork();
@@ -1001,4 +974,100 @@ public TokenInitiativeDto toDto() {
       return dto.build();
     }
   }
+
+  /*---------------------------------------------------------------------------------------------
+   * TokenInitiativeComparator Inner Class
+   *-------------------------------------------------------------------------------------------*/
+
+  /**
+   * Comparator in a helper class to control the ordering of the TokenInitiative list.
+   *
+   * <p>Uses regex {@link Pattern} and {@link Matcher} to try and extract a leading number (to
+   * convert to a {@link Double}) and/or a trailing string from the token's initiative state. Then
+   * it compares in descending fashion by:
+   *
+   * <ol>
+   *   <li>{@link Double}s are always considered bigger than the {@link String} values.
+   *   <li>{@link String} values are considered bigger than any <code>null</code> values.
+   *   <li>if all else is the same, finally compare by token name and then by token ID to get a
+   *       deterministic order to prevent flip-flopping on repeated sorts and/or tokens not changing
+   *       order when sort order is reversed.
+   * </ol>
+   *
+   * @see TokenInitiative
+   */
+  public class TokenInitiativeDescComparator implements Comparator<TokenInitiative> {
+    @Override
+    public int compare(TokenInitiative o1, TokenInitiative o2) {
+      Pattern p = Pattern.compile("^(-?\\d+\\.?\\d*)?(.*)");
+
+      // Get a number and/or string, or a null for first parameter
+      Object one = o1.getState();
+      Double oneDouble = null;
+      String oneString = null;
+      try {
+        Matcher m1 = p.matcher(o1.state);
+        if (m1.matches()) {
+          oneString = m1.group(2).trim();
+          oneDouble = Double.valueOf(m1.group(1));
+        }
+      } catch (NumberFormatException | NullPointerException e) {
+        // either null or could not convert to a Double
+      }
+
+      // Repeat for second param
+      Object two = o2.getState();
+      Double twoDouble = null;
+      String twoString = null;
+      try {
+        Matcher m2 = p.matcher(o2.state);
+        if (m2.matches()) {
+          twoString = m2.group(2).trim();
+          twoDouble = Double.valueOf(m2.group(1));
+        }
+      } catch (NumberFormatException | NullPointerException e) {
+        // either null or could not convert to a Double
+      }
+
+      int comparison = 0;
+      if (Objects.equals(one, two)) {
+        comparison = 0;
+      } else if (one == null) {
+        comparison = 1; // nulls come last
+      } else if (two == null) {
+        comparison = -1;
+      } else if (!Objects.equals(twoDouble, oneDouble)) {
+        // Numbers are different
+        if (oneDouble == null) {
+          comparison = 1;
+        } else if (twoDouble == null) {
+          comparison = -1;
+        } else {
+          comparison = twoDouble.compareTo(oneDouble);
+        }
+      } else {
+        // Numbers are the same, so sort by strings
+        if (oneString == null) {
+          comparison = 1;
+        } else if (twoString == null) {
+          comparison = -1;
+        } else {
+          comparison = twoString.compareToIgnoreCase(oneString);
+          if (comparison == 0) {
+            comparison = twoString.compareTo(oneString);
+          }
+        }
+      }
+
+      // If still tied, compare the names and then the GUID to get a deterministic sort order.
+      if (comparison == 0) {
+        comparison = o1.getToken().getName().compareTo(o2.getToken().getName());
+        if (comparison == 0) {
+          comparison = o1.getToken().getId().compareTo(o2.getToken().getId());
+        }
+      }
+
+      return comparison;
+    }
+  }
 }