From 54ea0c89a8b3eb2befb6f0480c7d1b4f0812931a Mon Sep 17 00:00:00 2001 From: yairm210 Date: Fri, 17 Sep 2021 14:47:03 +0300 Subject: [PATCH] Ruleset check finds errors in uniques of all kinds! --- core/src/com/unciv/models/ruleset/Ruleset.kt | 66 ++++++++++++------- .../com/unciv/models/ruleset/UniqueType.kt | 14 ++-- .../unciv/models/ruleset/tile/TileResource.kt | 8 ++- 3 files changed, 58 insertions(+), 30 deletions(-) diff --git a/core/src/com/unciv/models/ruleset/Ruleset.kt b/core/src/com/unciv/models/ruleset/Ruleset.kt index 00c825b105..c4aad507a7 100644 --- a/core/src/com/unciv/models/ruleset/Ruleset.kt +++ b/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, + 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() 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) { - 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 }} !" - } - } + checkUniques(building, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetInvariant) } @@ -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>() @@ -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 (wonder in era.value.startingObsoleteWonders) + for (era in eras.values) { + for (wonder in era.startingObsoleteWonders) if (wonder !in buildings) - lines += "Nonexistent wonder $wonder obsoleted when starting in ${era.key}!" - for (building in era.value.settlerBuildings) + lines += "Nonexistent wonder $wonder obsoleted when starting in ${era.name}!" + for (building in era.settlerBuildings) if (building !in buildings) - lines += "Nonexistent building $building built by settlers when starting in ${era.key}" - if (era.value.startingMilitaryUnit !in units) - lines += "Nonexistent unit ${era.value.startingMilitaryUnit} marked as starting unit when starting in ${era.key}" - 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) - lines += "Unexpected negative number found while parsing era ${era.key}" - if (era.value.settlerPopulation <= 0) - lines += "Population in cities from settlers must be strictly positive! Found value ${era.value.settlerPopulation} for era ${era.key}" + lines += "Nonexistent building $building built by settlers when starting in ${era.name}" + if (era.startingMilitaryUnit !in units) + lines += "Nonexistent unit ${era.startingMilitaryUnit} marked as starting unit when starting in ${era.name}" + 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.name}" + if (era.settlerPopulation <= 0) + 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) diff --git a/core/src/com/unciv/models/ruleset/UniqueType.kt b/core/src/com/unciv/models/ruleset/UniqueType.kt index cb77d69c2a..806136e42e 100644 --- a/core/src/com/unciv/models/ruleset/UniqueType.kt +++ b/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" */ - RulesetInvariant, + /** This is for filters that can also potentially accept free text, like UnitFilter and TileFilter */ + 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 diff --git a/core/src/com/unciv/models/ruleset/tile/TileResource.kt b/core/src/com/unciv/models/ruleset/tile/TileResource.kt index 7f0564f32c..15569a9652 100644 --- a/core/src/com/unciv/models/ruleset/tile/TileResource.kt +++ b/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 = 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 = arrayListOf() + override val uniqueObjects: List by lazy { uniques.map { Unique(it) } } override var civilopediaText = listOf() @@ -91,6 +94,7 @@ class TileResource : NamedStats(), ICivilopediaText { return textList } + }