Fix "Attackers and defenders collections must be distinct".

This was caused by cantMoveUnits containing duplicates in some cases. Changed to a Set to prevent that. Also cleans up some logic. Fixes: #11851
triplea-game · Sep 13, 2023 · b20f546 · b20f546
1 parent 12fdfda
commit b20f546
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 26 deletions.
diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProNonCombatMoveAi.java b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProNonCombatMoveAi.java
@@ -235,7 +235,6 @@ private void findUnitsThatCantMove(
 
     Map<Territory, ProTerritory> moveMap = territoryManager.getDefendOptions().getTerritoryMap();
     Map<Unit, Set<Territory>> unitMoveMap = territoryManager.getDefendOptions().getUnitMoveMap();
-    List<ProTransport> transportMapList = territoryManager.getDefendOptions().getTransportList();
 
     // Add all units that can't move (to be consumed, allied units, 0 move units, etc)
     for (final Territory t : moveMap.keySet()) {
@@ -252,26 +251,13 @@ private void findUnitsThatCantMove(
     // Add all units that only have 1 move option and can't be transported
     for (final Iterator<Unit> it = unitMoveMap.keySet().iterator(); it.hasNext(); ) {
       final Unit u = it.next();
-      if (unitMoveMap.get(u).size() == 1) {
-        final Territory onlyTerritory = CollectionUtils.getAny(unitMoveMap.get(u));
-        if (onlyTerritory.equals(unitTerritoryMap.get(u))) {
-          boolean canBeTransported = false;
-          for (final ProTransport pad : transportMapList) {
-            for (final Territory t : pad.getTransportMap().keySet()) {
-              if (pad.getTransportMap().get(t).contains(onlyTerritory)) {
-                canBeTransported = true;
-              }
-            }
-            for (final Territory t : pad.getSeaTransportMap().keySet()) {
-              if (pad.getSeaTransportMap().get(t).contains(onlyTerritory)) {
-                canBeTransported = true;
-              }
-            }
-          }
-          if (!canBeTransported) {
-            moveMap.get(onlyTerritory).addCantMoveUnit(u);
-            it.remove();
-          }
+      final Set<Territory> territories = unitMoveMap.get(u);
+      if (territories.size() == 1) {
+        final Territory onlyTerritory = CollectionUtils.getAny(territories);
+        if (onlyTerritory.equals(unitTerritoryMap.get(u))
+            && !canBePotentiallyBeTransported(onlyTerritory)) {
+          moveMap.get(onlyTerritory).addCantMoveUnit(u);
+          it.remove();
         }
       }
     }
@@ -325,6 +311,24 @@ private Map<Unit, Set<Territory>> findInfraUnitsThatCanMove() {
     return infraUnitMoveMap;
   }
 
+  private boolean canBePotentiallyBeTransported(Territory unitTerritory) {
+    final List<ProTransport> transportMapList =
+        territoryManager.getDefendOptions().getTransportList();
+    for (final ProTransport pad : transportMapList) {
+      for (final Territory t : pad.getTransportMap().keySet()) {
+        if (pad.getTransportMap().get(t).contains(unitTerritory)) {
+          return true;
+        }
+      }
+      for (final Territory t : pad.getSeaTransportMap().keySet()) {
+        if (pad.getSeaTransportMap().get(t).contains(unitTerritory)) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
   private List<Territory> moveOneDefenderToLandTerritoriesBorderingEnemy() {
 
     ProLogger.info("Determine which territories to defend with one land unit");

diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/data/ProTerritory.java b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/data/ProTerritory.java
@@ -52,7 +52,7 @@ public class ProTerritory {
   private ProBattleResult battleResult;
 
   // Non-combat move variables
-  private final List<Unit> cantMoveUnits;
+  private final Set<Unit> cantMoveUnits;
   private List<Unit> maxEnemyUnits;
   private Set<Unit> maxEnemyBombardUnits;
   private ProBattleResult minBattleResult;
@@ -89,7 +89,7 @@ public ProTerritory(final Territory territory, final ProData proData) {
     currentlyWins = false;
     battleResult = null;
 
-    cantMoveUnits = new ArrayList<>();
+    cantMoveUnits = new HashSet<>();
     maxEnemyUnits = new ArrayList<>();
     maxEnemyBombardUnits = new HashSet<>();
     minBattleResult = new ProBattleResult();
@@ -126,7 +126,7 @@ public ProTerritory(final Territory territory, final ProData proData) {
     currentlyWins = patd.isCurrentlyWins();
     battleResult = patd.getBattleResult();
 
-    cantMoveUnits = new ArrayList<>(patd.getCantMoveUnits());
+    cantMoveUnits = new HashSet<>(patd.getCantMoveUnits());
     maxEnemyUnits = new ArrayList<>(patd.getMaxEnemyUnits());
     maxEnemyBombardUnits = new HashSet<>(patd.getMaxEnemyBombardUnits());
     minBattleResult = patd.getMinBattleResult();
@@ -332,8 +332,8 @@ public String getResultString() {
     return sb.toString();
   }
 
-  public List<Unit> getCantMoveUnits() {
-    return Collections.unmodifiableList(cantMoveUnits);
+  public Collection<Unit> getCantMoveUnits() {
+    return Collections.unmodifiableCollection(cantMoveUnits);
   }
 
   public void addCantMoveUnit(final Unit unit) {