Fix another cause of "Could not find step name" (2.6 bug). (#11780)

This one is caused by non-determinism when choose an arbitrary unit type in a firing group to name that group. When the ordering of the provided units changed during the rounds of a battle, it resulted in different step names generated. This PR fixes that problem by choosing the first name alphabetically.

Adds test coverage.

game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/steps/fire/general/FiringGroupSplitterGeneral.java

@@ -15,6 +15,7 @@
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Comparator;
 import java.util.List;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -154,14 +155,17 @@ private void generateNamedGroups(
       final Collection<TargetGroup> targetGroups,
       final Collection<Unit> canFire,
       final Collection<Unit> enemyUnits) {
-
     if (targetGroups.size() == 1) {
       firingGroups.addAll(
           buildFiringGroups(name, canFire, enemyUnits, CollectionUtils.getAny(targetGroups)));
     } else {
       // use the first unitType name of each TargetGroup as a suffix for the FiringGroup name
       for (final TargetGroup targetGroup : targetGroups) {
-        final UnitType type = CollectionUtils.getAny(targetGroup.getFiringUnits(canFire)).getType();
+        // Sort units first, before getting the unit type to use for the name, so that we choose
+        // the same name regardless of the order of units provided.
+        List<Unit> firingUnits = targetGroup.getFiringUnits(canFire);
+        firingUnits.sort(Comparator.comparing(u -> u.getType().getName()));
+        UnitType type = CollectionUtils.getAny(firingUnits).getType();
         firingGroups.addAll(
             buildFiringGroups(name + " " + type.getName(), canFire, enemyUnits, targetGroup));
       }

game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/steps/fire/general/TargetGroup.java

@@ -27,7 +27,7 @@ class TargetGroup {
     this.targetUnitTypes = targetUnitTypes;
   }
 
-  public Collection<Unit> getFiringUnits(final Collection<Unit> units) {
+  public List<Unit> getFiringUnits(final Collection<Unit> units) {
     return CollectionUtils.getMatches(units, Matches.unitIsOfTypes(firingUnitTypes));
   }
 

game-app/game-core/src/test/java/games/strategy/triplea/delegate/battle/steps/fire/general/FiringGroupSplitterGeneralTest.java

@@ -13,6 +13,7 @@
 import static games.strategy.triplea.delegate.battle.steps.MockGameData.givenGameData;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
@@ -338,6 +339,14 @@ void noFiringGroupIfAllTargetsAreExcluded() {
     assertThat(firingGroups, is(empty()));
   }
 
+  private Unit givenUnitWithCannotTarget(String name, Set<UnitType> cannotTarget) {
+    final Unit fireUnit = givenAnyUnit();
+    lenient().when(fireUnit.getType().getName()).thenReturn(name);
+    lenient().when(fireUnit.toString()).thenReturn(name);
+    when(fireUnit.getUnitAttachment().getCanNotTarget()).thenReturn(cannotTarget);
+    return fireUnit;
+  }
+
   @Test
   void twoFiringGroupsWithCanNotTarget() {
     final Unit targetUnit = givenAnyUnit();
@@ -347,39 +356,38 @@ void twoFiringGroupsWithCanNotTarget() {
     final Unit targetUnit3 = givenAnyUnit();
     final UnitType targetUnit3Type = targetUnit3.getType();
 
-    final Unit fireUnit = givenAnyUnit();
-    when(fireUnit.getType().getName()).thenReturn("fireUnit");
-    final UnitAttachment unitAttachment =
-        (UnitAttachment) fireUnit.getType().getAttachment(UNIT_ATTACHMENT_NAME);
-    when(unitAttachment.getCanNotTarget()).thenReturn(Set.of(targetUnit2Type, targetUnit3Type));
-
-    final Unit fireUnit2 = givenAnyUnit();
-    when(fireUnit2.getType().getName()).thenReturn("fireUnit2");
-    final UnitAttachment unitAttachment2 =
-        (UnitAttachment) fireUnit2.getType().getAttachment(UNIT_ATTACHMENT_NAME);
-    when(unitAttachment2.getCanNotTarget()).thenReturn(Set.of(targetUnitType));
-
-    final List<FiringGroup> firingGroups =
-        FiringGroupSplitterGeneral.of(OFFENSE, FiringGroupSplitterGeneral.Type.NORMAL, UNITS)
-            .apply(
-                givenBattleStateBuilder()
-                    .gameData(givenGameData().withAlliedAirIndependent(true).build())
-                    .attacker(attacker)
-                    .defender(defender)
-                    .attackingUnits(List.of(fireUnit, fireUnit2))
-                    .defendingUnits(List.of(targetUnit, targetUnit2, targetUnit3))
-                    .build());
-
-    assertThat(firingGroups, hasSize(2));
-    assertThat(firingGroups.get(0).getDisplayName(), is(UNITS + " fireUnit"));
-    assertThat(firingGroups.get(0).getFiringUnits(), contains(fireUnit));
-    assertThat(firingGroups.get(0).getTargetUnits(), contains(targetUnit));
-    assertThat(firingGroups.get(0).isSuicideOnHit(), is(false));
-
-    assertThat(firingGroups.get(1).getDisplayName(), is(UNITS + " fireUnit2"));
-    assertThat(firingGroups.get(1).getFiringUnits(), contains(fireUnit2));
-    assertThat(firingGroups.get(1).getTargetUnits(), contains(targetUnit2, targetUnit3));
-    assertThat(firingGroups.get(1).isSuicideOnHit(), is(false));
+    final Unit fireUnit =
+        givenUnitWithCannotTarget("fireUnit", Set.of(targetUnit2Type, targetUnit3Type));
+    final Unit fireUnit2 = givenUnitWithCannotTarget("fireUnit2", Set.of(targetUnitType));
+    final Unit fireUnit3 = givenUnitWithCannotTarget("fireUnit3", Set.of(targetUnitType));
+
+    // Iterate over several different orderings of attackers to ensure step names are deterministic
+    // regardless of the order of units passed in.
+    List<Unit> attackersOrdering1 = List.of(fireUnit, fireUnit2, fireUnit3);
+    List<Unit> attackersOrdering2 = List.of(fireUnit, fireUnit3, fireUnit2);
+    for (List<Unit> attackingUnits : List.of(attackersOrdering1, attackersOrdering2)) {
+      final List<FiringGroup> firingGroups =
+          FiringGroupSplitterGeneral.of(OFFENSE, FiringGroupSplitterGeneral.Type.NORMAL, UNITS)
+              .apply(
+                  givenBattleStateBuilder()
+                      .gameData(givenGameData().withAlliedAirIndependent(true).build())
+                      .attacker(attacker)
+                      .defender(defender)
+                      .attackingUnits(attackingUnits)
+                      .defendingUnits(List.of(targetUnit, targetUnit2, targetUnit3))
+                      .build());
+
+      assertThat(firingGroups, hasSize(2));
+      assertThat(firingGroups.get(0).getDisplayName(), is(UNITS + " fireUnit"));
+      assertThat(firingGroups.get(0).getFiringUnits(), contains(fireUnit));
+      assertThat(firingGroups.get(0).getTargetUnits(), contains(targetUnit));
+      assertThat(firingGroups.get(0).isSuicideOnHit(), is(false));
+
+      assertThat(firingGroups.get(1).getDisplayName(), is(UNITS + " fireUnit2"));
+      assertThat(firingGroups.get(1).getFiringUnits(), containsInAnyOrder(fireUnit2, fireUnit3));
+      assertThat(firingGroups.get(1).getTargetUnits(), contains(targetUnit2, targetUnit3));
+      assertThat(firingGroups.get(1).isSuicideOnHit(), is(false));
+    }
   }
 
   @Test