Step 3 - new uniqueTypes, and removed old getStatsFromUniques entirely!

And we can FINALLY remove that ugly double statsPerPopulation check, since we take ALL uniques and filter them by building/wonder type instead of local/nonlocal!
@xlenstra perhaps the lower two new unique types can be replaced with conditionals?

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

@@ -72,7 +72,7 @@ object ChooseBeliefsAutomation {
                 "[]% attacking Strength for cities" -> unique.params[0].toFloat() / 10f // Modified by personality
                 "[] Units adjacent to this city heal [] HP per turn when healing" -> unique.params[1].toFloat() / 10f
                 "+[]% Production when constructing []" -> unique.params[0].toFloat() / 3f
-                "[] in cities on [] tiles" -> 
+                UniqueType.StatsFromCitiesOnSpecificTiles.placeholderText ->
                     if (city.getCenterTile().matchesFilter(unique.params[1])) 
                         unique.stats.values.sum() // Modified by personality 
                     else 0f
@@ -92,7 +92,7 @@ object ChooseBeliefsAutomation {
                         }
                         else -> 0f
                     }
-                "[] in cities with [] or more population" ->
+                UniqueType.StatsFromXPopulation.placeholderText ->
                     unique.stats.values.sum() // Modified by personality
                 "[] from each Trade Route" ->
                     unique.stats.values.sum() *

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

