Various performance improvements (#9296)

* Move caches for passThrough and movementCost into the parent method.

* Reuse path calculated for reaching enemy city if still far away instead of recalculating it for the "landing tile".

* Cache getDistanceToTilesWithinTurn by removing tilesToIgnore from the call and doing that filtering later. Also simplify caller side with some transformations around differences for the first iteration and subsequent iterations.

* Check whether a player is spectator by comparing their civName directly with the Constant rather than going through the lazily initialized property of the nation. This is significantly faster (10x ?) and we're calling this method a lot (tens of millions of times).

Also check whether a tile is explored directly on the tile, not the other way round.

* Revert "Cache getDistanceToTilesWithinTurn by removing tilesToIgnore from the call and doing that filtering later. Also simplify caller side with some transformations around differences for the first iteration and subsequent iterations."

This reverts commit f75ce00d83.

* Simplify UnitMovement.getShortestPath

core/src/com/unciv/logic/automation/unit/UnitAutomation.kt

@@ -557,13 +557,22 @@ object UnitAutomation {
                 .firstOrNull { unit.movement.canReach(it.getCenterTile()) }
 
         if (closestReachableEnemyCity != null) {
-            return headTowardsEnemyCity(unit, closestReachableEnemyCity.getCenterTile())
+            return headTowardsEnemyCity(
+                unit,
+                closestReachableEnemyCity.getCenterTile(),
+                // This should be cached after the `canReach` call above.
+                unit.movement.getShortestPath(closestReachableEnemyCity.getCenterTile())
+            )
         }
         return false
     }
 
 
-    private fun headTowardsEnemyCity(unit: MapUnit, closestReachableEnemyCity: Tile): Boolean {
+    private fun headTowardsEnemyCity(
+        unit: MapUnit,
+        closestReachableEnemyCity: Tile,
+        shortestPath: List<Tile>
+    ): Boolean {
         val unitDistanceToTiles = unit.movement.getDistanceToTiles()
         val unitRange = unit.getRange()
 
@@ -586,6 +595,18 @@ object UnitAutomation {
             return false
         }
 
+        // None of the stuff below is relevant if we're still quite far away from the city, so we
+        // short-circuit here for performance reasons.
+        val minDistanceFromCityToConsiderForLandingArea = 3
+        val maxDistanceFromCityToConsiderForLandingArea = 5
+        if (unit.currentTile.aerialDistanceTo(closestReachableEnemyCity) > maxDistanceFromCityToConsiderForLandingArea
+                // Even in the worst case of only being able to move 1 tile per turn, we would still
+                // not overshoot.
+                && shortestPath.size > minDistanceFromCityToConsiderForLandingArea ) {
+            unit.movement.moveToTile(shortestPath[0])
+            return true
+        }
+
         val ourUnitsAroundEnemyCity = closestReachableEnemyCity.getTilesInDistance(6)
             .flatMap { it.getUnits() }
             .filter { it.isMilitary() && it.civ == unit.civ }
@@ -614,7 +635,7 @@ object UnitAutomation {
             return true
         }
 
-        unit.movement.headTowards(closestReachableEnemyCity) // go for it!
+        unit.movement.moveToTile(shortestPath[0]) // go for it!
 
         return true
     }
@@ -682,7 +703,12 @@ object UnitAutomation {
                 .firstOrNull { unit.movement.canReach(it) }
 
         if (closestReachableCapturedCity != null) {
-            return headTowardsEnemyCity(unit, closestReachableCapturedCity)
+            return headTowardsEnemyCity(
+                unit,
+                closestReachableCapturedCity,
+                // This should be cached after the `canReach` call above.
+                unit.movement.getShortestPath(closestReachableCapturedCity)
+            )
         }
         return false
 

core/src/com/unciv/logic/map/mapunit/UnitMovement.kt

@@ -153,7 +153,14 @@ class UnitMovement(val unit: MapUnit) {
      * Does not consider if tiles can actually be entered, use canMoveTo for that.
      * If a tile can be reached within the turn, but it cannot be passed through, the total distance to it is set to unitMovement
      */
-    fun getDistanceToTilesWithinTurn(origin: Vector2, unitMovement: Float, considerZoneOfControl: Boolean = true, tilesToIgnore: HashSet<Tile>? = null): PathsToTilesWithinTurn {
+    fun getDistanceToTilesWithinTurn(
+                origin: Vector2,
+                unitMovement: Float,
+                considerZoneOfControl: Boolean = true,
+                tilesToIgnore: HashSet<Tile>? = null,
+                passThroughCache: HashMap<Tile, Boolean> = HashMap(),
+                movementCostCache: HashMap<Pair<Tile, Tile>, Float> = HashMap()
+            ): PathsToTilesWithinTurn {
         val distanceToTiles = PathsToTilesWithinTurn()
         if (unitMovement == 0f) return distanceToTiles
 
@@ -163,16 +170,13 @@ class UnitMovement(val unit: MapUnit) {
         distanceToTiles[unitTile] = ParentTileAndTotalDistance(unitTile, unitTile, 0f)
         var tilesToCheck = listOf(unitTile)
 
-        val passThroughCache = HashMap<Tile, Boolean>() // Cache for canPassThrough
-        val movementCostCache = HashMap<Pair<Tile, Tile>, Float>() // Cache for getMovementCostBetweenAdjacentTiles
-
         while (tilesToCheck.isNotEmpty()) {
             val updatedTiles = ArrayList<Tile>()
             for (tileToCheck in tilesToCheck)
                 for (neighbor in tileToCheck.neighbors) {
                     if (tilesToIgnore?.contains(neighbor) == true) continue // ignore this tile
                     var totalDistanceToTile: Float = when {
-                        !unit.civ.hasExplored(neighbor) ->
+                        !neighbor.isExplored(unit.civ) ->
                             distanceToTiles[tileToCheck]!!.totalDistance + 1f  // If we don't know then we just guess it to be 1.
                         !passThroughCache.getOrPut(neighbor) { canPassThrough(neighbor) } -> unitMovement // Can't go here.
                         // The reason that we don't just "return" is so that when calculating how to reach an enemy,
@@ -239,18 +243,17 @@ class UnitMovement(val unit: MapUnit) {
         val movementTreeParents = HashMap<Tile, Tile?>() // contains a map of "you can get from X to Y in that turn"
         movementTreeParents[currentTile] = null
 
-        var movementThisTurn = unit.currentMovement
         var distance = 1
+        val unitMaxMovement = unit.getMaxMovement().toFloat()
         val newTilesToCheck = ArrayList<Tile>()
-        var considerZoneOfControl = true // only for first distance!
         val visitedTiles: HashSet<Tile> = hashSetOf(currentTile)
         val civilization = unit.civ
 
+        val passThroughCache = HashMap<Tile, Boolean>()
+        val movementCostCache = HashMap<Pair<Tile, Tile>, Float>()
+        val canMoveToCache = HashMap<Tile, Boolean>()
+
         while (true) {
-            if (distance == 2) { // only set this once after distance > 1
-                movementThisTurn = unit.getMaxMovement().toFloat()
-                considerZoneOfControl = false  // by then units would have moved around, we don't need to consider untenable futures when it harms performance!
-            }
             newTilesToCheck.clear()
 
             var tilesByPreference = tilesToCheck.sortedBy { it.aerialDistanceTo(destination) }
@@ -260,10 +263,17 @@ class UnitMovement(val unit: MapUnit) {
 
             for (tileToCheck in tilesByPreference) {
                 val distanceToTilesThisTurn = if (distance == 1) {
-                    getDistanceToTiles(considerZoneOfControl) // check cache
+                    getDistanceToTiles(true, passThroughCache, movementCostCache) // check cache
                 }
                 else {
-                    getDistanceToTilesWithinTurn(tileToCheck.position, movementThisTurn, considerZoneOfControl, visitedTiles)
+                    getDistanceToTilesWithinTurn(
+                        tileToCheck.position,
+                        unitMaxMovement,
+                        false,
+                        visitedTiles,
+                        passThroughCache,
+                        movementCostCache
+                    )
                 }
                 for (reachableTile in distanceToTilesThisTurn.keys) {
                     // Avoid damaging terrain on first pass
@@ -287,7 +297,9 @@ class UnitMovement(val unit: MapUnit) {
                     } else {
                         if (movementTreeParents.containsKey(reachableTile)) continue // We cannot be faster than anything existing...
                         if (!isUnknownTileWeShouldAssumeToBePassable(reachableTile) &&
-                                !canMoveTo(reachableTile)) continue // This is a tile that we can't actually enter - either an intermediary tile containing our unit, or an enemy unit/city
+                                !canMoveToCache.getOrPut(reachableTile) { canMoveTo(reachableTile) })
+                            // This is a tile that we can't actually enter - either an intermediary tile containing our unit, or an enemy unit/city
+                            continue
                         movementTreeParents[reachableTile] = tileToCheck
                         newTilesToCheck.add(reachableTile)
                     }
@@ -758,12 +770,23 @@ class UnitMovement(val unit: MapUnit) {
     }
 
 
-    fun getDistanceToTiles(considerZoneOfControl: Boolean = true): PathsToTilesWithinTurn {
+    fun getDistanceToTiles(
+                considerZoneOfControl: Boolean = true,
+                passThroughCache: HashMap<Tile, Boolean> = HashMap(),
+                movementCostCache: HashMap<Pair<Tile, Tile>, Float> = HashMap())
+            : PathsToTilesWithinTurn {
         val cacheResults = pathfindingCache.getDistanceToTiles(considerZoneOfControl)
         if (cacheResults != null) {
             return cacheResults
         }
-        val distanceToTiles = getDistanceToTilesWithinTurn(unit.currentTile.position, unit.currentMovement, considerZoneOfControl)
+        val distanceToTiles = getDistanceToTilesWithinTurn(
+            unit.currentTile.position,
+            unit.currentMovement,
+            considerZoneOfControl,
+            null,
+            passThroughCache,
+            movementCostCache
+        )
         pathfindingCache.setDistanceToTiles(considerZoneOfControl, distanceToTiles)
         return distanceToTiles
     }

core/src/com/unciv/logic/map/tile/Tile.kt

@@ -232,7 +232,7 @@ open class Tile : IsPartOfGameInfoSerialization {
     }
 
     fun isExplored(player: Civilization): Boolean {
-        if (DebugUtils.VISIBLE_MAP || player.isSpectator())
+        if (DebugUtils.VISIBLE_MAP || player.civName == Constants.spectator)
             return true
         return exploredBy.contains(player.civName)
     }