From 862f3680751352986a5c1641a0981db8bf9e9a15 Mon Sep 17 00:00:00 2001
From: Jack Rainy <JackRainy@users.noreply.github.com>
Date: Sun, 5 Jun 2022 23:17:59 +0300
Subject: [PATCH] Bugfixes to the units' teleportation (#7071)

* Allow to pass other units when teleporting

* Teleport to the closest city tile

* Additional unit tests
---
 .../unciv/logic/map/UnitMovementAlgorithms.kt | 15 +++----
 .../logic/map/UnitMovementAlgorithmsTests.kt  | 41 ++++++++++++++++---
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt
index b490cf26b7..0561069cf1 100644
--- a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt
+++ b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt
@@ -385,18 +385,15 @@ class UnitMovementAlgorithms(val unit: MapUnit) {
                 // can the unit be placed safely there?
                 .filter { canMoveTo(it) }
                 // out of those where it can be placed, can it reach them in any meaningful way?
-                .firstOrNull { getPathBetweenTiles(unit.currentTile, it).size > 1 }
+                .firstOrNull { getPathBetweenTiles(unit.currentTile, it).contains(it) }
         }
 
         // No tile within 4 spaces? move him to a city.
-        if (allowedTile == null) {
-            for (city in unit.civInfo.cities) {
-                allowedTile = city.getTiles()
-                        .firstOrNull { canMoveTo(it) }
-                if (allowedTile != null) break
-            }
-        }
         val origin = unit.getTile()
+        if (allowedTile == null)
+            allowedTile = unit.civInfo.cities.flatMap { it.getTiles() }
+                .sortedBy { it.aerialDistanceTo(origin) }.firstOrNull{ canMoveTo(it) }
+
         if (allowedTile != null) {
             unit.removeFromTile() // we "teleport" them away
             unit.putInTile(allowedTile)
@@ -740,7 +737,7 @@ class UnitMovementAlgorithms(val unit: MapUnit) {
     private fun getPathBetweenTiles(from: TileInfo, to: TileInfo): MutableSet<TileInfo> {
         val tmp = unit.canEnterForeignTerrain
         unit.canEnterForeignTerrain = true // the trick to ignore tiles owners
-        val bfs = BFS(from) { canMoveTo(it) }
+        val bfs = BFS(from) { canPassThrough(it) }
         bfs.stepUntilDestination(to)
         unit.canEnterForeignTerrain = tmp
         return bfs.getReachedTiles()
diff --git a/tests/src/com/unciv/logic/map/UnitMovementAlgorithmsTests.kt b/tests/src/com/unciv/logic/map/UnitMovementAlgorithmsTests.kt
index 4078acaa9f..dee8d73cab 100644
--- a/tests/src/com/unciv/logic/map/UnitMovementAlgorithmsTests.kt
+++ b/tests/src/com/unciv/logic/map/UnitMovementAlgorithmsTests.kt
@@ -353,6 +353,33 @@ class UnitMovementAlgorithmsTests {
         Assert.assertTrue("Unit must be teleported to new location", unit.currentTile == newTiles.last())
     }
 
+    @Test
+    fun `can teleport water unit over other unit`() {
+        // this is needed for unit.putInTile(), unit.moveThroughTile() to avoid using Uncivgame.Current.viewEntireMapForDebug
+        civInfo.nation.name = Constants.spectator
+
+        tile.baseTerrain = Constants.ocean
+        tile.position.set(0f, 0f)
+        tile.setTransients()
+        createOpponentCivAndCity()
+        val newTiles = generateTileCopies(3)
+
+        // Other unit on the way
+        val otherUnit = MapUnit()
+        otherUnit.civInfo = civInfo
+        otherUnit.owner = civInfo.civName
+        otherUnit.baseUnit = BaseUnit().apply { unitType = "Melee Water"; strength = 1; ruleset = ruleSet }
+        otherUnit.currentTile = newTiles[0]
+        newTiles[0].militaryUnit = otherUnit
+        otherUnit.name = "Friend Unit"
+
+        setupMilitaryUnitInTheCurrentTile("Melee Water")
+
+        unit.movement.teleportToClosestMoveableTile()
+
+        Assert.assertTrue("Unit must be teleported to new location", unit.currentTile == newTiles.last())
+    }
+
     @Test
     fun `can teleport air unit`() {
         // this is needed for unit.putInTile(), unit.moveThroughTile() to avoid using Uncivgame.Current.viewEntireMapForDebug
@@ -380,17 +407,19 @@ class UnitMovementAlgorithmsTests {
         tile.position.set(0f, 0f)
         tile.setTransients()
         createOpponentCivAndCity()
-        val newTiles = generateTileCopies(6)
+        val newTiles = generateTileCopies(7)
         // create obstacle
-        newTiles[4].baseTerrain = "Grand Mesa"
-        newTiles[4].setTransients()
+        newTiles[3].baseTerrain = "Grand Mesa"
+        newTiles[3].setTransients()
         // create our city
         CityInfo().apply {
             this.civInfo = this@UnitMovementAlgorithmsTests.civInfo
             location = newTiles.last().position.cpy()
             tiles.add(location)
+            tiles.add(newTiles[5].position)
             tileMap = tile.tileMap
             civInfo.cities = listOf(this)
+            newTiles[5].setOwningCity(this)
             newTiles.last().setOwningCity(this)
         }
 
@@ -398,7 +427,7 @@ class UnitMovementAlgorithmsTests {
 
         unit.movement.teleportToClosestMoveableTile()
 
-        Assert.assertTrue("Unit must be teleported to the city", unit.currentTile == newTiles.last())
+        Assert.assertTrue("Unit must be teleported to the city", unit.currentTile == newTiles[5])
     }
 
     @Test
@@ -412,8 +441,8 @@ class UnitMovementAlgorithmsTests {
         createOpponentCivAndCity()
         val newTiles = generateTileCopies(3)
         // create obstacle
-        newTiles[0].baseTerrain = Constants.grassland
-        newTiles[0].setTransients()
+        newTiles[1].baseTerrain = Constants.grassland
+        newTiles[1].setTransients()
 
         setupMilitaryUnitInTheCurrentTile("Melee Water")