From 6ce685b71997160128977d2f4f1b056cb555d27d Mon Sep 17 00:00:00 2001 From: Yair Morgenstern Date: Mon, 30 Oct 2023 13:22:58 +0200 Subject: [PATCH] Added test for both sides of filtering unique, to not raise mod warnings on both the filter and the filtered unique --- .../com/unciv/models/ruleset/unique/Unique.kt | 2 ++ .../ruleset/unique/UniqueParameterType.kt | 10 +++---- .../unciv/models/ruleset/unique/UniqueType.kt | 2 +- .../ruleset/validation/RulesetValidator.kt | 3 +- .../ruleset/validation/UniqueValidator.kt | 30 +++++++++++++++++-- 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/core/src/com/unciv/models/ruleset/unique/Unique.kt b/core/src/com/unciv/models/ruleset/unique/Unique.kt index 30c09a4f36..a051aa1149 100644 --- a/core/src/com/unciv/models/ruleset/unique/Unique.kt +++ b/core/src/com/unciv/models/ruleset/unique/Unique.kt @@ -21,6 +21,7 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s /** This is so the heavy regex-based parsing is only activated once per unique, instead of every time it's called * - for instance, in the city screen, we call every tile unique for every tile, which can lead to ANRs */ val placeholderText = text.getPlaceholderText() + /** Does not include conditional params */ val params = text.getPlaceholderParameters() val type = UniqueType.uniqueTypeMap[placeholderText] @@ -37,6 +38,7 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s || conditionals.any { it.type == UniqueType.ConditionalTimedUnique } ) + /** Includes conditional params */ val allParams = params + conditionals.flatMap { it.params } val isLocalEffect = params.contains("in this city") || conditionals.any { it.type == UniqueType.ConditionalInThisCity } diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt index 2e57781fa7..1b3425057c 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt @@ -124,7 +124,7 @@ enum class UniqueParameterType( if (ruleset.unitTypes.containsKey(parameterText)) return null if (ruleset.eras.containsKey(parameterText)) return null if (ruleset.unitTypes.values.any { it.uniques.contains(parameterText) }) return null - return UniqueType.UniqueParameterErrorSeverity.WarningOnly + return UniqueType.UniqueParameterErrorSeverity.PossibleFilteringUnique } override fun isTranslationWriterGuess(parameterText: String, ruleset: Ruleset) = @@ -194,7 +194,7 @@ enum class UniqueParameterType( if (parameterText in knownValues) return null if (ruleset.nations.containsKey(parameterText)) return null if (ruleset.nations.values.any { it.hasUnique(parameterText) }) return null - return UniqueType.UniqueParameterErrorSeverity.RulesetSpecific + return UniqueType.UniqueParameterErrorSeverity.PossibleFilteringUnique } }, @@ -251,7 +251,7 @@ enum class UniqueParameterType( if (parameterText in knownValues) return null if (BuildingName.getErrorSeverity(parameterText, ruleset) == null) return null if (ruleset.buildings.values.any { it.hasUnique(parameterText) }) return null - return UniqueType.UniqueParameterErrorSeverity.RulesetSpecific + return UniqueType.UniqueParameterErrorSeverity.PossibleFilteringUnique } override fun isTranslationWriterGuess(parameterText: String, ruleset: Ruleset) = @@ -285,7 +285,7 @@ enum class UniqueParameterType( in ruleset.tileResources -> null in ruleset.terrains.values.asSequence().flatMap { it.uniques } -> null in ruleset.tileResources.values.asSequence().flatMap { it.uniques } -> null - else -> UniqueType.UniqueParameterErrorSeverity.RulesetSpecific + else -> UniqueType.UniqueParameterErrorSeverity.PossibleFilteringUnique } override fun isTranslationWriterGuess(parameterText: String, ruleset: Ruleset) = parameterText in ruleset.terrains || parameterText != "All" && parameterText in knownValues @@ -398,7 +398,7 @@ enum class UniqueParameterType( if (parameterText in knownValues) return null if (ImprovementName.getErrorSeverity(parameterText, ruleset) == null) return null if (ruleset.tileImprovements.values.any { it.hasUnique(parameterText) }) return null - return UniqueType.UniqueParameterErrorSeverity.RulesetSpecific + return UniqueType.UniqueParameterErrorSeverity.PossibleFilteringUnique } override fun isTranslationWriterGuess(parameterText: String, ruleset: Ruleset) = parameterText != "All" && getErrorSeverity(parameterText, ruleset) == null diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt index f9804d0680..17239f5605 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt @@ -1173,7 +1173,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 { + PossibleFilteringUnique { override fun getRulesetErrorSeverity() = RulesetErrorSeverity.WarningOptionsOnly }, diff --git a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt index 00fb87adbf..208b7fbb2d 100644 --- a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt @@ -60,8 +60,9 @@ class RulesetValidator(val ruleset: Ruleset) { private fun getBaseRulesetErrorList(tryFixUnknownUniques: Boolean): RulesetErrorList{ - val lines = RulesetErrorList() + uniqueValidator.populateFilteringUniqueHashsets() + val lines = RulesetErrorList() uniqueValidator.checkUniques(ruleset.globalUniques, lines, true, tryFixUnknownUniques) addUnitErrorsBaseRuleset(lines, tryFixUnknownUniques) diff --git a/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt b/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt index 8fc08d8479..f53f1ce6f3 100644 --- a/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt @@ -13,6 +13,29 @@ import com.unciv.models.stats.INamed class UniqueValidator(val ruleset: Ruleset) { + + private val allNonTypedUniques = HashSet() + private val allUniqueParameters = HashSet() + + private fun addToHashsets(uniqueHolder: IHasUniques) { + for (unique in uniqueHolder.uniqueObjects){ + if (unique.type == null) allNonTypedUniques.add(unique.text) + else allUniqueParameters.addAll(unique.allParams) + } + } + + fun populateFilteringUniqueHashsets(){ + addToHashsets(ruleset.globalUniques) + ruleset.units.values.forEach { addToHashsets(it) } + ruleset.buildings.values.forEach { addToHashsets(it) } + ruleset.unitPromotions.values.forEach { addToHashsets(it) } + ruleset.technologies.values.forEach { addToHashsets(it) } + ruleset.nations.values.forEach { addToHashsets(it) } + ruleset.tileResources.values.forEach { addToHashsets(it) } + ruleset.terrains.values.forEach { addToHashsets(it) } + ruleset.tileImprovements.values.forEach { addToHashsets(it) } + } + fun checkUniques( uniqueContainer: IHasUniques, lines: RulesetErrorList, @@ -150,6 +173,9 @@ class UniqueValidator(val ruleset: Ruleset) { val errorTypesForAcceptableParameters = acceptableParamTypes.map { getParamTypeErrorSeverityCached(it, param) } if (errorTypesForAcceptableParameters.any { it == null }) continue // This matches one of the types! + if (errorTypesForAcceptableParameters.contains(UniqueType.UniqueParameterErrorSeverity.PossibleFilteringUnique) + && param in allUniqueParameters) + continue // This is a filtering param, and the unique it's filtering for actually exists, no problem here! val leastSevereWarning = errorTypesForAcceptableParameters.minByOrNull { it!!.ordinal }!! errorList += UniqueComplianceError(param, acceptableParamTypes, leastSevereWarning) @@ -185,7 +211,7 @@ class UniqueValidator(val ruleset: Ruleset) { } return listOf(RulesetError( - "$prefix unique \"${unique.text}\" not found in Unciv's unique types.", + "$prefix unique \"${unique.text}\" not found in Unciv's unique types, and is not used as a filtering unique.", RulesetErrorSeverity.OK)) } @@ -193,7 +219,7 @@ class UniqueValidator(val ruleset: Ruleset) { // Isolate this decision, to allow easy change of approach // This says: Must have no conditionals or parameters, and is contained in GlobalUniques if (unique.conditionals.isNotEmpty() || unique.params.isNotEmpty()) return false - return unique.text in ruleset.globalUniques.uniqueMap + return unique.text in allUniqueParameters // referenced at least once from elsewhere } private fun tryFixUnknownUnique(unique: Unique, prefix: String): List {