Stats rework part 2 (#4983)

- Nicer iterators
- Callers adapted to simpler syntax
- CityStats changed to non-serializable

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

@@ -162,7 +162,7 @@ object Automation {
 
         // Improvements are good: less points
         if (tile.improvement != null &&
-            tile.getImprovementStats(tile.getTileImprovement()!!, cityInfo.civInfo, cityInfo).toHashMap().values.sum() > 0f
+            tile.getImprovementStats(tile.getTileImprovement()!!, cityInfo.civInfo, cityInfo).values.sum() > 0f
         ) score -= 5
 
         // The original checks if the tile has a road, but adds a score of 0 if it does.
@@ -171,7 +171,7 @@ object Automation {
         if (tile.naturalWonder != null) score -= 105
 
         // Straight up take the sum of all yields
-        score -= tile.getTileStats(null, cityInfo.civInfo).toHashMap().values.sum().toInt()
+        score -= tile.getTileStats(null, cityInfo.civInfo).values.sum().toInt()
 
         // Check if we get access to better tiles from this tile
         var adjacentNaturalWonder = false

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

@@ -218,12 +218,12 @@ object SpecificUnitAutomation {
     fun automateImprovementPlacer(unit: MapUnit) {
         val improvementName = unit.getMatchingUniques("Can construct []").first().params[0]
         val improvement = unit.civInfo.gameInfo.ruleSet.tileImprovements[improvementName]!!
-        val relatedStat = improvement.toHashMap().maxByOrNull { it.value }!!.key
+        val relatedStat = improvement.maxByOrNull { it.value }!!.key
 
         val citiesByStatBoost = unit.civInfo.cities.sortedByDescending {
             val stats = Stats()
             for (bonus in it.cityStats.statPercentBonusList.values) stats.add(bonus)
-            stats.toHashMap()[relatedStat]!!
+            stats[relatedStat]
         }
 
 

core/src/com/unciv/logic/battle/Battle.kt

@@ -234,13 +234,13 @@ object Battle {
         }
 
         val civ = plunderingUnit.getCivInfo()
-        plunderedGoods.toHashMap().filterNot { it.value == 0f }.forEach {
+        for ((key, value) in plunderedGoods) {
-            val plunderedAmount = it.value.toInt()
+            val plunderedAmount = value.toInt()
-            civ.addStat(it.key, plunderedAmount)
+            civ.addStat(key, plunderedAmount)
             civ.addNotification(
-                "Your [${plunderingUnit.getName()}] plundered [${plunderedAmount}] [${it.key.name}] from [${plunderedUnit.getName()}]",
+                "Your [${plunderingUnit.getName()}] plundered [${plunderedAmount}] [${key.name}] from [${plunderedUnit.getName()}]",
                 plunderedUnit.getTile().position,
-                plunderingUnit.getName(), NotificationIcon.War, "StatIcons/${it.key.name}",
+                plunderingUnit.getName(), NotificationIcon.War, "StatIcons/${key.name}",
                 if (plunderedUnit is CityCombatant) NotificationIcon.City else plunderedUnit.getName()
             )
         }

core/src/com/unciv/logic/city/CityConstructions.kt

@@ -286,8 +286,7 @@ class CityConstructions {
               we get all sorts of fun concurrency problems when accessing various parts of the cityStats.
             SO, we create an entirely new CityStats and iterate there - problem solve!
             */
-            val cityStats = CityStats()
+            val cityStats = CityStats(cityInfo)
-            cityStats.cityInfo = cityInfo
             val construction = cityInfo.cityConstructions.getConstruction(constructionName)
             cityStats.update(construction)
             cityStatsForConstruction = cityStats.currentCityStats

core/src/com/unciv/logic/city/CityInfo.kt

@@ -55,7 +55,9 @@ class CityInfo {
     var population = PopulationManager()
     var cityConstructions = CityConstructions()
     var expansion = CityExpansionManager()
-    var cityStats = CityStats()
+    
+    @Transient  // CityStats has no serializable fields
+    var cityStats = CityStats(this)
 
     /** All tiles that this city controls */
     var tiles = HashSet<Vector2>()
@@ -432,7 +434,6 @@ class CityInfo {
         population.cityInfo = this
         expansion.cityInfo = this
         expansion.setTransients()
-        cityStats.cityInfo = this
         cityConstructions.cityInfo = this
         cityConstructions.setTransients()
         religion.setTransients(this)

core/src/com/unciv/logic/city/CityReligion.kt

@@ -121,8 +121,8 @@ class CityInfoReligionManager {
                 "[] when a city adopts this religion for the first time" -> unique.stats
                 else -> continue
             }
-            for (stat in statsGranted.toHashMap())
+            for ((key, value) in statsGranted)
-                religionOwningCiv.addStat(stat.key, stat.value.toInt())
+                religionOwningCiv.addStat(key, value.toInt())
             if (cityInfo.location in religionOwningCiv.exploredTiles)
                 religionOwningCiv.addNotification(
                     "You gained [$statsGranted] as your religion was spread to [${cityInfo.name}]",

core/src/com/unciv/logic/city/CityStats.kt

@@ -17,31 +17,30 @@ import com.unciv.ui.utils.toPercent
 import kotlin.math.min
 
 
-class CityStats {
+/** Holds and calculates [Stats] for a city.
+ * 
+ * No field needs to be saved, all are calculated on the fly,
+ * so its field in [CityInfo] is @Transient and no such annotation is needed here.
+ */
+class CityStats(val cityInfo: CityInfo) {
+    //region Fields, Transient 
 
-    @Transient
     var baseStatList = LinkedHashMap<String, Stats>()
 
-    @Transient
     var statPercentBonusList = LinkedHashMap<String, Stats>()
 
     // Computed from baseStatList and statPercentBonusList - this is so the players can see a breakdown
-    @Transient
     var finalStatList = LinkedHashMap<String, Stats>()
 
-    @Transient
     var happinessList = LinkedHashMap<String, Float>()
 
-    @Transient
     var foodEaten = 0f
 
-    @Transient
     var currentCityStats: Stats = Stats()  // This is so we won't have to calculate this multiple times - takes a lot of time, especially on phones
 
-    @Transient
+    //endregion
-    lateinit var cityInfo: CityInfo
+    //region Pure Functions
 
-    //region pure fuctions
     private fun getStatsFromTiles(): Stats {
         val stats = Stats()
         for (cell in cityInfo.tilesInRange
@@ -58,7 +57,7 @@ class CityStats {
             stats.gold = civInfo.getCapital().population.population * 0.15f + cityInfo.population.population * 1.1f - 1 // Calculated by http://civilization.wikia.com/wiki/Trade_route_(Civ5)
             for (unique in civInfo.getMatchingUniques("[] from each Trade Route"))
                 stats.add(unique.stats)
-            if (civInfo.hasUnique("Gold from all trade routes +25%")) stats.gold *= 1.25f // Machu Pichu speciality
+            if (civInfo.hasUnique("Gold from all trade routes +25%")) stats.gold *= 1.25f // Machu Picchu speciality
         }
         return stats
     }
@@ -180,7 +179,7 @@ class CityStats {
         return stats
     }
 
-    fun getGrowthBonusFromPoliciesAndWonders(): Float {
+    private fun getGrowthBonusFromPoliciesAndWonders(): Float {
         var bonus = 0f
         // "+[amount]% growth [cityFilter]"
         for (unique in cityInfo.getMatchingUniques("+[]% growth []"))
@@ -192,70 +191,6 @@ class CityStats {
         return bonus / 100
     }
 
-    // needs to be a separate function because we need to know the global happiness state
-    // in order to determine how much food is produced in a city!
-    fun updateCityHappiness() {
-        val civInfo = cityInfo.civInfo
-        val newHappinessList = LinkedHashMap<String, Float>()
-        var unhappinessModifier = civInfo.getDifficulty().unhappinessModifier
-        if (!civInfo.isPlayerCivilization())
-            unhappinessModifier *= civInfo.gameInfo.getDifficulty().aiUnhappinessModifier
-
-        var unhappinessFromCity = -3f // -3 happiness per city
-        if (civInfo.hasUnique("Unhappiness from number of Cities doubled"))
-            unhappinessFromCity *= 2f //doubled for the Indian
-
-        newHappinessList["Cities"] = unhappinessFromCity * unhappinessModifier
-
-        var unhappinessFromCitizens = cityInfo.population.population.toFloat()
-        var unhappinessFromSpecialists = cityInfo.population.getNumberOfSpecialists().toFloat()
-
-        for (unique in civInfo.getMatchingUniques("Specialists only produce []% of normal unhappiness")) {
-            unhappinessFromSpecialists *= (1f - unique.params[0].toFloat() / 100f)
-        }
-
-        unhappinessFromCitizens -= cityInfo.population.getNumberOfSpecialists().toFloat() - unhappinessFromSpecialists
-
-        if (cityInfo.isPuppet)
-            unhappinessFromCitizens *= 1.5f
-        else if (hasExtraAnnexUnhappiness())
-            unhappinessFromCitizens *= 2f
-
-        for (unique in civInfo.getMatchingUniques("Unhappiness from population decreased by []%"))
-            unhappinessFromCitizens *= (1 - unique.params[0].toFloat() / 100)
-
-        for (unique in civInfo.getMatchingUniques("Unhappiness from population decreased by []% []"))
-            if (cityInfo.matchesFilter(unique.params[1]))
-                unhappinessFromCitizens *= (1 - unique.params[0].toFloat() / 100)
-
-        newHappinessList["Population"] = -unhappinessFromCitizens * unhappinessModifier
-
-        val happinessFromPolicies = getStatsFromUniques(civInfo.policies.policyUniques.getAllUniques()).happiness
-
-        newHappinessList["Policies"] = happinessFromPolicies
-
-        if (hasExtraAnnexUnhappiness()) newHappinessList["Occupied City"] = -2f //annexed city
-
-        val happinessFromSpecialists = getStatsFromSpecialists(cityInfo.population.getNewSpecialists()).happiness.toInt().toFloat()
-        if (happinessFromSpecialists > 0) newHappinessList["Specialists"] = happinessFromSpecialists
-
-        val happinessFromBuildings = cityInfo.cityConstructions.getStats().happiness.toInt().toFloat()
-        newHappinessList["Buildings"] = happinessFromBuildings
-
-        newHappinessList["National ability"] = getStatsFromUniques(cityInfo.civInfo.nation.uniqueObjects.asSequence()).happiness
-
-        newHappinessList["Wonders"] = getStatsFromUniques(civInfo.getCivWideBuildingUniques()).happiness
-
-        newHappinessList["Religion"] = getStatsFromUniques(cityInfo.religion.getUniques()).happiness
-        
-        newHappinessList["Tile yields"] = getStatsFromTiles().happiness
-
-        // we don't want to modify the existing happiness list because that leads
-        // to concurrency problems if we iterate on it while changing
-        happinessList = newHappinessList
-    }
-
-
     fun hasExtraAnnexUnhappiness(): Boolean {
         if (cityInfo.civInfo.civName == cityInfo.foundingCiv || cityInfo.foundingCiv == "" || cityInfo.isPuppet) return false
         return !cityInfo.containsBuildingUnique("Remove extra unhappiness from annexed cities")
@@ -263,7 +198,7 @@ class CityStats {
 
     fun getStatsOfSpecialist(specialistName: String): Stats {
         val specialist = cityInfo.getRuleset().specialists[specialistName]
-        if (specialist == null) return Stats()
+            ?: return Stats()
         val stats = specialist.clone()
         for (unique in cityInfo.civInfo.getMatchingUniques("[] from every specialist"))
             stats.add(unique.stats)
@@ -288,7 +223,7 @@ class CityStats {
             if (unique.placeholderText == "[] []" && cityInfo.matchesFilter(unique.params[1]))
                 stats.add(unique.stats)
 
-            // "[stats] per [amount] population [cityfilter]"
+            // "[stats] per [amount] population [cityFilter]"
             if (unique.placeholderText == "[] per [] population []" && cityInfo.matchesFilter(unique.params[2])) {
                 val amountOfEffects = (cityInfo.population.population / unique.params[1].toInt()).toFloat()
                 stats.add(unique.stats.times(amountOfEffects))
@@ -310,7 +245,6 @@ class CityStats {
         return stats
     }
 
-
     private fun getStatPercentBonusesFromGoldenAge(isGoldenAge: Boolean): Stats {
         val stats = Stats()
         if (isGoldenAge) {
@@ -400,16 +334,96 @@ class CityStats {
                 }
             else cityInfo.isConnectedToCapital()
     }
+
+    private fun getBuildingMaintenanceCosts(citySpecificUniques: Sequence<Unique>): Float {
+        // Same here - will have a different UI display.
+        var buildingsMaintenance = cityInfo.cityConstructions.getMaintenanceCosts().toFloat() // this is AFTER the bonus calculation!
+        if (!cityInfo.civInfo.isPlayerCivilization()) {
+            buildingsMaintenance *= cityInfo.civInfo.gameInfo.getDifficulty().aiBuildingMaintenanceModifier
+        }
+
+        // e.g. "-[50]% maintenance costs for buildings [in this city]"
+        for (unique in cityInfo.getMatchingUniques("-[]% maintenance cost for buildings []", citySpecificUniques)) {
+            buildingsMaintenance *= (1f - unique.params[0].toFloat() / 100)
+        }
+
+        return buildingsMaintenance
+    }
+
     //endregion
+    //region State-Changing Methods
+
+    // needs to be a separate function because we need to know the global happiness state
+    // in order to determine how much food is produced in a city!
+    fun updateCityHappiness() {
+        val civInfo = cityInfo.civInfo
+        val newHappinessList = LinkedHashMap<String, Float>()
+        var unhappinessModifier = civInfo.getDifficulty().unhappinessModifier
+        if (!civInfo.isPlayerCivilization())
+            unhappinessModifier *= civInfo.gameInfo.getDifficulty().aiUnhappinessModifier
+
+        var unhappinessFromCity = -3f // -3 happiness per city
+        if (civInfo.hasUnique("Unhappiness from number of Cities doubled"))
+            unhappinessFromCity *= 2f //doubled for the Indian
+
+        newHappinessList["Cities"] = unhappinessFromCity * unhappinessModifier
+
+        var unhappinessFromCitizens = cityInfo.population.population.toFloat()
+        var unhappinessFromSpecialists = cityInfo.population.getNumberOfSpecialists().toFloat()
+
+        for (unique in civInfo.getMatchingUniques("Specialists only produce []% of normal unhappiness")) {
+            unhappinessFromSpecialists *= (1f - unique.params[0].toFloat() / 100f)
+        }
+
+        unhappinessFromCitizens -= cityInfo.population.getNumberOfSpecialists().toFloat() - unhappinessFromSpecialists
+
+        if (cityInfo.isPuppet)
+            unhappinessFromCitizens *= 1.5f
+        else if (hasExtraAnnexUnhappiness())
+            unhappinessFromCitizens *= 2f
+
+        for (unique in civInfo.getMatchingUniques("Unhappiness from population decreased by []%"))
+            unhappinessFromCitizens *= (1 - unique.params[0].toFloat() / 100)
+
+        for (unique in civInfo.getMatchingUniques("Unhappiness from population decreased by []% []"))
+            if (cityInfo.matchesFilter(unique.params[1]))
+                unhappinessFromCitizens *= (1 - unique.params[0].toFloat() / 100)
+
+        newHappinessList["Population"] = -unhappinessFromCitizens * unhappinessModifier
+
+        val happinessFromPolicies = getStatsFromUniques(civInfo.policies.policyUniques.getAllUniques()).happiness
+
+        newHappinessList["Policies"] = happinessFromPolicies
+
+        if (hasExtraAnnexUnhappiness()) newHappinessList["Occupied City"] = -2f //annexed city
+
+        val happinessFromSpecialists = getStatsFromSpecialists(cityInfo.population.getNewSpecialists()).happiness.toInt().toFloat()
+        if (happinessFromSpecialists > 0) newHappinessList["Specialists"] = happinessFromSpecialists
+
+        val happinessFromBuildings = cityInfo.cityConstructions.getStats().happiness.toInt().toFloat()
+        newHappinessList["Buildings"] = happinessFromBuildings
+
+        newHappinessList["National ability"] = getStatsFromUniques(cityInfo.civInfo.nation.uniqueObjects.asSequence()).happiness
+
+        newHappinessList["Wonders"] = getStatsFromUniques(civInfo.getCivWideBuildingUniques()).happiness
+
+        newHappinessList["Religion"] = getStatsFromUniques(cityInfo.religion.getUniques()).happiness
+
+        newHappinessList["Tile yields"] = getStatsFromTiles().happiness
+
+        // we don't want to modify the existing happiness list because that leads
+        // to concurrency problems if we iterate on it while changing
+        happinessList = newHappinessList
+    }
 
     private fun updateBaseStatList() {
         val newBaseStatList = LinkedHashMap<String, Stats>() // we don't edit the existing baseStatList directly, in order to avoid concurrency exceptions
         val civInfo = cityInfo.civInfo
 
-        newBaseStatList["Population"] = Stats().apply {
+        newBaseStatList["Population"] = Stats(
-            science = cityInfo.population.population.toFloat()
+            science = cityInfo.population.population.toFloat(),
             production = cityInfo.population.getFreePopulation().toFloat()
-        }
+        )
         newBaseStatList["Tile yields"] = getStatsFromTiles()
         newBaseStatList["Specialists"] = getStatsFromSpecialists(cityInfo.population.getNewSpecialists())
         newBaseStatList["Trade routes"] = getStatsFromTradeRoute()
@@ -439,7 +453,7 @@ class CityStats {
 
         if (UncivGame.Current.superchargedForDebug) {
             val stats = Stats()
-            for (stat in Stat.values()) stats.add(stat, 10000f)
+            for (stat in Stat.values()) stats[stat] = 10000f
             newStatPercentBonusList["Supercharged"] = stats
         }
 
@@ -494,7 +508,7 @@ class CityStats {
             val amountConverted = (newFinalStatList.values.sumByDouble { it.gold.toDouble() }
                     * cityInfo.civInfo.tech.goldPercentConvertedToScience).toInt().toFloat()
             if (amountConverted > 0) // Don't want you converting negative gold to negative science yaknow
-                newFinalStatList["Gold -> Science"] = Stats().apply { science = amountConverted; gold = -amountConverted }
+                newFinalStatList["Gold -> Science"] = Stats(science = amountConverted, gold = -amountConverted)
         }
         for (entry in newFinalStatList.values) {
             entry.science *= statPercentBonusesSum.science.toPercent()
@@ -516,7 +530,7 @@ class CityStats {
         var totalFood = newFinalStatList.values.map { it.food }.sum()
 
         if (isUnhappy && totalFood > 0) { // Reduce excess food to 1/4 per the same
-            val foodReducedByUnhappiness = Stats().apply { food = totalFood * (-3 / 4f) }
+            val foodReducedByUnhappiness = Stats(food = totalFood * (-3 / 4f))
             baseStatList = LinkedHashMap(baseStatList).apply { put("Unhappiness", foodReducedByUnhappiness) } // concurrency-safe addition
             newFinalStatList["Unhappiness"] = foodReducedByUnhappiness
         }
@@ -531,36 +545,20 @@ class CityStats {
         }
 
         val buildingsMaintenance = getBuildingMaintenanceCosts(citySpecificUniques) // this is AFTER the bonus calculation!
-        newFinalStatList["Maintenance"] = Stats().apply { gold -= buildingsMaintenance.toInt() }
+        newFinalStatList["Maintenance"] = Stats(gold = -buildingsMaintenance.toInt().toFloat())
-
 
         if (totalFood > 0 && constructionMatchesFilter(currentConstruction, "Excess Food converted to Production when under construction")) {
-            newFinalStatList["Excess food to production"] = Stats().apply { production = totalFood; food = -totalFood }
+            newFinalStatList["Excess food to production"] = Stats(production = totalFood, food = -totalFood)
         }
 
         if (cityInfo.isInResistance())
             newFinalStatList.clear()  // NOPE
 
         if (newFinalStatList.values.map { it.production }.sum() < 1)  // Minimum production for things to progress
-            newFinalStatList["Production"] = Stats().apply { production = 1f }
+            newFinalStatList["Production"] = Stats(production = 1f)
         finalStatList = newFinalStatList
     }
 
-    private fun getBuildingMaintenanceCosts(citySpecificUniques: Sequence<Unique>): Float {
-        // Same here - will have a different UI display.
-        var buildingsMaintenance = cityInfo.cityConstructions.getMaintenanceCosts().toFloat() // this is AFTER the bonus calculation!
-        if (!cityInfo.civInfo.isPlayerCivilization()) {
-            buildingsMaintenance *= cityInfo.civInfo.gameInfo.getDifficulty().aiBuildingMaintenanceModifier
-        }
-
-        // e.g. "-[50]% maintenance costs for buildings [in this city]"
-        for (unique in cityInfo.getMatchingUniques("-[]% maintenance cost for buildings []", citySpecificUniques)) {
-            buildingsMaintenance *= (1f - unique.params[0].toFloat() / 100)
-        }
-
-        return buildingsMaintenance
-    }
-
     private fun updateFoodEaten() {
         foodEaten = cityInfo.population.population.toFloat() * 2
         var foodEatenBySpecialists = 2f * cityInfo.population.getNumberOfSpecialists()
@@ -570,4 +568,6 @@ class CityStats {
 
         foodEaten -= 2f * cityInfo.population.getNumberOfSpecialists() - foodEatenBySpecialists
     }
+
+    //endregion
 }

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

@@ -124,21 +124,21 @@ class CivInfoStats(val civInfo: CivilizationInfo) {
                         "City-States",
                         Stats().add(
                             Stat.valueOf(unique.params[0]),
-                            otherCiv.statsForNextTurn.get(Stat.valueOf(unique.params[0])) * unique.params[1].toFloat() / 100f
+                            otherCiv.statsForNextTurn[Stat.valueOf(unique.params[0])] * unique.params[1].toFloat() / 100f
                         )
                     )
                 }
         }
 
-        statMap["Transportation upkeep"] = Stats().apply { gold = -getTransportationUpkeep().toFloat() }
+        statMap["Transportation upkeep"] = Stats(gold = -getTransportationUpkeep().toFloat())
-        statMap["Unit upkeep"] = Stats().apply { gold = -getUnitMaintenance().toFloat() }
+        statMap["Unit upkeep"] = Stats(gold = -getUnitMaintenance().toFloat())
 
         if (civInfo.religionManager.religion != null) {
             for (unique in civInfo.religionManager.religion!!.getFounderBeliefs().flatMap { it.uniqueObjects }) {
                 if (unique.placeholderText == "[] for each global city following this religion") {
                     statMap.add(
                         "Religion", 
-                        unique.stats.times(civInfo.religionManager.numberOfCitiesFollowingThisReligion().toFloat())
+                        unique.stats.times(civInfo.religionManager.numberOfCitiesFollowingThisReligion())
                     )
                 }
             }
@@ -154,7 +154,7 @@ class CivInfoStats(val civInfo: CivilizationInfo) {
 
         if (civInfo.hasUnique("50% of excess happiness added to culture towards policies")) {
             val happiness = civInfo.getHappiness()
-            if (happiness > 0) statMap.add("Policies", Stats().apply { culture = happiness / 2f })
+            if (happiness > 0) statMap.add("Policies", Stats(culture = happiness / 2f))
         }
 
         // negative gold hurts science
@@ -163,11 +163,11 @@ class CivInfoStats(val civInfo: CivilizationInfo) {
         if (statMap.values.map { it.gold }.sum() < 0 && civInfo.gold < 0) {
             val scienceDeficit = max(statMap.values.map { it.gold }.sum(),
                     1 - statMap.values.map { it.science }.sum())// Leave at least 1
-            statMap["Treasury deficit"] = Stats().apply { science = scienceDeficit }
+            statMap["Treasury deficit"] = Stats(science = scienceDeficit)
         }
         val goldDifferenceFromTrade = civInfo.diplomacy.values.sumBy { it.goldPerTurn() }
         if (goldDifferenceFromTrade != 0)
-            statMap["Trade"] = Stats().apply { gold = goldDifferenceFromTrade.toFloat() }
+            statMap["Trade"] = Stats(gold = goldDifferenceFromTrade.toFloat())
 
         return statMap
     }

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

@@ -379,7 +379,7 @@ class CivilizationInfo {
 
         val cityStateLocation = if (cities.isEmpty()) null else getCapital().location
 
-        val giftAmount = Stats().add(Stat.Gold, 15f)
+        val giftAmount = Stats(gold = 15f)
         // Later, religious city-states will also gift gold, making this the better implementation
         // For now, it might be overkill though.
         var meetString = "[${civName}] has given us [${giftAmount}] as a token of goodwill for meeting us"
@@ -392,8 +392,8 @@ class CivilizationInfo {
         else
             otherCiv.addNotification(meetString, NotificationIcon.Gold)
 
-        for (stat in giftAmount.toHashMap().filter { it.value != 0f })
+        for ((key, value) in giftAmount)
-            otherCiv.addStat(stat.key, stat.value.toInt())
+            otherCiv.addStat(key, value.toInt())
     }
 
     fun discoverNaturalWonder(naturalWonderName: String) {

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

@@ -24,7 +24,7 @@ class GreatPersonManager {
 
         fun statsToGreatPersonCounter(stats: Stats): Counter<String> {
             val counter = Counter<String>()
-            for ((key, value) in stats.toHashMap())
+            for ((key, value) in stats)
                 if (statToGreatPersonMapping.containsKey(key))
                     counter.add(statToGreatPersonMapping[key]!!, value.toInt())
             return counter

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

@@ -81,8 +81,8 @@ class Building : NamedStats(), INonPerpetualConstruction, ICivilopediaText {
     fun getShortDescription(ruleset: Ruleset): String { // should fit in one line
         val infoList = mutableListOf<String>()
         getStats(null).toString().also { if (it.isNotEmpty()) infoList += it }
-        for (stat in getStatPercentageBonuses(null).toHashMap())
+        for ((key, value) in getStatPercentageBonuses(null))
-            if (stat.value != 0f) infoList += "+${stat.value.toInt()}% ${stat.key.name.tr()}"
+            infoList += "+${value.toInt()}% ${key.name.tr()}"
 
         if (requiredNearbyImprovedResources != null)
             infoList += "Requires worked [" + requiredNearbyImprovedResources!!.joinToString("/") { it.tr() } + "] near city"
@@ -143,7 +143,7 @@ class Building : NamedStats(), INonPerpetualConstruction, ICivilopediaText {
         if (!stats.isEmpty())
             lines += stats.toString()
 
-        for ((stat, value) in getStatPercentageBonuses(cityInfo).toHashMap())
+        for ((stat, value) in getStatPercentageBonuses(cityInfo))
             if (value != 0f) lines += "+${value.toInt()}% {${stat.name}}\n"
 
         for ((greatPersonName, value) in greatPersonPoints)
@@ -193,7 +193,7 @@ class Building : NamedStats(), INonPerpetualConstruction, ICivilopediaText {
     }
 
     fun getStatPercentageBonuses(cityInfo: CityInfo?): Stats {
-        val stats = if (percentStatBonus == null) Stats() else percentStatBonus!!.clone()
+        val stats = percentStatBonus?.clone() ?: Stats()
         val civInfo = cityInfo?.civInfo ?: return stats  // initial stats
 
         val baseBuildingName = getBaseBuilding(civInfo.gameInfo.ruleSet).name
@@ -320,7 +320,7 @@ class Building : NamedStats(), INonPerpetualConstruction, ICivilopediaText {
         }
 
         if (!percentStats.isEmpty()) {
-            for ( (key, value) in percentStats.toHashMap()) {
+            for ((key, value) in percentStats) {
                 if (value == 0f) continue
                 textList += FormattedLine(value.formatSignedInt() + "% {$key}")
             }
@@ -676,8 +676,8 @@ class Building : NamedStats(), INonPerpetualConstruction, ICivilopediaText {
 
     fun isStatRelated(stat: Stat): Boolean {
         if (get(stat) > 0) return true
-        if (getStatPercentageBonuses(null).get(stat) > 0) return true
+        if (getStatPercentageBonuses(null)[stat] > 0) return true
-        if (uniqueObjects.any { it.placeholderText == "[] per [] population []" && it.stats.get(stat) > 0 }) return true
+        if (uniqueObjects.any { it.placeholderText == "[] per [] population []" && it.stats[stat] > 0 }) return true
         return false
     }
 

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

@@ -124,10 +124,9 @@ class Nation : INamed, ICivilopediaText, IHasUniques {
                 val originalBuilding = ruleset.buildings[building.replaces!!]!!
 
                 textList += building.name.tr() + " - " + "Replaces [${originalBuilding.name}]".tr()
-                val originalBuildingStatMap = originalBuilding.toHashMap()
+                for ((key, value) in building)
-                for (stat in building.toHashMap())
+                    if (value != originalBuilding[key])
-                    if (stat.value != originalBuildingStatMap[stat.key])
+                        textList += "  " + key.name.tr() + " " + "[${value.toInt()}] vs [${originalBuilding[key].toInt()}]".tr()
-                        textList += "  " + stat.key.toString().tr() + " " + "[${stat.value.toInt()}] vs [${originalBuildingStatMap[stat.key]!!.toInt()}]".tr()
 
                 for (unique in building.uniques.filter { it !in originalBuilding.uniques })
                     textList += "  " + unique.tr()
@@ -245,13 +244,9 @@ class Nation : INamed, ICivilopediaText, IHasUniques {
                 val originalBuilding = ruleset.buildings[building.replaces!!]!!
                 textList += FormattedLine("Replaces [${originalBuilding.name}]", link=originalBuilding.makeLink(), indent=1)
 
-                val originalBuildingStatMap = originalBuilding.toHashMap()
+                for ((key, value) in building)
-                for (stat in building.toHashMap())
+                    if (value != originalBuilding[key])
-                    if (stat.value != originalBuildingStatMap[stat.key])
+                        textList += FormattedLine( key.name.tr() + " " +"[${value.toInt()}] vs [${originalBuilding[key].toInt()}]".tr(), indent=1)
-                        textList += FormattedLine(
-                            stat.key.toString().tr() + " " +
-                            "[${stat.value.toInt()}] vs [${originalBuildingStatMap[stat.key]!!.toInt()}]".tr(),
-                            indent=1)
 
                 for (unique in building.uniques.filter { it !in originalBuilding.uniques })
                     textList += FormattedLine(unique, indent=1)

core/src/com/unciv/models/stats/Stats.kt

@@ -6,6 +6,10 @@ import kotlin.reflect.KMutableProperty0
 /**
  * A container for the seven basic ["currencies"][Stat] in Unciv,
  * **Mutable**, allowing for easy merging of sources and applying bonuses.
+ * 
+ * Supports e.g. `for ((key,value) in <Stats>)` - the [iterator] will skip zero values automatically.
+ * 
+ * Also possible: `<Stats>`.[values].sum() and similar aggregates over a Sequence<Float>.
  */
 open class Stats(
     var production: Float = 0f,
@@ -15,7 +19,7 @@ open class Stats(
     var culture: Float = 0f,
     var happiness: Float = 0f,
     var faith: Float = 0f
-) {
+): Iterable<Stats.StatValuePair> {
     // This is what facilitates indexed access by [Stat] or add(Stat,Float)
     // without additional memory allocation or expensive conditionals
     @delegate:Transient
@@ -126,23 +130,44 @@ open class Stats(
      * Example output: `+1 Production, -1 Food`.
      */
     override fun toString(): String {
-        return toHashMap().filter { it.value != 0f }
+        return this.joinToString {
-                .map { (if (it.value > 0) "+" else "") + it.value.toInt() + " " + it.key.toString().tr() }.joinToString()
+            (if (it.value > 0) "+" else "") + it.value.toInt() + " " + it.key.toString().tr()
+        }
     }
 
-    /** @return a Map copy of the values in this instance, can be used to iterate over the values */
+    /** Represents one [key][Stat]/[value][Float] pair returned by the [iterator] */
-    fun toHashMap(): HashMap<Stat, Float> {
+    data class StatValuePair (val key: Stat, val value: Float)
-        return linkedMapOf(
+
-            Stat.Production to production,
+    /** Enables iteration over the non-zero [Stat]/value [pairs][StatValuePair].
-            Stat.Food to food,
+     * Explicit use unnecessary - [Stats] is [iterable][Iterable] directly.
-            Stat.Gold to gold,
+     * @see iterator */
-            Stat.Science to science,
+    fun asSequence() = sequence {
-            Stat.Culture to culture,
+        if (production != 0f) yield(StatValuePair(Stat.Production, production))
-            Stat.Happiness to happiness,
+        if (food != 0f) yield(StatValuePair(Stat.Food, food))
-            Stat.Faith to faith
+        if (gold != 0f) yield(StatValuePair(Stat.Gold, gold))
-        )
+        if (science != 0f) yield(StatValuePair(Stat.Science, science))
+        if (culture != 0f) yield(StatValuePair(Stat.Culture, culture))
+        if (happiness != 0f) yield(StatValuePair(Stat.Happiness, happiness))
+        if (faith != 0f) yield(StatValuePair(Stat.Faith, faith))
     }
 
+    /** Enables aggregates over the values, never empty */
+    // Property syntax to emulate Map.values pattern
+    // Doesn't skip zero values as it's meant for sum() or max() where the overhead would be higher than any gain
+    val values
+        get() = sequence {
+            yield(production)
+            yield(food)
+            yield(gold)
+            yield(science)
+            yield(culture)
+            yield(happiness)
+            yield(faith)
+        }
+
+    /** Returns an iterator over the elements of this object, wrapped as [StatValuePair]s */
+    override fun iterator(): Iterator<StatValuePair> = asSequence().iterator()
+
 
     companion object {
         private val allStatNames = Stat.values().joinToString("|") { it.name }

core/src/com/unciv/ui/cityscreen/CityInfoTable.kt

@@ -150,7 +150,7 @@ class CityInfoTable(private val cityScreen: CityScreen) : Table(CameraStageBaseS
 
             if (stat != Stat.Happiness)
                 for ((key, value) in cityStats.baseStatList)
-                    relevantBaseStats[key] = value.get(stat)
+                    relevantBaseStats[key] = value[stat]
             else relevantBaseStats.putAll(cityStats.happinessList)
             for (key in relevantBaseStats.keys.toList())
                 if (relevantBaseStats[key] == 0f) relevantBaseStats.remove(key)
@@ -172,12 +172,12 @@ class CityInfoTable(private val cityScreen: CityScreen) : Table(CameraStageBaseS
             statValuesTable.add("Total".toLabel())
             statValuesTable.add(sumOfAllBaseValues.toOneDecimalLabel()).row()
 
-            val relevantBonuses = cityStats.statPercentBonusList.filter { it.value.get(stat) != 0f }
+            val relevantBonuses = cityStats.statPercentBonusList.filter { it.value[stat] != 0f }
             if (relevantBonuses.isNotEmpty()) {
                 statValuesTable.add("Bonuses".toLabel(fontSize = FONT_SIZE_STAT_INFO_HEADER)).colspan(2).padTop(20f).row()
                 var sumOfBonuses = 0f
                 for (entry in relevantBonuses) {
-                    val specificStatValue = entry.value.get(stat)
+                    val specificStatValue = entry.value[stat]
                     sumOfBonuses += specificStatValue
                     statValuesTable.add(entry.key.toLabel())
                     statValuesTable.add(specificStatValue.toPercentLabel()).row() // negative bonus
@@ -191,7 +191,7 @@ class CityInfoTable(private val cityScreen: CityScreen) : Table(CameraStageBaseS
                 statValuesTable.add("Final".toLabel(fontSize = FONT_SIZE_STAT_INFO_HEADER)).colspan(2).padTop(20f).row()
                 var finalTotal = 0f
                 for (entry in cityStats.finalStatList) {
-                    val specificStatValue = entry.value.get(stat)
+                    val specificStatValue = entry.value[stat]
                     finalTotal += specificStatValue
                     if (specificStatValue == 0f) continue
                     statValuesTable.add(entry.key.toLabel())

core/src/com/unciv/ui/cityscreen/CityScreenTileTable.kt

@@ -132,9 +132,9 @@ class CityScreenTileTable(private val cityScreen: CityScreen): Table() {
     private fun getTileStatsTable(stats: Stats): Table {
         val statsTable = Table()
         statsTable.defaults().pad(2f)
-        for (entry in stats.toHashMap().filterNot { it.value == 0f }) {
+        for ((key, value) in stats) {
-            statsTable.add(ImageGetter.getStatIcon(entry.key.toString())).size(20f)
+            statsTable.add(ImageGetter.getStatIcon(key.name)).size(20f)
-            statsTable.add(entry.value.roundToInt().toString().toLabel()).padRight(5f)
+            statsTable.add(value.roundToInt().toLabel()).padRight(5f)
         }
         return statsTable
     }

core/src/com/unciv/ui/cityscreen/CityStatsTable.kt

@@ -28,7 +28,7 @@ class CityStatsTable(val cityScreen: CityScreen): Table() {
         innerTable.clear()
 
         val miniStatsTable = Table()
-        for ((stat, amount) in cityInfo.cityStats.currentCityStats.toHashMap()) {
+        for ((stat, amount) in cityInfo.cityStats.currentCityStats) {
             if (stat == Stat.Faith && !cityInfo.civInfo.gameInfo.hasReligionEnabled()) continue
             miniStatsTable.add(ImageGetter.getStatIcon(stat.name)).size(20f).padRight(5f)
             val valueToDisplay = if (stat == Stat.Happiness) cityInfo.cityStats.happinessList.values.sum() else amount

core/src/com/unciv/ui/cityscreen/SpecialistAllocationTable.kt

@@ -77,11 +77,11 @@ class SpecialistAllocationTable(val cityScreen: CityScreen): Table(CameraStageBa
 
     private fun getSpecialistStatsTable(specialistName: String): Table {
         val specialistStatTable = Table().apply { defaults().pad(5f) }
-        val specialistStats = cityInfo.cityStats.getStatsOfSpecialist(specialistName).toHashMap()
+        val specialistStats = cityInfo.cityStats.getStatsOfSpecialist(specialistName)
-        for (entry in specialistStats) {
+        for ((key, value) in specialistStats) {
-            if (entry.value == 0f) continue
+            if (value == 0f) continue
-            specialistStatTable.add(ImageGetter.getStatIcon(entry.key.name)).size(20f)
+            specialistStatTable.add(ImageGetter.getStatIcon(key.name)).size(20f)
-            specialistStatTable.add(entry.value.toInt().toLabel()).padRight(10f)
+            specialistStatTable.add(value.toInt().toLabel()).padRight(10f)
         }
         return specialistStatTable
     }

core/src/com/unciv/ui/cityscreen/YieldGroup.kt

@@ -12,13 +12,13 @@ class YieldGroup : HorizontalGroup() {
         isTransform = false // performance helper - nothing here is rotated or scaled
     }
 
-    var currentStats=Stats()
+    var currentStats = Stats()
 
     fun setStats(stats: Stats) {
         if (currentStats.equals(stats)) return // don't need to update - this is a memory and time saver!
         currentStats = stats
         clearChildren()
-        for ((stat, amount) in stats.toHashMap().asSequence().filter { it.value > 0 }) {
+        for ((stat, amount) in stats) {
             addActor(getStatIconsTable(stat.name, amount.toInt()))
         }
         pack()

core/src/com/unciv/ui/overviewscreen/StatsOverviewTable.kt

@@ -92,13 +92,13 @@ class StatsOverviewTable (
         scienceTable.add(scienceHeader).colspan(2).row()
         scienceTable.addSeparator()
         val scienceStats = viewingPlayer.stats().getStatMapForNextTurn()
-            .filter { it.value.science!=0f }
+            .filter { it.value.science != 0f }
         for (entry in scienceStats) {
             scienceTable.add(entry.key.tr())
             scienceTable.add(entry.value.science.roundToInt().toString()).right().row()
         }
         scienceTable.add("Total".tr())
-        scienceTable.add(scienceStats.values.map { it.science }.sum().roundToInt().toString()).right()
+        scienceTable.add(scienceStats.map { it.value.science }.sum().roundToInt().toString()).right()
         scienceTable.pack()
         return scienceTable
     }
@@ -108,9 +108,8 @@ class StatsOverviewTable (
 
         val greatPersonPoints = GreatPersonManager
             .greatPersonCounterToStats(viewingPlayer.greatPeople.greatPersonPointsCounter)
-            .toHashMap()
         val greatPersonPointsPerTurn = GreatPersonManager
-            .greatPersonCounterToStats(viewingPlayer.getGreatPersonPointsForNextTurn()).toHashMap()
+            .greatPersonCounterToStats(viewingPlayer.getGreatPersonPointsForNextTurn())
         val pointsToGreatPerson = viewingPlayer.greatPeople.pointsForNextGreatPerson
 
         greatPeopleTable.defaults().pad(5f)
@@ -128,11 +127,11 @@ class StatsOverviewTable (
         val mapping = GreatPersonManager.statToGreatPersonMapping
         for(entry in mapping){
             greatPeopleTable.add(entry.value.tr())
-            greatPeopleTable.add(greatPersonPoints[entry.key]!!.toInt().toString()+"/"+pointsToGreatPerson)
+            greatPeopleTable.add(greatPersonPoints[entry.key].toInt().toString()+"/"+pointsToGreatPerson)
-            greatPeopleTable.add(greatPersonPointsPerTurn[entry.key]!!.toInt().toString()).row()
+            greatPeopleTable.add(greatPersonPointsPerTurn[entry.key].toInt().toString()).row()
         }
-        val pointsForGreatGeneral = viewingPlayer.greatPeople.greatGeneralPoints.toString()
+        val pointsForGreatGeneral = viewingPlayer.greatPeople.greatGeneralPoints
-        val pointsForNextGreatGeneral = viewingPlayer.greatPeople.pointsForNextGreatGeneral.toString()
+        val pointsForNextGreatGeneral = viewingPlayer.greatPeople.pointsForNextGreatGeneral
         greatPeopleTable.add("Great General".tr())
         greatPeopleTable.add("$pointsForGreatGeneral/$pointsForNextGreatGeneral").row()
         greatPeopleTable.pack()

core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt

@@ -17,6 +17,7 @@ import com.unciv.models.translations.tr
 import com.unciv.ui.utils.*
 import com.unciv.ui.utils.UncivTooltip.Companion.addTooltip
 import kotlin.math.round
+import kotlin.math.roundToInt
 
 class ImprovementPickerScreen(val tileInfo: TileInfo, unit: MapUnit, val onAccept: ()->Unit) : PickerScreen() {
     private var selectedImprovement: TileImprovement? = null
@@ -164,13 +165,13 @@ class ImprovementPickerScreen(val tileInfo: TileInfo, unit: MapUnit, val onAccep
     // icons of benefits (food, gold, etc) by improvement
     private fun getStatsTable(stats: Stats): Table {
         val statsTable = Table()
-        for (stat in stats.toHashMap()) {
+        for ((key, value) in stats) {
-            val statValue = round(stat.value).toInt()
+            val statValue = value.roundToInt()
             if (statValue == 0) continue
 
-            statsTable.add(ImageGetter.getStatIcon(stat.key.name)).size(20f).padRight(3f)
+            statsTable.add(ImageGetter.getStatIcon(key.name)).size(20f).padRight(3f)
 
-            val valueLabel = statValue.toString().toLabel()
+            val valueLabel = statValue.toLabel()
             valueLabel.color = if (statValue < 0) Color.RED else Color.WHITE
 
             statsTable.add(valueLabel).padRight(13f)

core/src/com/unciv/ui/worldscreen/bottombar/TileInfoTable.kt

@@ -38,11 +38,10 @@ class TileInfoTable(private val viewingCiv :CivilizationInfo) : Table(CameraStag
 
         // padLeft = padRight + 5: for symmetry. An extra 5 for the distance yield number to
         // tile text comes from the pad up there in updateTileTable
-        for (entry in tile.getTileStats(viewingCiv).toHashMap()
+        for ((key, value) in tile.getTileStats(viewingCiv)) {
-                .filterNot { it.value == 0f || it.key.toString() == "" }) {
+            table.add(ImageGetter.getStatIcon(key.name))
-            table.add(ImageGetter.getStatIcon(entry.key.toString()))
                 .size(20f).align(Align.right).padLeft(10f)
-            table.add(entry.value.toInt().toLabel())
+            table.add(value.toInt().toLabel())
                 .align(Align.left).padRight(5f)
             table.row()
         }

tests/src/com/unciv/testing/BasicTests.kt

@@ -72,7 +72,7 @@ class BasicTests {
         Assert.assertTrue(Stats.isStats("+1 Gold, +2 Production"))
         Assert.assertFalse(Stats.isStats("+1 Gold from tree"))
 
-        val statsThatShouldBe = Stats().add(Stat.Gold,1f).add(Stat.Production, 2f)
+        val statsThatShouldBe = Stats(gold = 1f, production = 2f)
         Assert.assertTrue(Stats.parse("+1 Gold, +2 Production").equals(statsThatShouldBe))
 
         UncivGame.Current = UncivGame("")
@@ -100,15 +100,16 @@ class BasicTests {
     @Test
     fun statMathRandomResultTest() {
         val iterations = 42
-        val expectedStats = Stats().apply {
+        val expectedStats = Stats(
-            production = 12970.174f
+            production = 212765.08f,
-            food = -153216.12f
+            food = 776.8394f,
-            gold = 28614.738f
+            gold = -4987.297f,
-            science = 142650.89f
+            science = 14880.18f,
-            culture = -45024.03f
+            culture = -49435.21f,
-            happiness = -7081.2495f
+            happiness = -13046.4375f,
-            faith = -14933.622f
+            faith = 7291.375f
-        }
+        )
+        // This is dependent on iterator order, so when that changes the expected values must change too
         val stats = statMathRunner(iterations)
         Assert.assertTrue(stats.equals(expectedStats))
     }
@@ -121,14 +122,14 @@ class BasicTests {
         for (i in 0 until iterations) {
             val value: Float = random.nextDouble(-10.0, 10.0).toFloat()
             stats.add( Stats(gold = value) )
-            stats.toHashMap().forEach {
+            stats.forEach {
                 val stat = Stat.values()[(it.key.ordinal + random.nextInt(1,statCount)).rem(statCount)]
                 stats.add(stat, -it.value)
             }
             val stat = Stat.values()[random.nextInt(statCount)]
-            stats.add(stat, stats.times(4).get(stat))
+            stats.add(stat, stats.times(4)[stat])
             stats.timesInPlace(0.8f)
-            if (abs(stats.toHashMap().maxOfOrNull { it.value }!!) > 1000000f)
+            if (abs(stats.values.maxOrNull()!!) > 1000000f)
                 stats.timesInPlace(0.1f)
         }
         return stats