Performance Improvements to Construction Automation (#8508)

* Improvements to construction automation

* Remove early return in work boat consideration

* Reviews and minor addition

* Move cache init to correct file

* I swear I didn't delete that line during the merge

* Redundancy

core/src/com/unciv/logic/automation/Automation.kt

@@ -107,7 +107,9 @@ object Automation {
 
     fun tryTrainMilitaryUnit(city: City) {
         if (city.isPuppet) return
-        val chosenUnitName = chooseMilitaryUnit(city)
+        if ((city.cityConstructions.getCurrentConstruction() as? BaseUnit)?.isMilitary() == true)
+            return // already training a military unit
+        val chosenUnitName = chooseMilitaryUnit(city, city.civInfo.gameInfo.ruleSet.units.values.asSequence())
         if (chosenUnitName != null)
             city.cityConstructions.currentConstructionFromQueue = chosenUnitName
     }
@@ -133,35 +135,36 @@ object Automation {
         return totalCarriableUnits < totalCarryingSlots
     }
 
-    fun chooseMilitaryUnit(city: City): String? {
+    fun chooseMilitaryUnit(city: City, availableUnits: Sequence<BaseUnit>): String? {
         val currentChoice = city.cityConstructions.getCurrentConstruction()
         if (currentChoice is BaseUnit && !currentChoice.isCivilian()) return city.cityConstructions.currentConstructionFromQueue
 
-        var militaryUnits = city.getRuleset().units.values
+        // if not coastal, removeShips == true so don't even consider ships
-            .filter { !it.isCivilian() }
+        var removeShips = true
-            .filter { allowSpendingResource(city.civInfo, it) }
+        if (city.isCoastal()) {
-
+            // in the future this could be simplified by assigning every distinct non-lake body of
+            // water their own ID like a continent ID
             val findWaterConnectedCitiesAndEnemies =
                     BFS(city.getCenterTile()) { it.isWater || it.isCityCenter() }
             findWaterConnectedCitiesAndEnemies.stepToEnd()
-        if (findWaterConnectedCitiesAndEnemies.getReachedTiles().none {
+            removeShips = findWaterConnectedCitiesAndEnemies.getReachedTiles().none {
                         (it.isCityCenter() && it.getOwner() != city.civInfo)
                                 || (it.militaryUnit != null && it.militaryUnit!!.civInfo != city.civInfo)
-            }) // there is absolutely no reason for you to make water units on this body of water.
+                    } // there is absolutely no reason for you to make water units on this body of water.
-            militaryUnits = militaryUnits.filter { !it.isWaterUnit() }
-
-
-        val carryingOnlyUnits = militaryUnits.filter {
-            it.hasUnique(UniqueType.CarryAirUnits)
-                && it.hasUnique(UniqueType.CannotAttack)
         }
 
-        for (unit in carryingOnlyUnits)
+        val militaryUnits = availableUnits
-            if (providesUnneededCarryingSlots(unit, city.civInfo))
+            .filter { it.isMilitary() }
-                militaryUnits = militaryUnits.filterNot { it == unit }
+            .filterNot { removeShips && it.isWaterUnit() }
-
+            .filter { allowSpendingResource(city.civInfo, it) }
+            .filterNot {
+                // filter out carrier-type units that can't attack if we don't need them
+                (it.hasUnique(UniqueType.CarryAirUnits) && it.hasUnique(UniqueType.CannotAttack))
+                        && providesUnneededCarryingSlots(it, city.civInfo)
+            }
             // Only now do we filter out the constructable units because that's a heavier check
-        militaryUnits = militaryUnits.filter { it.isBuildable(city.cityConstructions) } // gather once because we have a .any afterwards
+            .filter { it.isBuildable(city.cityConstructions) }
+            .toList()
 
         val chosenUnit: BaseUnit
         if (!city.civInfo.isAtWar()
@@ -172,18 +175,15 @@ object Automation {
                 .filter { it.isRanged() }
                 .maxByOrNull { it.cost }!!
         } else { // randomize type of unit and take the most expensive of its kind
-            val availableTypes = militaryUnits
+            val bestUnitsForType = hashMapOf<String, BaseUnit>()
-                .map { it.unitType }
+            for (unit in militaryUnits) {
-                .distinct()
+                if (bestUnitsForType[unit.unitType] == null || bestUnitsForType[unit.unitType]!!.cost < unit.cost) {
-            if (availableTypes.none()) return null
+                    bestUnitsForType[unit.unitType] = unit
-            val bestUnitsForType = availableTypes.map { type ->
+                }
-                militaryUnits
-                    .filter { unit -> unit.unitType == type }
-                    .maxByOrNull { unit -> unit.cost }!!
             }
             // Check the maximum force evaluation for the shortlist so we can prune useless ones (ie scouts)
-            val bestForce = bestUnitsForType.maxOf { it.getForceEvaluation() }
+            val bestForce = bestUnitsForType.maxOfOrNull { it.value.getForceEvaluation() } ?: return null
-            chosenUnit = bestUnitsForType.filter { it.uniqueTo != null || it.getForceEvaluation() > bestForce / 3 }.random()
+            chosenUnit = bestUnitsForType.filterValues { it.uniqueTo != null || it.getForceEvaluation() > bestForce / 3 }.values.random()
         }
         return chosenUnit.name
     }

core/src/com/unciv/logic/automation/city/ConstructionAutomation.kt

@@ -24,11 +24,17 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
     private val cityInfo = cityConstructions.city
     private val civInfo = cityInfo.civInfo
 
-    private val buildings = cityInfo.getRuleset().buildings.values
+    private val buildableBuildings = hashMapOf<String, Boolean>()
+    private val buildableUnits = hashMapOf<String, Boolean>()
+    private val buildings = cityInfo.getRuleset().buildings.values.asSequence()
+
     private val nonWonders = buildings.filterNot { it.isAnyWonder() }
+        .filterNot { buildableBuildings[it.name] == false } // if we already know that this building can't be built here then don't even consider it
+    private val statBuildings = nonWonders.filter { !it.isEmpty() && Automation.allowAutomatedConstruction(civInfo, cityInfo, it) }
     private val wonders = buildings.filter { it.isAnyWonder() }
 
-    private val units = cityInfo.getRuleset().units.values
+    private val units = cityInfo.getRuleset().units.values.asSequence()
+        .filterNot { buildableUnits[it.name] == false } // if we already know that this unit can't be built here then don't even consider it
 
     private val civUnits = civInfo.units.getCivUnits()
     private val militaryUnits = civUnits.count { it.baseUnit.isMilitary() }
@@ -57,8 +63,14 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
         choices.add(ConstructionChoice(choice, choiceModifier, cityConstructions.getRemainingWork(choice)))
     }
 
-    private fun Collection<INonPerpetualConstruction>.isBuildable(): Collection<INonPerpetualConstruction> {
+    private fun Sequence<INonPerpetualConstruction>.filterBuildable(): Sequence<INonPerpetualConstruction> {
-        return this.filter { it.isBuildable(cityConstructions) }
+        return this.filter {
+            val cache = if (it is Building) buildableBuildings else buildableUnits
+            if (cache[it.name] == null) {
+                cache[it.name] = it.isBuildable(cityConstructions)
+            }
+            cache[it.name]!!
+        }
     }
 
 
@@ -115,7 +127,7 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
         if (!isAtWar && (civInfo.stats.statsForNextTurn.gold < 0 || militaryUnits > max(5, cities * 2))) return
         if (civInfo.gold < -50) return
 
-        val militaryUnit = Automation.chooseMilitaryUnit(cityInfo) ?: return
+        val militaryUnit = Automation.chooseMilitaryUnit(cityInfo, units) ?: return
         val unitsToCitiesRatio = cities.toFloat() / (militaryUnits + 1)
         // most buildings and civ units contribute the the civ's growth, military units are anti-growth
         var modifier = sqrt(unitsToCitiesRatio) / 2
@@ -138,7 +150,7 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
             .filter {
                 it.hasUnique(UniqueType.CreateWaterImprovements)
                     && Automation.allowAutomatedConstruction(civInfo, cityInfo, it)
-            }.isBuildable()
+            }.filterBuildable()
         val alreadyHasWorkBoat = buildableWorkboatUnits.any()
             && !cityInfo.getTiles().any {
                 it.civilianUnit?.hasUnique(UniqueType.CreateWaterImprovements) == true
@@ -170,7 +182,7 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
             .filter {
                 it.hasUnique(UniqueType.BuildImprovements)
                         && Automation.allowAutomatedConstruction(civInfo, cityInfo, it)
-            }.isBuildable()
+            }.filterBuildable()
         if (workerEquivalents.none()) return // for mods with no worker units
 
         // For the first 3 cities, dedicate a worker, from then on only build another worker if you have 12 cities.
@@ -184,10 +196,9 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
     }
 
     private fun addCultureBuildingChoice() {
-        val cultureBuilding = nonWonders
+        val cultureBuilding = statBuildings
-            .filter { it.isStatRelated(Stat.Culture)
+            .filter { it.isStatRelated(Stat.Culture) }
-                    && Automation.allowAutomatedConstruction(civInfo, cityInfo, it)
+            .filterBuildable()
-            }.isBuildable()
             .minByOrNull { it.cost }
         if (cultureBuilding != null) {
             var modifier = 0.5f
@@ -199,17 +210,19 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
     }
 
     private fun addSpaceshipPartChoice() {
-        val spaceshipPart = (nonWonders + units).filter { it.name in spaceshipParts }.isBuildable()
+        if (!civInfo.hasUnique(UniqueType.EnablesConstructionOfSpaceshipParts)) return
-        if (spaceshipPart.isNotEmpty()) {
+        val spaceshipPart = (nonWonders + units).filter { it.name in spaceshipParts }.filterBuildable().firstOrNull()
+        if (spaceshipPart != null) {
             val modifier = 2f
-            addChoice(relativeCostEffectiveness, spaceshipPart.first().name, modifier)
+            addChoice(relativeCostEffectiveness, spaceshipPart.name, modifier)
         }
     }
 
     private fun addOtherBuildingChoice() {
         val otherBuilding = nonWonders
             .filter { Automation.allowAutomatedConstruction(civInfo, cityInfo, it) }
-            .isBuildable().minByOrNull { it.cost }
+            .filterBuildable()
+            .minByOrNull { it.cost }
         if (otherBuilding != null) {
             val modifier = 0.6f
             addChoice(relativeCostEffectiveness, otherBuilding.name, modifier)
@@ -236,7 +249,7 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
             return if (civInfo.wantsToFocusOn(Victory.Focus.Science)) 1.5f
             else 1.3f
         }
-        if (wonder.name == "Manhattan Project") {
+        if (wonder.hasUnique(UniqueType.EnablesNuclearWeapons)) {
             return if (civInfo.wantsToFocusOn(Victory.Focus.Military)) 2f
             else 1.3f
         }
@@ -250,7 +263,8 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
 
         val highestPriorityWonder = wonders
             .filter { Automation.allowAutomatedConstruction(civInfo, cityInfo, it) }
-            .isBuildable().maxByOrNull { getWonderPriority(it as Building) }
+            .filterBuildable()
+            .maxByOrNull { getWonderPriority(it as Building) }
             ?: return
 
         val citiesBuildingWonders = civInfo.cities
@@ -265,7 +279,8 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
         val unitTrainingBuilding = nonWonders
             .filter { it.hasUnique(UniqueType.UnitStartingExperience)
                     && Automation.allowAutomatedConstruction(civInfo, cityInfo, it)
-            }.isBuildable()
+            }
+            .filterBuildable()
             .minByOrNull { it.cost }
         if (unitTrainingBuilding != null && (!civInfo.wantsToFocusOn(Victory.Focus.Culture) || isAtWar)) {
             var modifier = if (cityIsOverAverageProduction) 0.5f else 0.1f // You shouldn't be cranking out units anytime soon
@@ -280,7 +295,8 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
         val defensiveBuilding = nonWonders
             .filter { it.cityStrength > 0
                     && Automation.allowAutomatedConstruction(civInfo, cityInfo, it)
-            }.isBuildable()
+            }
+            .filterBuildable()
             .minByOrNull { it.cost }
         if (defensiveBuilding != null && (isAtWar || !civInfo.wantsToFocusOn(Victory.Focus.Culture))) {
             var modifier = 0.2f
@@ -301,7 +317,7 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
             .filter { (it.isStatRelated(Stat.Happiness)
                     || it.hasUnique(UniqueType.RemoveAnnexUnhappiness))
                     && Automation.allowAutomatedConstruction(civInfo, cityInfo, it) }
-            .isBuildable()
+            .filterBuildable()
             .minByOrNull { it.cost }
         if (happinessBuilding != null) {
             var modifier = 1f
@@ -315,10 +331,11 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
 
     private fun addScienceBuildingChoice() {
         if (allTechsAreResearched) return
-        val scienceBuilding = nonWonders
+        val scienceBuilding = statBuildings
             .filter { it.isStatRelated(Stat.Science)
                     && Automation.allowAutomatedConstruction(civInfo, cityInfo, it) }
-            .isBuildable().minByOrNull { it.cost }
+            .filterBuildable()
+            .minByOrNull { it.cost }
         if (scienceBuilding != null) {
             var modifier = 1.1f
             if (civInfo.wantsToFocusOn(Victory.Focus.Science))
@@ -328,9 +345,9 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
     }
 
     private fun addGoldBuildingChoice() {
-        val goldBuilding = nonWonders.filter { it.isStatRelated(Stat.Gold)
+        val goldBuilding = statBuildings.filter { it.isStatRelated(Stat.Gold) }
-            && Automation.allowAutomatedConstruction(civInfo, cityInfo, it) }
+            .filterBuildable()
-            .isBuildable().minByOrNull { it.cost }
+            .minByOrNull { it.cost }
         if (goldBuilding != null) {
             val modifier = if (civInfo.stats.statsForNextTurn.gold < 0) 3f else 1.2f
             addChoice(relativeCostEffectiveness, goldBuilding.name, modifier)
@@ -338,9 +355,10 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
     }
 
     private fun addProductionBuildingChoice() {
-        val productionBuilding = nonWonders
+        val productionBuilding = statBuildings
-            .filter { it.isStatRelated(Stat.Production) && Automation.allowAutomatedConstruction(civInfo, cityInfo, it) }
+            .filter { it.isStatRelated(Stat.Production) }
-            .isBuildable().minByOrNull { it.cost }
+            .filterBuildable()
+            .minByOrNull { it.cost }
         if (productionBuilding != null) {
             addChoice(relativeCostEffectiveness, productionBuilding.name, 1.5f)
         }
@@ -353,7 +371,7 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
                 (it.isStatRelated(Stat.Food)
                     || it.hasUnique(UniqueType.CarryOverFood, conditionalState)
                 ) && Automation.allowAutomatedConstruction(civInfo, cityInfo, it)
-            }.isBuildable().minByOrNull { it.cost }
+            }.filterBuildable().minByOrNull { it.cost }
         if (foodBuilding != null) {
             var modifier = 1f
             if (cityInfo.population.population < 5) modifier = 1.3f

core/src/com/unciv/logic/automation/civilization/NextTurnAutomation.kt

@@ -89,7 +89,7 @@ object NextTurnAutomation {
             chooseReligiousBeliefs(civInfo)
         }
 
-        reassignWorkedTiles(civInfo)  // second most expensive
+        automateCities(civInfo)  // second most expensive
         trainSettler(civInfo)
         tryVoteForDiplomaticVictory(civInfo)
     }
@@ -892,7 +892,7 @@ object NextTurnAutomation {
         for (city in civInfo.cities) UnitAutomation.tryBombardEnemy(city)
     }
 
-    private fun reassignWorkedTiles(civInfo: Civilization) {
+    private fun automateCities(civInfo: Civilization) {
         for (city in civInfo.cities) {
             if (city.isPuppet && city.population.population > 9
                     && !city.isInResistance()) {
@@ -901,9 +901,13 @@ object NextTurnAutomation {
 
             city.reassignAllPopulation()
 
+            if (city.health < city.getMaxHealth()) {
+                Automation.tryTrainMilitaryUnit(city) // need defenses if city is under attack
+                if (city.cityConstructions.constructionQueue.isNotEmpty())
+                    continue // found a unit to build so move on
+            }
+
             city.cityConstructions.chooseNextConstruction()
-            if (city.health < city.getMaxHealth())
-                Automation.tryTrainMilitaryUnit(city) // override previous decision if city is under attack
         }
     }
 

core/src/com/unciv/logic/civilization/Civilization.kt

@@ -476,8 +476,8 @@ class Civilization : IsPartOfGameInfoSerialization {
         if (baseBuilding.replaces != null)
             return getEquivalentBuilding(baseBuilding.replaces!!)
 
-        for (building in gameInfo.ruleSet.buildings.values)
+        for (building in cache.uniqueBuildings)
-            if (building.replaces == baseBuilding.name && building.uniqueTo == civName)
+            if (building.replaces == baseBuilding.name)
                 return building
         return baseBuilding
     }
@@ -492,8 +492,8 @@ class Civilization : IsPartOfGameInfoSerialization {
         if (baseUnit.replaces != null)
             return getEquivalentUnit(baseUnit.replaces!!) // Equivalent of unique unit is the equivalent of the replaced unit
 
-        for (unit in gameInfo.ruleSet.units.values)
+        for (unit in cache.uniqueUnits)
-            if (unit.replaces == baseUnit.name && unit.uniqueTo == civName)
+            if (unit.replaces == baseUnit.name)
                 return unit
         return baseUnit
     }

core/src/com/unciv/logic/civilization/transients/CivInfoTransientCache.kt

@@ -9,12 +9,14 @@ import com.unciv.logic.civilization.PlayerType
 import com.unciv.logic.civilization.Proximity
 import com.unciv.logic.map.MapShape
 import com.unciv.logic.map.tile.Tile
+import com.unciv.models.ruleset.Building
 import com.unciv.models.ruleset.tile.ResourceSupplyList
 import com.unciv.models.ruleset.tile.ResourceType
 import com.unciv.models.ruleset.unique.StateForConditionals
 import com.unciv.models.ruleset.unique.UniqueTarget
 import com.unciv.models.ruleset.unique.UniqueTriggerActivation
 import com.unciv.models.ruleset.unique.UniqueType
+import com.unciv.models.ruleset.unit.BaseUnit
 
 /** CivInfo class was getting too crowded */
 class CivInfoTransientCache(val civInfo: Civilization) {
@@ -25,6 +27,13 @@ class CivInfoTransientCache(val civInfo: Civilization) {
     @Transient
     val lastEraResourceUsedForUnit = java.util.HashMap<String, Int>()
 
+    /** Easy way to look up a Civilization's unique units and buildings */
+    @Transient
+    val uniqueUnits = hashSetOf<BaseUnit>()
+
+    @Transient
+    val uniqueBuildings = hashSetOf<Building>()
+
     fun setTransients(){
         val ruleset = civInfo.gameInfo.ruleSet
         for (resource in ruleset.tileResources.values.asSequence().filter { it.resourceType == ResourceType.Strategic }.map { it.name }) {
@@ -39,6 +48,18 @@ class CivInfoTransientCache(val civInfo: Civilization) {
             if (lastEraForUnit != null)
                 lastEraResourceUsedForUnit[resource] = lastEraForUnit
         }
+
+        for (building in ruleset.buildings.values) {
+            if (building.uniqueTo == civInfo.civName) {
+                uniqueBuildings.add(building)
+            }
+        }
+
+        for (unit in ruleset.units.values) {
+            if (unit.uniqueTo == civInfo.civName) {
+                uniqueUnits.add(unit)
+            }
+        }
     }
 
     fun updateSightAndResources() {

core/src/com/unciv/models/ruleset/Building.kt

@@ -577,7 +577,7 @@ class Building : RulesetStatsObject(), INonPerpetualConstruction {
         if (uniqueTo != null && uniqueTo != civInfo.civName)
             yield(RejectionReasonType.UniqueToOtherNation.toInstance("Unique to $uniqueTo"))
 
-        if (civInfo.gameInfo.ruleSet.buildings.values.any { it.uniqueTo == civInfo.civName && it.replaces == name })
+        if (civInfo.cache.uniqueBuildings.any { it.replaces == name })
             yield(RejectionReasonType.ReplacedByOurUnique.toInstance())
 
         if (requiredTech != null && !civInfo.tech.isResearched(requiredTech!!))

core/src/com/unciv/models/ruleset/unit/BaseUnit.kt

@@ -147,7 +147,7 @@ class BaseUnit : RulesetObject(), INonPerpetualConstruction {
 
         if (uniqueTo != null && uniqueTo != civInfo.civName)
             yield(RejectionReasonType.UniqueToOtherNation.toInstance("Unique to $uniqueTo"))
-        if (ruleSet.units.values.any { it.uniqueTo == civInfo.civName && it.replaces == name })
+        if (civInfo.cache.uniqueUnits.any { it.replaces == name })
             yield(RejectionReasonType.ReplacedByOurUnique.toInstance("Our unique unit replaces this"))
 
         if (!civInfo.gameInfo.gameParameters.nuclearWeaponsEnabled && isNuclearWeapon())