Settler re-picks a destination when its chosen spot becomes unreachable (#213)

C7Engine/AI/UnitAI/SettlerLocationAI.cs

@@ -30,7 +30,7 @@ private static Dictionary<Tile, float> AssignTileScores(Tile startTile, Player p
 			Dictionary<Tile, float> scores = new();
 			var memo = new Dictionary<string, float>();
 
-			candidates = candidates.Where(t => !SettlerAlreadyMovingTowardsTile(t, playerSettlers) && t.IsAllowCities());
+			candidates = candidates.Where(t => !SettlerAlreadyMovingTowardsTile(t, playerSettlers) && t.IsAllowCities() && !HasForeignUnitOnTile(t, player));
 
 			foreach (Tile t in candidates) {
 				float score = GetTileYieldScore(t, player, memo);
@@ -144,5 +144,17 @@ private static bool SettlerAlreadyMovingTowardsTile(Tile tile, List<MapUnit> pla
 			}
 			return false;
 		}
+
+		/// <summary>
+		/// Returns true if a foreign unit is on the supplied tile. A settler
+		/// cannot share a tile with another civ's unit, so it can never build
+		/// a city there, and it must not be chosen as a destination.
+		/// </summary>
+		/// <param name="tile">The tile under consideration for a future city.</param>
+		/// <param name="player">The settler's owner.</param>
+		/// <returns></returns>
+		private static bool HasForeignUnitOnTile(Tile tile, Player player) {
+			return tile.unitsOnTile.Any(u => u.owner != player);
+		}
 	}
 }

EngineTests/AI/Pathing/RivalCityRoutingTest.cs

