diff --git a/core/src/com/unciv/models/ruleset/RulesetCache.kt b/core/src/com/unciv/models/ruleset/RulesetCache.kt index 9c998bfc22..606a908d40 100644 --- a/core/src/com/unciv/models/ruleset/RulesetCache.kt +++ b/core/src/com/unciv/models/ruleset/RulesetCache.kt @@ -9,6 +9,7 @@ import com.unciv.models.metadata.GameParameters import com.unciv.models.ruleset.validation.RulesetError import com.unciv.models.ruleset.validation.RulesetErrorList import com.unciv.models.ruleset.validation.RulesetErrorSeverity +import com.unciv.models.ruleset.validation.getRelativeTextDistance import com.unciv.utils.Log import com.unciv.utils.debug @@ -171,9 +172,7 @@ object RulesetCache : HashMap() { } catch (ex: UncivShowableException) { // This happens if a building is dependent on a tech not in the base ruleset // because newRuleset.updateBuildingCosts() in getComplexRuleset() throws an error - RulesetErrorList() - .apply { add(ex.message, RulesetErrorSeverity.Error) } + RulesetErrorList.of(ex.message, RulesetErrorSeverity.Error) } } - } diff --git a/core/src/com/unciv/models/ruleset/unique/Conditionals.kt b/core/src/com/unciv/models/ruleset/unique/Conditionals.kt index b1b586dd8e..312b31b35e 100644 --- a/core/src/com/unciv/models/ruleset/unique/Conditionals.kt +++ b/core/src/com/unciv/models/ruleset/unique/Conditionals.kt @@ -18,7 +18,7 @@ object Conditionals { ): Boolean { if (condition.type?.targetTypes?.any { it.modifierType == UniqueTarget.ModifierType.Other } == true) - return true // not a filtering condition + return true // not a filtering condition, includes e.g. ModifierHiddenFromUsers val relevantUnit by lazy { if (state.ourCombatant != null && state.ourCombatant is MapUnitCombatant) state.ourCombatant.unit @@ -109,7 +109,6 @@ object Conditionals { return when (condition.type) { // These are 'what to do' and not 'when to do' conditionals UniqueType.ConditionalTimedUnique -> true - UniqueType.ModifierHiddenFromUsers -> true // allowed to be attached to any Unique to hide it, no-op otherwise UniqueType.ConditionalChance -> stateBasedRandom.nextFloat() < condition.params[0].toFloat() / 100f UniqueType.ConditionalEveryTurns -> checkOnGameInfo { turns % condition.params[0].toInt() == 0 } diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueTarget.kt b/core/src/com/unciv/models/ruleset/unique/UniqueTarget.kt index 380d2bdf49..576b91c6d8 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueTarget.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueTarget.kt @@ -7,7 +7,7 @@ package com.unciv.models.ruleset.unique * @param inheritsFrom means that all such uniques are acceptable as well. For example, all Global uniques are acceptable for Nations, Eras, etc. */ enum class UniqueTarget( - val documentationString:String = "", + val documentationString: String = "", val inheritsFrom: UniqueTarget? = null, val modifierType: ModifierType = ModifierType.None ) { @@ -64,6 +64,7 @@ enum class UniqueTarget( TriggerCondition("Special conditionals that can be added to Triggerable uniques, to make them activate upon specific actions.", inheritsFrom = Global, modifierType = ModifierType.Other), UnitTriggerCondition("Special conditionals that can be added to UnitTriggerable uniques, to make them activate upon specific actions.", inheritsFrom = TriggerCondition, modifierType = ModifierType.Other), UnitActionModifier("Modifiers that can be added to UnitAction uniques as conditionals", modifierType = ModifierType.Other), + MetaModifier("Modifiers that can be added to other uniques changing user experience, not their behavior", modifierType = ModifierType.Other), ; /** Whether a UniqueType is allowed in the `` part - or not. diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt index a0bcd1b740..593d14ea97 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt @@ -809,7 +809,7 @@ enum class UniqueType( HiddenAfterGreatProphet("Hidden after generating a Great Prophet", UniqueTarget.Ruins), HiddenWithoutVictoryType("Hidden when [victoryType] Victory is disabled", UniqueTarget.Building, UniqueTarget.Unit, flags = UniqueFlag.setOfHiddenToUsers), HiddenFromCivilopedia("Will not be displayed in Civilopedia", *UniqueTarget.Displayable, flags = UniqueFlag.setOfHiddenToUsers), - ModifierHiddenFromUsers("hidden from users", UniqueTarget.Conditional), + ModifierHiddenFromUsers("hidden from users", UniqueTarget.MetaModifier), Comment("Comment [comment]", *UniqueTarget.Displayable, docDescription = "Allows displaying arbitrary text in a Unique listing. Only the text within the '[]' brackets will be displayed, the rest serves to allow Ruleset validation to recognize the intent."), diff --git a/core/src/com/unciv/models/ruleset/validation/RulesetErrorList.kt b/core/src/com/unciv/models/ruleset/validation/RulesetErrorList.kt index ffe97c9df7..a046fcecbf 100644 --- a/core/src/com/unciv/models/ruleset/validation/RulesetErrorList.kt +++ b/core/src/com/unciv/models/ruleset/validation/RulesetErrorList.kt @@ -1,6 +1,8 @@ package com.unciv.models.ruleset.validation import com.badlogic.gdx.graphics.Color +import com.unciv.models.ruleset.Ruleset +import com.unciv.models.ruleset.unique.IHasUniques class RulesetError(val text: String, val errorSeverityToReport: RulesetErrorSeverity) @@ -11,32 +13,61 @@ enum class RulesetErrorSeverity(val color: Color) { Error(Color.RED), } -class RulesetErrorList : ArrayList() { - operator fun plusAssign(text: String) { - add(text, RulesetErrorSeverity.Error) +/** + * A container collecting errors in a [Ruleset] + * + * While this is based a standard collection, please do not use the standard add, [plusAssign] or [addAll]. + * Mod-controlled warning suppression is handled in [add] overloads that provide a source object, which can host suppression uniques. + * Bypassing these add methods means suppression is ignored. Thus using [addAll] is fine when the elements to add are all already checked. + * + * //todo This version prepares suppression, but does not actually implement it + * + * @param ruleset The ruleset being validated (needed to check modOptions for suppression uniques). Leave `null` only for validation results that need no suppression checks. + */ +class RulesetErrorList( + ruleset: Ruleset? = null +) : ArrayList() { + /** Add an [element], preventing duplicates (in which case the highest severity wins). + * + * [sourceObject] is for future use and should be the originating object. When it is not known or not a [IHasUniques], pass `null`. + */ + fun add(element: RulesetError, sourceObject: IHasUniques?): Boolean { + // Suppression to be checked here + return addWithDuplicateCheck(element) } - fun add(text: String, errorSeverityToReport: RulesetErrorSeverity) { - add(RulesetError(text, errorSeverityToReport)) - } + /** Shortcut: Add a new [RulesetError] built from [text] and [errorSeverityToReport]. + * + * [sourceObject] is for future use and should be the originating object. When it is not known or not a [IHasUniques], pass `null`. + */ + fun add(text: String, errorSeverityToReport: RulesetErrorSeverity = RulesetErrorSeverity.Error, sourceObject: IHasUniques?) = + add(RulesetError(text, errorSeverityToReport), sourceObject) - override fun add(element: RulesetError): Boolean { - // Suppress duplicates due to the double run of some checks for invariant/specific, - // Without changing collection type or making RulesetError obey the equality contract - val existing = firstOrNull { it.text == element.text } - ?: return super.add(element) - if (existing.errorSeverityToReport >= element.errorSeverityToReport) return false - remove(existing) - return super.add(element) - } + @Deprecated("No adding without explicit source object", ReplaceWith("add(element, sourceObject)")) + override fun add(element: RulesetError) = super.add(element) + /** Add all [elements] with duplicate check, but without suppression check */ override fun addAll(elements: Collection): Boolean { var result = false for (element in elements) - if (add(element)) result = true + if (addWithDuplicateCheck(element)) result = true return result } + private fun addWithDuplicateCheck(element: RulesetError) = + removeLowerSeverityDuplicate(element) && super.add(element) + + /** @return `true` if the element is not present, or it was removed due to having a lower severity */ + private fun removeLowerSeverityDuplicate(element: RulesetError): Boolean { + // Suppress duplicates due to the double run of some checks for invariant/specific, + // Without changing collection type or making RulesetError obey the equality contract + val existing = firstOrNull { it.text == element.text } + ?: return true + if (existing.errorSeverityToReport >= element.errorSeverityToReport) return false + remove(existing) + return true + } + fun getFinalSeverity(): RulesetErrorSeverity { if (isEmpty()) return RulesetErrorSeverity.OK return this.maxOf { it.errorSeverityToReport } @@ -60,4 +91,17 @@ class RulesetErrorList : ArrayList() { // out of place. Prevent via kludge: it.text.replace('<','〈').replace('>','〉') } + + companion object { + fun of( + text: String, + severity: RulesetErrorSeverity = RulesetErrorSeverity.Error, + ruleset: Ruleset? = null, + sourceObject: IHasUniques? = null + ): RulesetErrorList { + val result = RulesetErrorList(ruleset) + result.add(text, severity, sourceObject) + return result + } + } } diff --git a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt index 7550e2f465..cb29c6b9f2 100644 --- a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt @@ -36,7 +36,7 @@ class RulesetValidator(val ruleset: Ruleset) { } private fun getNonBaseRulesetErrorList(tryFixUnknownUniques: Boolean): RulesetErrorList { - val lines = RulesetErrorList() + val lines = RulesetErrorList(ruleset) // When not checking the entire ruleset, we can only really detect ruleset-invariant errors in uniques addModOptionsErrors(lines, tryFixUnknownUniques) @@ -62,7 +62,7 @@ class RulesetValidator(val ruleset: Ruleset) { uniqueValidator.populateFilteringUniqueHashsets() - val lines = RulesetErrorList() + val lines = RulesetErrorList(ruleset) addModOptionsErrors(lines, tryFixUnknownUniques) uniqueValidator.checkUniques(ruleset.globalUniques, lines, true, tryFixUnknownUniques) @@ -106,11 +106,12 @@ class RulesetValidator(val ruleset: Ruleset) { UniqueType.ModIsAudioVisualOnly, UniqueType.ModIsNotAudioVisual ) + // modOptions is a valid sourceObject, but unnecessary if (ruleset.modOptions.uniqueObjects.count { it.type in audioVisualUniqueTypes } > 1) - lines += "A mod should only specify one of the 'can/should/cannot be used as permanent audiovisual mod' options." + lines.add("A mod should only specify one of the 'can/should/cannot be used as permanent audiovisual mod' options.", sourceObject = null) if (!ruleset.modOptions.isBaseRuleset) return for (unique in ruleset.modOptions.getMatchingUniques(UniqueType.ModRequires)) { - lines += "Mod option '${unique.text}' is invalid for a base ruleset." + lines.add("Mod option '${unique.text}' is invalid for a base ruleset.", sourceObject = null) } } @@ -132,32 +133,34 @@ class RulesetValidator(val ruleset: Ruleset) { } private fun addDifficultyErrors(lines: RulesetErrorList) { + // A Difficulty is not a IHasUniques, so not suitable as sourceObject for (difficulty in ruleset.difficulties.values) { for (unitName in difficulty.aiCityStateBonusStartingUnits + difficulty.aiMajorCivBonusStartingUnits + difficulty.playerBonusStartingUnits) if (unitName != Constants.eraSpecificUnit && !ruleset.units.containsKey(unitName)) - lines += "Difficulty ${difficulty.name} contains starting unit $unitName which does not exist!" + lines.add("Difficulty ${difficulty.name} contains starting unit $unitName which does not exist!", sourceObject = null) } } private fun addVictoryTypeErrors(lines: RulesetErrorList) { + // Victory and Milestone aren't IHasUniques and are unsuitable as sourceObject for (victoryType in ruleset.victories.values) { for (requiredUnit in victoryType.requiredSpaceshipParts) if (!ruleset.units.contains(requiredUnit)) lines.add( "Victory type ${victoryType.name} requires adding the non-existant unit $requiredUnit to the capital to win!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, sourceObject = null ) for (milestone in victoryType.milestoneObjects) if (milestone.type == null) lines.add( "Victory type ${victoryType.name} has milestone ${milestone.uniqueDescription} that is of an unknown type!", - RulesetErrorSeverity.Error + RulesetErrorSeverity.Error, sourceObject = null ) for (victory in ruleset.victories.values) if (victory.name != victoryType.name && victory.milestones == victoryType.milestones) lines.add( "Victory types ${victoryType.name} and ${victory.name} have the same requirements!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, sourceObject = null ) } } @@ -183,12 +186,12 @@ class RulesetValidator(val ruleset: Ruleset) { if (!ruleset.unitPromotions.containsKey(prereq)) lines.add( "${promotion.name} requires promotion $prereq which does not exist!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, promotion ) for (unitType in promotion.unitTypes) checkUnitType(unitType) { lines.add( "${promotion.name} references unit type $unitType, which does not exist!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, promotion ) } uniqueValidator.checkUniques(promotion, lines, true, tryFixUnknownUniques) @@ -202,10 +205,10 @@ class RulesetValidator(val ruleset: Ruleset) { ) { for (reward in ruleset.ruinRewards.values) { @Suppress("KotlinConstantConditions") // data is read from json, so any assumptions may be wrong - if (reward.weight < 0) lines += "${reward.name} has a negative weight, which is not allowed!" + if (reward.weight < 0) lines.add("${reward.name} has a negative weight, which is not allowed!", sourceObject = reward) for (difficulty in reward.excludedDifficulties) if (!ruleset.difficulties.containsKey(difficulty)) - lines += "${reward.name} references difficulty ${difficulty}, which does not exist!" + lines.add("${reward.name} references difficulty ${difficulty}, which does not exist!", sourceObject = reward) uniqueValidator.checkUniques(reward, lines, true, tryFixUnknownUniques) } } @@ -218,18 +221,18 @@ class RulesetValidator(val ruleset: Ruleset) { if (policy.requires != null) for (prereq in policy.requires!!) if (!ruleset.policies.containsKey(prereq)) - lines += "${policy.name} requires policy $prereq which does not exist!" + lines.add("${policy.name} requires policy $prereq which does not exist!", sourceObject = policy) uniqueValidator.checkUniques(policy, lines, true, tryFixUnknownUniques) } for (branch in ruleset.policyBranches.values) if (branch.era !in ruleset.eras) - lines += "${branch.name} requires era ${branch.era} which does not exist!" + lines.add("${branch.name} requires era ${branch.era} which does not exist!", sourceObject = branch) for (policy in ruleset.policyBranches.values.flatMap { it.policies + it }) if (policy != ruleset.policies[policy.name]) - lines += "More than one policy with the name ${policy.name} exists!" + lines.add("More than one policy with the name ${policy.name} exists!", sourceObject = policy) } @@ -243,9 +246,9 @@ class RulesetValidator(val ruleset: Ruleset) { 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!" + lines.add("${nation.name} is of city-state type ${nation.cityStateType} which does not exist!", sourceObject = nation) if (nation.favoredReligion != null && nation.favoredReligion !in ruleset.religions) - lines += "${nation.name} has ${nation.favoredReligion} as their favored religion, which does not exist!" + lines.add("${nation.name} has ${nation.favoredReligion} as their favored religion, which does not exist!", sourceObject = nation) } } @@ -255,7 +258,7 @@ class RulesetValidator(val ruleset: Ruleset) { ) { for (belief in ruleset.beliefs.values) { if (belief.type == BeliefType.Any || belief.type == BeliefType.None) - lines += "${belief.name} type is {belief.type}, which is not allowed!" + lines.add("${belief.name} type is ${belief.type}, which is not allowed!", sourceObject = belief) uniqueValidator.checkUniques(belief, lines, true, tryFixUnknownUniques) } } @@ -263,9 +266,9 @@ class RulesetValidator(val ruleset: Ruleset) { private fun addSpeedErrors(lines: RulesetErrorList) { for (speed in ruleset.speeds.values) { if (speed.modifier < 0f) - lines += "Negative speed modifier for game speed ${speed.name}" + lines.add("Negative speed modifier for game speed ${speed.name}", sourceObject = speed) if (speed.yearsPerTurn.isEmpty()) - lines += "Empty turn increment list for game speed ${speed.name}" + lines.add("Empty turn increment list for game speed ${speed.name}", sourceObject = speed) } } @@ -274,7 +277,7 @@ class RulesetValidator(val ruleset: Ruleset) { tryFixUnknownUniques: Boolean ) { if (ruleset.eras.isEmpty()) { - lines += "Eras file is empty! This will likely lead to crashes. Ask the mod maker to update this mod!" + lines.add("Eras file is empty! This will likely lead to crashes. Ask the mod maker to update this mod!", sourceObject = null) } val allDifficultiesStartingUnits = hashSetOf() @@ -287,38 +290,38 @@ class RulesetValidator(val ruleset: Ruleset) { for (era in ruleset.eras.values) { for (wonder in era.startingObsoleteWonders) if (wonder !in ruleset.buildings) - lines += "Nonexistent wonder $wonder obsoleted when starting in ${era.name}!" + lines.add("Nonexistent wonder $wonder obsoleted when starting in ${era.name}!", sourceObject = era) for (building in era.settlerBuildings) if (building !in ruleset.buildings) - lines += "Nonexistent building $building built by settlers when starting in ${era.name}" + lines.add("Nonexistent building $building built by settlers when starting in ${era.name}", sourceObject = era) // todo the whole 'starting unit' thing needs to be redone, there's no reason we can't have a single list containing all the starting units. if (era.startingSettlerUnit !in ruleset.units && ruleset.units.values.none { it.isCityFounder() } ) - lines += "Nonexistent unit ${era.startingSettlerUnit} marked as starting unit when starting in ${era.name}" + lines.add("Nonexistent unit ${era.startingSettlerUnit} marked as starting unit when starting in ${era.name}", sourceObject = era) if (era.startingWorkerCount != 0 && era.startingWorkerUnit !in ruleset.units && ruleset.units.values.none { it.hasUnique(UniqueType.BuildImprovements) } ) - lines += "Nonexistent unit ${era.startingWorkerUnit} marked as starting unit when starting in ${era.name}" + lines.add("Nonexistent unit ${era.startingWorkerUnit} marked as starting unit when starting in ${era.name}", sourceObject = era) val grantsStartingMilitaryUnit = era.startingMilitaryUnitCount != 0 || allDifficultiesStartingUnits.contains(Constants.eraSpecificUnit) if (grantsStartingMilitaryUnit && era.startingMilitaryUnit !in ruleset.units) - lines += "Nonexistent unit ${era.startingMilitaryUnit} marked as starting unit when starting in ${era.name}" + lines.add("Nonexistent unit ${era.startingMilitaryUnit} marked as starting unit when starting in ${era.name}", sourceObject = era) 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}" + lines.add("Unexpected negative number found while parsing era ${era.name}", sourceObject = era) if (era.settlerPopulation <= 0) - lines += "Population in cities from settlers must be strictly positive! Found value ${era.settlerPopulation} for era ${era.name}" + lines.add("Population in cities from settlers must be strictly positive! Found value ${era.settlerPopulation} for era ${era.name}", sourceObject = era) if (era.allyBonus.isNotEmpty()) lines.add( "Era ${era.name} contains city-state bonuses. City-state bonuses are now defined in CityStateType.json", - RulesetErrorSeverity.WarningOptionsOnly + RulesetErrorSeverity.WarningOptionsOnly, era ) if (era.friendBonus.isNotEmpty()) lines.add( "Era ${era.name} contains city-state bonuses. City-state bonuses are now defined in CityStateType.json", - RulesetErrorSeverity.WarningOptionsOnly + RulesetErrorSeverity.WarningOptionsOnly, era ) uniqueValidator.checkUniques(era, lines, true, tryFixUnknownUniques) @@ -332,20 +335,20 @@ class RulesetValidator(val ruleset: Ruleset) { for (tech in ruleset.technologies.values) { for (prereq in tech.prerequisites) { if (!ruleset.technologies.containsKey(prereq)) - lines += "${tech.name} requires tech $prereq which does not exist!" + lines.add("${tech.name} requires tech $prereq which does not exist!", sourceObject = tech) if (tech.prerequisites.any { it != prereq && getPrereqTree(it).contains(prereq) }) { lines.add( "No need to add $prereq as a prerequisite of ${tech.name} - it is already implicit from the other prerequisites!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, tech ) } if (getPrereqTree(prereq).contains(tech.name)) - lines += "Techs ${tech.name} and $prereq require each other!" + lines.add("Techs ${tech.name} and $prereq require each other!", sourceObject = tech) } if (tech.era() !in ruleset.eras) - lines += "Unknown era ${tech.era()} referenced in column of tech ${tech.name}" + lines.add("Unknown era ${tech.era()} referenced in column of tech ${tech.name}", sourceObject = tech) uniqueValidator.checkUniques(tech, lines, true, tryFixUnknownUniques) } } @@ -355,25 +358,25 @@ class RulesetValidator(val ruleset: Ruleset) { tryFixUnknownUniques: Boolean ) { if (ruleset.terrains.values.none { it.type == TerrainType.Land && !it.impassable }) - lines += "No passable land terrains exist!" + lines.add("No passable land terrains exist!", sourceObject = null) for (terrain in ruleset.terrains.values) { for (baseTerrainName in terrain.occursOn) { val baseTerrain = ruleset.terrains[baseTerrainName] if (baseTerrain == null) - lines += "${terrain.name} occurs on terrain $baseTerrainName which does not exist!" + lines.add("${terrain.name} occurs on terrain $baseTerrainName which does not exist!", sourceObject = terrain) else if (baseTerrain.type == TerrainType.NaturalWonder) - lines.add("${terrain.name} occurs on natural wonder $baseTerrainName: Unsupported.", RulesetErrorSeverity.WarningOptionsOnly) + lines.add("${terrain.name} occurs on natural wonder $baseTerrainName: Unsupported.", RulesetErrorSeverity.WarningOptionsOnly, terrain) } if (terrain.type == TerrainType.NaturalWonder) { if (terrain.turnsInto == null) - lines += "Natural Wonder ${terrain.name} is missing the turnsInto attribute!" + lines.add("Natural Wonder ${terrain.name} is missing the turnsInto attribute!", sourceObject = terrain) val baseTerrain = ruleset.terrains[terrain.turnsInto] if (baseTerrain == null) - lines += "${terrain.name} turns into terrain ${terrain.turnsInto} which does not exist!" + lines.add("${terrain.name} turns into terrain ${terrain.turnsInto} which does not exist!", sourceObject = terrain) else if (!baseTerrain.type.isBaseTerrain) // 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) + lines.add("${terrain.name} turns into terrain ${terrain.turnsInto} which is not a base terrain!", RulesetErrorSeverity.Warning, terrain) } uniqueValidator.checkUniques(terrain, lines, true, tryFixUnknownUniques) } @@ -385,10 +388,10 @@ class RulesetValidator(val ruleset: Ruleset) { ) { for (improvement in ruleset.tileImprovements.values) { if (improvement.techRequired != null && !ruleset.technologies.containsKey(improvement.techRequired!!)) - lines += "${improvement.name} requires tech ${improvement.techRequired} which does not exist!" + lines.add("${improvement.name} requires tech ${improvement.techRequired} which does not exist!", sourceObject = improvement) for (terrain in improvement.terrainsCanBeBuiltOn) if (!ruleset.terrains.containsKey(terrain) && terrain != "Land" && terrain != "Water") - lines += "${improvement.name} can be built on terrain $terrain which does not exist!" + lines.add("${improvement.name} can be built on terrain $terrain which does not exist!", sourceObject = improvement) if (improvement.terrainsCanBeBuiltOn.isEmpty() && !improvement.hasUnique(UniqueType.CanOnlyImproveResource) && !improvement.hasUnique(UniqueType.Unbuildable) @@ -398,7 +401,7 @@ class RulesetValidator(val ruleset: Ruleset) { ) { lines.add( "${improvement.name} has an empty `terrainsCanBeBuiltOn`, isn't allowed to only improve resources and isn't unbuildable! Support for this will soon end. Either give this the unique \"Unbuildable\", \"Can only be built to improve a resource\" or add \"Land\", \"Water\" or any other value to `terrainsCanBeBuiltOn`.", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, improvement ) } for (unique in improvement.uniqueObjects @@ -407,7 +410,7 @@ class RulesetValidator(val ruleset: Ruleset) { val params = Stats.parse(unique.params[0]) if (params.values.any { it < 0 }) lines.add( "${improvement.name} cannot have a negative value for a pillage yield!", - RulesetErrorSeverity.Error + RulesetErrorSeverity.Error, improvement ) } @@ -416,7 +419,7 @@ class RulesetValidator(val ruleset: Ruleset) { if (hasPillageUnique && improvement.hasUnique(UniqueType.Unpillagable, StateForConditionals.IgnoreConditionals)) { lines.add( "${improvement.name} has both an `Unpillagable` unique type and a `PillageYieldRandom` or `PillageYieldFixed` unique type!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, improvement ) } uniqueValidator.checkUniques(improvement, lines, true, tryFixUnknownUniques) @@ -429,26 +432,27 @@ class RulesetValidator(val ruleset: Ruleset) { ) { for (resource in ruleset.tileResources.values) { if (resource.revealedBy != null && !ruleset.technologies.containsKey(resource.revealedBy!!)) - lines += "${resource.name} revealed by tech ${resource.revealedBy} which does not exist!" + lines.add("${resource.name} revealed by tech ${resource.revealedBy} which does not exist!", sourceObject = resource) if (resource.improvement != null && !ruleset.tileImprovements.containsKey(resource.improvement!!)) - lines += "${resource.name} improved by improvement ${resource.improvement} which does not exist!" + lines.add("${resource.name} improved by improvement ${resource.improvement} which does not exist!", sourceObject = resource) for (improvement in resource.improvedBy) if (!ruleset.tileImprovements.containsKey(improvement)) - lines += "${resource.name} improved by improvement $improvement which does not exist!" + lines.add("${resource.name} improved by improvement $improvement which does not exist!", sourceObject = resource) for (terrain in resource.terrainsCanBeFoundOn) if (!ruleset.terrains.containsKey(terrain)) - lines += "${resource.name} can be found on terrain $terrain which does not exist!" + lines.add("${resource.name} can be found on terrain $terrain which does not exist!", sourceObject = resource) uniqueValidator.checkUniques(resource, lines, true, tryFixUnknownUniques) } } private fun addSpecialistErrors(lines: RulesetErrorList) { + // Specialist is not a IHasUniques and unsuitable as sourceObject for (specialist in ruleset.specialists.values) { for (gpp in specialist.greatPersonPoints) if (gpp.key !in ruleset.units) lines.add( "Specialist ${specialist.name} has greatPersonPoints for ${gpp.key}, which is not a unit in the ruleset!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, sourceObject = null ) } } @@ -462,17 +466,17 @@ class RulesetValidator(val ruleset: Ruleset) { for (requiredTech: String in building.requiredTechs()) if (!ruleset.technologies.containsKey(requiredTech)) - lines += "${building.name} requires tech $requiredTech which does not exist!" + lines.add("${building.name} requires tech $requiredTech which does not exist!", sourceObject = building) for (specialistName in building.specialistSlots.keys) if (!ruleset.specialists.containsKey(specialistName)) - lines += "${building.name} provides specialist $specialistName which does not exist!" + lines.add("${building.name} provides specialist $specialistName which does not exist!", sourceObject = building) for (resource in building.getResourceRequirementsPerTurn(StateForConditionals.IgnoreConditionals).keys) if (!ruleset.tileResources.containsKey(resource)) - lines += "${building.name} requires resource $resource which does not exist!" + lines.add("${building.name} requires resource $resource which does not exist!", sourceObject = building) if (building.replaces != null && !ruleset.buildings.containsKey(building.replaces!!)) - lines += "${building.name} replaces ${building.replaces} which does not exist!" + lines.add("${building.name} replaces ${building.replaces} which does not exist!", sourceObject = building) if (building.requiredBuilding != null && !ruleset.buildings.containsKey(building.requiredBuilding!!)) - lines += "${building.name} requires ${building.requiredBuilding} which does not exist!" + lines.add("${building.name} requires ${building.requiredBuilding} which does not exist!", sourceObject = building) uniqueValidator.checkUniques(building, lines, true, tryFixUnknownUniques) } } @@ -493,7 +497,7 @@ class RulesetValidator(val ruleset: Ruleset) { tryFixUnknownUniques: Boolean ) { if (ruleset.units.values.none { it.isCityFounder() }) - lines += "No city-founding units in ruleset!" + lines.add("No city-founding units in ruleset!", sourceObject = null) for (unit in ruleset.units.values) { checkUnitRulesetInvariant(unit, lines) @@ -523,12 +527,12 @@ class RulesetValidator(val ruleset: Ruleset) { } 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) lines.add("Promotion ${promotion.name} has invalid row value: ${promotion.row}", sourceObject = promotion) + if (promotion.column < 0) lines.add("Promotion ${promotion.name} has invalid column value: ${promotion.column}", sourceObject = promotion) 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}" + lines.add("Promotions ${promotion.name} and ${otherPromotion.name} have the same position: ${promotion.row}/${promotion.column}", sourceObject = promotion) } private fun addNationErrorsRulesetInvariant( @@ -543,23 +547,20 @@ class RulesetValidator(val ruleset: Ruleset) { 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!" + lines.add("${nation.name} can settle cities, but has no city names!", sourceObject = nation) } // https://www.w3.org/TR/WCAG20/#visual-audio-contrast-contrast val constrastRatio = nation.getContrastRatio() if (constrastRatio < 3) { - val suggestedColors = getSuggestedColors(nation) - val newOuterColor = suggestedColors.outerColor - val newInnerColor = suggestedColors.innerColor + val (newInnerColor, newOuterColor) = getSuggestedColors(nation) var text = "${nation.name}'s colors do not contrast enough - it is unreadable!" 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()}]," - lines.add(text, RulesetErrorSeverity.WarningOptionsOnly) - lines.add(text, RulesetErrorSeverity.WarningOptionsOnly) + lines.add(text, RulesetErrorSeverity.WarningOptionsOnly, nation) } } @@ -609,37 +610,38 @@ class RulesetValidator(val ruleset: Ruleset) { ) lines.add( "${building.name} is buildable and therefore should either have an explicit cost or reference an existing tech!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, building ) 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 + RulesetErrorSeverity.Warning, building ) } private fun addTechColumnErrorsRulesetInvariant(lines: RulesetErrorList) { + // TechColumn is not a IHasUniques and unsuitable as sourceObject for (techColumn in ruleset.techColumns) { if (techColumn.columnNumber < 0) - lines += "Tech Column number ${techColumn.columnNumber} is negative" + lines.add("Tech Column number ${techColumn.columnNumber} is negative", sourceObject = null) if (techColumn.buildingCost == -1) lines.add( "Tech Column number ${techColumn.columnNumber} has no explicit building cost", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, sourceObject = null ) if (techColumn.wonderCost == -1) lines.add( "Tech Column number ${techColumn.columnNumber} has no explicit wonder cost", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, sourceObject = null ) } 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}!" + lines.add("${tech.name} is in the same row and column as ${otherTech.name}!", sourceObject = tech) } } } @@ -673,23 +675,23 @@ class RulesetValidator(val ruleset: Ruleset) { private fun checkUnitRulesetInvariant(unit: BaseUnit, lines: RulesetErrorList) { for (upgradesTo in unit.getUpgradeUnits(StateForConditionals.IgnoreConditionals)) { if (upgradesTo == unit.name || (upgradesTo == unit.replaces)) - lines += "${unit.name} upgrades to itself!" + lines.add("${unit.name} upgrades to itself!", sourceObject = unit) } if (unit.isMilitary() && unit.strength == 0) // Should only match ranged units with 0 strength - lines += "${unit.name} is a military unit but has no assigned strength!" + lines.add("${unit.name} is a military unit but has no assigned strength!", sourceObject = unit) } /** Collects all RulesetSpecific checks for a BaseUnit */ private fun checkUnitRulesetSpecific(unit: BaseUnit, lines: RulesetErrorList) { for (requiredTech: String in unit.requiredTechs()) if (!ruleset.technologies.containsKey(requiredTech)) - lines += "${unit.name} requires tech $requiredTech which does not exist!" + lines.add("${unit.name} requires tech $requiredTech which does not exist!", sourceObject = unit) for (obsoleteTech: String in unit.techsAtWhichNoLongerAvailable()) if (!ruleset.technologies.containsKey(obsoleteTech)) - lines += "${unit.name} obsoletes at tech $obsoleteTech which does not exist!" + lines.add("${unit.name} obsoletes at tech $obsoleteTech which does not exist!", sourceObject = unit) for (upgradesTo in unit.getUpgradeUnits(StateForConditionals.IgnoreConditionals)) if (!ruleset.units.containsKey(upgradesTo)) - lines += "${unit.name} upgrades to unit $upgradesTo which does not exist!" + lines.add("${unit.name} upgrades to unit $upgradesTo which does not exist!", sourceObject = unit) // Check that we don't obsolete ourselves before we can upgrade for (obsoleteTech: String in unit.techsAtWhichAutoUpgradeInProduction()) @@ -702,20 +704,20 @@ class RulesetValidator(val ruleset: Ruleset) { lines.add( "${unit.name} is supposed to automatically upgrade at tech ${obsoleteTech}," + " and therefore $requiredTech for its upgrade ${upgradedUnit.name} may not yet be researched!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, unit ) } for (resource in unit.getResourceRequirementsPerTurn(StateForConditionals.IgnoreConditionals).keys) if (!ruleset.tileResources.containsKey(resource)) - lines += "${unit.name} requires resource $resource which does not exist!" + lines.add("${unit.name} requires resource $resource which does not exist!", sourceObject = unit) if (unit.replaces != null && !ruleset.units.containsKey(unit.replaces!!)) - lines += "${unit.name} replaces ${unit.replaces} which does not exist!" + lines.add("${unit.name} replaces ${unit.replaces} which does not exist!", sourceObject = unit) for (promotion in unit.promotions) if (!ruleset.unitPromotions.containsKey(promotion)) - lines += "${unit.name} contains promotion $promotion which does not exist!" + lines.add("${unit.name} contains promotion $promotion which does not exist!", sourceObject = unit) checkUnitType(unit.unitType) { - lines += "${unit.name} is of type ${unit.unitType}, which does not exist!" + lines.add("${unit.name} is of type ${unit.unitType}, which does not exist!", sourceObject = unit) } // We should ignore conditionals here - there are condition implementations on this out there that require a game state (and will test false without) @@ -726,7 +728,7 @@ class RulesetValidator(val ruleset: Ruleset) { unit.isCivilian() && !unit.isGreatPersonOfType("War")) { lines.add("${unit.name} can place improvement $improvementName which has no stats, preventing unit automation!", - RulesetErrorSeverity.WarningOptionsOnly) + RulesetErrorSeverity.WarningOptionsOnly, unit) } } } @@ -763,31 +765,31 @@ class RulesetValidator(val ruleset: Ruleset) { allFallbacks.add(config.fallbackTileSet!!) } catch (ex: Exception) { // Our fromJsonFile wrapper already intercepts Exceptions and gives them a generalized message, so go a level deeper for useful details (like "unmatched brace") - lines.add("Tileset config '${file.name()}' cannot be loaded (${ex.cause?.message})", RulesetErrorSeverity.Warning) + lines.add("Tileset config '${file.name()}' cannot be loaded (${ex.cause?.message})", RulesetErrorSeverity.Warning, sourceObject = null) } } // Folder should not contain subdirectories, non-json files, or be empty if (folderContentBad) - lines.add("The Mod tileset config folder contains non-json files or subdirectories", RulesetErrorSeverity.Warning) + lines.add("The Mod tileset config folder contains non-json files or subdirectories", RulesetErrorSeverity.Warning, sourceObject = null) if (configTilesets.isEmpty()) - lines.add("The Mod tileset config folder contains no json files", RulesetErrorSeverity.Warning) + lines.add("The Mod tileset config folder contains no json files", RulesetErrorSeverity.Warning, sourceObject = null) // There should be atlas images corresponding to each json name val atlasTilesets = getTilesetNamesFromAtlases() val configOnlyTilesets = configTilesets - atlasTilesets if (configOnlyTilesets.isNotEmpty()) - lines.add("Mod has no graphics for configured tilesets: ${configOnlyTilesets.joinToString()}", RulesetErrorSeverity.Warning) + lines.add("Mod has no graphics for configured tilesets: ${configOnlyTilesets.joinToString()}", RulesetErrorSeverity.Warning, sourceObject = null) // For all atlas images matching "TileSets/*" there should be a json val atlasOnlyTilesets = atlasTilesets - configTilesets if (atlasOnlyTilesets.isNotEmpty()) - lines.add("Mod has no configuration for tileset graphics: ${atlasOnlyTilesets.joinToString()}", RulesetErrorSeverity.Warning) + lines.add("Mod has no configuration for tileset graphics: ${atlasOnlyTilesets.joinToString()}", RulesetErrorSeverity.Warning, sourceObject = null) // All fallbacks should exist (default added because TileSetCache is not loaded when running as unit test) val unknownFallbacks = allFallbacks - TileSetCache.keys - Constants.defaultFallbackTileset if (unknownFallbacks.isNotEmpty()) - lines.add("Fallback tileset invalid: ${unknownFallbacks.joinToString()}", RulesetErrorSeverity.Warning) + lines.add("Fallback tileset invalid: ${unknownFallbacks.joinToString()}", RulesetErrorSeverity.Warning, sourceObject = null) } private fun getTilesetNamesFromAtlases(): Set { @@ -810,7 +812,7 @@ class RulesetValidator(val ruleset: Ruleset) { fun recursiveCheck(history: HashSet, promotion: Promotion, level: Int) { if (promotion in history) { lines.add("Circular Reference in Promotions: ${history.joinToString("→") { it.name }}→${promotion.name}", - RulesetErrorSeverity.Warning) + RulesetErrorSeverity.Warning, promotion) return } if (level > 99) return diff --git a/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt b/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt index aae99ef81c..946754edf6 100644 --- a/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt @@ -60,30 +60,29 @@ class UniqueValidator(val ruleset: Ruleset) { tryFixUnknownUniques: Boolean, uniqueContainer: IHasUniques?, reportRulesetSpecificErrors: Boolean - ): List { - val prefix by lazy { (if (uniqueContainer is IRulesetObject) "${uniqueContainer.originRuleset}: " else "") + - (if (uniqueContainer == null) "The" else "(${uniqueContainer.getUniqueTarget().name}) ${uniqueContainer.name}'s") } - if (unique.type == null) return checkUntypedUnique(unique, tryFixUnknownUniques, prefix) + ): RulesetErrorList { + val prefix by lazy { getUniqueContainerPrefix(uniqueContainer) + "\"${unique.text}\"" } + if (unique.type == null) return checkUntypedUnique(unique, tryFixUnknownUniques, uniqueContainer, prefix) - val rulesetErrors = RulesetErrorList() + val rulesetErrors = RulesetErrorList(ruleset) if (uniqueContainer != null && !unique.type.canAcceptUniqueTarget(uniqueContainer.getUniqueTarget())) - rulesetErrors.add(RulesetError("$prefix unique \"${unique.text}\" is not allowed on its target type", RulesetErrorSeverity.Warning)) + rulesetErrors.add("$prefix is not allowed on its target type", RulesetErrorSeverity.Warning, uniqueContainer) val typeComplianceErrors = getComplianceErrors(unique) for (complianceError in typeComplianceErrors) { if (!reportRulesetSpecificErrors && complianceError.errorSeverity == UniqueType.UniqueParameterErrorSeverity.RulesetSpecific) continue - rulesetErrors.add(RulesetError("$prefix unique \"${unique.text}\" contains parameter ${complianceError.parameterName}," + + rulesetErrors.add("$prefix contains parameter ${complianceError.parameterName}," + " which does not fit parameter type" + " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !", - complianceError.errorSeverity.getRulesetErrorSeverity() - )) + complianceError.errorSeverity.getRulesetErrorSeverity(), uniqueContainer + ) } for (conditional in unique.conditionals) { - addConditionalErrors(conditional, rulesetErrors, prefix, unique, reportRulesetSpecificErrors) + addConditionalErrors(conditional, rulesetErrors, prefix, unique, uniqueContainer, reportRulesetSpecificErrors) } if (unique.type in MapUnitCache.UnitMovementUniques @@ -91,14 +90,17 @@ class UniqueValidator(val ruleset: Ruleset) { ) // (Stay silent if the only conditional is `` - as in G&K Denmark) // Not necessarily even a problem, but yes something mod maker should be aware of - rulesetErrors.add("$prefix unique \"${unique.text}\" contains a conditional on a unit movement unique. " + + rulesetErrors.add( + "$prefix contains a conditional on a unit movement unique. " + "Due to performance considerations, this unique is cached on the unit," + - " and the conditional may not always limit the unique correctly.", RulesetErrorSeverity.OK) + " and the conditional may not always limit the unique correctly.", + RulesetErrorSeverity.OK, uniqueContainer + ) 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. - addDeprecationAnnotationErrors(unique, prefix, rulesetErrors) + addDeprecationAnnotationErrors(unique, prefix, rulesetErrors, uniqueContainer) return rulesetErrors } @@ -108,40 +110,41 @@ class UniqueValidator(val ruleset: Ruleset) { rulesetErrors: RulesetErrorList, prefix: String, unique: Unique, + uniqueContainer: IHasUniques?, reportRulesetSpecificErrors: Boolean ) { if (unique.hasFlag(UniqueFlag.NoConditionals)) { rulesetErrors.add( - "$prefix unique \"${unique.text}\" contains the conditional \"${conditional.text}\"," + + "$prefix contains the conditional \"${conditional.text}\"," + " but the unique does not accept conditionals!", - RulesetErrorSeverity.Error + RulesetErrorSeverity.Error, uniqueContainer ) return } if (conditional.type == null) { rulesetErrors.add( - "$prefix unique \"${unique.text}\" contains the conditional \"${conditional.text}\"," + + "$prefix contains the conditional \"${conditional.text}\"," + " which is of an unknown type!", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, uniqueContainer ) return } if (conditional.type.targetTypes.none { it.modifierType != UniqueTarget.ModifierType.None }) rulesetErrors.add( - "$prefix unique \"${unique.text}\" contains the conditional \"${conditional.text}\"," + + "$prefix contains the conditional \"${conditional.text}\"," + " which is a Unique type not allowed as conditional or trigger.", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, uniqueContainer ) if (conditional.type.targetTypes.contains(UniqueTarget.UnitActionModifier) && unique.type!!.targetTypes.none { UniqueTarget.UnitAction.canAcceptUniqueTarget(it) } ) rulesetErrors.add( - "$prefix unique \"${unique.text}\" contains the conditional \"${conditional.text}\"," + + "$prefix contains the conditional \"${conditional.text}\"," + " which as a UnitActionModifier is only allowed on UnitAction uniques.", - RulesetErrorSeverity.Warning + RulesetErrorSeverity.Warning, uniqueContainer ) val conditionalComplianceErrors = @@ -152,12 +155,10 @@ class UniqueValidator(val ruleset: Ruleset) { continue rulesetErrors.add( - RulesetError( - "$prefix unique \"${unique.text}\" contains the conditional \"${conditional.text}\"." + - " This contains the parameter ${complianceError.parameterName} which does not fit parameter type" + - " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !", - complianceError.errorSeverity.getRulesetErrorSeverity() - ) + "$prefix contains the conditional \"${conditional.text}\"." + + " This contains the parameter ${complianceError.parameterName} which does not fit parameter type" + + " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !", + complianceError.errorSeverity.getRulesetErrorSeverity(), uniqueContainer ) } } @@ -165,19 +166,20 @@ class UniqueValidator(val ruleset: Ruleset) { private fun addDeprecationAnnotationErrors( unique: Unique, prefix: String, - rulesetErrors: RulesetErrorList + rulesetErrors: RulesetErrorList, + uniqueContainer: IHasUniques? ) { val deprecationAnnotation = unique.getDeprecationAnnotation() if (deprecationAnnotation != null) { val replacementUniqueText = unique.getReplacementText(ruleset) val deprecationText = - "$prefix unique \"${unique.text}\" is deprecated ${deprecationAnnotation.message}," + + "$prefix is deprecated ${deprecationAnnotation.message}," + if (deprecationAnnotation.replaceWith.expression != "") " replace with \"${replacementUniqueText}\"" else "" val severity = if (deprecationAnnotation.level == DeprecationLevel.WARNING) RulesetErrorSeverity.WarningOptionsOnly // Not user-visible else RulesetErrorSeverity.Warning // User visible - rulesetErrors.add(deprecationText, severity) + rulesetErrors.add(deprecationText, severity, uniqueContainer) } } @@ -185,7 +187,7 @@ class UniqueValidator(val ruleset: Ruleset) { private fun getComplianceErrors( unique: Unique, ): List { - if (unique.type==null) return emptyList() + if (unique.type == null) return emptyList() val errorList = ArrayList() for ((index, param) in unique.params.withIndex()) { val acceptableParamTypes = unique.type.parameterTypeMap[index] @@ -215,24 +217,26 @@ class UniqueValidator(val ruleset: Ruleset) { return severity } - private fun checkUntypedUnique(unique: Unique, tryFixUnknownUniques: Boolean, prefix: String ): List { + private fun checkUntypedUnique(unique: Unique, tryFixUnknownUniques: Boolean, uniqueContainer: IHasUniques?, prefix: String): RulesetErrorList { // Malformed conditional is always bad if (unique.text.count { it == '<' } != unique.text.count { it == '>' }) - return listOf(RulesetError( - "$prefix unique \"${unique.text}\" contains mismatched conditional braces!", - RulesetErrorSeverity.Warning)) + return RulesetErrorList.of( + "$prefix contains mismatched conditional braces!", + RulesetErrorSeverity.Warning, ruleset, uniqueContainer + ) // Support purely filtering Uniques without actual implementation - if (isFilteringUniqueAllowed(unique)) return emptyList() + if (isFilteringUniqueAllowed(unique)) return RulesetErrorList() if (tryFixUnknownUniques) { - val fixes = tryFixUnknownUnique(unique, prefix) + val fixes = tryFixUnknownUnique(unique, uniqueContainer, prefix) if (fixes.isNotEmpty()) return fixes } - return listOf(RulesetError( - "$prefix unique \"${unique.text}\" not found in Unciv's unique types, and is not used as a filtering unique.", - if (unique.params.isEmpty()) RulesetErrorSeverity.OK else RulesetErrorSeverity.Warning - )) + return RulesetErrorList.of( + "$prefix not found in Unciv's unique types, and is not used as a filtering unique.", + if (unique.params.isEmpty()) RulesetErrorSeverity.OK else RulesetErrorSeverity.Warning, + ruleset, uniqueContainer + ) } private fun isFilteringUniqueAllowed(unique: Unique): Boolean { @@ -242,7 +246,7 @@ class UniqueValidator(val ruleset: Ruleset) { return unique.text in allUniqueParameters // referenced at least once from elsewhere } - private fun tryFixUnknownUnique(unique: Unique, prefix: String): List { + private fun tryFixUnknownUnique(unique: Unique, uniqueContainer: IHasUniques?, prefix: String): RulesetErrorList { val similarUniques = UniqueType.values().filter { getRelativeTextDistance( it.placeholderText, @@ -253,13 +257,15 @@ class UniqueValidator(val ruleset: Ruleset) { similarUniques.filter { it.placeholderText == unique.placeholderText } return when { // This should only ever happen if a bug is or has been introduced that prevents Unique.type from being set for a valid UniqueType, I think.\ - equalUniques.isNotEmpty() -> listOf(RulesetError( - "$prefix unique \"${unique.text}\" looks like it should be fine, but for some reason isn't recognized.", - RulesetErrorSeverity.OK)) + equalUniques.isNotEmpty() -> RulesetErrorList.of( + "$prefix looks like it should be fine, but for some reason isn't recognized.", + RulesetErrorSeverity.OK, + ruleset, uniqueContainer + ) similarUniques.isNotEmpty() -> { val text = - "$prefix unique \"${unique.text}\" looks like it may be a misspelling of:\n" + + "$prefix looks like it may be a misspelling of:\n" + similarUniques.joinToString("\n") { uniqueType -> var text = "\"${uniqueType.text}" if (unique.conditionals.isNotEmpty()) @@ -268,9 +274,16 @@ class UniqueValidator(val ruleset: Ruleset) { if (uniqueType.getDeprecationAnnotation() != null) text += " (Deprecated)" return@joinToString text }.prependIndent("\t") - listOf(RulesetError(text, RulesetErrorSeverity.OK)) + RulesetErrorList.of(text, RulesetErrorSeverity.OK, ruleset, uniqueContainer) } - else -> emptyList() + else -> RulesetErrorList() } } + + companion object { + internal fun getUniqueContainerPrefix(uniqueContainer: IHasUniques?) = + (if (uniqueContainer is IRulesetObject) "${uniqueContainer.originRuleset}: " else "") + + (if (uniqueContainer == null) "The" else "(${uniqueContainer.getUniqueTarget().name}) ${uniqueContainer.name}'s") + + " unique " + } } diff --git a/core/src/com/unciv/ui/popups/options/ModCheckTab.kt b/core/src/com/unciv/ui/popups/options/ModCheckTab.kt index 45cbe8504f..3bebda5713 100644 --- a/core/src/com/unciv/ui/popups/options/ModCheckTab.kt +++ b/core/src/com/unciv/ui/popups/options/ModCheckTab.kt @@ -8,7 +8,6 @@ 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.validation.RulesetError import com.unciv.models.ruleset.validation.RulesetErrorSeverity import com.unciv.models.ruleset.validation.UniqueValidator import com.unciv.models.translations.tr @@ -101,8 +100,8 @@ class ModCheckTab( else RulesetCache.checkCombinedModLinks(linkedSetOf(mod.name), base, tryFixUnknownUniques = true) modLinks.sortByDescending { it.errorSeverityToReport } val noProblem = !modLinks.isNotOK() - if (modLinks.isNotEmpty()) modLinks += RulesetError("", RulesetErrorSeverity.OK) - if (noProblem) modLinks += RulesetError("No problems found.".tr(), RulesetErrorSeverity.OK) + if (modLinks.isNotEmpty()) modLinks.add("", RulesetErrorSeverity.OK, sourceObject = null) + if (noProblem) modLinks.add("No problems found.".tr(), RulesetErrorSeverity.OK, sourceObject = null) launchOnGLThread { // When the options popup is already closed before this postRunnable is run, diff --git a/docs/Modders/uniques.md b/docs/Modders/uniques.md index 793ad8aba1..ea7d23be8e 100644 --- a/docs/Modders/uniques.md +++ b/docs/Modders/uniques.md @@ -2170,9 +2170,6 @@ Due to performance considerations, this unique is cached, thus conditionals may Applicable to: Conditional -??? example "<hidden from users>" - Applicable to: Conditional - ## TriggerCondition uniques !!! note "" @@ -2306,6 +2303,14 @@ Due to performance considerations, this unique is cached, thus conditionals may ??? example "<after which this unit is consumed>" Applicable to: UnitActionModifier +## MetaModifier uniques +!!! note "" + + Modifiers that can be added to other uniques changing user experience, not their behavior + +??? example "<hidden from users>" + Applicable to: MetaModifier + *[amount]: This indicates a whole number, possibly with a + or - sign, such as `2`, `+13`, or `-3`. *[baseTerrain]: The name of any terrain that is a base terrain according to the json file.