Validation warning Suppression as Unique or modifier (#11142)

* Make source object available to RulesetErrorList.add

* Make ruleset available to RulesetErrorList

* Introduce UniqueTarget.MetaModifier - ModifierHiddenFromUsers won't stay alone

* There was no use of prefix without the unique name - fold

* Introduce UniqueTarget.MetaModifier - doc

* Pass context down even inside UniqueValidator, convenience factory for limited RulesetErrorList's

* Clean up RulesetErrorList.of factory

* Reorder parameters of RulesetErrorList.add

* Suppression Unique and implementation

* Remove logging
This commit is contained in:
SomeTroglodyte
2024-03-03 19:02:54 +01:00
committed by GitHub
parent 657ec94f4a
commit 69c047790e
8 changed files with 194 additions and 29 deletions

View File

@ -7,8 +7,8 @@ enum class UniqueFlag {
NoConditionals,
;
companion object {
val none: EnumSet<UniqueFlag> = EnumSet.noneOf(UniqueFlag::class.java)
val setOfHiddenToUsers: EnumSet<UniqueFlag> = EnumSet.of(HiddenToUsers)
val setOfNoConditionals: EnumSet<UniqueFlag> = EnumSet.of(NoConditionals)
val setOfHiddenNoConditionals: EnumSet<UniqueFlag> = EnumSet.of(HiddenToUsers, NoConditionals)
}
}

View File

@ -8,6 +8,7 @@ import com.unciv.models.ruleset.Ruleset
import com.unciv.models.ruleset.RulesetCache
import com.unciv.models.ruleset.tile.ResourceType
import com.unciv.models.ruleset.unique.UniqueParameterType.Companion.guessTypeForTranslationWriter
import com.unciv.models.ruleset.validation.Suppression
import com.unciv.models.stats.Stat
import com.unciv.models.translations.TranslationFileWriter
@ -600,6 +601,13 @@ enum class UniqueParameterType(
override fun getTranslationWriterStringsForOutput() = scanExistingValues(this)
},
/** Suppress RulesetValidator warnings: Parameter check delegated to RulesetValidator, and auto-translation off. */
ValidationWarning("validationWarning", Suppression.parameterDocExample, Suppression.parameterDocDescription, "Mod-check warning") {
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? =
if (Suppression.isValidFilter(parameterText)) null
else UniqueType.UniqueParameterErrorSeverity.RulesetInvariant
},
/** Behaves like [Unknown], but states explicitly the parameter is OK and its contents are ignored */
Comment("comment", "comment", null, "Unique Specials") {
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset):

View File

@ -86,5 +86,11 @@ enum class UniqueTarget(
Building, Unit, UnitType, Improvement, Tech, FollowerBelief, FounderBelief,
Terrain, Resource, Policy, Promotion, Nation, Ruins, Speed
)
val CanIncludeSuppression = arrayOf(
Triggerable, // Includes Global and covers most IHasUnique's
Terrain, Speed, // IHasUnique targets without inheritsFrom
ModOptions, // For suppressions that target something that doesn't have Uniques
MetaModifier // Allows use as Conditional-like syntax
)
}
}

View File