@@ -0,0 +1,186 @@
+using System.Collections.Generic;
+using System.Linq;
+using C7Engine;
+using C7Engine.Pathing;
+using C7GameData;
+using C7GameData.Save;
+using EngineTests.Utils;
+using Xunit;
+
+namespace EngineTests.AI.Pathing;
+
+/// <summary>
+/// Regression tests for https://github.com/C7-Game/OpenCiv3/issues/371:
+/// units should not route through rival cities. A city tile owned by another
+/// civ must be excluded from the middle of a path, while still remaining a
+/// valid destination for a combat unit that wants to attack it.
+/// </summary>
+public sealed class LandUnitAvoidsRivalCityTest : MapBase {
+	// A 3x4 patch of plains tiles arranged so that the direct line from the
+	// start to the destination passes through the rival city, but the city
+	// can be flanked along the top or bottom row:
+	//      (48,48)  (50,48)  (52,48)  (54,48)
+	//      (48,50)  (50,50)  (52,50)  (54,50)
+	//      (48,52)  (50,52)  (52,52)  (54,52)
+	// start = (50,50), rival city = (52,50), destination = (54,50)
+	private readonly List<Tile> grid = new();
+	private readonly Player human;
+	private readonly Player rival;
+	private readonly Tile cityTile;
+	private readonly Tile destinationTile;
+
+	public LandUnitAvoidsRivalCityTest() {
+		int[,] coordinates = {
+			{ 48, 48 }, { 50, 48 }, { 52, 48 }, { 54, 48 },
+			{ 48, 50 }, { 50, 50 }, { 52, 50 }, { 54, 50 },
+			{ 48, 52 }, { 50, 52 }, { 52, 52 }, { 54, 52 },
+		};
+
+		for (int i = 0; i < coordinates.GetLength(0); i++) {
+			Tile tile = MakePlainsTile();
+			tile.XCoordinate = coordinates[i, 0];
+			tile.YCoordinate = coordinates[i, 1];
+			grid.Add(tile);
+		}
+
+		startTile = grid.Single(t => t.XCoordinate == 50 && t.YCoordinate == 50);
+		cityTile = grid.Single(t => t.XCoordinate == 52 && t.YCoordinate == 50);
+		destinationTile = grid.Single(t => t.XCoordinate == 54 && t.YCoordinate == 50);
+
+		ComputeAllNeighbors(grid.ToHashSet());
+
+		human = MakeHumanPlayer("human-1");
+		rival = MakeCivPlayer("rival-1");
+
+		// The human has explored the whole patch, including the rival city.
+		foreach (Tile tile in grid) {
+			human.tileKnowledge.knownTiles.Add(tile);
+		}
+
+		// Belongs to a rival civ.
+		cityTile.cityAtTile = new City(cityTile, rival, "Rival City", ID.None("city"));
+	}
+
+	private static Player MakeCivPlayer(string id) {
+		Player player = new Player();
+		player.id = ID.FromString(id);
+		player.civilization = new Civilization();
+		return player;
+	}
+
+	private static Player MakeHumanPlayer(string id) {
+		Player player = MakeCivPlayer(id);
+		player.isHuman = true;
+		return player;
+	}
+
+	private MapUnit MakeLandUnitOnStart(Player owner, bool combat) {
+		MapUnit unit = MakeLandUnit(2);
+		if (combat) {
+			unit.unitType.attack = 1;
+		}
+		unit.owner = owner;
+		unit.location = startTile;
+		startTile.unitsOnTile.Add(unit);
+		return unit;
+	}
+
+	private static TilePath ComputePath(MapUnit unit, Tile destination) {
+		return PathingAlgorithmChooser.GetAlgorithm(unit).PathFrom(unit.location, destination, unit);
+	}
+
+	[Fact]
+	private void TestHumanCombatUnitDoesNotPathThroughRivalCityAtPeace() {
+		EngineStorage.InitializeGameDataForTests(new C7GameData.GameData(1234));
+		MapUnit unit = MakeLandUnitOnStart(human, combat: true);
+
+		TilePath path = ComputePath(unit, destinationTile);
+
+		// It can still reach the destination...
+		Assert.NotEmpty(path.path);
+		Assert.Contains(destinationTile, path.path);
+		// ...but must not route through the rival city.
+		Assert.DoesNotContain(cityTile, path.path);
+	}
+
+	[Fact]
+	private void TestHumanCombatUnitDoesNotPathThroughRivalCityAtWar() {
+		EngineStorage.InitializeGameDataForTests(new C7GameData.GameData(1235));
+		human.DeclareWarOn(rival, currentTurn: 1);
+		MapUnit unit = MakeLandUnitOnStart(human, combat: true);
+
+		TilePath path = ComputePath(unit, destinationTile);
+
+		Assert.NotEmpty(path.path);
+		Assert.Contains(destinationTile, path.path);
+		Assert.DoesNotContain(cityTile, path.path);
+	}
+
+	[Fact]
+	private void TestAiCombatUnitDoesNotPathThroughRivalCityAtPeace() {
+		EngineStorage.InitializeGameDataForTests(new C7GameData.GameData(1236));
+		Player aiPlayer = MakeCivPlayer("ai-1");
+		MapUnit unit = MakeLandUnitOnStart(aiPlayer, combat: true);
+
+		TilePath path = ComputePath(unit, destinationTile);
+
+		Assert.NotEmpty(path.path);
+		Assert.Contains(destinationTile, path.path);
+		Assert.DoesNotContain(cityTile, path.path);
+	}
+
+	[Fact]
+	private void TestAiCombatUnitDoesNotPathThroughRivalCityAtWar() {
+		EngineStorage.InitializeGameDataForTests(new C7GameData.GameData(1237));
+		Player aiPlayer = MakeCivPlayer("ai-1");
+		aiPlayer.DeclareWarOn(rival, currentTurn: 1);
+		MapUnit unit = MakeLandUnitOnStart(aiPlayer, combat: true);
+
+		TilePath path = ComputePath(unit, destinationTile);
+
+		Assert.NotEmpty(path.path);
+		Assert.Contains(destinationTile, path.path);
+		Assert.DoesNotContain(cityTile, path.path);
+	}
+
+	[Fact]
+	private void TestAiSettlerDoesNotPathThroughRivalCity() {
+		// The scenario described in #371: an AI settler routes through Athens.
+		EngineStorage.InitializeGameDataForTests(new C7GameData.GameData(1240));
+		Player aiPlayer = MakeCivPlayer("ai-1");
+		MapUnit unit = MakeLandUnitOnStart(aiPlayer, combat: false);
+
+		TilePath path = ComputePath(unit, destinationTile);
+
+		// A non-combat unit cannot enter the rival city at all, so no path
+		// through it (or to tiles beyond it) is possible.
+		Assert.DoesNotContain(cityTile, path.path);
+	}
+
+	[Fact]
+	private void TestCombatUnitCanTargetRivalCityAsDestination() {
+		// Attacking a rival city is legitimate - only traversal is banned.
+		EngineStorage.InitializeGameDataForTests(new C7GameData.GameData(1238));
+		Player aiPlayer = MakeCivPlayer("ai-1");
+		aiPlayer.DeclareWarOn(rival, currentTurn: 1);
+		MapUnit unit = MakeLandUnitOnStart(aiPlayer, combat: true);
+
+		TilePath path = ComputePath(unit, cityTile);
+
+		// A combat unit should be able to move onto (attack) a rival city.
+		Assert.NotEmpty(path.path);
+		Assert.Contains(cityTile, path.path);
+	}
+
+	[Fact]
+	private void TestNonCombatUnitCannotEnterRivalCityEvenAsDestination() {
+		EngineStorage.InitializeGameDataForTests(new C7GameData.GameData(1239));
+		Player aiPlayer = MakeCivPlayer("ai-1");
+		MapUnit unit = MakeLandUnitOnStart(aiPlayer, combat: false);
+
+		TilePath path = ComputePath(unit, cityTile);
+
+		// A settler or worker cannot be ordered into a rival city at all.
+		Assert.Empty(path.path);
+	}
+}

EngineTests/AI/UnitAI/SettlerDestinationBlockedTest.cs