@@ -88,33 +88,7 @@ class CityConstructions {
         val stats = Stats()
         for (building in getBuiltBuildings())
             stats.add(building.getStats(cityInfo))
-        
+
-        // Why only the local matching uniques you ask? Well, the non-local uniques are evaluated in
-        // CityStats.getStatsFromUniques(uniques). This function gets a list of uniques and one of
-        // the placeholderTexts it filters for is "[] per [] population []". Why doesn't that function
-        // then not also handle the local (i.e. cityFilter == "in this city") versions?
-        // This is because of what it is used for. The only time time a unique with this placeholderText
-        // is actually contained in `uniques`, is when `getStatsFromUniques` is called for determining
-        // the stats a city receives from wonders. It is then called with `unique` being the list
-        // of all specifically non-local uniques of all cities.
-        // 
-        // This averts the problem, albeit barely, and it might change in the future without
-        // anyone noticing, which might lead to further bugs. So why can't these two unique checks
-        // just be merged then? Because of another problem.
-        //
-        // As noted earlier, `getStatsFromUniques` is called in that case to calculate the stats
-        // this city received from wonders, both local and non-local. This function, `getStats`,
-        // is only called to calculate the stats the city receives from local buildings.
-        // In the current codebase with the current JSON objects it just so happens to be that
-        // all non-local uniques with this placeholderText from other cities belong to wonders, 
-        // while the local uniques with this placeholderText are from buildings, but this is in no
-        // way a given. In reality, there should be functions getBuildingStats and getWonderStats,
-        // to solve this, with getStats merely adding these two together. 
-        for (unique in cityInfo.getLocalMatchingUniques(UniqueType.StatsPerPopulation)
-                .filter { cityInfo.matchesFilter(it.params[2])}
-        ) {
-            stats.add(unique.stats.times(cityInfo.population.population / unique.params[1].toFloat()))
-        }
         
         // Deprecated since 3.17.11
             for (unique in cityInfo.getLocalMatchingUniques(UniqueType.StatsWithTech)) {

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

@@ -88,9 +88,6 @@ class CityStats(val cityInfo: CityInfo) {
         return stats
     }
 
-    private fun getStatsFromNationUnique(): Stats {
-        return getStatsFromUniques(cityInfo.civInfo.nation.uniqueObjects.asSequence())
-    }
 
     private fun getStatsFromCityStates(): Stats {
         val stats = Stats()
@@ -186,36 +183,38 @@ class CityStats(val cityInfo: CityInfo) {
     }
 
 
-    /**
-     * Intended to replace getStatsFromUniques
-     * getStatsFromUniques is one of many functions that takes getCivWideBuildingUniques as an input
-     * That function is too performance-heavy and we need to get rid of it
-     * The proper way is by converting the "get uniques by object type then filter by uniquetype and map to stats"
-     *   to "get uniques by uniquetype then map to stats and sort by object type"
-     * That way we use the nice hashmaps we have everywhere to only get the relevant uniques
-     *   and not iterate on ALL of them
-     */
     private fun getStatsFromUniquesBySource():StatMap {
         val sourceToStats = StatMap()
         val cityConditionals = StateForConditionals(cityInfo.civInfo, cityInfo)
         fun addUniqueStats(unique:Unique) =
             sourceToStats.add(unique.sourceObjectType?.name ?: "", unique.stats)
 
-        for (unique in cityInfo.civInfo.getMatchingUniques(UniqueType.Stats, cityConditionals))
+        for (unique in cityInfo.getMatchingUniques(UniqueType.Stats, cityConditionals))
             addUniqueStats(unique)
 
-        for (unique in cityInfo.civInfo.getMatchingUniques(UniqueType.StatsPerCity, cityConditionals))
+        for (unique in cityInfo.getMatchingUniques(UniqueType.StatsPerCity, cityConditionals))
             if (cityInfo.matchesFilter(unique.params[1]))
                 addUniqueStats(unique)
 
-
         // "[stats] per [amount] population [cityFilter]"
-        for (unique in cityInfo.civInfo.getMatchingUniques(UniqueType.StatsPerPopulation, cityConditionals))
+        for (unique in cityInfo.getMatchingUniques(UniqueType.StatsPerPopulation, cityConditionals))
             if (cityInfo.matchesFilter(unique.params[2])) {
                 val amountOfEffects = (cityInfo.population.population / unique.params[1].toInt()).toFloat()
                 sourceToStats.add(unique.sourceObjectType?.name ?: "", unique.stats.times(amountOfEffects))
             }
 
+        for (unique in cityInfo.getMatchingUniques(UniqueType.StatsFromXPopulation, cityConditionals))
+            if (cityInfo.population.population >= unique.params[1].toInt())
+                addUniqueStats(unique)
+
+        for (unique in cityInfo.getMatchingUniques(UniqueType.StatsFromCitiesOnSpecificTiles, cityConditionals))
+            if (cityInfo.getCenterTile().matchesTerrainFilter(unique.params[1]))
+                addUniqueStats(unique)
+
+        for (unique in cityInfo.getMatchingUniques(UniqueType.StatsFromCitiesBefore, cityConditionals))
+            if (!cityInfo.civInfo.hasTechOrPolicy(unique.params[1]))
+                addUniqueStats(unique)
+
 
         fun rename(source: String, displayedSource: String) {
             if (!sourceToStats.containsKey(source)) return
@@ -232,19 +231,6 @@ class CityStats(val cityInfo: CityInfo) {
     private fun getStatsFromUniques(uniques: Sequence<Unique>): Stats {
         val stats = Stats()
 
-        for (unique in uniques.toList()) { // Should help mitigate getConstructionButtonDTOs concurrency problems.
-
-            // "[stats] in cities with [amount] or more population
-            if (unique.placeholderText == "[] in cities with [] or more population" && cityInfo.population.population >= unique.params[1].toInt())
-                stats.add(unique.stats)
-
-            // "[stats] in cities on [tileFilter] tiles"
-            if (unique.placeholderText == "[] in cities on [] tiles" && cityInfo.getCenterTile().matchesTerrainFilter(unique.params[1]))
-                stats.add(unique.stats)
-
-            if (unique.placeholderText == "[] per turn from cities before []" && !cityInfo.civInfo.hasTechOrPolicy(unique.params[1]))
-                stats.add(unique.stats)
-        }
 
         return stats
     }
@@ -458,11 +444,6 @@ class CityStats(val cityInfo: CityInfo) {
 
         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 =
@@ -472,20 +453,12 @@ class CityStats(val cityInfo: CityInfo) {
 
         newHappinessList["Buildings"] = statsFromBuildings.happiness.toInt().toFloat()
 
-        newHappinessList["National ability"] =
-            getStatsFromUniques(cityInfo.civInfo.nation.uniqueObjects.asSequence()).happiness
-
-        newHappinessList["Wonders"] =
-            getStatsFromUniques(civInfo.getCivWideBuildingUniques(cityInfo)).happiness
-
-        newHappinessList["Religion"] = getStatsFromUniques(cityInfo.religion.getUniques()).happiness
-
         newHappinessList["Tile yields"] = statsFromTiles.happiness
 
         val happinessBySource = getStatsFromUniquesBySource()
         for ((source, stats) in happinessBySource)
             if (stats.happiness != 0f) {
-                if (newHappinessList.containsKey(source)) newHappinessList[source] = 0f
+                if (!newHappinessList.containsKey(source)) newHappinessList[source] = 0f
                 newHappinessList[source] = newHappinessList[source]!! + stats.happiness
             }
 
@@ -495,25 +468,22 @@ class CityStats(val cityInfo: CityInfo) {
     }
 
     private fun updateBaseStatList(statsFromBuildings: Stats) {
-        val newBaseStatList = StatMap() // we don't edit the existing baseStatList directly, in order to avoid concurrency exceptions
+        val newBaseStatList =
-        val civInfo = cityInfo.civInfo
+            StatMap() // we don't edit the existing baseStatList directly, in order to avoid concurrency exceptions
 
         newBaseStatList["Population"] = Stats(
             science = cityInfo.population.population.toFloat(),
             production = cityInfo.population.getFreePopulation().toFloat()
         )
         newBaseStatList["Tile yields"] = statsFromTiles
-        newBaseStatList["Specialists"] = getStatsFromSpecialists(cityInfo.population.getNewSpecialists())
+        newBaseStatList["Specialists"] =
+            getStatsFromSpecialists(cityInfo.population.getNewSpecialists())
         newBaseStatList["Trade routes"] = getStatsFromTradeRoute()
         newBaseStatList["Buildings"] = statsFromBuildings
-        newBaseStatList["Policies"] = getStatsFromUniques(civInfo.policies.policyUniques.getAllUniques())
-        newBaseStatList["National ability"] = getStatsFromNationUnique()
-        newBaseStatList["Wonders"] = getStatsFromUniques(civInfo.getCivWideBuildingUniques(cityInfo))
         newBaseStatList["City-States"] = getStatsFromCityStates()
-        newBaseStatList["Religion"] = getStatsFromUniques(cityInfo.religion.getUniques())
 
         val statMap = getStatsFromUniquesBySource()
-        for((source, stats) in statMap)
+        for ((source, stats) in statMap)
             newBaseStatList.add(source, stats)
 
         baseStatList = newBaseStatList
@@ -577,7 +547,7 @@ class CityStats(val cityInfo: CityInfo) {
     }
 
     private fun updateFinalStatList(currentConstruction: IConstruction, citySpecificUniques: Sequence<Unique>) {
-        val newFinalStatList = LinkedHashMap<String, Stats>() // again, we don't edit the existing currentCityStats directly, in order to avoid concurrency exceptions
+        val newFinalStatList = StatMap() // again, we don't edit the existing currentCityStats directly, in order to avoid concurrency exceptions
 
         for (entry in baseStatList)
             newFinalStatList[entry.key] = entry.value.clone()
@@ -636,7 +606,7 @@ class CityStats(val cityInfo: CityInfo) {
         // Since growth bonuses are special, (applied afterwards) they will be displayed separately in the user interface as well.
         if (totalFood > 0 && !isUnhappy) { // Percentage Growth bonus revoked when unhappy per https://forums.civfanatics.com/resources/complete-guide-to-happiness-vanilla.25584/
             val foodFromGrowthBonuses = getGrowthBonusFromPoliciesAndWonders() * totalFood
-            newFinalStatList["Policies"]!!.food += foodFromGrowthBonuses // Why Policies? Wonders can also provide this?
+            newFinalStatList.add("Growth bonus", Stats(food = foodFromGrowthBonuses))  // Why Policies? Wonders can also provide this?
             totalFood = newFinalStatList.values.map { it.food }.sum() // recalculate again
         }
 

core/src/com/unciv/models/ruleset/unique/UniqueType.kt

@@ -76,6 +76,10 @@ enum class UniqueType(val text:String, vararg targets: UniqueTarget, val flags:
     StatsFromSpecialistDeprecated("[stats] from every specialist", UniqueTarget.Global),
 
     StatsPerPopulation("[stats] per [amount] population [cityFilter]", UniqueTarget.Global),
+    StatsFromXPopulation("[stats] in cities with [amount] or more population", UniqueTarget.Global, UniqueTarget.FollowerBelief),
+    StatsFromCitiesOnSpecificTiles("[stats] in cities on [terrainFilter] tiles", UniqueTarget.Global, UniqueTarget.FollowerBelief),
+    StatsFromCitiesBefore("[stats] per turn from cities before [tech/policy]", UniqueTarget.Global),
+
 
     StatsSpendingGreatPeople("[stats] whenever a Great Person is expended", UniqueTarget.Global),
     StatsFromTiles("[stats] from [tileFilter] tiles [cityFilter]", UniqueTarget.Global),