@ -3,9 +3,9 @@ package com.unciv.models.ruleset.unique
import com.unciv.Constants
import com.unciv.models.ruleset.validation.RulesetErrorSeverity
import com.unciv.models.ruleset.validation.RulesetValidator
import com.unciv.models.ruleset.validation.Suppression
import com.unciv.models.translations.getPlaceholderParameters
import com.unciv.models.translations.getPlaceholderText
import java.util.EnumSet
// I didn't put this in a companion object because APPARENTLY doing that means you can't use it in the init function.
private val numberRegex = Regex("\\d+$") // Any number of trailing digits
@ -13,7 +13,7 @@ private val numberRegex = Regex("\\d+$") // Any number of trailing digits
enum class UniqueType(
val text: String,
vararg targets: UniqueTarget,
val flags: EnumSet<UniqueFlag> = UniqueFlag.none,
val flags: Set<UniqueFlag> = emptySet(),
val docDescription: String? = null
) {
@ -835,6 +835,8 @@ enum class UniqueType(
AllowRazeCapital("Allow raze capital", UniqueTarget.ModOptions, flags = UniqueFlag.setOfNoConditionals),
AllowRazeHolyCity("Allow raze holy city", UniqueTarget.ModOptions, flags = UniqueFlag.setOfNoConditionals),
SuppressWarnings("Suppress warning [validationWarning]", *UniqueTarget.CanIncludeSuppression, flags = UniqueFlag.setOfHiddenNoConditionals, docDescription = Suppression.uniqueDocDescription),
// Declarative Mod compatibility (see [ModCompatibility]):
// Note there is currently no display for these, but UniqueFlag.HiddenToUsers is not set.
// That means we auto-template and ask our translators for a translation that is currently unused.

View File

@ -3,6 +3,9 @@ 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
import com.unciv.models.ruleset.unique.StateForConditionals
import com.unciv.models.ruleset.unique.Unique
import com.unciv.models.ruleset.unique.UniqueType
class RulesetError(val text: String, val errorSeverityToReport: RulesetErrorSeverity)
@ -20,30 +23,40 @@ enum class RulesetErrorSeverity(val color: Color) {
* 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<RulesetError>() {
private val globalSuppressionFilters: Set<String> =
ruleset?.modOptions
?.getMatchingUniques(UniqueType.SuppressWarnings, StateForConditionals.IgnoreConditionals)
?.map { it.params[0] }
?.toSet()
?: emptySet()
/** 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`.
* @param sourceObject the originating object, which can host suppressions. When it is not known or not a [IHasUniques], pass `null`.
* @param sourceUnique the originating unique, so look for suppression modifiers. Leave `null` if unavailable.
*/
fun add(element: RulesetError, sourceObject: IHasUniques?): Boolean {
// Suppression to be checked here
return addWithDuplicateCheck(element)
fun add(element: RulesetError, sourceObject: IHasUniques?, sourceUnique: Unique? = null): Boolean {
// The dupe check may be faster than the Suppression check, so do it first?
if (!removeLowerSeverityDuplicate(element)) return false
if (Suppression.isErrorSuppressed(globalSuppressionFilters, sourceObject, sourceUnique, element))
return false
return super.add(element)
}
/** 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`.
* @param sourceObject the originating object, which can host suppressions. When it is not known or not a [IHasUniques], pass `null`.
* @param sourceUnique the originating unique, so look for suppression modifiers. Leave `null` if unavailable.
*/
fun add(text: String, errorSeverityToReport: RulesetErrorSeverity = RulesetErrorSeverity.Error, sourceObject: IHasUniques?) =
add(RulesetError(text, errorSeverityToReport), sourceObject)
fun add(text: String, errorSeverityToReport: RulesetErrorSeverity = RulesetErrorSeverity.Error, sourceObject: IHasUniques? = null, sourceUnique: Unique? = null) =
add(RulesetError(text, errorSeverityToReport), sourceObject, sourceUnique)
@Deprecated("No adding without explicit source object", ReplaceWith("add(element, sourceObject)"))
@Deprecated("No adding without explicit source object", ReplaceWith("add(element, sourceObject, sourceUnique)"))
override fun add(element: RulesetError) = super.add(element)
/** Add all [elements] with duplicate check, but without suppression check */
@ -93,14 +106,17 @@ class RulesetErrorList(
}
companion object {
/** Helper factory for a single entry list (which can result in an empty list due to suppression)
* Note: Valid source for [addAll] since suppression is already taken care of. */
fun of(
text: String,
severity: RulesetErrorSeverity = RulesetErrorSeverity.Error,
ruleset: Ruleset? = null,
sourceObject: IHasUniques? = null
sourceObject: IHasUniques? = null,
sourceUnique: Unique? = null
): RulesetErrorList {
val result = RulesetErrorList(ruleset)
result.add(text, severity, sourceObject)
result.add(text, severity, sourceObject, sourceUnique)
return result
}
}

View File

@ -0,0 +1,125 @@
package com.unciv.models.ruleset.validation
import com.unciv.json.json
import com.unciv.logic.UncivShowableException
import com.unciv.models.ruleset.ModOptions
import com.unciv.models.ruleset.Ruleset
import com.unciv.models.ruleset.unique.IHasUniques
import com.unciv.models.ruleset.unique.StateForConditionals
import com.unciv.models.ruleset.unique.Unique
import com.unciv.models.ruleset.unique.UniqueParameterType
import com.unciv.models.ruleset.unique.UniqueType
import com.unciv.models.translations.fillPlaceholders
/**
* All public methods dealing with how Mod authors can suppress RulesetValidator output.
*
* This allows the outside code to be agnostic about how each entry operates, it's all here, and can easily be expanded.
* [uniqueDocDescription], [parameterDocDescription], [parameterDocExample], [isErrorSuppressed] and [isValidFilter] need to agree on the rules!
* Note there is minor influence on the rules where [RulesetErrorList.add] is called, as supplying null for its source parameters limits the scope where suppressions are found.
*
* Current decisions:
* * You cannot suppress [RulesetErrorSeverity.Error] level messages.
* * Each suppression entry is either compared verbatim or as primitive wildcard pattern with '*' on both ends, case-insensitive.
* * Minimum selectivity of [minimumSelectivity] characters matching.
* * Validation of the suppression entries themselves is rudimentary.
*/
object Suppression {
/** minimum match length for a valid suppression filter */
private const val minimumSelectivity = 12 // arbitrary
/** Delegated from [UniqueType.SuppressWarnings] */
const val uniqueDocDescription = "Allows suppressing specific validation warnings." +
" Errors, deprecation warnings, or warnings about untyped and non-filtering uniques should be heeded, not suppressed, and are therefore not accepted." +
" Note that this can be used in ModOptions, in the uniques a warning is about, or as modifier on the unique triggering a warning -" +
" but you still need to be specific. Even in the modifier case you will need to specify a sufficiently selective portion of the warning text as parameter."
/** Delegated from [UniqueParameterType.ValidationWarning] */
const val parameterDocDescription = "Suppresses one specific Ruleset validation warning. " +
"This can specify the full text verbatim including correct upper/lower case, " +
"or it can be a wildcard case-insensitive simple pattern starting and ending in an asterisk ('*'). " +
"If the suppression unique is used within an object or as modifier (not ModOptions), " +
"the wildcard symbols can be omitted, as selectivity is better due to the limited scope."
/** Delegated from [UniqueParameterType.ValidationWarning] */
const val parameterDocExample = "Tinman is supposed to automatically upgrade at tech Clockwork, and therefore Servos for its upgrade Mecha may not yet be researched!" +
" -or- *is supposed to automatically upgrade*"
private const val deprecationWarningPattern = """unique "~" is deprecated as of ~, replace with"""
private const val untypedWarningPattern = """unique "~" not found in Unciv's unique types, and is not used as a filtering unique"""
/** Determine whether [parameterText] is a valid Suppression filter as implemented by [isErrorSuppressed] */
fun isValidFilter(parameterText: String) = when {
// Cannot contain {} or <>
'{' in parameterText || '<' in parameterText -> false
// Must not be a deprecation - these should be implemented by their replacement not suppressed
hasCommonSubstringLength(parameterText, deprecationWarningPattern, minimumSelectivity) -> false
// Must not be a untyped/nonfiltering warning (a case for the Comment UniqueType instead)
hasCommonSubstringLength(parameterText, untypedWarningPattern, minimumSelectivity) -> false
// Check wildcard suppression - '*' on both ends, rest of pattern selective enough
parameterText.startsWith('*') != parameterText.endsWith('*') -> false
parameterText.length < minimumSelectivity + 2 -> false
// More rules here???
else -> true
}
private fun matchesFilter(error: RulesetError, filter: String): Boolean {
if (error.text == filter) return true
if (!filter.endsWith('*') || !filter.startsWith('*')) return false
return error.text.contains(filter.removeSurrounding("*", "*"), ignoreCase = true)
}
/** Determine if [error] matches any suppression Unique in [ModOptions] or the [sourceObject], or any suppression modifier in [sourceUnique] */
internal fun isErrorSuppressed(
globalSuppressionFilters: Collection<String>,
sourceObject: IHasUniques?,
sourceUnique: Unique?,
error: RulesetError
): Boolean {
if (error.errorSeverityToReport >= RulesetErrorSeverity.Error) return false
if (sourceObject == null && globalSuppressionFilters.isEmpty()) return false
fun getWildcardFilter(unique: Unique) = unique.params[0].let {
if (it.startsWith('*')) it else "*$it*"
}
// Allow suppressing from ModOptions
var suppressions = globalSuppressionFilters.asSequence()
// Allow suppressing from suppression uniques in the same Unique collection
if (sourceObject != null)
suppressions += sourceObject.getMatchingUniques(UniqueType.SuppressWarnings, StateForConditionals.IgnoreConditionals).map { getWildcardFilter(it) }
// Allow suppressing from modifiers in the same Unique
if (sourceUnique != null)
suppressions += sourceUnique.conditionals.filter { it.type == UniqueType.SuppressWarnings }.map { getWildcardFilter(it) }
for (filter in suppressions)
if (matchesFilter(error, filter)) return true
return false
}
/** Whosoever maketh this here methyd available to evil Mod whytches shall forever be cursed and damned to all Hell's tortures */
@Suppress("unused") // Debug tool
fun autoSuppressAllWarnings(ruleset: Ruleset, toModOptions: ModOptions) {
if (ruleset.folderLocation == null)
throw UncivShowableException("autoSuppressAllWarnings needs Ruleset.folderLocation")
for (error in RulesetValidator(ruleset).getErrorList()) {
if (error.errorSeverityToReport >= RulesetErrorSeverity.Error) continue
toModOptions.uniques += UniqueType.SuppressWarnings.text.fillPlaceholders(error.text)
}
json().toJson(toModOptions, ruleset.folderLocation!!.child("jsons/ModOptions.json"))
}
@Suppress("SameParameterValue") // Yes we're using a constant up there, still reads clearer
private fun hasCommonSubstringLength(x: String, y: String, minCommonLength: Int): Boolean {
// This is brute-force, but adapting a public "longest-common-substring" algorithm was complex and still slooow.
// Using the knowledge that we're only interested in the common length exceeding a threshold saves time.
// This uses the fact that Int.until will _not_ throw on upper < lower.
for (xIndex in 0 until x.length - minCommonLength) {
val xSub = x.substring(xIndex)
for (yIndex in 0 until y.length - minCommonLength) {
if (xSub.commonPrefixWith(y.substring(yIndex), true).length >= minCommonLength)
return true
}
}
return false
}
}

View File

@ -67,17 +67,18 @@ class UniqueValidator(val ruleset: Ruleset) {
val rulesetErrors = RulesetErrorList(ruleset)
if (uniqueContainer != null && !unique.type.canAcceptUniqueTarget(uniqueContainer.getUniqueTarget()))
rulesetErrors.add("$prefix is not allowed on its target type", RulesetErrorSeverity.Warning, uniqueContainer)
rulesetErrors.add("$prefix is not allowed on its target type", RulesetErrorSeverity.Warning, uniqueContainer, unique)
val typeComplianceErrors = getComplianceErrors(unique)
for (complianceError in typeComplianceErrors) {
if (!reportRulesetSpecificErrors && complianceError.errorSeverity == UniqueType.UniqueParameterErrorSeverity.RulesetSpecific)
continue
rulesetErrors.add("$prefix 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(), uniqueContainer
complianceError.errorSeverity.getRulesetErrorSeverity(), uniqueContainer, unique
)
}
@ -94,7 +95,7 @@ class UniqueValidator(val ruleset: Ruleset) {
"$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, uniqueContainer
RulesetErrorSeverity.OK, uniqueContainer, unique
)
if (reportRulesetSpecificErrors)
@ -117,7 +118,7 @@ class UniqueValidator(val ruleset: Ruleset) {
rulesetErrors.add(
"$prefix contains the conditional \"${conditional.text}\"," +
" but the unique does not accept conditionals!",
RulesetErrorSeverity.Error, uniqueContainer
RulesetErrorSeverity.Error, uniqueContainer, unique
)
return
}
@ -136,7 +137,7 @@ class UniqueValidator(val ruleset: Ruleset) {
text += " May be a misspelling of \""+ similarConditionals.joinToString("\", or \"") { it.text } +"\""
rulesetErrors.add(
text,
RulesetErrorSeverity.Warning, uniqueContainer
RulesetErrorSeverity.Warning, uniqueContainer, unique
)
return
}
@ -145,7 +146,7 @@ class UniqueValidator(val ruleset: Ruleset) {
rulesetErrors.add(
"$prefix contains the conditional \"${conditional.text}\"," +
" which is a Unique type not allowed as conditional or trigger.",
RulesetErrorSeverity.Warning, uniqueContainer
RulesetErrorSeverity.Warning, uniqueContainer, unique
)
if (conditional.type.targetTypes.contains(UniqueTarget.UnitActionModifier)
@ -154,7 +155,7 @@ class UniqueValidator(val ruleset: Ruleset) {
rulesetErrors.add(
"$prefix contains the conditional \"${conditional.text}\"," +
" which as a UnitActionModifier is only allowed on UnitAction uniques.",
RulesetErrorSeverity.Warning, uniqueContainer
RulesetErrorSeverity.Warning, uniqueContainer, unique
)
val conditionalComplianceErrors =
@ -168,7 +169,7 @@ class UniqueValidator(val ruleset: Ruleset) {
"$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
complianceError.errorSeverity.getRulesetErrorSeverity(), uniqueContainer, unique
)
}
}
@ -189,7 +190,7 @@ class UniqueValidator(val ruleset: Ruleset) {
RulesetErrorSeverity.WarningOptionsOnly // Not user-visible
else RulesetErrorSeverity.Warning // User visible
rulesetErrors.add(deprecationText, severity, uniqueContainer)
rulesetErrors.add(deprecationText, severity, uniqueContainer, unique)
}
}
@ -232,7 +233,7 @@ class UniqueValidator(val ruleset: Ruleset) {
if (unique.text.count { it == '<' } != unique.text.count { it == '>' })
return RulesetErrorList.of(
"$prefix contains mismatched conditional braces!",
RulesetErrorSeverity.Warning, ruleset, uniqueContainer
RulesetErrorSeverity.Warning, ruleset, uniqueContainer, unique
)
// Support purely filtering Uniques without actual implementation
@ -245,7 +246,7 @@ class UniqueValidator(val ruleset: Ruleset) {
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
ruleset, uniqueContainer, unique
)
}
@ -270,7 +271,7 @@ class UniqueValidator(val ruleset: Ruleset) {
equalUniques.isNotEmpty() -> RulesetErrorList.of(
"$prefix looks like it should be fine, but for some reason isn't recognized.",
RulesetErrorSeverity.OK,
ruleset, uniqueContainer
ruleset, uniqueContainer, unique
)
similarUniques.isNotEmpty() -> {
@ -284,7 +285,7 @@ class UniqueValidator(val ruleset: Ruleset) {
if (uniqueType.getDeprecationAnnotation() != null) text += " (Deprecated)"
return@joinToString text
}.prependIndent("\t")
RulesetErrorList.of(text, RulesetErrorSeverity.OK, ruleset, uniqueContainer)
RulesetErrorList.of(text, RulesetErrorSeverity.OK, ruleset, uniqueContainer, unique)
}
else -> RulesetErrorList()
}

View File

@ -159,6 +159,12 @@ Simple unique parameters are explained by mouseover. Complex parameters are expl
Applicable to: Triggerable
??? example "Suppress warning [validationWarning]"
Allows suppressing specific validation warnings. Errors, deprecation warnings, or warnings about untyped and non-filtering uniques should be heeded, not suppressed, and are therefore not accepted. Note that this can be used in ModOptions, in the uniques a warning is about, or as modifier on the unique triggering a warning - but you still need to be specific. Even in the modifier case you will need to specify a sufficiently selective portion of the warning text as parameter.
Example: "Suppress warning [Tinman is supposed to automatically upgrade at tech Clockwork, and therefore Servos for its upgrade Mecha may not yet be researched! -or- *is supposed to automatically upgrade*]"
Applicable to: Triggerable, Terrain, Speed, ModOptions, MetaModifier
## UnitTriggerable uniques
!!! note ""
@ -2369,4 +2375,5 @@ Simple unique parameters are explained by mouseover. Complex parameters are expl
*[stockpiledResource]: The name of any stockpiled.
*[tech]: The name of any tech.
*[tileFilter]: Anything that can be used either in an improvementFilter or in a terrainFilter can be used here, plus 'unimproved'
*[validationWarning]: Suppresses one specific Ruleset validation warning. This can specify the full text verbatim including correct upper/lower case, or it can be a wildcard case-insensitive simple pattern starting and ending in an asterisk ('*'). If the suppression unique is used within an object or as modifier (not ModOptions), the wildcard symbols can be omitted, as selectivity is better due to the limited scope.
*[victoryType]: The name of any victory type: 'Neutral', 'Cultural', 'Diplomatic', 'Domination', 'Scientific', 'Time'