From 3220d3c52bcd3a1ce2ec1e9e35c3794581161f50 Mon Sep 17 00:00:00 2001 From: yairm210 Date: Fri, 1 Aug 2025 16:05:04 +0300 Subject: [PATCH] chore(purity): UniqueValidator --- .../unciv/models/ruleset/unique/Countables.kt | 2 +- .../com/unciv/models/ruleset/unique/Unique.kt | 3 +- .../ruleset/unique/expressions/Expressions.kt | 2 + .../ruleset/validation/UniqueValidator.kt | 98 ++++++++++++------- 4 files changed, 66 insertions(+), 39 deletions(-) diff --git a/core/src/com/unciv/models/ruleset/unique/Countables.kt b/core/src/com/unciv/models/ruleset/unique/Countables.kt index f37fdbcd58..74500a0cdc 100644 --- a/core/src/com/unciv/models/ruleset/unique/Countables.kt +++ b/core/src/com/unciv/models/ruleset/unique/Countables.kt @@ -285,7 +285,7 @@ enum class Countables( /** Leave this only for Countables without any parameters - they can rely on [matches] having validated enough */ open fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? = null - fun getDeprecationAnnotation(): Deprecated? = declaringJavaClass.getField(name).getAnnotation(Deprecated::class.java) + @Readonly fun getDeprecationAnnotation(): Deprecated? = declaringJavaClass.getField(name).getAnnotation(Deprecated::class.java) protected fun UniqueParameterType.getTranslatedErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? = getErrorSeverity(parameterText.getPlaceholderParameters().first(), ruleset) diff --git a/core/src/com/unciv/models/ruleset/unique/Unique.kt b/core/src/com/unciv/models/ruleset/unique/Unique.kt index 74f3a294f3..5c96075de0 100644 --- a/core/src/com/unciv/models/ruleset/unique/Unique.kt +++ b/core/src/com/unciv/models/ruleset/unique/Unique.kt @@ -169,6 +169,7 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s return -1 // Not found } + @Readonly fun getReplacementText(ruleset: Ruleset): String { val deprecationAnnotation = getDeprecationAnnotation() ?: return "" val replacementUniqueText = deprecationAnnotation.replaceWith.expression @@ -180,7 +181,7 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s // note this is only done for the replacement, not the deprecated unique, thus parameters of // conditionals on the deprecated unique are ignored - val finalPossibleUniques = ArrayList() + @LocalState val finalPossibleUniques = ArrayList() for (possibleUnique in possibleUniques) { var resultingUnique = possibleUnique diff --git a/core/src/com/unciv/models/ruleset/unique/expressions/Expressions.kt b/core/src/com/unciv/models/ruleset/unique/expressions/Expressions.kt index 148fc89f64..4c8c38cf2c 100644 --- a/core/src/com/unciv/models/ruleset/unique/expressions/Expressions.kt +++ b/core/src/com/unciv/models/ruleset/unique/expressions/Expressions.kt @@ -58,9 +58,11 @@ class Expressions { } } + @Readonly fun getParsingError(parameterText: String): Parser.ParsingError? = parse(parameterText).exception + @Readonly fun getCountableErrors(parameterText: String, ruleset: Ruleset): List { val parseResult = parse(parameterText) return if (parseResult.node == null) emptyList() else parseResult.node.getErrors(ruleset) diff --git a/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt b/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt index 46ab9c4bad..13eb92a51a 100644 --- a/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt @@ -15,6 +15,9 @@ import com.unciv.models.ruleset.unique.UniqueParameterType import com.unciv.models.ruleset.unique.UniqueTarget import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.ruleset.unique.expressions.Expressions +import yairm210.purity.annotations.Cache +import yairm210.purity.annotations.LocalState +import yairm210.purity.annotations.Readonly class UniqueValidator(val ruleset: Ruleset) { @@ -63,6 +66,7 @@ class UniqueValidator(val ruleset: Ruleset) { UniqueType.ConditionalNotAdjacentTo ) + @Readonly fun checkUnique( unique: Unique, tryFixUnknownUniques: Boolean, @@ -72,7 +76,7 @@ class UniqueValidator(val ruleset: Ruleset) { val prefix by lazy { getUniqueContainerPrefix(uniqueContainer) + "\"${unique.text}\"" } if (unique.type == null) return checkUntypedUnique(unique, tryFixUnknownUniques, uniqueContainer, prefix, reportRulesetSpecificErrors) - val rulesetErrors = RulesetErrorList(ruleset) + @LocalState val rulesetErrors = RulesetErrorList(ruleset) if (uniqueContainer != null && !(unique.type.canAcceptUniqueTarget(uniqueContainer.getUniqueTarget()) || @@ -105,14 +109,14 @@ class UniqueValidator(val ruleset: Ruleset) { complianceError.errorSeverity.getRulesetErrorSeverity(), uniqueContainer, unique ) - addExpressionParseErrors(complianceError, rulesetErrors, uniqueContainer, unique) + rulesetErrors += getExpressionParseErrors(complianceError, uniqueContainer, unique) } for (conditional in unique.modifiers) { - addConditionalErrors(conditional, rulesetErrors, prefix, unique, uniqueContainer, reportRulesetSpecificErrors) + rulesetErrors += getConditionalErrors(conditional, prefix, unique, uniqueContainer, reportRulesetSpecificErrors) } - addUniqueTypeSpecificErrors(rulesetErrors, prefix, unique, uniqueContainer, reportRulesetSpecificErrors) + rulesetErrors += getUniqueTypeSpecificErrors(prefix, unique, uniqueContainer, reportRulesetSpecificErrors) val conditionals = unique.modifiers.filter { it.type?.canAcceptUniqueTarget(UniqueTarget.Conditional) == true } if (conditionals.size > 1){ @@ -142,18 +146,19 @@ class UniqueValidator(val ruleset: Ruleset) { 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, uniqueContainer) + rulesetErrors += addDeprecationAnnotationErrors(unique, prefix, uniqueContainer) return rulesetErrors } - private fun addExpressionParseErrors( + @Readonly + private fun getExpressionParseErrors( complianceError: UniqueComplianceError, - rulesetErrors: RulesetErrorList, uniqueContainer: IHasUniques?, unique: Unique - ) { - if (!complianceError.acceptableParameterTypes.contains(UniqueParameterType.Countable)) return + ): RulesetErrorList { + @LocalState val rulesetErrors = RulesetErrorList(ruleset) + if (!complianceError.acceptableParameterTypes.contains(UniqueParameterType.Countable)) return rulesetErrors val parseError = Expressions.getParsingError(complianceError.parameterName) if (parseError != null) { @@ -165,7 +170,7 @@ class UniqueValidator(val ruleset: Ruleset) { val text = "\"${complianceError.parameterName}\" could not be parsed as an expression due to:" + " ${parseError.message}. \n$parameterWithErrorLocationMarked" rulesetErrors.add(text, RulesetErrorSeverity.WarningOptionsOnly, uniqueContainer, unique) - return + return rulesetErrors } val countableErrors = Expressions.getCountableErrors(complianceError.parameterName, ruleset) @@ -174,6 +179,7 @@ class UniqueValidator(val ruleset: Ruleset) { " ${countableErrors.joinToString(", ")}" rulesetErrors.add(text, RulesetErrorSeverity.WarningOptionsOnly, uniqueContainer, unique) } + return rulesetErrors } private val resourceUniques = setOf(UniqueType.ProvidesResources, UniqueType.ConsumesResources, @@ -186,21 +192,37 @@ class UniqueValidator(val ruleset: Ruleset) { UniqueType.ConditionalWhenBelowAmountStatResource, ) - private fun addConditionalErrors( + @Readonly + private fun getUniqueTypeSpecificErrors( + prefix: String, unique: Unique, uniqueContainer: IHasUniques?, reportRulesetSpecificErrors: Boolean + ): RulesetErrorList { + @LocalState val rulesetErrors = RulesetErrorList(ruleset) + when (unique.type) { + UniqueType.RuinsUpgrade -> { + if (reportRulesetSpecificErrors && !anyAncientRuins) + rulesetErrors.add("$prefix is pointless - there are no ancient ruins", RulesetErrorSeverity.Warning, uniqueContainer, unique) + } + else -> {} + } + return rulesetErrors + } + + @Readonly + private fun getConditionalErrors( conditional: Unique, - rulesetErrors: RulesetErrorList, prefix: String, unique: Unique, uniqueContainer: IHasUniques?, reportRulesetSpecificErrors: Boolean - ) { + ): RulesetErrorList { + @LocalState val rulesetErrors = RulesetErrorList(ruleset) if (unique.hasFlag(UniqueFlag.NoConditionals)) { rulesetErrors.add( "$prefix contains the conditional \"${conditional.text}\"," + " but the unique does not accept conditionals!", RulesetErrorSeverity.Error, uniqueContainer, unique ) - return + return rulesetErrors } if (conditional.type == null) { @@ -219,7 +241,7 @@ class UniqueValidator(val ruleset: Ruleset) { text, RulesetErrorSeverity.Warning, uniqueContainer, unique ) - return + return rulesetErrors } if (conditional.type.targetTypes.none { it.modifierType != UniqueTarget.ModifierType.None }) @@ -274,30 +296,21 @@ class UniqueValidator(val ruleset: Ruleset) { complianceError.errorSeverity.getRulesetErrorSeverity(), uniqueContainer, unique ) - addExpressionParseErrors(complianceError, rulesetErrors, uniqueContainer, unique) + rulesetErrors += getExpressionParseErrors(complianceError, uniqueContainer, unique) } - addDeprecationAnnotationErrors(conditional, "$prefix contains modifier \"${conditional.text}\" which", rulesetErrors, uniqueContainer) - } - - private fun addUniqueTypeSpecificErrors( - rulesetErrors: RulesetErrorList, prefix: String, unique: Unique, uniqueContainer: IHasUniques?, reportRulesetSpecificErrors: Boolean - ) { - when(unique.type) { - UniqueType.RuinsUpgrade -> { - if (reportRulesetSpecificErrors && !anyAncientRuins) - rulesetErrors.add("$prefix is pointless - there are no ancient ruins", RulesetErrorSeverity.Warning, uniqueContainer, unique) - } - else -> return - } + addDeprecationAnnotationErrors(conditional, "$prefix contains modifier \"${conditional.text}\" which", uniqueContainer) + + return rulesetErrors } + @Readonly private fun addDeprecationAnnotationErrors( unique: Unique, prefix: String, - rulesetErrors: RulesetErrorList, uniqueContainer: IHasUniques? - ) { + ): RulesetErrorList { + @LocalState val rulesetErrors = RulesetErrorList(ruleset) val deprecationAnnotation = unique.getDeprecationAnnotation() if (deprecationAnnotation != null) { val replacementUniqueText = unique.getReplacementText(ruleset) @@ -312,12 +325,13 @@ class UniqueValidator(val ruleset: Ruleset) { } // Check for deprecated Countables - if (unique.type == null) return + if (unique.type == null) return rulesetErrors val countables = unique.type.parameterTypeMap.withIndex() .filter { UniqueParameterType.Countable in it.value } .map { unique.params[it.index] } .mapNotNull { Countables.getMatching(it, ruleset) } + for (countable in countables) { val deprecation = countable.getDeprecationAnnotation() ?: continue // This is less flexible than unique.getReplacementText(ruleset) @@ -329,14 +343,18 @@ class UniqueValidator(val ruleset: Ruleset) { else RulesetErrorSeverity.ErrorOptionsOnly // User visible in new game and red in options rulesetErrors.add(text, severity, uniqueContainer, unique) } + + return rulesetErrors } /** Maps uncompliant parameters to their required types */ + @Readonly private fun getComplianceErrors( unique: Unique, ): List { if (unique.type == null) return emptyList() - val errorList = ArrayList() + @LocalState val errorList = ArrayList() + for ((index, param) in unique.params.withIndex()) { // Trying to catch the error at #11404 if (unique.type.parameterTypeMap.size != unique.params.size) { @@ -361,11 +379,13 @@ class UniqueValidator(val ruleset: Ruleset) { return errorList } - private val paramTypeErrorSeverityCache = HashMap>() + @Cache private val paramTypeErrorSeverityCache = HashMap>() + @Readonly private fun getParamTypeErrorSeverityCached(uniqueParameterType: UniqueParameterType, param: String): UniqueType.UniqueParameterErrorSeverity? { if (!paramTypeErrorSeverityCache.containsKey(uniqueParameterType)) paramTypeErrorSeverityCache[uniqueParameterType] = hashMapOf() - val uniqueParamCache = paramTypeErrorSeverityCache[uniqueParameterType]!! + + @LocalState val uniqueParamCache = paramTypeErrorSeverityCache[uniqueParameterType]!! if (uniqueParamCache.containsKey(param)) return uniqueParamCache[param] @@ -374,6 +394,7 @@ class UniqueValidator(val ruleset: Ruleset) { return severity } + @Readonly private fun checkUntypedUnique( unique: Unique, tryFixUnknownUniques: Boolean, @@ -389,7 +410,7 @@ class UniqueValidator(val ruleset: Ruleset) { ) // Support purely filtering Uniques without actual implementation - if (isFilteringUniqueAllowed(unique, reportRulesetSpecificErrors)) return RulesetErrorList() + if (isFilteringUniqueAllowed(unique, reportRulesetSpecificErrors)) return RulesetErrorList(ruleset) if (tryFixUnknownUniques) { val fixes = tryFixUnknownUnique(unique, uniqueContainer, prefix) @@ -403,6 +424,7 @@ class UniqueValidator(val ruleset: Ruleset) { ) } + @Readonly private fun isFilteringUniqueAllowed(unique: Unique, reportRulesetSpecificErrors: Boolean): Boolean { // Isolate this decision, to allow easy change of approach // This says: Must have no conditionals or parameters, and is used in any "filtering" parameter of another Unique @@ -411,6 +433,7 @@ class UniqueValidator(val ruleset: Ruleset) { return unique.text in allUniqueParameters // referenced at least once from elsewhere } + @Readonly private fun tryFixUnknownUnique(unique: Unique, uniqueContainer: IHasUniques?, prefix: String): RulesetErrorList { val similarUniques = UniqueType.entries.filter { getRelativeTextDistance( @@ -441,13 +464,14 @@ class UniqueValidator(val ruleset: Ruleset) { }.prependIndent("\t") RulesetErrorList.of(text, RulesetErrorSeverity.OK, ruleset, uniqueContainer, unique) } - else -> RulesetErrorList() + else -> RulesetErrorList(ruleset) } } companion object { const val whichDoesNotFitParameterType = "which does not fit parameter type" + @Readonly internal fun getUniqueContainerPrefix(uniqueContainer: IHasUniques?) = (if (uniqueContainer is IRulesetObject) "${uniqueContainer.originRuleset}: " else "") + (if (uniqueContainer == null) "The" else "(${uniqueContainer.getUniqueTarget().name}) ${uniqueContainer.name}'s") +