Skip to content

Commit

Permalink
Added test for both sides of filtering unique, to not raise mod warni…
Browse files Browse the repository at this point in the history
…ngs on both the filter and the filtered unique
  • Loading branch information
yairm210 committed Oct 30, 2023
1 parent d51b6a6 commit 6ce685b
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 9 deletions.
2 changes: 2 additions & 0 deletions core/src/com/unciv/models/ruleset/unique/Unique.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand All @@ -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 }
Expand Down
10 changes: 5 additions & 5 deletions core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand Down Expand Up @@ -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
}
},

Expand Down Expand Up @@ -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) =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/src/com/unciv/models/ruleset/unique/UniqueType.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 28 additions & 2 deletions core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,29 @@ import com.unciv.models.stats.INamed

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(
uniqueContainer: IHasUniques,
lines: RulesetErrorList,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -185,15 +211,15 @@ 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))
}

private fun isFilteringUniqueAllowed(unique: Unique): Boolean {
// 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<RulesetError> {
Expand Down

0 comments on commit 6ce685b

Please sign in to comment.