From 06a3271df6b8ffdfb42a5cf86ed2e1023c8fa9cc Mon Sep 17 00:00:00 2001 From: asvitkine Date: Sun, 27 Aug 2023 15:43:45 -0400 Subject: [PATCH] Use BFS class for GameMap.getDistance(). --- .../games/strategy/engine/data/GameMap.java | 35 ++------------- .../engine/data/util/BreadthFirstSearch.java | 41 ++++++++++++++++-- .../triplea/ai/pro/ProNonCombatMoveAi.java | 27 ++++++------ .../ai/pro/util/ProTerritoryValueUtils.java | 13 +++--- .../strategy/engine/data/GameMapTest.java | 35 +++++++++++++++ .../data/util/BreadthFirstSearchTest.java | 43 +++++++++++++++++++ 6 files changed, 136 insertions(+), 58 deletions(-) create mode 100644 game-app/game-core/src/test/java/games/strategy/engine/data/GameMapTest.java create mode 100644 game-app/game-core/src/test/java/games/strategy/engine/data/util/BreadthFirstSearchTest.java diff --git a/game-app/game-core/src/main/java/games/strategy/engine/data/GameMap.java b/game-app/game-core/src/main/java/games/strategy/engine/data/GameMap.java index 3aaf5227468..095f811e10d 100644 --- a/game-app/game-core/src/main/java/games/strategy/engine/data/GameMap.java +++ b/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 searched, - final Set frontier, - final Territory target, - final BiPredicate routeCond) { - - // add the frontier to the searched - searched.addAll(frontier); - // find the new frontier - - final Set 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 getDistance( diff --git a/game-app/game-core/src/main/java/games/strategy/engine/data/util/BreadthFirstSearch.java b/game-app/game-core/src/main/java/games/strategy/engine/data/util/BreadthFirstSearch.java index f550db2796c..e119692c6c3 100644 --- a/game-app/game-core/src/main/java/games/strategy/engine/data/util/BreadthFirstSearch.java +++ b/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 visited; private final ArrayDeque territoriesToCheck; - private final Predicate neighborCondition; + private final BiPredicate 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. + * + *

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 startTerritories, Predicate neighborCondition) { + Collection startTerritories, BiPredicate neighborCondition) { this.map = CollectionUtils.getAny(startTerritories).getData().getMap(); this.visited = new HashSet<>(startTerritories); this.territoriesToCheck = new ArrayDeque<>(startTerritories); this.neighborCondition = neighborCondition; } + public BreadthFirstSearch( + Collection startTerritories, Predicate neighborCondition) { + this(startTerritories, (it, it2) -> neighborCondition.test(it2)); + } + public BreadthFirstSearch(Territory startTerritory, Predicate 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; 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 index 4977e834a9d..9314e26d34a 100644 --- 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 @@ -2405,24 +2405,21 @@ private Optional findDestinationOrSafeTerritoryOnTheWay( MutableObject 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()) { diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/util/ProTerritoryValueUtils.java b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/util/ProTerritoryValueUtils.java index 40e32e42173..8c09dc03f50 100644 --- a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/util/ProTerritoryValueUtils.java +++ b/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; diff --git a/game-app/game-core/src/test/java/games/strategy/engine/data/GameMapTest.java b/game-app/game-core/src/test/java/games/strategy/engine/data/GameMapTest.java new file mode 100644 index 00000000000..c04f15ab267 --- /dev/null +++ b/game-app/game-core/src/test/java/games/strategy/engine/data/GameMapTest.java @@ -0,0 +1,35 @@ +package games.strategy.engine.data; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import games.strategy.triplea.xml.TestMapGameData; +import org.junit.jupiter.api.Test; + +public class GameMapTest { + private final GameData gameData = TestMapGameData.REVISED.getGameData(); + private final Territory caucasus = gameData.getMap().getTerritory("Caucasus"); + private final Territory germany = gameData.getMap().getTerritory("Germany"); + private final Territory russia = gameData.getMap().getTerritory("Russia"); + private final Territory uk = gameData.getMap().getTerritory("United Kingdom"); + + private int getLandDistance(Territory from, Territory to) { + return gameData.getMap().getLandDistance(from, to); + } + + @Test + void testLandDistance() { + assertThat(getLandDistance(caucasus, russia), is(1)); + assertThat(getLandDistance(caucasus, germany), is(3)); + } + + @Test + void testLandDistanceNotFound() { + assertThat(getLandDistance(caucasus, uk), is(-1)); + } + + @Test + void testLandDistanceSameTerritory() { + assertThat(getLandDistance(caucasus, caucasus), is(0)); + } +} diff --git a/game-app/game-core/src/test/java/games/strategy/engine/data/util/BreadthFirstSearchTest.java b/game-app/game-core/src/test/java/games/strategy/engine/data/util/BreadthFirstSearchTest.java new file mode 100644 index 00000000000..816e4fccbf7 --- /dev/null +++ b/game-app/game-core/src/test/java/games/strategy/engine/data/util/BreadthFirstSearchTest.java @@ -0,0 +1,43 @@ +package games.strategy.engine.data.util; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import games.strategy.engine.data.GameData; +import games.strategy.engine.data.Territory; +import games.strategy.triplea.delegate.Matches; +import games.strategy.triplea.xml.TestMapGameData; +import org.junit.jupiter.api.Test; + +public class BreadthFirstSearchTest { + private final GameData gameData = TestMapGameData.REVISED.getGameData(); + private final Territory caucasus = gameData.getMap().getTerritory("Caucasus"); + private final Territory germany = gameData.getMap().getTerritory("Germany"); + private final Territory russia = gameData.getMap().getTerritory("Russia"); + private final Territory uk = gameData.getMap().getTerritory("United Kingdom"); + + private int getLandDistance(Territory from, Territory to) { + var territoryFinder = new BreadthFirstSearch.TerritoryFinder(to); + new BreadthFirstSearch(from, Matches.territoryIsLand()).traverse(territoryFinder); + return territoryFinder.getDistanceFound(); + } + + @Test + void testLandDistance() { + assertThat(getLandDistance(caucasus, russia), is(1)); + assertThat(getLandDistance(caucasus, germany), is(3)); + } + + @Test + void testLandDistanceNotFound() { + assertThat(getLandDistance(caucasus, uk), is(-1)); + } + + @Test + void testLandDistanceSameTerritory() { + // Note: This is testing the limitation described in the API doc. + // This is a test for the low-level helper class, but the high level API, which is tested by + // GameMapTest.testLandDistanceSameTerritory() returns the expected result of 0. + assertThat(getLandDistance(caucasus, caucasus), is(-1)); + } +}