From cf2ff124cf6cb6ad0b23fd6cbd8d36496df605d3 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Sun, 29 Oct 2023 18:00:09 +0100 Subject: [PATCH] Fix Mod checker crash on RekMod (#10349) * Verbose exception for the original RekMod's Civilian crash * Make ConstructImprovementInstantly validation gameInfo-agnostic * Helpers to encapsulate repetitive Conditional test patterns, eliminating ruleset(), all ruleset access guarded * Make ConditionalFirstCivToResearch actually work on Policies - ??? * Linting * Revert params accessor helpers and reduce ConditionalFirstCivToResearch to tech only * Implement ConditionalFirstCivToAdopt for nicer symmetry --- .../com/unciv/models/ruleset/unique/Unique.kt | 193 +++++++++--------- .../unciv/models/ruleset/unique/UniqueType.kt | 1 + .../ruleset/validation/RulesetValidator.kt | 8 +- docs/Modders/uniques.md | 3 + 4 files changed, 110 insertions(+), 95 deletions(-) diff --git a/core/src/com/unciv/models/ruleset/unique/Unique.kt b/core/src/com/unciv/models/ruleset/unique/Unique.kt index 675c9e1d8d..90ca621a4f 100644 --- a/core/src/com/unciv/models/ruleset/unique/Unique.kt +++ b/core/src/com/unciv/models/ruleset/unique/Unique.kt @@ -140,8 +140,6 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s if (condition.type?.targetTypes?.any { it.modifierType == UniqueTarget.ModifierType.Other } == true) return true // not a filtering condition - fun ruleset() = state.civInfo!!.gameInfo.ruleset - val relevantUnit by lazy { if (state.ourCombatant != null && state.ourCombatant is MapUnitCombatant) state.ourCombatant.unit else state.unit @@ -161,107 +159,110 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s return 0 } + /** Helper to simplify conditional tests requiring a Civilization */ + fun checkOnCiv(predicate: (Civilization.() -> Boolean)): Boolean { + if (state.civInfo == null) return false + return state.civInfo.predicate() + } + + /** Helper to simplify conditional tests requiring a City */ + fun checkOnCity(predicate: (City.() -> Boolean)): Boolean { + if (state.city == null) return false + return state.city.predicate() + } + + /** Helper to simplify the "compare civ's current era with named era" conditions */ + fun compareEra(eraParam: String, compare: (civEra: Int, paramEra: Int) -> Boolean): Boolean { + if (state.civInfo == null) return false + val era = state.civInfo.gameInfo.ruleset.eras[eraParam] ?: return false + return compare(state.civInfo.getEraNumber(), era.eraNumber) + } + + /** Helper for ConditionalWhenAboveAmountStatResource and its below counterpart */ + fun checkResourceOrStatAmount(compare: (current: Int, limit: Int) -> Boolean): Boolean { + if (state.civInfo == null) return false + val limit = condition.params[0].toInt() + val resourceOrStatName = condition.params[1] + if (state.civInfo.gameInfo.ruleset.tileResources.containsKey(resourceOrStatName)) + return compare(getResourceAmount(resourceOrStatName), limit) + val stat = Stat.safeValueOf(resourceOrStatName) + ?: return false + return compare(state.civInfo.getStatReserve(stat), limit) + } + + /** Helper for ConditionalWhenAboveAmountStatSpeed and its below counterpart */ + fun checkStatAmountWithSpeed(compare: (current: Int, limit: Float) -> Boolean): Boolean { + if (state.civInfo == null) return false + val stat = Stat.safeValueOf(condition.params[1]) + ?: return false + val limit = condition.params[0].toFloat() * state.civInfo.gameInfo.speed.statCostModifiers[stat]!! + return compare(state.civInfo.getStatReserve(stat), limit) + } + return when (condition.type) { // These are 'what to do' and not 'when to do' conditionals UniqueType.ConditionalTimedUnique -> true UniqueType.ConditionalChance -> stateBasedRandom.nextFloat() < condition.params[0].toFloat() / 100f - UniqueType.ConditionalBeforeTurns -> state.civInfo != null && state.civInfo.gameInfo.turns < condition.params[0].toInt() - UniqueType.ConditionalAfterTurns -> state.civInfo != null && state.civInfo.gameInfo.turns >= condition.params[0].toInt() + UniqueType.ConditionalBeforeTurns -> checkOnCiv { gameInfo.turns < condition.params[0].toInt() } + UniqueType.ConditionalAfterTurns -> checkOnCiv { gameInfo.turns >= condition.params[0].toInt() } - - UniqueType.ConditionalNationFilter -> state.civInfo?.nation?.matchesFilter(condition.params[0]) == true - UniqueType.ConditionalWar -> state.civInfo?.isAtWar() == true - UniqueType.ConditionalNotWar -> state.civInfo?.isAtWar() == false + UniqueType.ConditionalNationFilter -> checkOnCiv { nation.matchesFilter(condition.params[0]) } + UniqueType.ConditionalWar -> checkOnCiv { isAtWar() } + UniqueType.ConditionalNotWar -> checkOnCiv { !isAtWar() } UniqueType.ConditionalWithResource -> getResourceAmount(condition.params[0]) > 0 UniqueType.ConditionalWithoutResource -> getResourceAmount(condition.params[0]) <= 0 UniqueType.ConditionalWhenAboveAmountStatResource -> - if (ruleset().tileResources.containsKey(condition.params[1])) { - return getResourceAmount(condition.params[1]) > condition.params[0].toInt() - } else if (Stat.isStat(condition.params[1])) { - return state.civInfo != null && - state.civInfo.getStatReserve(Stat.valueOf(condition.params[1])) > condition.params[0].toInt() - } else { - return false - } - + checkResourceOrStatAmount { current, limit -> current > limit } UniqueType.ConditionalWhenBelowAmountStatResource -> - if (ruleset().tileResources.containsKey(condition.params[1])) { - return getResourceAmount(condition.params[1]) < condition.params[0].toInt() - } else if (Stat.isStat(condition.params[1])) { - return state.civInfo != null && - state.civInfo.getStatReserve(Stat.valueOf(condition.params[1])) < condition.params[0].toInt() - } else { - return false - } + checkResourceOrStatAmount { current, limit -> current < limit } + UniqueType.ConditionalWhenAboveAmountStatSpeed -> + checkStatAmountWithSpeed { current, limit -> current > limit } // Note: Int.compareTo(Float)! + UniqueType.ConditionalWhenBelowAmountStatSpeed -> + checkStatAmountWithSpeed { current, limit -> current < limit } // Note: Int.compareTo(Float)! - UniqueType.ConditionalWhenAboveAmountStatSpeed -> state.civInfo != null && - state.civInfo.getStatReserve(Stat.valueOf(condition.params[1])) > condition.params[0].toInt() * - state.civInfo.gameInfo.speed.statCostModifiers[Stat.valueOf(condition.params[1])]!! - - UniqueType.ConditionalWhenBelowAmountStatSpeed -> state.civInfo != null && - state.civInfo.getStatReserve(Stat.valueOf(condition.params[1])) < condition.params[0].toInt() * - state.civInfo.gameInfo.speed.statCostModifiers[Stat.valueOf(condition.params[1])]!! - - UniqueType.ConditionalHappy -> - state.civInfo != null && state.civInfo.stats.happiness >= 0 + UniqueType.ConditionalHappy -> checkOnCiv { stats.happiness >= 0 } UniqueType.ConditionalBetweenHappiness -> - state.civInfo != null - && condition.params[0].toInt() <= state.civInfo.stats.happiness - && state.civInfo.stats.happiness < condition.params[1].toInt() - UniqueType.ConditionalBelowHappiness -> - state.civInfo != null && state.civInfo.stats.happiness < condition.params[0].toInt() - UniqueType.ConditionalGoldenAge -> - state.civInfo != null && state.civInfo.goldenAges.isGoldenAge() - UniqueType.ConditionalWLTKD -> - state.city != null && state.city.isWeLoveTheKingDayActive() - UniqueType.ConditionalBeforeEra -> - state.civInfo != null && ruleset().eras.containsKey(condition.params[0]) - && state.civInfo.getEraNumber() < ruleset().eras[condition.params[0]]!!.eraNumber - UniqueType.ConditionalStartingFromEra -> - state.civInfo != null && ruleset().eras.containsKey(condition.params[0]) - && state.civInfo.getEraNumber() >= ruleset().eras[condition.params[0]]!!.eraNumber - UniqueType.ConditionalDuringEra -> - state.civInfo != null && ruleset().eras.containsKey(condition.params[0]) - && state.civInfo.getEraNumber() == ruleset().eras[condition.params[0]]!!.eraNumber - UniqueType.ConditionalIfStartingInEra -> - state.civInfo != null && state.civInfo.gameInfo.gameParameters.startingEra == condition.params[0] - UniqueType.ConditionalTech -> - state.civInfo != null && state.civInfo.tech.isResearched(condition.params[0]) - UniqueType.ConditionalNoTech -> - state.civInfo != null && !state.civInfo.tech.isResearched(condition.params[0]) + checkOnCiv { stats.happiness in condition.params[0].toInt() until condition.params[1].toInt() } + UniqueType.ConditionalBelowHappiness -> checkOnCiv { stats.happiness < condition.params[0].toInt() } + UniqueType.ConditionalGoldenAge -> checkOnCiv { goldenAges.isGoldenAge() } + UniqueType.ConditionalBeforeEra -> compareEra(condition.params[0]) { current, param -> current < param } + UniqueType.ConditionalStartingFromEra -> compareEra(condition.params[0]) { current, param -> current >= param } + UniqueType.ConditionalDuringEra -> compareEra(condition.params[0]) { current, param -> current == param } + UniqueType.ConditionalIfStartingInEra -> checkOnCiv { gameInfo.gameParameters.startingEra == condition.params[0] } + UniqueType.ConditionalTech -> checkOnCiv { tech.isResearched(condition.params[0]) } + UniqueType.ConditionalNoTech -> checkOnCiv { !tech.isResearched(condition.params[0]) } UniqueType.ConditionalAfterPolicyOrBelief -> - state.civInfo != null && (state.civInfo.policies.isAdopted(condition.params[0]) - || state.civInfo.religionManager.religion?.hasBelief(condition.params[0]) == true) + checkOnCiv { policies.isAdopted(condition.params[0]) || religionManager.religion?.hasBelief(condition.params[0]) == true } UniqueType.ConditionalBeforePolicyOrBelief -> - state.civInfo != null && !state.civInfo.policies.isAdopted(condition.params[0]) - && state.civInfo.religionManager.religion?.hasBelief(condition.params[0]) != true + checkOnCiv { !policies.isAdopted(condition.params[0]) && religionManager.religion?.hasBelief(condition.params[0]) != true } UniqueType.ConditionalBeforePantheon -> - state.civInfo != null && state.civInfo.religionManager.religionState == ReligionState.None + checkOnCiv { religionManager.religionState == ReligionState.None } UniqueType.ConditionalAfterPantheon -> - state.civInfo != null && state.civInfo.religionManager.religionState != ReligionState.None + checkOnCiv { religionManager.religionState != ReligionState.None } UniqueType.ConditionalBeforeReligion -> - state.civInfo != null && state.civInfo.religionManager.religionState < ReligionState.Religion + checkOnCiv { religionManager.religionState < ReligionState.Religion } UniqueType.ConditionalAfterReligion -> - state.civInfo != null && state.civInfo.religionManager.religionState >= ReligionState.Religion + checkOnCiv { religionManager.religionState >= ReligionState.Religion } UniqueType.ConditionalBeforeEnhancingReligion -> - state.civInfo != null && state.civInfo.religionManager.religionState < ReligionState.EnhancedReligion + checkOnCiv { religionManager.religionState < ReligionState.EnhancedReligion } UniqueType.ConditionalAfterEnhancingReligion -> - state.civInfo != null && state.civInfo.religionManager.religionState >= ReligionState.EnhancedReligion + checkOnCiv { religionManager.religionState >= ReligionState.EnhancedReligion } UniqueType.ConditionalBuildingBuilt -> - state.civInfo != null && state.civInfo.cities.any { it.cityConstructions.containsBuildingOrEquivalent(condition.params[0]) } + checkOnCiv { cities.any { it.cityConstructions.containsBuildingOrEquivalent(condition.params[0]) } } // Filtered via city.getMatchingUniques UniqueType.ConditionalInThisCity -> true + UniqueType.ConditionalWLTKD -> checkOnCity { isWeLoveTheKingDayActive() } UniqueType.ConditionalCityWithBuilding -> - state.city != null && state.city.cityConstructions.containsBuildingOrEquivalent(condition.params[0]) + checkOnCity { cityConstructions.containsBuildingOrEquivalent(condition.params[0]) } UniqueType.ConditionalCityWithoutBuilding -> - state.city != null && !state.city.cityConstructions.containsBuildingOrEquivalent(condition.params[0]) + checkOnCity { !cityConstructions.containsBuildingOrEquivalent(condition.params[0]) } UniqueType.ConditionalPopulationFilter -> - state.city != null && state.city.population.getPopulationFilterAmount(condition.params[1]) >= condition.params[0].toInt() + checkOnCity { population.getPopulationFilterAmount(condition.params[1]) >= condition.params[0].toInt() } UniqueType.ConditionalWhenGarrisoned -> - state.city != null && state.city.getCenterTile().militaryUnit != null && state.city.getCenterTile().militaryUnit!!.canGarrison() + checkOnCity { getCenterTile().militaryUnit?.canGarrison() == true } UniqueType.ConditionalVsCity -> state.theirCombatant?.matchesCategory("City") == true UniqueType.ConditionalVsUnits -> state.theirCombatant?.matchesCategory(condition.params[0]) == true @@ -301,12 +302,12 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s val theirCities = state.theirCombatant?.getCivInfo()?.cities?.size ?: 0 yourCities < theirCities } - UniqueType.ConditionalForeignContinent -> - state.civInfo != null && relevantTile != null - && (state.civInfo.cities.isEmpty() || state.civInfo.getCapital() == null - || state.civInfo.getCapital()!!.getCenterTile().getContinent() - != relevantTile!!.getContinent() + UniqueType.ConditionalForeignContinent -> checkOnCiv { + relevantTile != null && ( + cities.isEmpty() || getCapital() == null + || getCapital()!!.getCenterTile().getContinent() != relevantTile!!.getContinent() ) + } UniqueType.ConditionalAdjacentUnit -> state.civInfo != null && relevantUnit != null @@ -318,26 +319,33 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s } UniqueType.ConditionalNeighborTiles -> - relevantTile != null && - relevantTile!!.neighbors.count { - it.matchesFilter(condition.params[2], state.civInfo) - } in (condition.params[0].toInt())..(condition.params[1].toInt()) + relevantTile != null + && relevantTile!!.neighbors.count { + it.matchesFilter(condition.params[2], state.civInfo) + } in condition.params[0].toInt()..condition.params[1].toInt() UniqueType.ConditionalNeighborTilesAnd -> relevantTile != null && relevantTile!!.neighbors.count { it.matchesFilter(condition.params[2], state.civInfo) && it.matchesFilter(condition.params[3], state.civInfo) - } in (condition.params[0].toInt())..(condition.params[1].toInt()) + } in condition.params[0].toInt()..condition.params[1].toInt() UniqueType.ConditionalOnWaterMaps -> state.region?.continentID == -1 UniqueType.ConditionalInRegionOfType -> state.region?.type == condition.params[0] UniqueType.ConditionalInRegionExceptOfType -> state.region?.type != condition.params[0] - UniqueType.ConditionalFirstCivToResearch -> sourceObjectType == UniqueTarget.Tech - && state.civInfo != null - && state.civInfo.gameInfo.civilizations.none { - it != state.civInfo && it.isMajorCiv() && (it.tech.isResearched(sourceObjectName!!) || it.policies.isAdopted(sourceObjectName)) - } + UniqueType.ConditionalFirstCivToResearch -> + state.civInfo != null && sourceObjectType == UniqueTarget.Tech + && state.civInfo.gameInfo.civilizations.none { + it != state.civInfo && it.isMajorCiv() + && it.tech.isResearched(sourceObjectName!!) // guarded by the sourceObjectType check + } + UniqueType.ConditionalFirstCivToAdopt -> + state.civInfo != null && sourceObjectType == UniqueTarget.Policy + && state.civInfo.gameInfo.civilizations.none { + it != state.civInfo && it.isMajorCiv() + && it.policies.isAdopted(sourceObjectName!!) // guarded by the sourceObjectType check + } else -> false } @@ -432,9 +440,10 @@ class UniqueMap: HashMap>() { fun getAllUniques() = this.asSequence().flatMap { it.value.asSequence() } fun getTriggeredUniques(trigger: UniqueType, stateForConditionals: StateForConditionals): Sequence { - val result = getAllUniques().filter { it.conditionals.any { it.type == trigger } } - .filter { it.conditionalsApply(stateForConditionals) } - return result + return getAllUniques().filter { unique -> + unique.conditionals.any { it.type == trigger } + && unique.conditionalsApply(stateForConditionals) + } } } diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt index 8b90bc81d2..096b9ae71b 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt @@ -617,6 +617,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags: ConditionalTech("after discovering [tech]", UniqueTarget.Conditional), ConditionalNoTech("before discovering [tech]", UniqueTarget.Conditional), + ConditionalFirstCivToAdopt("if no other Civilization has adopted this", UniqueTarget.Conditional), ConditionalAfterPolicyOrBelief("after adopting [policy/belief]", UniqueTarget.Conditional), ConditionalBeforePolicyOrBelief("before adopting [policy/belief]", UniqueTarget.Conditional), diff --git a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt index 7af75fcaa6..45299a7c8d 100644 --- a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt @@ -660,10 +660,12 @@ class RulesetValidator(val ruleset: Ruleset) { checkUnitType(unit.unitType) { lines += "${unit.name} is of type ${unit.unitType}, which does not exist!" } - for (unique in unit.getMatchingUniques(UniqueType.ConstructImprovementInstantly)) { + + // We should ignore conditionals here - there are condition implementations on this out there that require a game state (and will test false without) + for (unique in unit.getMatchingUniques(UniqueType.ConstructImprovementInstantly, StateForConditionals.IgnoreConditionals)) { val improvementName = unique.params[0] - if (ruleset.tileImprovements[improvementName]==null) continue // this will be caught in the uniqueValidator.checkUniques - if ((ruleset.tileImprovements[improvementName] as Stats).none() && + if (ruleset.tileImprovements[improvementName] == null) continue // this will be caught in the uniqueValidator.checkUniques + if ((ruleset.tileImprovements[improvementName] as Stats).isEmpty() && unit.isCivilian() && !unit.isGreatPersonOfType("War")) { lines.add("${unit.name} can place improvement $improvementName which has no stats, preventing unit automation!", diff --git a/docs/Modders/uniques.md b/docs/Modders/uniques.md index 96de91a1c2..9756ca6cbf 100644 --- a/docs/Modders/uniques.md +++ b/docs/Modders/uniques.md @@ -1835,6 +1835,9 @@ Simple unique parameters are explained by mouseover. Complex parameters are expl Applicable to: Conditional +??? example "<if no other Civilization has adopted this>" + Applicable to: Conditional + ??? example "<after adopting [policy/belief]>" Example: "<after adopting [Oligarchy]>"