From 14c4f2d57077b30226585c39659e38dfec9ce704 Mon Sep 17 00:00:00 2001 From: yairm210 Date: Sun, 21 Nov 2021 22:43:51 +0200 Subject: [PATCH] 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? --- .../automation/ChooseBeliefsAutomation.kt | 4 +- .../com/unciv/logic/city/CityConstructions.kt | 28 +------ core/src/com/unciv/logic/city/CityStats.kt | 76 ++++++------------- .../unciv/models/ruleset/unique/UniqueType.kt | 4 + 4 files changed, 30 insertions(+), 82 deletions(-) diff --git a/core/src/com/unciv/logic/automation/ChooseBeliefsAutomation.kt b/core/src/com/unciv/logic/automation/ChooseBeliefsAutomation.kt index b70d5d8aed..e13586a2c6 100644 --- a/core/src/com/unciv/logic/automation/ChooseBeliefsAutomation.kt +++ b/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() * diff --git a/core/src/com/unciv/logic/city/CityConstructions.kt b/core/src/com/unciv/logic/city/CityConstructions.kt index 61ebe05c19..0d3bf1488b 100644 --- a/core/src/com/unciv/logic/city/CityConstructions.kt +++ b/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)) { diff --git a/core/src/com/unciv/logic/city/CityStats.kt b/core/src/com/unciv/logic/city/CityStats.kt index cf505f2cf6..75ca0bbca5 100644 --- a/core/src/com/unciv/logic/city/CityStats.kt +++ b/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): 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 civInfo = cityInfo.civInfo + val newBaseStatList = + 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) { - val newFinalStatList = LinkedHashMap() // 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 } diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt index d10fc7e650..d7beaf29e3 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt +++ b/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),