@@ -0,0 +1,125 @@
+using C7Engine;
+using C7GameData;
+using C7GameData.AIData;
+using C7GameData.Save;
+using EngineTests.Utils;
+using System.Collections.Generic;
+using Xunit;
+
+namespace EngineTests.AI.UnitAI;
+
+/// <summary>
+/// Tests for https://github.com/C7-Game/OpenCiv3/issues/213: an AI settler
+/// whose path destination becomes unreachable must not stay stuck forever.
+/// The original crash ("Could not get next part of path") is gone, but
+/// FindSettlerLocation still re-picks a destination the settler can no longer
+/// reach (e.g. because a rival unit parked on it), so the settler re-evaluates
+/// the same impossible task every turn.
+/// </summary>
+public sealed class SettlerDestinationBlockedTest : MapBase {
+	// start = (50,50), destination = (52,50), one tile east
+	private readonly Player aiPlayer;
+	private readonly Player rival;
+	private readonly Tile start;
+	private readonly Tile destination;
+
+	public SettlerDestinationBlockedTest() {
+		InitilizeStartTile(MakeDesertTile(), new TileLocation(50, 50));
+		start = startTile;
+
+		destination = MakeHillTile();
+		destination.XCoordinate = 52;
+		destination.YCoordinate = 50;
+		AddNeighborsAndUpdateMap(start, destination, TileDirection.EAST);
+		AddNeighborsAndUpdateMap(destination, start, TileDirection.WEST);
+
+		aiPlayer = MakeAiPlayer();
+		rival = MakeCivPlayer();
+
+		foreach (Tile tile in new List<Tile> { start, destination }) {
+			aiPlayer.tileKnowledge.knownTiles.Add(tile);
+		}
+
+		// The AI already has a home, so its settler goes looking for a spot.
+		Tile homeTile = MakePlainsTile();
+		homeTile.XCoordinate = 10;
+		homeTile.YCoordinate = 10;
+		aiPlayer.cities.Add(new City(homeTile, aiPlayer, "Home", ID.None("")));
+	}
+
+	private static Player MakeCivPlayer() {
+		Player player = new Player();
+		player.id = ID.FromString("rival-1");
+		player.civilization = new Civilization();
+		return player;
+	}
+
+	private static Player MakeAiPlayer() {
+		Player player = new Player();
+		player.id = ID.FromString("ai-1");
+		player.civilization = new Civilization();
+		player.government = new Government();
+		player.rules = MakeTestRules();
+		return player;
+	}
+
+	private MapUnit MakeSettlerOnStart() {
+		MapUnit settler = MakeLandUnit(1);
+		settler.unitType.name = "Settler";
+		settler.owner = aiPlayer;
+		settler.nationality = aiPlayer.civilization;
+		settler.location = start;
+		start.unitsOnTile.Add(settler);
+		aiPlayer.units.Add(settler);
+		return settler;
+	}
+
+	private void ParkRivalUnitOnDestination() {
+		MapUnit blocker = MakeLandUnit(1);
+		blocker.unitType.attack = 1;
+		blocker.owner = rival;
+		blocker.location = destination;
+		destination.unitsOnTile.Add(blocker);
+	}
+
+	[Fact]
+	private void SettlerDoesNotRetargetTheUnreachableDestination() {
+		EngineStorage.InitializeGameDataForTests(new C7GameData.GameData(1));
+
+		// Sanity check: with the destination clear, it is the chosen spot.
+		Assert.Equal(destination, SettlerLocationAI.FindSettlerLocation(start, aiPlayer));
+
+		// A rival unit parks on the destination (the #213 scenario).
+		ParkRivalUnitOnDestination();
+
+		// It must no longer be chosen, or the settler will re-evaluate this
+		// same impossible task forever.
+		Tile chosen = SettlerLocationAI.FindSettlerLocation(start, aiPlayer);
+		Assert.NotEqual(destination, chosen);
+	}
+
+	[Fact]
+	private void SettlerFailsGracefullyWhenDestinationBlockedMidJourney() {
+		EngineStorage.InitializeGameDataForTests(new C7GameData.GameData(2));
+
+		MapUnit settler = MakeSettlerOnStart();
+		settler.movementPoints.reset(settler.unitType.movement);
+
+		// The settler heads off while the destination is still clear.
+		SettlerAIData data = SettlerAI.MakeAiData(settler, aiPlayer);
+		Assert.Equal(SettlerAIData.SettlerGoal.BUILD_CITY, data.goal);
+		Assert.Equal(destination, data.destination);
+		Assert.NotEmpty(data.pathToDestination.path);
+
+		// The destination gets blocked while en route.
+		ParkRivalUnitOnDestination();
+
+		SettlerAI settlerAi = new SettlerAI(data);
+		C7GameData.UnitAI.MoveResult result = settlerAi.TryToMoveAlongPath(settler, ref data.pathToDestination);
+
+		// The move fails gracefully (no exception) and the repath also cannot
+		// reach the blocked destination.
+		Assert.Equal(C7GameData.UnitAI.Result.Error, result.Result);
+		Assert.Equal(Tile.NONE, data.pathToDestination.Next());
+	}
+}