RulesetValidator reorg - instead of running 2 sets of unique checks for base rulesets and filtering out results from the first to get the second, we now separate and have one function for non-base-rulesets, and one for base rulesets.

core/src/com/unciv/models/ruleset/Ruleset.kt

@@ -383,7 +383,7 @@ class Ruleset {
                                 Unique(it),
                                 false,
                                 cityStateType,
-                                UniqueType.UniqueParameterErrorSeverity.RulesetSpecific
+                                true
                             ).isEmpty()
                         })
                         allyBonusUniques = ArrayList(cityStateType.allyBonusUniques.filter {
@@ -391,7 +391,7 @@ class Ruleset {
                                 Unique(it),
                                 false,
                                 cityStateType,
-                                UniqueType.UniqueParameterErrorSeverity.RulesetSpecific
+                                true
                             ).isEmpty()
                         })
                     }

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

@@ -122,7 +122,7 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s
             val unique = Unique(it)
             val errors = UniqueValidator(ruleset).checkUnique(
                 unique, true, null,
-                UniqueType.UniqueParameterErrorSeverity.RulesetSpecific
+                true
             )
             errors.isEmpty()
         }

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

@@ -47,6 +47,13 @@ enum class UniqueParameterType(
         }
     },
 
+    Fraction("fraction", docExample = "0.5", "Indicates a fractional number, which can be negative"){
+        override fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? {
+            return if (parameterText.toFloatOrNull () == null) UniqueType.UniqueParameterErrorSeverity.RulesetInvariant
+            else null
+        }
+    },
+
     RelativeNumber("relativeAmount", "+20", "This indicates a number, usually with a + or - sign, such as `+25` (this kind of parameter is often followed by '%' which is nevertheless not part of the value)") {
         override fun getErrorSeverity(parameterText: String, ruleset: Ruleset):
                 UniqueType.UniqueParameterErrorSeverity? {

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

@@ -529,7 +529,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
     HasQuality("Considered [terrainQuality] when determining start locations", UniqueTarget.Terrain, flags = UniqueFlag.setOfHiddenToUsers),
 
     NoNaturalGeneration("Doesn't generate naturally", UniqueTarget.Terrain, UniqueTarget.Resource, flags = UniqueFlag.setOfHiddenToUsers),
-    TileGenerationConditions("Occurs at temperature between [amount] and [amount] and humidity between [amount] and [amount]", UniqueTarget.Terrain, UniqueTarget.Resource, flags = UniqueFlag.setOfHiddenToUsers),
+    TileGenerationConditions("Occurs at temperature between [fraction] and [amount] and humidity between [amount] and [amount]", UniqueTarget.Terrain, UniqueTarget.Resource, flags = UniqueFlag.setOfHiddenToUsers),
     OccursInChains("Occurs in chains at high elevations", UniqueTarget.Terrain, flags = UniqueFlag.setOfHiddenToUsers),
     OccursInGroups("Occurs in groups around high elevations", UniqueTarget.Terrain, flags = UniqueFlag.setOfHiddenToUsers),
     MajorStrategicFrequency("Every [amount] tiles with this terrain will receive a major deposit of a strategic resource.", UniqueTarget.Terrain, flags = UniqueFlag.setOfHiddenToUsers),
@@ -1174,8 +1174,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
         /** This is a warning, regardless of what ruleset we're in.
          * This is for filters that can also potentially accept free text, like UnitFilter and TileFilter */
         WarningOnly {
-            override fun getRulesetErrorSeverity(severityToReport: UniqueParameterErrorSeverity) =
+            override fun getRulesetErrorSeverity() = RulesetErrorSeverity.WarningOptionsOnly
-                RulesetErrorSeverity.WarningOptionsOnly
         },
 
         /** An error, but only because of other information in the current ruleset.
@@ -1184,15 +1183,13 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
         RulesetSpecific {
             // Report Warning on the first pass of RulesetValidator only, where mods are checked standalone
             // but upgrade to error when the econd pass asks, which runs only for combined or base rulesets.
-            override fun getRulesetErrorSeverity(severityToReport: UniqueParameterErrorSeverity) =
+            override fun getRulesetErrorSeverity() = RulesetErrorSeverity.Warning
-                RulesetErrorSeverity.Warning
         },
 
         /** An error, regardless of the ruleset we're in.
          * This is a problem like "numbers don't parse", "stat isn't stat", "city filter not applicable" */
         RulesetInvariant {
-            override fun getRulesetErrorSeverity(severityToReport: UniqueParameterErrorSeverity) =
+            override fun getRulesetErrorSeverity() = RulesetErrorSeverity.Error
-                RulesetErrorSeverity.Error
         },
         ;
 
@@ -1201,7 +1198,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
          *  first pass that also runs for extension mods without a base mixed in; the complex check
          *  runs with [severityToReport]==[RulesetSpecific].
          */
-        abstract fun getRulesetErrorSeverity(severityToReport: UniqueParameterErrorSeverity): RulesetErrorSeverity
+        abstract fun getRulesetErrorSeverity(): RulesetErrorSeverity
     }
 
     fun getDeprecationAnnotation(): Deprecated? = declaringJavaClass.getField(name)

core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt

@@ -8,8 +8,10 @@ import com.unciv.json.fromJsonFile
 import com.unciv.json.json
 import com.unciv.logic.map.tile.RoadStatus
 import com.unciv.models.metadata.BaseRuleset
+import com.unciv.models.ruleset.Building
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.RulesetCache
+import com.unciv.models.ruleset.nation.Nation
 import com.unciv.models.ruleset.nation.getContrastRatio
 import com.unciv.models.ruleset.nation.getRelativeLuminance
 import com.unciv.models.ruleset.tile.TerrainType
@@ -23,66 +25,75 @@ import com.unciv.models.tilesets.TileSetConfig
 
 class RulesetValidator(val ruleset: Ruleset) {
 
-    val uniqueValidator = UniqueValidator(ruleset)
+    private val uniqueValidator = UniqueValidator(ruleset)
 
     fun getErrorList(tryFixUnknownUniques: Boolean = false): RulesetErrorList {
+        // When no base ruleset is loaded - references cannot be checked
+        if (!ruleset.modOptions.isBaseRuleset) return getNonBaseRulesetErrorList(tryFixUnknownUniques)
+
+        return getBaseRulesetErrorList(tryFixUnknownUniques)
+    }
+
+    private fun getNonBaseRulesetErrorList(tryFixUnknownUniques: Boolean): RulesetErrorList {
         val lines = RulesetErrorList()
 
-        /********************** Ruleset Invariant Part **********************/
-        // Checks for ALL MODS - only those that can succeed without loading a base ruleset
         // When not checking the entire ruleset, we can only really detect ruleset-invariant errors in uniques
-
+        uniqueValidator.checkUniques(ruleset.globalUniques, lines, false, tryFixUnknownUniques)
-        val rulesetInvariant = UniqueType.UniqueParameterErrorSeverity.RulesetInvariant
+        addUnitErrorsRulesetInvariant(lines, tryFixUnknownUniques)
-        val rulesetSpecific = UniqueType.UniqueParameterErrorSeverity.RulesetSpecific
+        addTechErrorsRulesetInvariant(lines, tryFixUnknownUniques)
-
-        uniqueValidator.checkUniques(ruleset.globalUniques, lines, rulesetInvariant, tryFixUnknownUniques)
-        addUnitErrorsRulesetInvariant(lines, rulesetInvariant, tryFixUnknownUniques)
-        addTechErrorsRulesetInvariant(lines, rulesetInvariant, tryFixUnknownUniques)
         addTechColumnErrorsRulesetInvariant(lines)
-        addBuildingErrorsRulesetInvariant(lines, rulesetInvariant, tryFixUnknownUniques)
+        addBuildingErrorsRulesetInvariant(lines, tryFixUnknownUniques)
-        addNationErrorsRulesetInvariant(lines, rulesetInvariant, tryFixUnknownUniques)
+        addNationErrorsRulesetInvariant(lines, tryFixUnknownUniques)
-        addPromotionErrorsRulesetInvariant(lines, rulesetInvariant, tryFixUnknownUniques)
+        addPromotionErrorsRulesetInvariant(lines, tryFixUnknownUniques)
-        addResourceErrorsRulesetInvariant(lines, rulesetInvariant, tryFixUnknownUniques)
+        addResourceErrorsRulesetInvariant(lines, tryFixUnknownUniques)
 
-        /********************** Tileset tests **********************/
+        /**********************  **********************/
         // e.g. json configs complete and parseable
+        // Check for mod or Civ_V_GnK to avoid running the same test twice (~200ms for the builtin assets)
+        if (ruleset.folderLocation != null) {
+            checkTilesetSanity(lines)
+        }
+
+        return lines
+    }
+
+
+    private fun getBaseRulesetErrorList(tryFixUnknownUniques: Boolean): RulesetErrorList{
+
+        val lines = RulesetErrorList()
+
+        uniqueValidator.checkUniques(ruleset.globalUniques, lines, true, tryFixUnknownUniques)
+
+        addUnitErrorsBaseRuleset(lines, tryFixUnknownUniques)
+        addBuildingErrors(lines, tryFixUnknownUniques)
+        addSpecialistErrors(lines)
+        addResourceErrors(lines, tryFixUnknownUniques)
+        addImprovementErrors(lines, tryFixUnknownUniques)
+        addTerrainErrors(lines, tryFixUnknownUniques)
+        addTechErrors(lines, tryFixUnknownUniques)
+        addTechColumnErrorsRulesetInvariant(lines)
+        addEraErrors(lines, tryFixUnknownUniques)
+        addSpeedErrors(lines)
+        addBeliefErrors(lines, tryFixUnknownUniques)
+        addNationErrors(lines, tryFixUnknownUniques)
+        addPolicyErrors(lines, tryFixUnknownUniques)
+        addRuinsErrors(lines, tryFixUnknownUniques)
+        addPromotionErrors(lines, tryFixUnknownUniques)
+        addUnitTypeErrors(lines, tryFixUnknownUniques)
+        addVictoryTypeErrors(lines)
+        addDifficutlyErrors(lines)
+        addCityStateTypeErrors(tryFixUnknownUniques, lines)
+
         // Check for mod or Civ_V_GnK to avoid running the same test twice (~200ms for the builtin assets)
         if (ruleset.folderLocation != null || ruleset.name == BaseRuleset.Civ_V_GnK.fullName) {
             checkTilesetSanity(lines)
         }
 
-        // Quit here when no base ruleset is loaded - references cannot be checked
-        if (!ruleset.modOptions.isBaseRuleset) return lines
-
-        /********************** Ruleset Specific Part **********************/
-
-        uniqueValidator.checkUniques(ruleset.globalUniques, lines, rulesetSpecific, tryFixUnknownUniques)
-
-        addUnitErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addBuildingErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addSpecialistErrors(lines)
-        addResourceErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addImprovementErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addTerrainErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addTechErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addEraErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addSpeedErrors(lines)
-        addBeliefErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addNationErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addPolicyErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addRuinsErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addPromotionErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addUnitTypeErrors(lines, rulesetSpecific, tryFixUnknownUniques)
-        addVictoryTypeErrors(lines)
-        addDifficutlyErrors(lines)
-        addCityStateTypeErrors(tryFixUnknownUniques, rulesetSpecific, lines)
-
         return lines
     }
 
     private fun addCityStateTypeErrors(
         tryFixUnknownUniques: Boolean,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         lines: RulesetErrorList
     ) {
         for (cityStateType in ruleset.cityStateTypes.values) {
@@ -91,7 +102,7 @@ class RulesetValidator(val ruleset: Ruleset) {
                     unique,
                     tryFixUnknownUniques,
                     cityStateType,
-                    rulesetSpecific
+                    true
                 )
                 lines.addAll(errors)
             }
@@ -131,20 +142,20 @@ class RulesetValidator(val ruleset: Ruleset) {
 
     private fun addUnitTypeErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (unitType in ruleset.unitTypes.values) {
-            uniqueValidator.checkUniques(unitType, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(unitType, lines, true, tryFixUnknownUniques)
         }
     }
 
     private fun addPromotionErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (promotion in ruleset.unitPromotions.values) {
+            addPromotionErrorRulesetInvariant(promotion, lines)
+
             // These are warning as of 3.17.5 to not break existing mods and give them time to correct, should be upgraded to error in the future
             for (prereq in promotion.prerequisites)
                 if (!ruleset.unitPromotions.containsKey(prereq))
@@ -158,27 +169,25 @@ class RulesetValidator(val ruleset: Ruleset) {
                     RulesetErrorSeverity.Warning
                 )
             }
-            uniqueValidator.checkUniques(promotion, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(promotion, lines, true, tryFixUnknownUniques)
         }
         checkPromotionCircularReferences(lines)
     }
 
     private fun addRuinsErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (reward in ruleset.ruinRewards.values) {
             for (difficulty in reward.excludedDifficulties)
                 if (!ruleset.difficulties.containsKey(difficulty))
                     lines += "${reward.name} references difficulty ${difficulty}, which does not exist!"
-            uniqueValidator.checkUniques(reward, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(reward, lines, true, tryFixUnknownUniques)
         }
     }
 
     private fun addPolicyErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (policy in ruleset.policies.values) {
@@ -186,7 +195,7 @@ class RulesetValidator(val ruleset: Ruleset) {
                 for (prereq in policy.requires!!)
                     if (!ruleset.policies.containsKey(prereq))
                         lines += "${policy.name} requires policy $prereq which does not exist!"
-            uniqueValidator.checkUniques(policy, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(policy, lines, true, tryFixUnknownUniques)
         }
 
         for (branch in ruleset.policyBranches.values)
@@ -202,11 +211,12 @@ class RulesetValidator(val ruleset: Ruleset) {
 
     private fun addNationErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (nation in ruleset.nations.values) {
-            uniqueValidator.checkUniques(nation, lines, rulesetSpecific, tryFixUnknownUniques)
+            addNationErrorRulesetInvariant(nation, lines)
+
+            uniqueValidator.checkUniques(nation, lines, true, tryFixUnknownUniques)
 
             if (nation.cityStateType != null && nation.cityStateType !in ruleset.cityStateTypes)
                 lines += "${nation.name} is of city-state type ${nation.cityStateType} which does not exist!"
@@ -217,11 +227,10 @@ class RulesetValidator(val ruleset: Ruleset) {
 
     private fun addBeliefErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (belief in ruleset.beliefs.values) {
-            uniqueValidator.checkUniques(belief, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(belief, lines, true, tryFixUnknownUniques)
         }
     }
 
@@ -236,7 +245,6 @@ class RulesetValidator(val ruleset: Ruleset) {
 
     private fun addEraErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         if (ruleset.eras.isEmpty()) {
@@ -288,13 +296,12 @@ class RulesetValidator(val ruleset: Ruleset) {
                     RulesetErrorSeverity.WarningOptionsOnly
                 )
 
-            uniqueValidator.checkUniques(era, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(era, lines, true, tryFixUnknownUniques)
         }
     }
 
     private fun addTechErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (tech in ruleset.technologies.values) {
@@ -316,13 +323,12 @@ class RulesetValidator(val ruleset: Ruleset) {
             }
             if (tech.era() !in ruleset.eras)
                 lines += "Unknown era ${tech.era()} referenced in column of tech ${tech.name}"
-            uniqueValidator.checkUniques(tech, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(tech, lines, true, tryFixUnknownUniques)
         }
     }
 
     private fun addTerrainErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         if (ruleset.terrains.values.none { it.type == TerrainType.Land && !it.impassable })
@@ -345,13 +351,12 @@ class RulesetValidator(val ruleset: Ruleset) {
                     // See https://github.com/hackedpassword/Z2/blob/main/HybridTileTech.md for a clever exploit
                     lines.add("${terrain.name} turns into terrain ${terrain.turnsInto} which is not a base terrain!", RulesetErrorSeverity.Warning)
             }
-            uniqueValidator.checkUniques(terrain, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(terrain, lines, true, tryFixUnknownUniques)
         }
     }
 
     private fun addImprovementErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (improvement in ruleset.tileImprovements.values) {
@@ -400,13 +405,12 @@ class RulesetValidator(val ruleset: Ruleset) {
                     RulesetErrorSeverity.Warning
                 )
             }
-            uniqueValidator.checkUniques(improvement, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(improvement, lines, true, tryFixUnknownUniques)
         }
     }
 
     private fun addResourceErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (resource in ruleset.tileResources.values) {
@@ -420,7 +424,7 @@ class RulesetValidator(val ruleset: Ruleset) {
             for (terrain in resource.terrainsCanBeFoundOn)
                 if (!ruleset.terrains.containsKey(terrain))
                     lines += "${resource.name} can be found on terrain $terrain which does not exist!"
-            uniqueValidator.checkUniques(resource, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(resource, lines, true, tryFixUnknownUniques)
         }
     }
 
@@ -437,13 +441,13 @@ class RulesetValidator(val ruleset: Ruleset) {
 
     private fun addBuildingErrors(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (building in ruleset.buildings.values) {
+            addBuildingErrorRulesetInvariant(building, lines)
+
             if (building.requiredTech != null && !ruleset.technologies.containsKey(building.requiredTech!!))
                 lines += "${building.name} requires tech ${building.requiredTech} which does not exist!"
-
             for (specialistName in building.specialistSlots.keys)
                 if (!ruleset.specialists.containsKey(specialistName))
                     lines += "${building.name} provides specialist $specialistName which does not exist!"
@@ -454,127 +458,147 @@ class RulesetValidator(val ruleset: Ruleset) {
                 lines += "${building.name} replaces ${building.replaces} which does not exist!"
             if (building.requiredBuilding != null && !ruleset.buildings.containsKey(building.requiredBuilding!!))
                 lines += "${building.name} requires ${building.requiredBuilding} which does not exist!"
-            uniqueValidator.checkUniques(building, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(building, lines, true, tryFixUnknownUniques)
         }
     }
 
-    private fun addUnitErrors(
+
+    private fun addUnitErrorsRulesetInvariant(
+        lines: RulesetErrorList,
+        tryFixUnknownUniques: Boolean
+    ) {
+        for (unit in ruleset.units.values) {
+            checkUnitRulesetInvariant(unit, lines)
+            uniqueValidator.checkUniques(unit, lines, false, tryFixUnknownUniques)
+        }
+    }
+
+    private fun addUnitErrorsBaseRuleset(
         lines: RulesetErrorList,
-        rulesetSpecific: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         if (ruleset.units.values.none { it.isCityFounder() })
             lines += "No city-founding units in ruleset!"
 
         for (unit in ruleset.units.values) {
+            checkUnitRulesetInvariant(unit, lines)
             checkUnitRulesetSpecific(unit, lines)
-            uniqueValidator.checkUniques(unit, lines, rulesetSpecific, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(unit, lines, false, tryFixUnknownUniques)
         }
     }
 
     private fun addResourceErrorsRulesetInvariant(
         lines: RulesetErrorList,
-        rulesetInvariant: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (resource in ruleset.tileResources.values) {
-            uniqueValidator.checkUniques(resource, lines, rulesetInvariant, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(resource, lines, false, tryFixUnknownUniques)
         }
     }
 
     private fun addPromotionErrorsRulesetInvariant(
         lines: RulesetErrorList,
-        rulesetInvariant: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (promotion in ruleset.unitPromotions.values) {
-            uniqueValidator.checkUniques(promotion, lines, rulesetInvariant, tryFixUnknownUniques)
+            uniqueValidator.checkUniques(promotion, lines, false, tryFixUnknownUniques)
-            if (promotion.row < -1) lines += "Promotion ${promotion.name} has invalid row value: ${promotion.row}"
+
-            if (promotion.column < 0) lines += "Promotion ${promotion.name} has invalid column value: ${promotion.column}"
+            addPromotionErrorRulesetInvariant(promotion, lines)
-            if (promotion.row == -1) continue
-            for (otherPromotion in ruleset.unitPromotions.values)
-                if (promotion != otherPromotion && promotion.column == otherPromotion.column && promotion.row == otherPromotion.row)
-                    lines += "Promotions ${promotion.name} and ${otherPromotion.name} have the same position: ${promotion.row}/${promotion.column}"
         }
     }
 
+    private fun addPromotionErrorRulesetInvariant(promotion: Promotion, lines: RulesetErrorList) {
+        if (promotion.row < -1) lines += "Promotion ${promotion.name} has invalid row value: ${promotion.row}"
+        if (promotion.column < 0) lines += "Promotion ${promotion.name} has invalid column value: ${promotion.column}"
+        if (promotion.row == -1) return
+        for (otherPromotion in ruleset.unitPromotions.values)
+            if (promotion != otherPromotion && promotion.column == otherPromotion.column && promotion.row == otherPromotion.row)
+                lines += "Promotions ${promotion.name} and ${otherPromotion.name} have the same position: ${promotion.row}/${promotion.column}"
+    }
+
     private fun addNationErrorsRulesetInvariant(
         lines: RulesetErrorList,
-        rulesetInvariant: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (nation in ruleset.nations.values) {
-            if (nation.cities.isEmpty() && !nation.isSpectator && !nation.isBarbarian) {
+            addNationErrorRulesetInvariant(nation, lines)
-                lines += "${nation.name} can settle cities, but has no city names!"
+
+            uniqueValidator.checkUniques(nation, lines, false, tryFixUnknownUniques)
+        }
+    }
+
+    private fun addNationErrorRulesetInvariant(nation: Nation, lines: RulesetErrorList) {
+        if (nation.cities.isEmpty() && !nation.isSpectator && !nation.isBarbarian) {
+            lines += "${nation.name} can settle cities, but has no city names!"
+        }
+
+        // https://www.w3.org/TR/WCAG20/#visual-audio-contrast-contrast
+        val constrastRatio = nation.getContrastRatio()
+        if (constrastRatio < 3) {
+            val innerColorLuminance = getRelativeLuminance(nation.getInnerColor())
+            val outerColorLuminance = getRelativeLuminance(nation.getOuterColor())
+
+            val innerLerpColor: Color
+            val outerLerpColor: Color
+
+            if (innerColorLuminance > outerColorLuminance) { // inner is brighter
+                innerLerpColor = Color.WHITE
+                outerLerpColor = Color.BLACK
+            } else {
+                innerLerpColor = Color.BLACK
+                outerLerpColor = Color.WHITE
             }
 
-            // https://www.w3.org/TR/WCAG20/#visual-audio-contrast-contrast
+            var text = "${nation.name}'s colors do not contrast enough - it is unreadable!"
-            val constrastRatio = nation.getContrastRatio()
-            if (constrastRatio < 3) {
-                val innerColorLuminance = getRelativeLuminance(nation.getInnerColor())
-                val outerColorLuminance = getRelativeLuminance(nation.getOuterColor())
 
-                val innerLerpColor: Color
+            for (i in 1..10) {
-                val outerLerpColor: Color
+                val newInnerColor = nation.getInnerColor().cpy().lerp(innerLerpColor, 0.05f * i)
+                val newOuterColor = nation.getOuterColor().cpy().lerp(outerLerpColor, 0.05f * i)
 
-                if (innerColorLuminance > outerColorLuminance) { // inner is brighter
+                if (getContrastRatio(newInnerColor, newOuterColor) > 3) {
-                    innerLerpColor = Color.WHITE
+                    text += "\nSuggested colors: "
-                    outerLerpColor = Color.BLACK
+                    text += "\n\t\t\"outerColor\": [${(newOuterColor.r * 255).toInt()}, ${(newOuterColor.g * 255).toInt()}, ${(newOuterColor.b * 255).toInt()}],"
-                } else {
+                    text += "\n\t\t\"innerColor\": [${(newInnerColor.r * 255).toInt()}, ${(newInnerColor.g * 255).toInt()}, ${(newInnerColor.b * 255).toInt()}],"
-                    innerLerpColor = Color.BLACK
+                    break
-                    outerLerpColor = Color.WHITE
                 }
-
-                var text = "${nation.name}'s colors do not contrast enough - it is unreadable!"
-
-                for (i in 1..10) {
-                    val newInnerColor = nation.getInnerColor().cpy().lerp(innerLerpColor, 0.05f * i)
-                    val newOuterColor = nation.getOuterColor().cpy().lerp(outerLerpColor, 0.05f * i)
-
-                    if (getContrastRatio(newInnerColor, newOuterColor) > 3) {
-                        text += "\nSuggested colors: "
-                        text += "\n\t\t\"outerColor\": [${(newOuterColor.r * 255).toInt()}, ${(newOuterColor.g * 255).toInt()}, ${(newOuterColor.b * 255).toInt()}],"
-                        text += "\n\t\t\"innerColor\": [${(newInnerColor.r * 255).toInt()}, ${(newInnerColor.g * 255).toInt()}, ${(newInnerColor.b * 255).toInt()}],"
-                        break
-                    }
-                }
-
-                lines.add(
-                    text, RulesetErrorSeverity.WarningOptionsOnly
-                )
             }
 
-            uniqueValidator.checkUniques(nation, lines, rulesetInvariant, tryFixUnknownUniques)
+            lines.add(
+                text, RulesetErrorSeverity.WarningOptionsOnly
+            )
         }
     }
 
     private fun addBuildingErrorsRulesetInvariant(
         lines: RulesetErrorList,
-        rulesetInvariant: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
         for (building in ruleset.buildings.values) {
-            if (building.requiredTech == null && building.cost == -1 && !building.hasUnique(
+            addBuildingErrorRulesetInvariant(building, lines)
-                    UniqueType.Unbuildable
-                )
-            )
-                lines.add(
-                    "${building.name} is buildable and therefore should either have an explicit cost or reference an existing tech!",
-                    RulesetErrorSeverity.Warning
-                )
 
-            for (gpp in building.greatPersonPoints)
+            uniqueValidator.checkUniques(building, lines, false, tryFixUnknownUniques)
-                if (gpp.key !in ruleset.units)
-                    lines.add(
-                        "Building ${building.name} has greatPersonPoints for ${gpp.key}, which is not a unit in the ruleset!",
-                        RulesetErrorSeverity.Warning
-                    )
-
-            uniqueValidator.checkUniques(building, lines, rulesetInvariant, tryFixUnknownUniques)
 
         }
     }
 
+    private fun addBuildingErrorRulesetInvariant(building: Building, lines: RulesetErrorList) {
+        if (building.requiredTech == null && building.cost == -1 && !building.hasUnique(
+                UniqueType.Unbuildable
+            )
+        )
+            lines.add(
+                "${building.name} is buildable and therefore should either have an explicit cost or reference an existing tech!",
+                RulesetErrorSeverity.Warning
+            )
+
+        for (gpp in building.greatPersonPoints)
+            if (gpp.key !in ruleset.units)
+                lines.add(
+                    "Building ${building.name} has greatPersonPoints for ${gpp.key}, which is not a unit in the ruleset!",
+                    RulesetErrorSeverity.Warning
+                )
+    }
+
     private fun addTechColumnErrorsRulesetInvariant(lines: RulesetErrorList) {
         for (techColumn in ruleset.techColumns) {
             if (techColumn.columnNumber < 0)
@@ -590,40 +614,25 @@ class RulesetValidator(val ruleset: Ruleset) {
                     RulesetErrorSeverity.Warning
                 )
         }
-    }
 
-    private fun addTechErrorsRulesetInvariant(
-        lines: RulesetErrorList,
-        rulesetInvariant: UniqueType.UniqueParameterErrorSeverity,
-        tryFixUnknownUniques: Boolean
-    ) {
         for (tech in ruleset.technologies.values) {
             for (otherTech in ruleset.technologies.values) {
                 if (tech != otherTech && otherTech.column?.columnNumber == tech.column?.columnNumber && otherTech.row == tech.row)
                     lines += "${tech.name} is in the same row and column as ${otherTech.name}!"
             }
-
-            uniqueValidator.checkUniques(tech, lines, rulesetInvariant, tryFixUnknownUniques)
         }
     }
 
-    private fun addUnitErrorsRulesetInvariant(
+    private fun addTechErrorsRulesetInvariant(
         lines: RulesetErrorList,
-        rulesetInvariant: UniqueType.UniqueParameterErrorSeverity,
         tryFixUnknownUniques: Boolean
     ) {
-        for (unit in ruleset.units.values) {
+        for (tech in ruleset.technologies.values) {
-            if (unit.upgradesTo == unit.name || (unit.upgradesTo != null && unit.upgradesTo == unit.replaces))
+            uniqueValidator.checkUniques(tech, lines, false, tryFixUnknownUniques)
-                lines += "${unit.name} upgrades to itself!"
-            if (!unit.isCivilian() && unit.strength == 0)
-                lines += "${unit.name} is a military unit but has no assigned strength!"
-            if (unit.isRanged() && unit.rangedStrength == 0 && !unit.hasUnique(UniqueType.CannotAttack))
-                lines += "${unit.name} is a ranged unit but has no assigned rangedStrength!"
-
-            uniqueValidator.checkUniques(unit, lines, rulesetInvariant, tryFixUnknownUniques)
         }
     }
 
+
     /** Collects known technology prerequisite paths: key is the technology name,
      *  value a Set of its prerequisites including indirect ones */
     private val prereqsHashMap = HashMap<String, HashSet<String>>()
@@ -640,6 +649,15 @@ class RulesetValidator(val ruleset: Ruleset) {
         return techHashSet
     }
 
+    private fun checkUnitRulesetInvariant(unit: BaseUnit, lines: RulesetErrorList) {
+        if (unit.upgradesTo == unit.name || (unit.upgradesTo != null && unit.upgradesTo == unit.replaces))
+            lines += "${unit.name} upgrades to itself!"
+        if (!unit.isCivilian() && unit.strength == 0)
+            lines += "${unit.name} is a military unit but has no assigned strength!"
+        if (unit.isRanged() && unit.rangedStrength == 0 && !unit.hasUnique(UniqueType.CannotAttack))
+            lines += "${unit.name} is a ranged unit but has no assigned rangedStrength!"
+    }
+
     /** Collects all RulesetSpecific checks for a BaseUnit */
     private fun checkUnitRulesetSpecific(unit: BaseUnit, lines: RulesetErrorList) {
         if (unit.requiredTech != null && !ruleset.technologies.containsKey(unit.requiredTech!!))

core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt

@@ -16,7 +16,7 @@ class UniqueValidator(val ruleset: Ruleset) {
     fun checkUniques(
         uniqueContainer: IHasUniques,
         lines: RulesetErrorList,
-        severityToReport: UniqueType.UniqueParameterErrorSeverity,
+        reportRulesetSpecificErrors: Boolean,
         tryFixUnknownUniques: Boolean
     ) {
         for (unique in uniqueContainer.uniqueObjects) {
@@ -24,7 +24,7 @@ class UniqueValidator(val ruleset: Ruleset) {
                 unique,
                 tryFixUnknownUniques,
                 uniqueContainer as? INamed,
-                severityToReport
+                reportRulesetSpecificErrors
             )
             lines.addAll(errors)
         }
@@ -34,7 +34,7 @@ class UniqueValidator(val ruleset: Ruleset) {
         unique: Unique,
         tryFixUnknownUniques: Boolean,
         namedObj: INamed?,
-        severityToReport: UniqueType.UniqueParameterErrorSeverity
+        reportRulesetSpecificErrors: Boolean
     ): List<RulesetError> {
         val prefix by lazy { (if (namedObj is IRulesetObject) "${namedObj.originRuleset}: " else "") +
             (if (namedObj == null) "The" else "${namedObj.name}'s") }
@@ -47,25 +47,25 @@ class UniqueValidator(val ruleset: Ruleset) {
 
         val typeComplianceErrors = getComplianceErrors(unique)
         for (complianceError in typeComplianceErrors) {
-            if (complianceError.errorSeverity <= severityToReport)
+            if (!reportRulesetSpecificErrors && complianceError.errorSeverity == UniqueType.UniqueParameterErrorSeverity.RulesetSpecific)
-                rulesetErrors.add(RulesetError("$prefix unique \"${unique.text}\" contains parameter ${complianceError.parameterName}," +
+                continue
-                    " which does not fit parameter type" +
+
-                    " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !",
+            rulesetErrors.add(RulesetError("$prefix unique \"${unique.text}\" contains parameter ${complianceError.parameterName}," +
-                    complianceError.errorSeverity.getRulesetErrorSeverity(severityToReport)
+                " which does not fit parameter type" +
-                ))
+                " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !",
+                complianceError.errorSeverity.getRulesetErrorSeverity()
+            ))
         }
 
         for (conditional in unique.conditionals) {
-            addConditionalErrors(conditional, rulesetErrors, prefix, unique, severityToReport)
+            addConditionalErrors(conditional, rulesetErrors, prefix, unique, reportRulesetSpecificErrors)
         }
 
 
-        if (severityToReport != UniqueType.UniqueParameterErrorSeverity.RulesetSpecific)
+        if (reportRulesetSpecificErrors)
         // If we don't filter these messages will be listed twice as this function is called twice on most objects
         // The tests are RulesetInvariant in nature, but RulesetSpecific is called for _all_ objects, invariant is not.
-            return rulesetErrors
+            addDeprecationAnnotationErrors(unique, prefix, rulesetErrors)
-
-        addDeprecationAnnotationErrors(unique, prefix, rulesetErrors)
 
         return rulesetErrors
     }
@@ -75,7 +75,7 @@ class UniqueValidator(val ruleset: Ruleset) {
         rulesetErrors: RulesetErrorList,
         prefix: String,
         unique: Unique,
-        severityToReport: UniqueType.UniqueParameterErrorSeverity
+        reportRulesetSpecificErrors: Boolean
     ) {
         if (conditional.type == null) {
             rulesetErrors.add(
@@ -104,16 +104,19 @@ class UniqueValidator(val ruleset: Ruleset) {
 
         val conditionalComplianceErrors =
             getComplianceErrors(conditional)
+
         for (complianceError in conditionalComplianceErrors) {
-            if (complianceError.errorSeverity == severityToReport)
+            if (!reportRulesetSpecificErrors && complianceError.errorSeverity == UniqueType.UniqueParameterErrorSeverity.RulesetSpecific)
-                rulesetErrors.add(
+                continue
-                    RulesetError(
+
-                        "$prefix unique \"${unique.text}\" contains the conditional \"${conditional.text}\"." +
+            rulesetErrors.add(
-                            " This contains the parameter ${complianceError.parameterName} which does not fit parameter type" +
+                RulesetError(
-                            " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !",
+                    "$prefix unique \"${unique.text}\" contains the conditional \"${conditional.text}\"." +
-                        complianceError.errorSeverity.getRulesetErrorSeverity(severityToReport)
+                        " This contains the parameter ${complianceError.parameterName} which does not fit parameter type" +
-                    )
+                        " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !",
+                    complianceError.errorSeverity.getRulesetErrorSeverity()
                 )
+            )
         }
     }
 

core/src/com/unciv/ui/popups/options/ModCheckTab.kt

@@ -8,19 +8,18 @@ import com.badlogic.gdx.utils.Align
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.ruleset.unique.Unique
-import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.ruleset.validation.RulesetError
 import com.unciv.models.ruleset.validation.RulesetErrorSeverity
 import com.unciv.models.ruleset.validation.UniqueValidator
 import com.unciv.models.translations.tr
-import com.unciv.ui.components.widgets.ExpanderTab
-import com.unciv.ui.components.widgets.TabbedPager
-import com.unciv.ui.components.widgets.TranslatedSelectBox
 import com.unciv.ui.components.extensions.surroundWithCircle
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
 import com.unciv.ui.components.input.onChange
 import com.unciv.ui.components.input.onClick
+import com.unciv.ui.components.widgets.ExpanderTab
+import com.unciv.ui.components.widgets.TabbedPager
+import com.unciv.ui.components.widgets.TranslatedSelectBox
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.popups.ToastPopup
 import com.unciv.ui.screens.basescreen.BaseScreen
@@ -75,7 +74,7 @@ class ModCheckTab(
         runAction()
     }
 
-    fun runModChecker(base: String = MOD_CHECK_WITHOUT_BASE) {
+    private fun runModChecker(base: String = MOD_CHECK_WITHOUT_BASE) {
 
         modCheckFirstRun = false
         if (modCheckBaseSelect == null) return
@@ -204,7 +203,7 @@ class ModCheckTab(
                 replacementUnique,
                 false,
                 null,
-                UniqueType.UniqueParameterErrorSeverity.RulesetInvariant
+                true
             )
             for (error in modInvariantErrors)
                 Log.error("ModInvariantError: %s - %s", error.text, error.errorSeverityToReport)
@@ -215,7 +214,7 @@ class ModCheckTab(
                     replacementUnique,
                     false,
                     null,
-                    UniqueType.UniqueParameterErrorSeverity.RulesetInvariant
+                    true
                 )
                 for (error in modSpecificErrors)
                     Log.error("ModSpecificError: %s - %s", error.text, error.errorSeverityToReport)