From b8bd1fc2c2a9a34610771bb8767937b4d7f7f6be Mon Sep 17 00:00:00 2001 From: WhoIsJohannes <126110113+WhoIsJohannes@users.noreply.github.com> Date: Wed, 26 Apr 2023 17:47:07 +0200 Subject: [PATCH] Performance improvements (#9271) * Speed up WorkerAutomation.findTileToWork - apparently tileCanBeImproved is quite expensive * Add cache for rankTileForCityWork in CityPopulationManager.autoAssignPopulation * Optimize NextTurnAutomation.declareWar by moving expensive BFSs to the end and potentially short-circuiting evaluation if result won't be promising anyways. * No need to throw if atLeast is negative. * Revert changes to CityPopulationManager.kt * Revert changes to CityPopulationManager.kt * Speed up WorkerAutomation.findTileToWork - apparently tileCanBeImproved is quite expensive * Add cache for rankTileForCityWork in CityPopulationManager.autoAssignPopulation * Optimize NextTurnAutomation.declareWar by moving expensive BFSs to the end and potentially short-circuiting evaluation if result won't be promising anyways. * No need to throw if atLeast is negative. * Revert changes to CityPopulationManager.kt * Revert changes to CityPopulationManager.kt --- .../civilization/NextTurnAutomation.kt | 59 ++++++++++++------- .../logic/automation/unit/WorkerAutomation.kt | 9 +-- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/core/src/com/unciv/logic/automation/civilization/NextTurnAutomation.kt b/core/src/com/unciv/logic/automation/civilization/NextTurnAutomation.kt index 680e6b2466..34313e1054 100644 --- a/core/src/com/unciv/logic/automation/civilization/NextTurnAutomation.kt +++ b/core/src/com/unciv/logic/automation/civilization/NextTurnAutomation.kt @@ -817,15 +817,18 @@ object NextTurnAutomation { if (enemyCivs.none()) return + val minMotivationToAttack = 20 val civWithBestMotivationToAttack = enemyCivs - .map { Pair(it, motivationToAttack(civInfo, it)) } + .map { Pair(it, hasAtLeastMotivationToAttack(civInfo, it, minMotivationToAttack)) } .maxByOrNull { it.second }!! - if (civWithBestMotivationToAttack.second >= 20) + if (civWithBestMotivationToAttack.second >= minMotivationToAttack) civInfo.getDiplomacyManager(civWithBestMotivationToAttack.first).declareWar() } - private fun motivationToAttack(civInfo: Civilization, otherCiv: Civilization): Int { + /** Will return the motivation to attack, but might short circuit if the value is guaranteed to + * be lower than `atLeast`. So any values below `atLeast` should not be used for comparison. */ + private fun hasAtLeastMotivationToAttack(civInfo: Civilization, otherCiv: Civilization, atLeast: Int): Int { val closestCities = getClosestCities(civInfo, otherCiv) ?: return 0 val baseForce = 30f @@ -859,12 +862,6 @@ object NextTurnAutomation { && (owner == otherCiv || owner == null || civInfo.diplomacyFunctions.canPassThroughTiles(owner)) } - val reachableEnemyCitiesBfs = BFS(civInfo.getCapital()!!.getCenterTile()) { isTileCanMoveThrough(it) } - reachableEnemyCitiesBfs.stepToEnd() - val reachableEnemyCities = otherCiv.cities.filter { reachableEnemyCitiesBfs.hasReachedTile(it.getCenterTile()) } - if (reachableEnemyCities.isEmpty()) return 0 // Can't even reach the enemy city, no point in war. - - val modifierMap = HashMap() val combatStrengthRatio = ourCombatStrength / theirCombatStrength val combatStrengthModifier = when { @@ -880,14 +877,6 @@ object NextTurnAutomation { if (closestCities.aerialDistance > 7) modifierMap["Far away cities"] = -10 - val landPathBFS = BFS(ourCity.getCenterTile()) { - it.isLand && isTileCanMoveThrough(it) - } - - landPathBFS.stepUntilDestination(theirCity.getCenterTile()) - if (!landPathBFS.hasReachedTile(theirCity.getCenterTile())) - modifierMap["No land path"] = -10 - val diplomacyManager = civInfo.getDiplomacyManager(otherCiv) if (diplomacyManager.hasFlag(DiplomacyFlags.ResearchAgreement)) modifierMap["Research Agreement"] = -5 @@ -924,7 +913,35 @@ object NextTurnAutomation { modifierMap["About to win"] = 15 } - return modifierMap.values.sum() + var motivationSoFar = modifierMap.values.sum() + + // We don't need to execute the expensive BFSs below if we're below the threshold here + // anyways, since it won't get better from those, only worse. + if (motivationSoFar < atLeast) { + return motivationSoFar + } + + + val landPathBFS = BFS(ourCity.getCenterTile()) { + it.isLand && isTileCanMoveThrough(it) + } + + landPathBFS.stepUntilDestination(theirCity.getCenterTile()) + if (!landPathBFS.hasReachedTile(theirCity.getCenterTile())) + motivationSoFar -= -10 + + // We don't need to execute the expensive BFSs below if we're below the threshold here + // anyways, since it won't get better from those, only worse. + if (motivationSoFar < atLeast) { + return motivationSoFar + } + + val reachableEnemyCitiesBfs = BFS(civInfo.getCapital()!!.getCenterTile()) { isTileCanMoveThrough(it) } + reachableEnemyCitiesBfs.stepToEnd() + val reachableEnemyCities = otherCiv.cities.filter { reachableEnemyCitiesBfs.hasReachedTile(it.getCenterTile()) } + if (reachableEnemyCities.isEmpty()) return 0 // Can't even reach the enemy city, no point in war. + + return motivationSoFar } @@ -941,8 +958,10 @@ object NextTurnAutomation { .filter { it.tradeRequests.none { tradeRequest -> tradeRequest.requestingCiv == civInfo.civName && tradeRequest.trade.isPeaceTreaty() } } for (enemy in enemiesCiv) { - val motivationToAttack = motivationToAttack(civInfo, enemy) - if (motivationToAttack >= 10) continue // We can still fight. Refuse peace. + if(hasAtLeastMotivationToAttack(civInfo, enemy, 10) >= 10) { + // We can still fight. Refuse peace. + continue + } // pay for peace val tradeLogic = TradeLogic(civInfo, enemy) diff --git a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt b/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt index baa8ab7c90..9b9b794f41 100644 --- a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt +++ b/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt @@ -285,7 +285,7 @@ class WorkerAutomation( .filter { (it.civilianUnit == null || it == currentTile) && (it.owningCity == null || it.getOwner()==civInfo) - && (tileCanBeImproved(unit, it) || it.isPillaged()) + && getPriority(it) > 1 && it.getTilesInDistance(2) // don't work in range of enemy cities .none { tile -> tile.isCityCenter() && tile.getCity()!!.civ.isAtWarWith(civInfo) } && it.getTilesInDistance(3) // don't work in range of enemy units @@ -293,13 +293,10 @@ class WorkerAutomation( } .sortedByDescending { getPriority(it) } - // the tile needs to be actually reachable - more difficult than it seems, - // which is why we DON'T calculate this for every possible tile in the radius, - // but only for the tile that's about to be chosen. - val selectedTile = workableTiles.firstOrNull { unit.movement.canReach(it) } + // These are the expensive calculations (tileCanBeImproved, canReach), so we only apply these filters after everything else it done. + val selectedTile = workableTiles.firstOrNull { unit.movement.canReach(it) && (tileCanBeImproved(unit, it) || it.isPillaged()) } return if (selectedTile != null - && getPriority(selectedTile) > 1 && (!workableTiles.contains(currentTile) || getPriority(selectedTile) > getPriority(currentTile))) selectedTile