From 6b469cb25b5b2b4392cfa037cea4bcad92d69369 Mon Sep 17 00:00:00 2001 From: Yair Morgenstern Date: Tue, 9 Jan 2024 12:31:37 +0200 Subject: [PATCH] Final performance improvements for the new algotihm, before we say 'goodnight sweet prince' - it underperforms drastically compared to current --- .../map/mapunit/movement/UnitMovement.kt | 113 +++++++++++------- 1 file changed, 67 insertions(+), 46 deletions(-) diff --git a/core/src/com/unciv/logic/map/mapunit/movement/UnitMovement.kt b/core/src/com/unciv/logic/map/mapunit/movement/UnitMovement.kt index 0ac70f90eb..db0df82afa 100644 --- a/core/src/com/unciv/logic/map/mapunit/movement/UnitMovement.kt +++ b/core/src/com/unciv/logic/map/mapunit/movement/UnitMovement.kt @@ -1,3 +1,5 @@ + + package com.unciv.logic.map.mapunit.movement import com.badlogic.gdx.math.Vector2 @@ -31,13 +33,13 @@ class UnitMovement(val unit: MapUnit) { * 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? = null, - passThroughCache: HashMap = HashMap(), - movementCostCache: HashMap, Float> = HashMap() - ): PathsToTilesWithinTurn { + origin: Vector2, + unitMovement: Float, + considerZoneOfControl: Boolean = true, + tilesToIgnore: HashSet? = null, + passThroughCache: HashMap = HashMap(), + movementCostCache: HashMap, Float> = HashMap() + ): PathsToTilesWithinTurn { val distanceToTiles = PathsToTilesWithinTurn() val currentUnitTile = unit.currentTile @@ -65,9 +67,9 @@ class UnitMovement(val unit: MapUnit) { else -> { val key = Pair(tileToCheck, neighbor) val movementCost = - movementCostCache.getOrPut(key) { - MovementCost.getMovementCostBetweenAdjacentTiles(unit, tileToCheck, neighbor, considerZoneOfControl) - } + movementCostCache.getOrPut(key) { + MovementCost.getMovementCostBetweenAdjacentTiles(unit, tileToCheck, neighbor, considerZoneOfControl) + } distanceToTiles[tileToCheck]!!.totalDistance + movementCost } } @@ -95,13 +97,15 @@ class UnitMovement(val unit: MapUnit) { data class MovementStepTotalCost(/** Turn 0 means the initial turn */ val turn: Int, val movementLeft: Float):Comparable { - override operator fun compareTo(other: MovementStepTotalCost) = - compareValuesBy(this, other, {it.turn}, {-it.movementLeft}) + override operator fun compareTo(other: MovementStepTotalCost): Int { + if (turn != other.turn) return turn.compareTo(other.turn) + return other.movementLeft.compareTo(movementLeft) // The higher the MovementLeft, the *lower* the turn cost + } } /** Problem and solution documented at https://yairm210.medium.com/multi-turn-pathfinding-7136bd0bdaf0 */ fun getShortestPathNew(destination: Tile, considerZoneOfControl: Boolean = true, - /** For allowing optional avoid of damaging tiles, tiles outside borders, etc */ shouldAvoidTile: (Tile) -> Boolean = {false}, + /** For allowing optional avoid of damaging tiles, tiles outside borders, etc */ shouldAvoidTile: ((Tile) -> Boolean)? = null, maxTurns: Int = 25, ): List { if (unit.cache.cannotMove) return listOf() @@ -115,6 +119,7 @@ class UnitMovement(val unit: MapUnit) { return listOf(initialStep) } + val tileToBestStep = HashMap() // contains a map of "you can get from X to Y in that turn" tileToBestStep[startingTile] = initialStep @@ -122,7 +127,11 @@ class UnitMovement(val unit: MapUnit) { val tStep = tileToBestStep[t]!! val t2Step = tileToBestStep[t2]!! // This last comparitor is REQUIRED otherwise the tree will think that tiles the same distance away are the same and will throw the second one away! - compareValuesBy(tStep, t2Step, {it.totalCost}, {it.tile.aerialDistanceTo(destination)}, {it.tile.position.hashCode()}) + val totalCostComparison = tStep.totalCost.compareTo(t2Step.totalCost) + if (totalCostComparison != 0) return@TreeSet totalCostComparison + val aerialDistanceComparison = t.aerialDistanceTo(destination).compareTo(t2.aerialDistanceTo(destination)) + if (aerialDistanceComparison != 0) return@TreeSet aerialDistanceComparison + return@TreeSet t.position.hashCode().compareTo(t2.position.hashCode()) } tilesToCheck.add(startingTile) @@ -138,13 +147,17 @@ class UnitMovement(val unit: MapUnit) { val currentTileStep = tileToBestStep[currentTileToCheck]!! for (neighbor in currentTileToCheck.neighbors){ - if (shouldAvoidTileCache.getOrPut(neighbor){ shouldAvoidTile(neighbor) }) continue + if (shouldAvoidTile != null && shouldAvoidTileCache.getOrPut(neighbor){ + shouldAvoidTile(neighbor) + }) continue val currentBestStepToNeighbor = tileToBestStep[neighbor] // If this tile can't beat the current best then no point checking if (currentBestStepToNeighbor!=null && (currentBestStepToNeighbor.totalCost < currentTileStep.totalCost)) continue - if (!canPassThroughCache.getOrPut(neighbor){ canPassThrough(neighbor) }) continue + if (!canPassThroughCache.getOrPut(neighbor){ + canPassThrough(neighbor) + }) continue val movementBetweenTiles: Float = if (!neighbor.isExplored(unit.civ)) 1f // If we don't know then we just guess it to be 1. else movementCostCache.getOrPut(currentTileToCheck to neighbor) { @@ -214,15 +227,6 @@ class UnitMovement(val unit: MapUnit) { * Returns an empty list if there's no way to get to the destination. */ fun getShortestPath(destination: Tile, avoidDamagingTerrain: Boolean = false): List { - if (UncivGame.Current.settings.experimentalMovement) { - fun shouldAvoidEnemyTile(tile:Tile) = unit.isCivilian() && unit.isAutomated() && tile.isEnemyTerritory(unit.civ) - if (avoidDamagingTerrain){ - val shortestPathWithoutDamagingTiles = getShortestPathNew(destination, - shouldAvoidTile = { shouldAvoidEnemyTile(it) || unit.getDamageFromTerrain(it) > 0 }) - if (shortestPathWithoutDamagingTiles.isNotEmpty()) return shortestPathWithoutDamagingTiles.toBackwardsCompatiblePath() - } - return getShortestPathNew(destination, shouldAvoidTile = ::shouldAvoidEnemyTile).toBackwardsCompatiblePath() - } if (unit.cache.cannotMove) return listOf() // First try and find a path without damaging terrain @@ -231,7 +235,7 @@ class UnitMovement(val unit: MapUnit) { if (damageFreePath.isNotEmpty()) return damageFreePath } if (unit.baseUnit.isWaterUnit() - && destination.neighbors.none { isUnknownTileWeShouldAssumeToBePassable(it) || it.isWater }) { + && destination.neighbors.none { isUnknownTileWeShouldAssumeToBePassable(it) || it.isWater }) { // edge case where this unit is a boat and all of the tiles around the destination are // explored and known to be land so we know a priori that no path exists pathfindingCache.setShortestPathCache(destination, listOf()) @@ -248,6 +252,19 @@ class UnitMovement(val unit: MapUnit) { return listOf(currentTile) } + if (UncivGame.Current.settings.experimentalMovement) { + if (avoidDamagingTerrain){ + val shouldAvoidTile: (Tile) -> Boolean = if (unit.isCivilian() && unit.isAutomated()) + {{unit.getDamageFromTerrain(it) > 0 || it.isEnemyTerritory(unit.civ)}} + else {{unit.getDamageFromTerrain(it) > 0}} + return getShortestPathNew(destination, + shouldAvoidTile = shouldAvoidTile).toBackwardsCompatiblePath() + } + val shouldAvoidTile :((Tile) -> Boolean)? = if (unit.isCivilian() && unit.isAutomated()) + {{it.isEnemyTerritory(unit.civ)}} else null + return getShortestPathNew(destination, shouldAvoidTile = shouldAvoidTile).toBackwardsCompatiblePath() + } + var tilesToCheck = listOf(currentTile) val movementTreeParents = HashMap() // contains a map of "you can get from X to Y in that turn" movementTreeParents[currentTile] = null @@ -306,8 +323,8 @@ class UnitMovement(val unit: MapUnit) { } else { if (movementTreeParents.containsKey(reachableTile)) continue // We cannot be faster than anything existing... if (!isUnknownTileWeShouldAssumeToBePassable(reachableTile) && - !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 + !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) @@ -345,9 +362,13 @@ class UnitMovement(val unit: MapUnit) { val distanceToTiles = getDistanceToTiles() // If the tile is far away, we need to build a path how to get there, and then take the first step - if (!distanceToTiles.containsKey(finalDestination)) - return getShortestPath(finalDestination).firstOrNull() + if (!distanceToTiles.containsKey(finalDestination)) { + val shortestDestination = getShortestPath(finalDestination).firstOrNull() ?: throw UnreachableDestinationException("$unit ${unit.currentTile} cannot reach $finalDestination") + if (shortestDestination !in distanceToTiles) + return distanceToTiles.keys.minBy { it.aerialDistanceTo(finalDestination) } + return shortestDestination + } // we should be able to get there this turn if (canMoveTo(finalDestination)) @@ -360,7 +381,7 @@ class UnitMovement(val unit: MapUnit) { else -> destinationNeighbors .filter { distanceToTiles.containsKey(it) && canMoveTo(it) } .minByOrNull { distanceToTiles.getValue(it).totalDistance } // we can get a little closer - ?: currentTile // We can't get closer... + ?: currentTile // We can't get closer... } } @@ -431,11 +452,11 @@ class UnitMovement(val unit: MapUnit) { reachableTile.civilianUnit else reachableTile.militaryUnit - ) ?: return false + ) ?: return false val ourPosition = unit.getTile() if (otherUnit.owner != unit.owner - || otherUnit.cache.cannotMove // redundant, line below would cover it too - || !otherUnit.movement.canReachInCurrentTurn(ourPosition)) return false + || otherUnit.cache.cannotMove // redundant, line below would cover it too + || !otherUnit.movement.canReachInCurrentTurn(ourPosition)) return false if (!canMoveTo(reachableTile, canSwap = true)) return false if (!otherUnit.movement.canMoveTo(ourPosition, canSwap = true)) return false @@ -633,7 +654,7 @@ class UnitMovement(val unit: MapUnit) { destination.civilianUnit else destination.militaryUnit - )?: return // The precondition guarantees that there is an eligible same-type unit at the destination + )?: return // The precondition guarantees that there is an eligible same-type unit at the destination val ourOldPosition = unit.getTile() val theirOldPosition = otherUnit.getTile() @@ -696,7 +717,7 @@ class UnitMovement(val unit: MapUnit) { else // can skip checking for airUnit since not a city (tile.militaryUnit == null || (canSwap && tile.militaryUnit!!.owner == unit.owner)) - && (tile.civilianUnit == null || tile.civilianUnit!!.owner == unit.owner || unit.civ.isAtWarWith(tile.civilianUnit!!.civ)) + && (tile.civilianUnit == null || tile.civilianUnit!!.owner == unit.owner || unit.civ.isAtWarWith(tile.civilianUnit!!.civ)) } private fun canAirUnitMoveTo(tile: Tile, unit: MapUnit): Boolean { @@ -741,14 +762,14 @@ class UnitMovement(val unit: MapUnit) { return false } if (tile.isLand - && unit.baseUnit.isWaterUnit() - && !tile.isCityCenter()) + && unit.baseUnit.isWaterUnit() + && !tile.isCityCenter()) return false val unitSpecificAllowOcean: Boolean by lazy { unit.civ.tech.specificUnitsCanEnterOcean && - unit.civ.getMatchingUniques(UniqueType.UnitsMayEnterOcean) - .any { unit.matchesFilter(it.params[0]) } + unit.civ.getMatchingUniques(UniqueType.UnitsMayEnterOcean) + .any { unit.matchesFilter(it.params[0]) } } if (tile.isWater && unit.baseUnit.isLandUnit() && !unit.cache.canMoveOnWater) { if (!unit.civ.tech.unitsCanEmbark) return false @@ -773,7 +794,7 @@ class UnitMovement(val unit: MapUnit) { // Allow movement through unguarded, at-war Civilian Unit. Capture on the way // But not for Embarked Units capturing on Water if (!(unit.baseUnit.isLandUnit() && tile.isWater && !unit.cache.canMoveOnWater) - && firstUnit.isCivilian() && unit.civ.isAtWarWith(firstUnit.civ)) + && firstUnit.isCivilian() && unit.civ.isAtWarWith(firstUnit.civ)) return true // Cannot enter hostile tile with any unit in there if (unit.civ.isAtWarWith(firstUnit.civ)) @@ -785,10 +806,10 @@ class UnitMovement(val unit: MapUnit) { fun getDistanceToTiles( - considerZoneOfControl: Boolean = true, - passThroughCache: HashMap = HashMap(), - movementCostCache: HashMap, Float> = HashMap()) - : PathsToTilesWithinTurn { + considerZoneOfControl: Boolean = true, + passThroughCache: HashMap = HashMap(), + movementCostCache: HashMap, Float> = HashMap()) + : PathsToTilesWithinTurn { val cacheResults = pathfindingCache.getDistanceToTiles(considerZoneOfControl) if (cacheResults != null) { return cacheResults @@ -819,7 +840,7 @@ class UnitMovement(val unit: MapUnit) { val newTilesToCheck = ArrayList() for (currentTileToCheck in tilesToCheck) { val reachableTiles = currentTileToCheck.getTilesInDistance(unit.getRange()) - .filter { unit.movement.canMoveTo(it) } + .filter { unit.movement.canMoveTo(it) } for (reachableTile in reachableTiles) { if (tilesReached.containsKey(reachableTile)) continue tilesReached[reachableTile] = currentTileToCheck