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

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<String, ArrayList<Unique>>() {
     fun getAllUniques() = this.asSequence().flatMap { it.value.asSequence() }
 
     fun getTriggeredUniques(trigger: UniqueType, stateForConditionals: StateForConditionals): Sequence<Unique> {
-        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)
+        }
     }
 }
 

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),
 

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!",

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]>"