Merge branch 'triplea-game:master' into master

build.gradle

@@ -80,10 +80,10 @@ subprojects {
         junitJupiterVersion = '5.10.0'
         junitPlatformLauncherVersion = '1.10.0'
         logbackClassicVersion = '1.2.11'
-        mockitoVersion = '5.4.0'
+        mockitoVersion = '5.5.0'
         openFeignVersion = '12.4'
         postgresqlVersion = '42.6.0'
-        snakeYamlVersion = '2.6'
+        snakeYamlVersion = '2.7'
         sonatypeGoodiesPrefsVersion = '2.3.6'
         substanceVersion = '4.5.0'
         wireMockJunit5Version = '1.3.1'

game-app/game-core/src/main/java/games/strategy/engine/data/GameMap.java

@@ -4,6 +4,7 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
+import games.strategy.engine.data.util.BreadthFirstSearch;
 import games.strategy.triplea.delegate.Matches;
 import java.math.BigDecimal;
 import java.util.ArrayList;
@@ -375,37 +376,9 @@ public int getDistance(
     if (t1.equals(t2)) {
       return 0;
     }
-    return getDistance(0, new HashSet<>(), Set.of(t1), t2, routeCond);
-  }
-
-  /**
-   * Guaranteed that frontier doesn't contain target. Territories on the frontier are not target.
-   * They represent the extent of paths already searched. Territories in searched have already been
-   * on the frontier.
-   */
-  private int getDistance(
-      final int distance,
-      final Set<Territory> searched,
-      final Set<Territory> frontier,
-      final Territory target,
-      final BiPredicate<Territory, Territory> routeCond) {
-
-    // add the frontier to the searched
-    searched.addAll(frontier);
-    // find the new frontier
-
-    final Set<Territory> newFrontier =
-        frontier.stream()
-            .flatMap(f -> getNeighbors(f, routeCond).stream())
-            .collect(Collectors.toSet());
-    if (newFrontier.contains(target)) {
-      return distance + 1;
-    }
-    newFrontier.removeAll(searched);
-    if (newFrontier.isEmpty()) {
-      return -1;
-    }
-    return getDistance(distance + 1, searched, newFrontier, target, routeCond);
+    var territoryFinder = new BreadthFirstSearch.TerritoryFinder(t2);
+    new BreadthFirstSearch(List.of(t1), routeCond).traverse(territoryFinder);
+    return territoryFinder.getDistanceFound();
   }
 
   public IntegerMap<Territory> getDistance(

game-app/game-core/src/main/java/games/strategy/engine/data/util/BreadthFirstSearch.java

@@ -7,7 +7,9 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.function.BiPredicate;
 import java.util.function.Predicate;
+import lombok.Getter;
 import org.triplea.java.ObjectUtils;
 import org.triplea.java.collections.CollectionUtils;
 
@@ -20,7 +22,7 @@ public final class BreadthFirstSearch {
   private final GameMap map;
   private final Set<Territory> visited;
   private final ArrayDeque<Territory> territoriesToCheck;
-  private final Predicate<Territory> neighborCondition;
+  private final BiPredicate<Territory, Territory> neighborCondition;
 
   @FunctionalInterface
   public interface Visitor {
@@ -34,19 +36,50 @@ public interface Visitor {
     boolean visit(Territory territory, int distance);
   }
 
+  /**
+   * Visitor implementation for finding a specific territory. The resulting distance can be accessed
+   * using the getDistanceFound() getter, which will return -1 when not found.
+   *
+   * <p>Note: The Visitor does not get called with any of the starting territories, so -1 will be
+   * returned when passing one of them. If handling such a case is needed, it should be done by the
+   * caller before running the BreadthFirstSearch.
+   */
+  public static class TerritoryFinder implements Visitor {
+    final Territory destination;
+    @Getter int distanceFound = -1;
+
+    public TerritoryFinder(Territory destination) {
+      this.destination = destination;
+    }
+
+    @Override
+    public boolean visit(Territory territory, int distance) {
+      if (destination.equals(territory)) {
+        distanceFound = distance;
+        return false;
+      }
+      return true;
+    }
+  }
+
   /**
    * @param startTerritories The territories from where to start the search.
    * @param neighborCondition Condition that neighboring territories must match to be considered
    *     neighbors.
    */
   public BreadthFirstSearch(
-      Collection<Territory> startTerritories, Predicate<Territory> neighborCondition) {
+      Collection<Territory> startTerritories, BiPredicate<Territory, Territory> neighborCondition) {
     this.map = CollectionUtils.getAny(startTerritories).getData().getMap();
     this.visited = new HashSet<>(startTerritories);
     this.territoriesToCheck = new ArrayDeque<>(startTerritories);
     this.neighborCondition = neighborCondition;
   }
 
+  public BreadthFirstSearch(
+      Collection<Territory> startTerritories, Predicate<Territory> neighborCondition) {
+    this(startTerritories, (it, it2) -> neighborCondition.test(it2));
+  }
+
   public BreadthFirstSearch(Territory startTerritory, Predicate<Territory> neighborCondition) {
     this(List.of(startTerritory), neighborCondition);
   }
@@ -82,10 +115,10 @@ private Territory checkNextTerritory(Visitor visitor, int currentDistance) {
     final Territory territory = territoriesToCheck.removeFirst();
     // Note: The condition isn't passed to getNeighbors() because that implementation is very slow.
     for (final Territory neighbor : map.getNeighbors(territory)) {
-      if (!visited.contains(neighbor) && neighborCondition.test(neighbor)) {
+      if (!visited.contains(neighbor) && neighborCondition.test(territory, neighbor)) {
         visited.add(neighbor);
 
-        final boolean shouldContinueSearch = visitor.visit(neighbor, currentDistance);
+        final boolean shouldContinueSearch = visitor.visit(neighbor, currentDistance + 1);
         if (!shouldContinueSearch) {
           territoriesToCheck.clear();
           break;

game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProCombatMoveAi.java

@@ -434,12 +434,12 @@ private void determineTerritoriesThatCanBeHeld(
       }
 
       // Set max enemy attackers
-      if (enemyAttackOptions.getMax(t) != null) {
-        final Set<Unit> enemyAttackingUnits =
-            new HashSet<>(enemyAttackOptions.getMax(t).getMaxUnits());
-        enemyAttackingUnits.addAll(enemyAttackOptions.getMax(t).getMaxAmphibUnits());
-        patd.setMaxEnemyUnits(new ArrayList<>(enemyAttackingUnits));
-        patd.setMaxEnemyBombardUnits(enemyAttackOptions.getMax(t).getMaxBombardUnits());
+      final ProTerritory enemyAttackMax = enemyAttackOptions.getMax(t);
+      if (enemyAttackMax != null) {
+        final Set<Unit> enemyAttackingUnits = new HashSet<>(enemyAttackMax.getMaxUnits());
+        enemyAttackingUnits.addAll(enemyAttackMax.getMaxAmphibUnits());
+        patd.setMaxEnemyUnits(enemyAttackingUnits);
+        patd.setMaxEnemyBombardUnits(enemyAttackMax.getMaxBombardUnits());
       }
 
       // Add strategic value for factories

game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProNonCombatMoveAi.java

@@ -394,17 +394,17 @@ private void determineIfMoveTerritoriesCanBeHeld() {
       final ProTerritory patd = moveMap.get(t);
 
       // Check if no enemy attackers
-      if (enemyAttackOptions.getMax(t) == null) {
+      final ProTerritory enemyAttackMax = enemyAttackOptions.getMax(t);
+      if (enemyAttackMax == null) {
         ProLogger.debug("Territory=" + t.getName() + ", CanHold=true since has no enemy attackers");
         continue;
       }
 
       // Check if min defenders can hold it (not considering AA)
-      final Set<Unit> enemyAttackingUnits =
-          new HashSet<>(enemyAttackOptions.getMax(t).getMaxUnits());
-      enemyAttackingUnits.addAll(enemyAttackOptions.getMax(t).getMaxAmphibUnits());
-      patd.setMaxEnemyUnits(new ArrayList<>(enemyAttackingUnits));
-      patd.setMaxEnemyBombardUnits(enemyAttackOptions.getMax(t).getMaxBombardUnits());
+      final Set<Unit> enemyAttackingUnits = new HashSet<>(enemyAttackMax.getMaxUnits());
+      enemyAttackingUnits.addAll(enemyAttackMax.getMaxAmphibUnits());
+      patd.setMaxEnemyUnits(enemyAttackingUnits);
+      patd.setMaxEnemyBombardUnits(enemyAttackMax.getMaxBombardUnits());
       final List<Unit> minDefendingUnitsAndNotAa =
           CollectionUtils.getMatches(
               patd.getCantMoveUnits(), Matches.unitIsAaForAnything().negate());
@@ -414,7 +414,7 @@ private void determineIfMoveTerritoriesCanBeHeld() {
               t,
               enemyAttackingUnits,
               minDefendingUnitsAndNotAa,
-              enemyAttackOptions.getMax(t).getMaxBombardUnits());
+              enemyAttackMax.getMaxBombardUnits());
       patd.setMinBattleResult(minResult);
       if (minResult.getTuvSwing() <= 0 && !minDefendingUnitsAndNotAa.isEmpty()) {
         ProLogger.debug(
@@ -2405,24 +2405,21 @@ private Optional<Territory> findDestinationOrSafeTerritoryOnTheWay(
     MutableObject<Territory> destination = new MutableObject<>();
     BreadthFirstSearch bfs = new BreadthFirstSearch(from, canMoveThrough);
     bfs.traverse(
-        new BreadthFirstSearch.Visitor() {
-          @Override
-          public boolean visit(Territory t, int distance) {
-            // If it's a desired final destination, see if we can move towards it.
-            if (finalDestinationTest.test(t)) {
-              Route r = data.getMap().getRouteForUnit(from, t, canMoveThrough, unit, player);
-              while (r != null && r.hasSteps()) {
-                final ProTerritory proDestination = proData.getProTerritory(moveMap, r.getEnd());
-                if (proDestination.isCanHold() && validateMove.test(r)) {
-                  destination.setValue(r.getEnd());
-                  // End the search.
-                  return false;
-                }
-                r = new Route(from, r.getMiddleSteps());
+        (t, distance) -> {
+          // If it's a desired final destination, see if we can move towards it.
+          if (finalDestinationTest.test(t)) {
+            Route r = data.getMap().getRouteForUnit(from, t, canMoveThrough, unit, player);
+            while (r != null && r.hasSteps()) {
+              final ProTerritory proDestination = proData.getProTerritory(moveMap, r.getEnd());
+              if (proDestination.isCanHold() && validateMove.test(r)) {
+                destination.setValue(r.getEnd());
+                // End the search.
+                return false;
               }
+              r = new Route(from, r.getMiddleSteps());
             }
-            return true;
           }
+          return true;
         });
     // If nothing chosen and we can't hold the current territory, try to move somewhere safe.
     if (destination.getValue() == null && !moveMap.get(from).isCanHold()) {

game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/data/ProTerritory.java

@@ -344,8 +344,8 @@ public void addCantMoveUnits(final Collection<Unit> units) {
     this.cantMoveUnits.addAll(units);
   }
 
-  public void setMaxEnemyUnits(final List<Unit> maxEnemyUnits) {
-    this.maxEnemyUnits = maxEnemyUnits;
+  public void setMaxEnemyUnits(final Collection<Unit> maxEnemyUnits) {
+    this.maxEnemyUnits = new ArrayList<>(maxEnemyUnits);
   }
 
   public List<Unit> getMaxEnemyUnits() {

game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/util/ProTerritoryValueUtils.java

@@ -187,13 +187,10 @@ static int findMaxLandMassSize(final GamePlayer player) {
         final int[] landMassSize = new int[1];
         new BreadthFirstSearch(t, cond)
             .traverse(
-                new BreadthFirstSearch.Visitor() {
-                  @Override
-                  public boolean visit(Territory territory, int distance) {
-                    visited.add(territory);
-                    landMassSize[0]++;
-                    return true;
-                  }
+                (territory, distance) -> {
+                  visited.add(territory);
+                  landMassSize[0]++;
+                  return true;
                 });
         if (landMassSize[0] > maxLandMassSize) {
           maxLandMassSize = landMassSize[0];
@@ -462,7 +459,7 @@ public boolean visit(Territory territory, int distance) {
               }
 
               public boolean shouldContinueSearch() {
-                return currentDistance < MIN_FACTORY_CHECK_DISTANCE || found.isEmpty();
+                return currentDistance <= MIN_FACTORY_CHECK_DISTANCE || found.isEmpty();
               }
             });
     return found;

game-app/game-core/src/main/java/games/strategy/triplea/attachments/CanalAttachment.java

@@ -12,7 +12,9 @@
 import games.strategy.triplea.Constants;
 import games.strategy.triplea.delegate.Matches;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import org.triplea.java.collections.CollectionUtils;
@@ -34,23 +36,34 @@ public CanalAttachment(final String name, final Attachable attachable, final Gam
     super(name, attachable, gameData);
   }
 
+  private static boolean hasCanal(final Territory t, final String canalName) {
+    return !get(t, canalAttachment -> canalAttachment.getCanalName().equals(canalName)).isEmpty();
+  }
+
+  public static List<CanalAttachment> get(final Territory t, final Route onRoute) {
+    return get(t, attachment -> isCanalOnRoute(attachment.getCanalName(), onRoute));
+  }
+
+  private static List<CanalAttachment> get(final Territory t, Predicate<CanalAttachment> cond) {
+    return t.getAttachments().values().stream()
+        .filter(attachment -> attachment.getName().startsWith(Constants.CANAL_ATTACHMENT_PREFIX))
+        .map(CanalAttachment.class::cast)
+        .filter(cond)
+        .collect(Collectors.toList());
+  }
+
+  static CanalAttachment get(final Territory t, final String nameOfAttachment) {
+    return getAttachment(t, nameOfAttachment, CanalAttachment.class);
+  }
+
   /**
    * Checks if the route contains both territories to pass through the given canal. If route is null
    * returns true.
    */
-  public static boolean isCanalOnRoute(final String canalName, final Route route) {
-    if (route == null) {
-      return true;
-    }
+  private static boolean isCanalOnRoute(final String canalName, final Route route) {
     boolean previousTerritoryHasCanal = false;
     for (final Territory t : route) {
-      boolean currentTerritoryHasCanal = false;
-      for (final CanalAttachment canalAttachment : get(t)) {
-        if (canalAttachment.getCanalName().equals(canalName)) {
-          currentTerritoryHasCanal = true;
-          break;
-        }
-      }
+      boolean currentTerritoryHasCanal = hasCanal(t, canalName);
       if (previousTerritoryHasCanal && currentTerritoryHasCanal) {
         return true;
       }
@@ -59,17 +72,6 @@ public static boolean isCanalOnRoute(final String canalName, final Route route)
     return false;
   }
 
-  public static Set<CanalAttachment> get(final Territory t) {
-    return t.getAttachments().values().stream()
-        .filter(attachment -> attachment.getName().startsWith(Constants.CANAL_ATTACHMENT_PREFIX))
-        .map(CanalAttachment.class::cast)
-        .collect(Collectors.toSet());
-  }
-
-  static CanalAttachment get(final Territory t, final String nameOfAttachment) {
-    return getAttachment(t, nameOfAttachment, CanalAttachment.class);
-  }
-
   private void setCanalName(final String name) {
     canalName = name.intern();
   }
@@ -162,11 +164,8 @@ public void validate(final GameState data) throws GameParseException {
     }
     final Set<Territory> territories = new HashSet<>();
     for (final Territory t : data.getMap()) {
-      final Set<CanalAttachment> canalAttachments = get(t);
-      for (final CanalAttachment canalAttachment : canalAttachments) {
-        if (canalAttachment.getCanalName().equals(canalName)) {
-          territories.add(t);
-        }
+      if (hasCanal(t, canalName)) {
+        territories.add(t);
       }
     }
     if (territories.size() != 2) {