Revert "Issues 12447 (Standardized Sorting of UnitCategory) (#12722)"

game-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/BattleCalculatorPanel.java

@@ -417,17 +417,12 @@ class BattleCalculatorPanel extends JPanel {
                   Matches.unitIsOwnedBy(getDefender())
                       .and(
                           Matches.unitCanBeInBattle(
-                              true,
-                              isLandBattle(),
-                              1,
-                              hasMaxRounds(isLandBattle(), data),
-                              true,
-                              List.of())));
+                              true, isLand(), 1, hasMaxRounds(isLand(), data), true, List.of())));
           final GamePlayer newDefender = getAttacker();
           final List<Unit> newDefenders =
               CollectionUtils.getMatches(
                   attackingUnitsPanel.getUnits(),
-                  Matches.unitCanBeInBattle(true, isLandBattle(), 1, true));
+                  Matches.unitCanBeInBattle(true, isLand(), 1, true));
           setAttacker(newAttacker);
           setDefender(newDefender);
           setAttackingUnits(newAttackers);
@@ -440,8 +435,8 @@ class BattleCalculatorPanel extends JPanel {
               new OrderOfLossesInputPanel(
                   attackerOrderOfLosses,
                   defenderOrderOfLosses,
-                  attackingUnitsPanel.getUnitCategories(),
-                  defendingUnitsPanel.getUnitCategories(),
+                  attackingUnitsPanel.getCategories(),
+                  defendingUnitsPanel.getCategories(),
                   landBattleCheckBox.isSelected(),
                   uiContext,
                   data);
@@ -564,12 +559,12 @@ private void updateStats() {
           try {
             final Territory location = findPotentialBattleSite();
             if (location == null) {
-              throw new IllegalStateException("No territory found that is land:" + isLandBattle());
+              throw new IllegalStateException("No territory found that is land:" + isLand());
             }
             final List<Unit> defending = defendingUnitsPanel.getUnits();
             final List<Unit> attacking = attackingUnitsPanel.getUnits();
             List<Unit> bombarding = new ArrayList<>();
-            if (isLandBattle()) {
+            if (isLand()) {
               bombarding =
                   CollectionUtils.getMatches(attacking, Matches.unitCanBombard(getAttacker()));
               attacking.removeAll(bombarding);
@@ -627,7 +622,7 @@ private void updateStats() {
     attackerWin.setText(formatPercentage(results.getAttackerWinPercent()));
     defenderWin.setText(formatPercentage(results.getDefenderWinPercent()));
     draw.setText(formatPercentage(results.getDrawPercent()));
-    final boolean isLand = isLandBattle();
+    final boolean isLand = isLand();
     final List<Unit> mainCombatAttackers =
         CollectionUtils.getMatches(
             attackers.get(), Matches.unitCanBeInBattle(true, isLand, 1, true));
@@ -663,9 +658,9 @@ private void updateStats() {
 
   private Territory findPotentialBattleSite() {
     Territory location = null;
-    if (this.location == null || this.location.isWater() == isLandBattle()) {
+    if (this.location == null || this.location.isWater() == isLand()) {
       for (final Territory t : data.getMap()) {
-        if (t.isWater() == !isLandBattle()) {
+        if (t.isWater() == !isLand()) {
           location = t;
           break;
         }
@@ -698,9 +693,8 @@ private void setAttackingUnits(final List<Unit> initialUnits) {
         CollectionUtils.getMatches(
             units,
             Matches.unitCanBeInBattle(
-                true, isLandBattle(), 1, hasMaxRounds(isLandBattle(), data), false, List.of())),
-        isLandBattle(),
-        location);
+                true, isLand(), 1, hasMaxRounds(isLand(), data), false, List.of())),
+        isLand());
   }
 
   void addDefendingUnits(final List<Unit> unitsToAdd) {
@@ -714,10 +708,8 @@ private void setDefendingUnits(final List<Unit> initialUnits) {
     final List<Unit> units = Optional.ofNullable(initialUnits).orElseGet(List::of);
     defendingUnitsPanel.init(
         getDefender(),
-        CollectionUtils.getMatches(
-            units, Matches.unitCanBeInBattle(false, isLandBattle(), 1, false)),
-        isLandBattle(),
-        location);
+        CollectionUtils.getMatches(units, Matches.unitCanBeInBattle(false, isLand(), 1, false)),
+        isLand());
   }
 
   public boolean hasAttackingUnitsAdded() {
@@ -728,7 +720,7 @@ public boolean hasDefendingUnitsAdded() {
     return !defendingUnitsPanel.isEmpty();
   }
 
-  private boolean isLandBattle() {
+  private boolean isLand() {
     return landBattleCheckBox.isSelected();
   }
 
@@ -750,7 +742,7 @@ private void setResultsToBlank() {
   private void setWidgetActivation() {
     keepOneAttackingLandUnitCheckBox.setEnabled(landBattleCheckBox.isSelected());
     amphibiousCheckBox.setEnabled(landBattleCheckBox.isSelected());
-    final boolean isLand = isLandBattle();
+    final boolean isLand = isLand();
     try (GameData.Unlocker ignored = data.acquireReadLock()) {
       final List<Unit> attackers =
           CollectionUtils.getMatches(

game-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/OrderOfLossesInputPanel.java

@@ -12,7 +12,6 @@
 import games.strategy.triplea.image.UnitImageFactory.ImageKey;
 import games.strategy.triplea.ui.UiContext;
 import games.strategy.triplea.util.UnitCategory;
-import games.strategy.triplea.util.UnitSeparator;
 import java.awt.Color;
 import java.awt.Component;
 import java.util.ArrayList;
@@ -253,13 +252,12 @@ private void layoutComponents() {
   }
 
   private JPanel getUnitButtonPanel(
-      final List<UnitCategory> unitCategories, final JTextField textField) {
+      final List<UnitCategory> categories, final JTextField textField) {
     final JPanel panel = new JPanel();
     panel.setLayout(new BoxLayout(panel, BoxLayout.X_AXIS));
-    if (unitCategories != null) {
-      UnitSeparator.sortUnitCategories(unitCategories, data);
+    if (categories != null) {
       final Set<UnitType> typesUsed = new HashSet<>();
-      for (final UnitCategory category : unitCategories) {
+      for (final UnitCategory category : categories) {
         // no duplicates or infrastructure allowed. no sea if land, no land if sea.
         if (typesUsed.contains(category.getType())
             || Matches.unitTypeIsInfrastructure().test(category.getType())

game-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/PlayerUnitsPanel.java

@@ -8,6 +8,7 @@
 import games.strategy.engine.data.Territory;
 import games.strategy.engine.data.Unit;
 import games.strategy.engine.data.UnitType;
+import games.strategy.triplea.attachments.UnitAttachment;
 import games.strategy.triplea.delegate.Matches;
 import games.strategy.triplea.ui.UiContext;
 import games.strategy.triplea.util.TuvCostsCalculator;
@@ -33,8 +34,8 @@ public class PlayerUnitsPanel extends JPanel {
   private final GameData data;
   private final UiContext uiContext;
   private final boolean defender;
-  private boolean isLandBattle = true;
-  @Getter private List<UnitCategory> unitCategories = null;
+  private boolean isLand = true;
+  @Getter private List<UnitCategory> categories = null;
   private final List<Runnable> listeners = new ArrayList<>();
   private final List<UnitPanel> unitPanels = new ArrayList<>();
 
@@ -58,17 +59,54 @@ public List<Unit> getUnits() {
   }
 
   /** Sets up components to an initial state. */
-  public void init(
-      final GamePlayer gamePlayer,
-      final List<Unit> units,
-      final boolean isLandBattle,
-      final Territory territory) {
-    this.isLandBattle = isLandBattle;
+  public void init(final GamePlayer gamePlayer, final List<Unit> units, final boolean land) {
+    isLand = land;
     unitPanels.clear();
+    categories = new ArrayList<>(categorize(gamePlayer, units));
+
+    categories.sort(
+        (c1, c2) -> {
+          if (!c1.isOwnedBy(c2.getOwner())) {
+            if (c1.isOwnedBy(gamePlayer)) {
+              return -1;
+            } else if (c2.isOwnedBy(gamePlayer)) {
+              return 1;
+            } else {
+              return c1.getOwner().getName().compareTo(c2.getOwner().getName());
+            }
+          }
+          final UnitType ut1 = c1.getType();
+          final UnitType ut2 = c2.getType();
+          final UnitAttachment u1 = ut1.getUnitAttachment();
+          final UnitAttachment u2 = ut2.getUnitAttachment();
+          // For land battles, sort by land, air, can't combat move (AA), bombarding
+          if (land) {
+            if (u1.getIsSea() != u2.getIsSea()) {
+              return u1.getIsSea() ? 1 : -1;
+            }
+            final boolean u1CanNotCombatMove =
+                Matches.unitTypeCanNotMoveDuringCombatMove().test(ut1)
+                    || !Matches.unitTypeCanMove(gamePlayer).test(ut1);
+            final boolean u2CanNotCombatMove =
+                Matches.unitTypeCanNotMoveDuringCombatMove().test(ut2)
+                    || !Matches.unitTypeCanMove(gamePlayer).test(ut2);
+            if (u1CanNotCombatMove != u2CanNotCombatMove) {
+              return u1CanNotCombatMove ? 1 : -1;
+            }
+            if (u1.getIsAir() != u2.getIsAir()) {
+              return u1.getIsAir() ? 1 : -1;
+            }
+          } else {
+            if (u1.getIsSea() != u2.getIsSea()) {
+              return u1.getIsSea() ? -1 : 1;
+            }
+          }
+          return u1.getName().compareTo(u2.getName());
+        });
 
     removeAll();
     final Predicate<UnitType> predicate;
-    if (isLandBattle) {
+    if (land) {
       if (defender) {
         predicate = Matches.unitTypeIsNotSea();
       } else {
@@ -82,11 +120,9 @@ public void init(
       costs = new TuvCostsCalculator().getCostsForTuv(gamePlayer);
     }
 
-    unitCategories = getAllUnitCategories(gamePlayer, units);
-    UnitSeparator.sortUnitCategories(unitCategories, territory, gamePlayer);
     GamePlayer previousPlayer = null;
     JPanel panel = null;
-    for (final UnitCategory category : unitCategories) {
+    for (final UnitCategory category : categories) {
       if (predicate.test(category.getType())) {
         if (!category.isOwnedBy(previousPlayer)) {
           panel = new JPanel();
@@ -95,12 +131,10 @@ public void init(
           add(panel);
           previousPlayer = category.getOwner();
         }
-        if (panel != null) {
-          final var unitPanel = new UnitPanel(uiContext, category, costs);
-          unitPanel.addChangeListener(this::notifyListeners);
-          panel.add(unitPanel);
-          unitPanels.add(unitPanel);
-        }
+        final var unitPanel = new UnitPanel(uiContext, category, costs);
+        unitPanel.addChangeListener(this::notifyListeners);
+        panel.add(unitPanel);
+        unitPanels.add(unitPanel);
       }
     }
 
@@ -116,11 +150,10 @@ public void init(
    * production frontier and then any unit types the player owns on the map. Then populate the list
    * of units into the categories.
    */
-  private List<UnitCategory> getAllUnitCategories(
-      final GamePlayer gamePlayer, final List<Unit> units) {
+  private Set<UnitCategory> categorize(final GamePlayer gamePlayer, final List<Unit> units) {
 
     // Get player unit types from production frontier and unit types on the map
-    final List<UnitCategory> categories = new ArrayList<>();
+    final Set<UnitCategory> categories = new LinkedHashSet<>();
     for (final UnitType t : getUnitTypes(gamePlayer)) {
       final UnitCategory category = new UnitCategory(t, gamePlayer);
       categories.add(category);
@@ -139,15 +172,15 @@ private List<UnitCategory> getAllUnitCategories(
     }
 
     // Populate units into each category then add any remaining categories (damaged units, etc)
-    final Set<UnitCategory> unitCategoriesWithUnits = UnitSeparator.categorize(units);
+    final Set<UnitCategory> unitCategories = UnitSeparator.categorize(units);
     for (final UnitCategory category : categories) {
-      for (final UnitCategory unitCategoryWithUnits : unitCategoriesWithUnits) {
-        if (category.equals(unitCategoryWithUnits)) {
-          category.getUnits().addAll(unitCategoryWithUnits.getUnits());
+      for (final UnitCategory unitCategory : unitCategories) {
+        if (category.equals(unitCategory)) {
+          category.getUnits().addAll(unitCategory.getUnits());
         }
       }
     }
-    categories.addAll(unitCategoriesWithUnits);
+    categories.addAll(unitCategories);
 
     return categories;
   }
@@ -183,10 +216,10 @@ private Collection<UnitType> getUnitTypes(final GamePlayer player) {
             unitTypes,
             Matches.unitTypeCanBeInBattle(
                 !defender,
-                isLandBattle,
+                isLand,
                 player,
                 1,
-                BattleCalculatorPanel.hasMaxRounds(isLandBattle, data),
+                BattleCalculatorPanel.hasMaxRounds(isLand, data),
                 false,
                 List.of()));
 

game-app/game-core/src/main/java/games/strategy/triplea/ui/BattleDisplay.java

@@ -47,8 +47,6 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Supplier;
-import java.util.stream.Collectors;
-import java.util.stream.StreamSupport;
 import javax.annotation.Nullable;
 import javax.swing.AbstractAction;
 import javax.swing.Action;
@@ -232,10 +230,10 @@ private Collection<Unit> updateKilledUnits(
     for (final Collection<Unit> dependentCollection : dependentsMap.values()) {
       dependentUnitsReturned.addAll(dependentCollection);
     }
-
-    List<UnitCategory> unitCategories =
-        UnitSeparator.getSortedUnitCategories(killedUnits, gameData, uiContext.getMapData());
-    for (final UnitCategory category : unitCategories) {
+    for (final UnitCategory category :
+        UnitSeparator.categorize(
+            killedUnits,
+            UnitSeparator.SeparatorCategories.builder().dependents(dependentsMap).build())) {
       final JPanel panel = new JPanel();
       JLabel unit = uiContext.newUnitImageLabel(category.getType(), category.getOwner());
       panel.add(unit);
@@ -777,14 +775,7 @@ void setNotificationShort(
 
     private void categorizeUnits(
         final Iterable<UnitCategory> categoryIter, final boolean damaged, final boolean disabled) {
-      final List<UnitCategory> unitCategories =
-          StreamSupport.stream(categoryIter.spliterator(), false).collect(Collectors.toList());
-      if (unitCategories.isEmpty()) {
-        return;
-      }
-      final GameData gameData = unitCategories.get(0).getUnitAttachment().getData();
-      UnitSeparator.sortUnitCategories(unitCategories, gameData);
-      for (final UnitCategory category : unitCategories) {
+      for (final UnitCategory category : categoryIter) {
         final JPanel panel = new JPanel();
         final ImageIcon unitImage =
             uiContext

game-app/game-core/src/main/java/games/strategy/triplea/ui/BattleModel.java

@@ -128,8 +128,7 @@ void refresh() {
       }
       unitPowerAndRollsMap = PowerStrengthAndRolls.build(units, combatValue);
     }
-    final List<UnitCategory> unitCategories =
-        UnitSeparator.getSortedUnitCategories(units, gameData, uiContext.getMapData());
+    final Collection<UnitCategory> unitCategories = UnitSeparator.categorize(units);
     for (final UnitCategory category : unitCategories) {
       final int[] shift = new int[gameData.getDiceSides() + 1];
       for (final Unit current : category.getUnits()) {

game-app/game-core/src/main/java/games/strategy/triplea/ui/SimpleUnitPanel.java

@@ -1,7 +1,5 @@
 package games.strategy.triplea.ui;
 
-import com.google.common.collect.Lists;
-import games.strategy.engine.data.GameData;
 import games.strategy.engine.data.GamePlayer;
 import games.strategy.engine.data.GameState;
 import games.strategy.engine.data.NamedAttachable;
@@ -15,9 +13,7 @@
 import games.strategy.triplea.delegate.Matches;
 import games.strategy.triplea.image.UnitImageFactory;
 import games.strategy.triplea.util.UnitCategory;
-import games.strategy.triplea.util.UnitSeparator;
 import java.awt.Image;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
@@ -124,13 +120,7 @@ public void setUnitsFromRepairRuleMap(
    */
   public void setUnitsFromCategories(final Collection<UnitCategory> categories) {
     removeAll();
-    if (categories.isEmpty()) {
-      return;
-    }
-    final GameData gameData = categories.iterator().next().getUnitAttachment().getData();
-    final ArrayList<UnitCategory> unitCategories = Lists.newArrayList(categories);
-    UnitSeparator.sortUnitCategories(unitCategories, gameData);
-    for (final UnitCategory category : unitCategories) {
+    for (final UnitCategory category : categories) {
       addUnits(
           category.getOwner(),
           category.getUnits().size(),