From 861a42e8818388e8fe16eaf3c13504a15a88413d Mon Sep 17 00:00:00 2001 From: Xander Lenstra <71121390+xlenstra@users.noreply.github.com> Date: Tue, 28 Sep 2021 21:42:18 +0200 Subject: [PATCH] Moved the check for conditionals applying to `getMatchingUniques` functions; rewrote `civInfo.getMatchingUniques` (#5342) * Moved the check for conditionals applying to `getMatchingUniques` functions. Rewrote `civInfo.getMatchingUniques`. * Clarified comment --- .../com/unciv/logic/city/CityConstructions.kt | 4 +- core/src/com/unciv/logic/city/CityInfo.kt | 38 +++++---- core/src/com/unciv/logic/city/CityStats.kt | 13 ++- .../unciv/logic/civilization/CivInfoStats.kt | 7 +- .../logic/civilization/CivilizationInfo.kt | 81 +++++++++---------- .../com/unciv/models/ruleset/IHasUniques.kt | 22 +++-- .../ruleset/unique/StateForConditionals.kt | 9 +++ .../com/unciv/models/ruleset/unique/Unique.kt | 24 +++--- .../unciv/models/ruleset/unique/UniqueType.kt | 1 + 9 files changed, 116 insertions(+), 83 deletions(-) create mode 100644 core/src/com/unciv/models/ruleset/unique/StateForConditionals.kt diff --git a/core/src/com/unciv/logic/city/CityConstructions.kt b/core/src/com/unciv/logic/city/CityConstructions.kt index b81f66bdb0..adc2b477cd 100644 --- a/core/src/com/unciv/logic/city/CityConstructions.kt +++ b/core/src/com/unciv/logic/city/CityConstructions.kt @@ -6,6 +6,7 @@ import com.unciv.logic.civilization.NotificationIcon import com.unciv.logic.civilization.PopupAlert import com.unciv.models.ruleset.Building import com.unciv.models.ruleset.Ruleset +import com.unciv.models.ruleset.unique.StateForConditionals import com.unciv.models.ruleset.unique.UniqueMap import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.ruleset.unit.BaseUnit @@ -162,8 +163,7 @@ class CityConstructions { fun addFreeBuildings() { // "Provides a free [buildingName] [cityFilter]" - for (unique in cityInfo.getLocalMatchingUniques(UniqueType.ProvidesFreeBuildings)) { - if (!unique.conditionalsApply(cityInfo.civInfo, cityInfo)) continue + for (unique in cityInfo.getLocalMatchingUniques(UniqueType.ProvidesFreeBuildings, StateForConditionals(cityInfo.civInfo, cityInfo))) { val freeBuildingName = cityInfo.civInfo.getEquivalentBuilding(unique.params[0]).name val citiesThatApply = when (unique.params[1]) { "in this city" -> listOf(cityInfo) diff --git a/core/src/com/unciv/logic/city/CityInfo.kt b/core/src/com/unciv/logic/city/CityInfo.kt index f9cc493234..58fe2998ab 100644 --- a/core/src/com/unciv/logic/city/CityInfo.kt +++ b/core/src/com/unciv/logic/city/CityInfo.kt @@ -14,6 +14,7 @@ import com.unciv.models.ruleset.unique.Unique import com.unciv.models.ruleset.unique.UniqueType 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.unit.BaseUnit import com.unciv.models.stats.Stat import java.util.* @@ -160,8 +161,7 @@ class CityInfo { civInfo.civConstructions.tryAddFreeBuildings() - for (unique in getMatchingUniques(UniqueType.GainFreeBuildings)) { - if (!unique.conditionalsApply(civInfo, this)) continue + for (unique in getMatchingUniques(UniqueType.GainFreeBuildings, stateForConditionals = StateForConditionals(civInfo, this))) { val freeBuildingName = unique.params[0] if (matchesFilter(unique.params[1])) { if (!cityConstructions.isBuilt(freeBuildingName)) @@ -723,14 +723,16 @@ class CityInfo { uniqueType: UniqueType, // We might have this cached to avoid concurrency problems. If we don't, just get it directly localUniques: Sequence = getLocalMatchingUniques(uniqueType), + stateForConditionals: StateForConditionals? = null, ): Sequence { // The localUniques might not be filtered when passed as a parameter, so we filter it anyway // The time loss shouldn't be that large I don't think - return civInfo.getMatchingUniques(uniqueType, this) + - localUniques.filter { - it.isOfType(uniqueType) - && it.params.none { param -> param == "in other cities" } - } + return civInfo.getMatchingUniques(uniqueType, stateForConditionals, this) + + localUniques.filter { + it.isOfType(uniqueType) + && it.conditionalsApply(stateForConditionals) + && it.params.none { param -> param == "in other cities" } + } } // Matching uniques provided by sources in the city itself @@ -740,10 +742,14 @@ class CityInfo { religion.getUniques().filter { it.placeholderText == placeholderText } } - fun getLocalMatchingUniques(uniqueType: UniqueType): Sequence { - return cityConstructions.builtBuildingUniqueMap.getUniques(uniqueType) - .filter { it.params.none { param -> param == "in other cities" } } + - religion.getUniques().filter { it.isOfType(uniqueType) } + fun getLocalMatchingUniques(uniqueType: UniqueType, stateForConditionals: StateForConditionals? = null): Sequence { + return ( + cityConstructions.builtBuildingUniqueMap.getUniques(uniqueType) + .filter { it.params.none { param -> param == "in other cities" } } + + religion.getUniques().filter { it.isOfType(uniqueType) } + ).filter { + it.conditionalsApply(stateForConditionals) + } } // Get all uniques that originate from this city @@ -755,21 +761,21 @@ class CityInfo { fun getMatchingUniquesWithNonLocalEffects(placeholderText: String): Sequence { return cityConstructions.builtBuildingUniqueMap.getUniques(placeholderText) .filter { it.params.none { param -> param == "in this city" } } - // Note that we don't query religion here, as those only have local effects (for now at least) + // Note that we don't query religion here, as those only have local effects } - fun getMatchingUniquesWithNonLocalEffects(uniqueType: UniqueType): Sequence { + fun getMatchingUniquesWithNonLocalEffects(uniqueType: UniqueType, stateForConditionals: StateForConditionals? = null): Sequence { return cityConstructions.builtBuildingUniqueMap.getUniques(uniqueType) - .filter { it.params.none { param -> param == "in this city" } } - // Note that we don't query religion here, as those only have local effects (for now at least) + .filter { it.params.none { param -> param == "in this city" } && it.conditionalsApply(stateForConditionals) } + // Note that we don't query religion here, as those only have local effects } // Get all uniques that don't apply to only this city fun getAllUniquesWithNonLocalEffects(): Sequence { return cityConstructions.builtBuildingUniqueMap.getAllUniques() .filter { it.params.none { param -> param == "in this city" } } - // Note that we don't query religion here, as those only have local effects (for now at least) + // Note that we don't query religion here, as those only have local effects } fun isHolyCity(): Boolean = religion.religionThisIsTheHolyCityOf != null diff --git a/core/src/com/unciv/logic/city/CityStats.kt b/core/src/com/unciv/logic/city/CityStats.kt index c92d502250..359b1d4b07 100644 --- a/core/src/com/unciv/logic/city/CityStats.kt +++ b/core/src/com/unciv/logic/city/CityStats.kt @@ -7,6 +7,7 @@ import com.unciv.logic.map.RoadStatus import com.unciv.models.Counter import com.unciv.models.ruleset.Building import com.unciv.models.ruleset.ModOptionsConstants +import com.unciv.models.ruleset.unique.StateForConditionals import com.unciv.models.ruleset.unique.Unique import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.ruleset.unit.BaseUnit @@ -222,8 +223,12 @@ class CityStats(val cityInfo: CityInfo) { stats.add(unique.stats) } - if (unique.isOfType(UniqueType.StatsPerCity) && cityInfo.matchesFilter(unique.params[1]) && unique.conditionalsApply(cityInfo.civInfo)) + if (unique.isOfType(UniqueType.StatsPerCity) + && cityInfo.matchesFilter(unique.params[1]) + && unique.conditionalsApply(cityInfo.civInfo) + ) { stats.add(unique.stats) + } // "[stats] per [amount] population [cityFilter]" if (unique.placeholderText == "[] per [] population []" && cityInfo.matchesFilter(unique.params[2])) { @@ -510,6 +515,12 @@ class CityStats(val cityInfo: CityInfo) { // We calculate this here for concurrency reasons // If something needs this, we pass this through as a parameter val localBuildingUniques = cityInfo.cityConstructions.builtBuildingUniqueMap.getAllUniques() + + // Is This line really necessary? There is only a single unique that actually uses this, + // and it is passed to functions at least 3 times for that + // It's the only reason `cityInfo.getMatchingUniques` has a localUniques parameter, + // which clutters readability, and also the only reason `CityInfo.getAllLocalUniques()` + // exists in the first place, though that could be useful for the previous line too. val citySpecificUniques = cityInfo.getAllLocalUniques() // We need to compute Tile yields before happiness diff --git a/core/src/com/unciv/logic/civilization/CivInfoStats.kt b/core/src/com/unciv/logic/civilization/CivInfoStats.kt index a3580370a0..8ce32d5dd2 100644 --- a/core/src/com/unciv/logic/civilization/CivInfoStats.kt +++ b/core/src/com/unciv/logic/civilization/CivInfoStats.kt @@ -7,6 +7,7 @@ import com.unciv.models.ruleset.BeliefType import com.unciv.models.ruleset.Policy import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.ruleset.tile.ResourceType +import com.unciv.models.ruleset.unique.StateForConditionals import com.unciv.models.stats.Stat import com.unciv.models.stats.StatMap import com.unciv.models.stats.Stats @@ -21,8 +22,7 @@ class CivInfoStats(val civInfo: CivilizationInfo) { private fun getUnitMaintenance(): Int { val baseUnitCost = 0.5f var freeUnits = 3 - for (unique in civInfo.getMatchingUniques(UniqueType.FreeUnits)) { - if (!unique.conditionalsApply(civInfo)) continue + for (unique in civInfo.getMatchingUniques(UniqueType.FreeUnits, StateForConditionals(civInfo))) { freeUnits += unique.params[0].toInt() } @@ -36,8 +36,7 @@ class CivInfoStats(val civInfo: CivilizationInfo) { var numberOfUnitsToPayFor = max(0f, unitsToPayFor.count().toFloat() - freeUnits) - for (unique in civInfo.getMatchingUniques(UniqueType.UnitMaintenanceDiscount)) { - if (!unique.conditionalsApply(civInfo)) continue + for (unique in civInfo.getMatchingUniques(UniqueType.UnitMaintenanceDiscount, StateForConditionals(civInfo))) { val numberOfUnitsWithDiscount = min( numberOfUnitsToPayFor, unitsToPayFor.count { it.matchesFilter(unique.params[1]) }.toFloat() diff --git a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt index 16932c01c0..70714ea922 100644 --- a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt +++ b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt @@ -18,6 +18,7 @@ import com.unciv.models.ruleset.* import com.unciv.models.ruleset.tile.ResourceSupplyList import com.unciv.models.ruleset.tile.ResourceType import com.unciv.models.ruleset.tile.TileResource +import com.unciv.models.ruleset.unique.StateForConditionals import com.unciv.models.ruleset.unique.Unique import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.ruleset.unit.BaseUnit @@ -337,48 +338,46 @@ class CivilizationInfo { fun hasUnique(uniqueType: UniqueType) = getMatchingUniques(uniqueType).any() fun hasUnique(unique: String) = getMatchingUniques(unique).any() - - /** Destined to replace getMatchingUniques, gradually, as we fill the enum */ - fun getMatchingUniques(uniqueType: UniqueType, cityToIgnore: CityInfo?=null): Sequence { - val ruleset = gameInfo.ruleSet - return nation.uniqueObjects.asSequence().filter { it.matches(uniqueType, ruleset) } + - cities.asSequence().filter { it != cityToIgnore }.flatMap { city -> - city.getMatchingUniquesWithNonLocalEffects(uniqueType) - } + - policies.policyUniques.getUniques(uniqueType) + - tech.techUniques.getUniques(uniqueType) + - temporaryUniques - .asSequence().map { it.first } - .filter { it.matches(uniqueType, ruleset) } + - getEra().getMatchingUniques(uniqueType) + - ( - if (religionManager.religion != null) - religionManager.religion!!.getFounderUniques() - .filter { it.isOfType(uniqueType) } - else sequenceOf() - ) - } - + // Does not return local uniques, only global ones. - fun getMatchingUniques(uniqueTemplate: String, cityToIgnore: CityInfo? = null): Sequence { - return nation.uniqueObjects.asSequence().filter { it.placeholderText == uniqueTemplate } + - cities.asSequence().filter { it != cityToIgnore}.flatMap { - city -> city.getMatchingUniquesWithNonLocalEffects(uniqueTemplate) - } + - policies.policyUniques.getUniques(uniqueTemplate) + - tech.techUniques.getUniques(uniqueTemplate) + - temporaryUniques - .asSequence() - .filter { it.first.placeholderText == uniqueTemplate }.map { it.first } + - getEra().getMatchingUniques(uniqueTemplate) - .asSequence() + - ( - if (religionManager.religion != null) - religionManager.religion!!.getFounderUniques() - .asSequence() - .filter { it.placeholderText == uniqueTemplate } - else sequenceOf() - ) + /** Destined to replace getMatchingUniques, gradually, as we fill the enum */ + fun getMatchingUniques(uniqueType: UniqueType, stateForConditionals: StateForConditionals? = null, cityToIgnore: CityInfo? = null) = sequence { + val ruleset = gameInfo.ruleSet + yieldAll(nation.uniqueObjects.asSequence().filter {it.matches(uniqueType, ruleset) }) + yieldAll(cities.asSequence() + .filter { it != cityToIgnore } + .flatMap { city -> city.getMatchingUniquesWithNonLocalEffects(uniqueType) } + ) + yieldAll(policies.policyUniques.getUniques(uniqueType)) + yieldAll(tech.techUniques.getUniques(uniqueType)) + yieldAll(temporaryUniques.asSequence() + .map { it.first } + .filter { it.matches(uniqueType, ruleset) } + ) + yieldAll(getEra().getMatchingUniques(uniqueType)) + if (religionManager.religion != null) + yieldAll(religionManager.religion!!.getFounderUniques().filter { it.isOfType(uniqueType) }) + }.filter { + it.conditionalsApply(stateForConditionals) + } + + fun getMatchingUniques(uniqueTemplate: String, cityToIgnore: CityInfo? = null) = sequence { + yieldAll(nation.uniqueObjects.asSequence().filter { it.placeholderText == uniqueTemplate }) + yieldAll(cities.asSequence() + .filter { it != cityToIgnore } + .flatMap { city -> city.getMatchingUniquesWithNonLocalEffects(uniqueTemplate) } + ) + yieldAll(policies.policyUniques.getUniques(uniqueTemplate)) + yieldAll(tech.techUniques.getUniques(uniqueTemplate)) + yieldAll(temporaryUniques.asSequence() + .filter { it.first.placeholderText == uniqueTemplate }.map { it.first } + ) + yieldAll(getEra().getMatchingUniques(uniqueTemplate).asSequence()) + if (religionManager.religion != null) + yieldAll(religionManager.religion!!.getFounderUniques() + .asSequence() + .filter { it.placeholderText == uniqueTemplate } + ) } //region Units diff --git a/core/src/com/unciv/models/ruleset/IHasUniques.kt b/core/src/com/unciv/models/ruleset/IHasUniques.kt index 1d1c8efcd8..7e17063a8d 100644 --- a/core/src/com/unciv/models/ruleset/IHasUniques.kt +++ b/core/src/com/unciv/models/ruleset/IHasUniques.kt @@ -1,5 +1,6 @@ package com.unciv.models.ruleset +import com.unciv.models.ruleset.unique.StateForConditionals import com.unciv.models.ruleset.unique.Unique import com.unciv.models.ruleset.unique.UniqueTarget import com.unciv.models.ruleset.unique.UniqueType @@ -10,15 +11,22 @@ import com.unciv.models.ruleset.unique.UniqueType interface IHasUniques { var uniques: ArrayList // Can not be a hashset as that would remove doubles // I bet there's a way of initializing these without having to override it everywhere... - val uniqueObjects: List + val uniqueObjects: List + /** Technically not currently needed, since the unique target can be retrieved from every unique in the uniqueObjects, * But making this a function is relevant for future "unify Unciv object" plans ;) * */ fun getUniqueTarget(): UniqueTarget - - fun getMatchingUniques(uniqueTemplate: String) = uniqueObjects.asSequence().filter { it.placeholderText == uniqueTemplate } - fun getMatchingUniques(uniqueType: UniqueType) = uniqueObjects.asSequence().filter { it.isOfType(uniqueType) } - - fun hasUnique(uniqueTemplate: String) = uniqueObjects.any { it.placeholderText == uniqueTemplate } - fun hasUnique(uniqueType: UniqueType) = uniqueObjects.any { it.isOfType(uniqueType) } + + fun getMatchingUniques(uniqueTemplate: String, stateForConditionals: StateForConditionals? = null) = + uniqueObjects.asSequence().filter { it.placeholderText == uniqueTemplate && it.conditionalsApply(stateForConditionals) } + + fun getMatchingUniques(uniqueType: UniqueType, stateForConditionals: StateForConditionals? = null) = + uniqueObjects.asSequence().filter { it.isOfType(uniqueType) && it.conditionalsApply(stateForConditionals) } + + fun hasUnique(uniqueTemplate: String, stateForConditionals: StateForConditionals? = null) = + uniqueObjects.any { it.placeholderText == uniqueTemplate && it.conditionalsApply(stateForConditionals) } + + fun hasUnique(uniqueType: UniqueType, stateForConditionals: StateForConditionals? = null) = + uniqueObjects.any { it.isOfType(uniqueType) && it.conditionalsApply(stateForConditionals) } } diff --git a/core/src/com/unciv/models/ruleset/unique/StateForConditionals.kt b/core/src/com/unciv/models/ruleset/unique/StateForConditionals.kt new file mode 100644 index 0000000000..cf75ee0df8 --- /dev/null +++ b/core/src/com/unciv/models/ruleset/unique/StateForConditionals.kt @@ -0,0 +1,9 @@ +package com.unciv.models.ruleset.unique + +import com.unciv.logic.city.CityInfo +import com.unciv.logic.civilization.CivilizationInfo + +data class StateForConditionals( + val civInfo: CivilizationInfo? = null, + val cityInfo: CityInfo? = null, +) \ No newline at end of file diff --git a/core/src/com/unciv/models/ruleset/unique/Unique.kt b/core/src/com/unciv/models/ruleset/unique/Unique.kt index acb1d93619..a9bf562682 100644 --- a/core/src/com/unciv/models/ruleset/unique/Unique.kt +++ b/core/src/com/unciv/models/ruleset/unique/Unique.kt @@ -27,29 +27,29 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s fun matches(uniqueType: UniqueType, ruleset: Ruleset) = isOfType(uniqueType) && uniqueType.getComplianceErrors(this, ruleset).isEmpty() - // This function will get LARGE, as it will basically check for all conditionals if they apply - // This will require a lot of parameters to be passed (attacking unit, tile, defending unit, civInfo, cityInfo, ...) - // I'm open for better ideas, but this was the first thing that I could think of that would - // work in all cases. fun conditionalsApply(civInfo: CivilizationInfo? = null, city: CityInfo? = null): Boolean { + return conditionalsApply(StateForConditionals(civInfo, city)) + } + + fun conditionalsApply(state: StateForConditionals?): Boolean { + if (state == null) return conditionals.isEmpty() for (condition in conditionals) { - if (!conditionalApplies(condition, civInfo, city)) return false + if (!conditionalApplies(condition, state)) return false } return true } - + private fun conditionalApplies( condition: Unique, - civInfo: CivilizationInfo? = null, - city: CityInfo? = null + state: StateForConditionals ): Boolean { return when (condition.placeholderText) { - UniqueType.ConditionalNotWar.placeholderText -> civInfo?.isAtWar() == false - UniqueType.ConditionalWar.placeholderText -> civInfo?.isAtWar() == true + UniqueType.ConditionalNotWar.placeholderText -> state.civInfo?.isAtWar() == false + UniqueType.ConditionalWar.placeholderText -> state.civInfo?.isAtWar() == true UniqueType.ConditionalSpecialistCount.placeholderText -> - city != null && city.population.getNumberOfSpecialists() >= condition.params[0].toInt() + state.cityInfo != null && state.cityInfo.population.getNumberOfSpecialists() >= condition.params[0].toInt() UniqueType.ConditionalHappy.placeholderText -> - civInfo != null && civInfo.statsForNextTurn.happiness >= 0 + state.civInfo != null && state.civInfo.statsForNextTurn.happiness >= 0 else -> false } } diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt index 64fd86d2d8..ff3e66318c 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt @@ -109,6 +109,7 @@ enum class UniqueType(val text:String, vararg targets: UniqueTarget) { NaturalWonderLatitude("Occurs on latitudes from [amount] to [amount] percent of distance equator to pole", UniqueTarget.Terrain), NaturalWonderGroups("Occurs in groups of [amount] to [amount] tiles", UniqueTarget.Terrain), NaturalWonderConvertNeighbors("Neighboring tiles will convert to [baseTerrain]", UniqueTarget.Terrain), + // The "Except [terrainFilter]" could theoretically be implemented with a conditional NaturalWonderConvertNeighborsExcept("Neighboring tiles except [terrainFilter] will convert to [baseTerrain]", UniqueTarget.Terrain), TerrainGrantsPromotion("Grants [promotion] ([comment]) to adjacent [mapUnitFilter] units for the rest of the game", UniqueTarget.Terrain),