Ruleset check finds errors in uniques of all kinds!

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

@@ -299,11 +299,29 @@ class Ruleset {
         fun isNotOK() = status != CheckModLinksStatus.OK
     }
 
+    fun checkUniques(uniqueContainer:IHasUniques, lines:ArrayList<String>,
+                     severityToReport: UniqueType.UniqueComplianceErrorSeverity) {
+        val name = if (uniqueContainer is INamed) uniqueContainer.name else ""
+
+        for (unique in uniqueContainer.uniqueObjects) {
+            if (unique.type == null) continue
+            val complianceErrors = unique.type.getComplianceErrors(unique, this)
+            for (complianceError in complianceErrors) {
+                if (complianceError.errorSeverity ==  severityToReport)
+                    lines += "$name's unique \"${unique.text}\" contains parameter ${complianceError.parameterName}," +
+                            " which does not fit parameter type" +
+                            " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !"
+            }
+        }
+    }
+
     fun checkModLinks(): CheckModLinksResult {
         val lines = ArrayList<String>()
         var warningCount = 0
 
         // 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
+
         for (unit in units.values) {
             if (unit.upgradesTo == unit.name)
                 lines += "${unit.name} upgrades to itself!"
@@ -311,6 +329,8 @@ class Ruleset {
                 lines += "${unit.name} is a military unit but has no assigned strength!"
             if (unit.isRanged() && unit.rangedStrength == 0 && "Cannot attack" !in unit.uniques)
                 lines += "${unit.name} is a ranged unit but has no assigned rangedStrength!"
+
+            checkUniques(unit, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetInvariant)
         }
 
         for (tech in technologies.values) {
@@ -318,23 +338,15 @@ class Ruleset {
                 if (tech != otherTech && otherTech.column == tech.column && otherTech.row == tech.row)
                     lines += "${tech.name} is in the same row as ${otherTech.name}!"
             }
+
+            checkUniques(tech, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetInvariant)
         }
 
         for (building in buildings.values) {
             if (building.requiredTech == null && building.cost == 0 && !building.uniques.contains("Unbuildable"))
                 lines += "${building.name} is buildable and therefore must either have an explicit cost or reference an existing tech!"
 
-            for (unique in building.uniqueObjects) {
+            checkUniques(building, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetInvariant)
-                if (unique.type == null) continue
-                val complianceErrors = unique.type.getComplianceErrors(unique, this)
-                for (complianceError in complianceErrors) {
-                    // When not checking the entire ruleset, we can only really detect ruleset-invariant errors
-                    if (complianceError.errorSeverity == UniqueType.UniqueComplianceErrorSeverity.RulesetInvariant)
-                        lines += "${building.name}'s unique \"${unique.text}\" contains parameter ${complianceError.parameterName}," +
-                                " which does not fit parameter type" +
-                                " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !"
-                }
-            }
 
         }
         
@@ -342,6 +354,8 @@ class Ruleset {
             if (nation.cities.isEmpty() && !nation.isSpectator() && !nation.isBarbarian()) {
                 lines += "${nation.name} can settle cities, but has no city names!"
             }
+
+            checkUniques(nation, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetInvariant)
         }
 
         // Quit here when no base ruleset is loaded - references cannot be checked
@@ -377,6 +391,8 @@ class Ruleset {
                     warningCount++
                 }
             }
+
+            checkUniques(unit, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific)
         }
 
         for (building in buildings.values) {
@@ -394,6 +410,7 @@ class Ruleset {
             for (unique in building.uniqueObjects)
                 if (unique.placeholderText == "Creates a [] improvement on a specific tile" && !tileImprovements.containsKey(unique.params[0]))
                     lines += "${building.name} creates a ${unique.params[0]} improvement which does not exist!"
+            checkUniques(building, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific)
         }
 
         for (resource in tileResources.values) {
@@ -404,6 +421,7 @@ class Ruleset {
             for (terrain in resource.terrainsCanBeFoundOn)
                 if (!terrains.containsKey(terrain))
                     lines += "${resource.name} can be found on terrain $terrain which does not exist!"
+            checkUniques(resource, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific)
         }
 
         for (improvement in tileImprovements.values) {
@@ -412,12 +430,14 @@ class Ruleset {
             for (terrain in improvement.terrainsCanBeBuiltOn)
                 if (!terrains.containsKey(terrain))
                     lines += "${improvement.name} can be built on terrain $terrain which does not exist!"
+            checkUniques(improvement, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific)
         }
 
         for (terrain in terrains.values) {
             for (baseTerrain in terrain.occursOn)
                 if (!terrains.containsKey(baseTerrain))
                     lines += "${terrain.name} occurs on terrain $baseTerrain which does not exist!"
+            checkUniques(terrain, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific)
         }
 
         val prereqsHashMap = HashMap<String,HashSet<String>>()
@@ -446,6 +466,7 @@ class Ruleset {
             }
             if (tech.era() !in eras)
                 lines += "Unknown era ${tech.era()} referenced in column of tech ${tech.name}"
+            checkUniques(tech, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific)
         }
 
         if (eras.isEmpty()) {
@@ -453,19 +474,20 @@ class Ruleset {
             warningCount++
         }
         
-        for (era in eras) {
+        for (era in eras.values) {
-            for (wonder in era.value.startingObsoleteWonders)
+            for (wonder in era.startingObsoleteWonders)
                 if (wonder !in buildings)
-                    lines += "Nonexistent wonder $wonder obsoleted when starting in ${era.key}!"
+                    lines += "Nonexistent wonder $wonder obsoleted when starting in ${era.name}!"
-            for (building in era.value.settlerBuildings)
+            for (building in era.settlerBuildings)
                 if (building !in buildings)
-                    lines += "Nonexistent building $building built by settlers when starting in ${era.key}"
+                    lines += "Nonexistent building $building built by settlers when starting in ${era.name}"
-            if (era.value.startingMilitaryUnit !in units)
+            if (era.startingMilitaryUnit !in units)
-                lines += "Nonexistent unit ${era.value.startingMilitaryUnit} marked as starting unit when starting in ${era.key}"
+                lines += "Nonexistent unit ${era.startingMilitaryUnit} marked as starting unit when starting in ${era.name}"
-            if (era.value.researchAgreementCost < 0 || era.value.startingSettlerCount < 0 || era.value.startingWorkerCount < 0 || era.value.startingMilitaryUnitCount < 0 || era.value.startingGold < 0 || era.value.startingCulture < 0)
+            if (era.researchAgreementCost < 0 || era.startingSettlerCount < 0 || era.startingWorkerCount < 0 || era.startingMilitaryUnitCount < 0 || era.startingGold < 0 || era.startingCulture < 0)
-                lines += "Unexpected negative number found while parsing era ${era.key}"
+                lines += "Unexpected negative number found while parsing era ${era.name}"
-            if (era.value.settlerPopulation <= 0)
+            if (era.settlerPopulation <= 0)
-                lines += "Population in cities from settlers must be strictly positive! Found value ${era.value.settlerPopulation} for era ${era.key}"
+                lines += "Population in cities from settlers must be strictly positive! Found value ${era.settlerPopulation} for era ${era.name}"
+            checkUniques(era, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific)
         }
 
         return CheckModLinksResult(warningCount, lines)

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

@@ -31,17 +31,19 @@ enum class UniqueType(val text:String, val replacedBy: UniqueType? = null) {
 
     val placeholderText = text.getPlaceholderText()
 
-    /** Ordinal determines severity - ordered from most severe at 0 */
+    /** Ordinal determines severity - ordered from least to most severe, so we can use Severity >=  */
     enum class UniqueComplianceErrorSeverity {
 
-        /** This is a problem like "numbers don't parse", "stat isn't stat", "city filter not applicable" */
+        /** This is for filters that can also potentially accept free text, like UnitFilter and TileFilter */
-        RulesetInvariant,
+        WarningOnly,
 
         /** This is a problem like "unit/resource/tech name doesn't exist in ruleset" - definite bug */
         RulesetSpecific,
 
-        /** This is for filters that can also potentially accept free text, like UnitFilter and TileFilter */
+
-        WarningOnly
+        /** This is a problem like "numbers don't parse", "stat isn't stat", "city filter not applicable" */
+        RulesetInvariant
+
     }
 
     /** Maps uncompliant parameters to their required types */
@@ -56,7 +58,7 @@ enum class UniqueType(val text:String, val replacedBy: UniqueType? = null) {
                 acceptableParamTypes.map { it.getErrorSeverity(param, ruleset) }
             if (errorTypesForAcceptableParameters.any { it == null }) continue // This matches one of the types!
             val leastSevereWarning =
-                errorTypesForAcceptableParameters.maxByOrNull { it!!.ordinal }!!
+                errorTypesForAcceptableParameters.minByOrNull { it!!.ordinal }!!
             errorList += UniqueComplianceError(param, acceptableParamTypes, leastSevereWarning)
         }
         return errorList

core/src/com/unciv/models/ruleset/tile/TileResource.kt

@@ -1,22 +1,25 @@
 package com.unciv.models.ruleset.tile
 
 import com.unciv.models.ruleset.Belief
+import com.unciv.models.ruleset.IHasUniques
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.Unique
 import com.unciv.models.stats.NamedStats
 import com.unciv.models.stats.Stats
 import com.unciv.ui.civilopedia.FormattedLine
 import com.unciv.ui.civilopedia.ICivilopediaText
-import java.util.*
 
-class TileResource : NamedStats(), ICivilopediaText {
+class TileResource : NamedStats(), ICivilopediaText, IHasUniques {
 
     var resourceType: ResourceType = ResourceType.Bonus
     var terrainsCanBeFoundOn: List<String> = listOf()
     var improvement: String? = null
     var improvementStats: Stats? = null
     var revealedBy: String? = null
+    @Deprecated("As of 3.16.16 - replaced by uniques")
     var unique: String? = null
+    override var uniques: ArrayList<String> = arrayListOf()
+    override val uniqueObjects: List<Unique> by lazy { uniques.map { Unique(it) } }
 
     override var civilopediaText = listOf<FormattedLine>()
 
@@ -91,6 +94,7 @@ class TileResource : NamedStats(), ICivilopediaText {
 
         return textList
     }
+
 }