Added test for both sides of filtering unique, to not raise mod warnings on both the filter and the filtered unique

This commit is contained in:
Yair Morgenstern 2023-10-30 13:22:58 +02:00
parent d51b6a679d
commit 6ce685b719
5 changed files with 38 additions and 9 deletions

View File

@ -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 /** 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 */ * - for instance, in the city screen, we call every tile unique for every tile, which can lead to ANRs */
val placeholderText = text.getPlaceholderText() val placeholderText = text.getPlaceholderText()
/** Does not include conditional params */
val params = text.getPlaceholderParameters() val params = text.getPlaceholderParameters()
val type = UniqueType.uniqueTypeMap[placeholderText] 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 } || conditionals.any { it.type == UniqueType.ConditionalTimedUnique }
) )
/** Includes conditional params */
val allParams = params + conditionals.flatMap { it.params } val allParams = params + conditionals.flatMap { it.params }
val isLocalEffect = params.contains("in this city") || conditionals.any { it.type == UniqueType.ConditionalInThisCity } val isLocalEffect = params.contains("in this city") || conditionals.any { it.type == UniqueType.ConditionalInThisCity }

View File

@ -124,7 +124,7 @@ enum class UniqueParameterType(
if (ruleset.unitTypes.containsKey(parameterText)) return null if (ruleset.unitTypes.containsKey(parameterText)) return null
if (ruleset.eras.containsKey(parameterText)) return null if (ruleset.eras.containsKey(parameterText)) return null
if (ruleset.unitTypes.values.any { it.uniques.contains(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) = override fun isTranslationWriterGuess(parameterText: String, ruleset: Ruleset) =
@ -194,7 +194,7 @@ enum class UniqueParameterType(
if (parameterText in knownValues) return null if (parameterText in knownValues) return null
if (ruleset.nations.containsKey(parameterText)) return null if (ruleset.nations.containsKey(parameterText)) return null
if (ruleset.nations.values.any { it.hasUnique(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 (parameterText in knownValues) return null
if (BuildingName.getErrorSeverity(parameterText, ruleset) == null) return null if (BuildingName.getErrorSeverity(parameterText, ruleset) == null) return null
if (ruleset.buildings.values.any { it.hasUnique(parameterText) }) 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) = override fun isTranslationWriterGuess(parameterText: String, ruleset: Ruleset) =
@ -285,7 +285,7 @@ enum class UniqueParameterType(
in ruleset.tileResources -> null in ruleset.tileResources -> null
in ruleset.terrains.values.asSequence().flatMap { it.uniques } -> null in ruleset.terrains.values.asSequence().flatMap { it.uniques } -> null
in ruleset.tileResources.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) = override fun isTranslationWriterGuess(parameterText: String, ruleset: Ruleset) =
parameterText in ruleset.terrains || parameterText != "All" && parameterText in knownValues parameterText in ruleset.terrains || parameterText != "All" && parameterText in knownValues
@ -398,7 +398,7 @@ enum class UniqueParameterType(
if (parameterText in knownValues) return null if (parameterText in knownValues) return null
if (ImprovementName.getErrorSeverity(parameterText, ruleset) == null) return null if (ImprovementName.getErrorSeverity(parameterText, ruleset) == null) return null
if (ruleset.tileImprovements.values.any { it.hasUnique(parameterText) }) 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) = override fun isTranslationWriterGuess(parameterText: String, ruleset: Ruleset) =
parameterText != "All" && getErrorSeverity(parameterText, ruleset) == null parameterText != "All" && getErrorSeverity(parameterText, ruleset) == null

View File

@ -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 a warning, regardless of what ruleset we're in.
* This is for filters that can also potentially accept free text, like UnitFilter and TileFilter */ * This is for filters that can also potentially accept free text, like UnitFilter and TileFilter */
WarningOnly { PossibleFilteringUnique {
override fun getRulesetErrorSeverity() = RulesetErrorSeverity.WarningOptionsOnly override fun getRulesetErrorSeverity() = RulesetErrorSeverity.WarningOptionsOnly
}, },

View File

@ -60,8 +60,9 @@ class RulesetValidator(val ruleset: Ruleset) {
private fun getBaseRulesetErrorList(tryFixUnknownUniques: Boolean): RulesetErrorList{ private fun getBaseRulesetErrorList(tryFixUnknownUniques: Boolean): RulesetErrorList{
val lines = RulesetErrorList() uniqueValidator.populateFilteringUniqueHashsets()
val lines = RulesetErrorList()
uniqueValidator.checkUniques(ruleset.globalUniques, lines, true, tryFixUnknownUniques) uniqueValidator.checkUniques(ruleset.globalUniques, lines, true, tryFixUnknownUniques)
addUnitErrorsBaseRuleset(lines, tryFixUnknownUniques) addUnitErrorsBaseRuleset(lines, tryFixUnknownUniques)

View File

@ -13,6 +13,29 @@ import com.unciv.models.stats.INamed
class UniqueValidator(val ruleset: Ruleset) { class UniqueValidator(val ruleset: Ruleset) {
private val allNonTypedUniques = HashSet<String>()
private val allUniqueParameters = HashSet<String>()
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( fun checkUniques(
uniqueContainer: IHasUniques, uniqueContainer: IHasUniques,
lines: RulesetErrorList, lines: RulesetErrorList,
@ -150,6 +173,9 @@ class UniqueValidator(val ruleset: Ruleset) {
val errorTypesForAcceptableParameters = val errorTypesForAcceptableParameters =
acceptableParamTypes.map { getParamTypeErrorSeverityCached(it, param) } acceptableParamTypes.map { getParamTypeErrorSeverityCached(it, param) }
if (errorTypesForAcceptableParameters.any { it == null }) continue // This matches one of the types! 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 = val leastSevereWarning =
errorTypesForAcceptableParameters.minByOrNull { it!!.ordinal }!! errorTypesForAcceptableParameters.minByOrNull { it!!.ordinal }!!
errorList += UniqueComplianceError(param, acceptableParamTypes, leastSevereWarning) errorList += UniqueComplianceError(param, acceptableParamTypes, leastSevereWarning)
@ -185,7 +211,7 @@ class UniqueValidator(val ruleset: Ruleset) {
} }
return listOf(RulesetError( 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)) RulesetErrorSeverity.OK))
} }
@ -193,7 +219,7 @@ class UniqueValidator(val ruleset: Ruleset) {
// Isolate this decision, to allow easy change of approach // Isolate this decision, to allow easy change of approach
// This says: Must have no conditionals or parameters, and is contained in GlobalUniques // This says: Must have no conditionals or parameters, and is contained in GlobalUniques
if (unique.conditionals.isNotEmpty() || unique.params.isNotEmpty()) return false 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<RulesetError> { private fun tryFixUnknownUnique(unique: Unique, prefix: String): List<RulesetError> {