AnarchistGamers/Unciv
1
0
Fork 0
mirror of https://github.com/yairm210/Unciv.git synced 2025-08-03 20:48:49 -04:00
Code Issues Packages Projects Releases Wiki Activity

Countables : Next "pass" (#13168)

Browse Source 
* Fix equalsPlaceholderText again

* Fix implementation details and linting

* Fix placeholder parameters validation

* Edit Kdoc to match new structure and improve clarity

* Add unit test checking the Class itself for convention violations

* Split off the ForEveryCountable test from testRulesetValidation and give it full coverage

* Silence warnings
This commit is contained in:
SomeTroglodyte 2025-04-08 09:56:05 +02:00 committed by GitHub
parent 15777b306f
commit 2025d2ae7a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 150 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

56
core/src/com/unciv/models/ruleset/unique/Countables.kt
View File

@ -5,13 +5,15 @@ import com.unciv.models.stats.Stat
import com.unciv.models.translations.equalsPlaceholderText
import com.unciv.models.translations.getPlaceholderParameters
import com.unciv.models.translations.getPlaceholderText
import org.jetbrains.annotations.VisibleForTesting
/**
* Prototype for each new [Countables] instance, to ensure a baseline.
* Prototype for each new [Countables] instance, core functionality, to ensure a baseline.
*
* Notes:
* - Each instance ***must*** implement _either_ overload of [matches] and indicate which one via [matchesWithRuleset].
* - [matches] is used to look up which instance implements a given string, [getErrorSeverity] is responsible for validating placeholders.
* - [matches] is used to look up which instance implements a given string, **without** validating its placeholders.
* - [getErrorSeverity] is responsible for validating placeholders, _and can assume [matches] was successful_.
* - Override [getKnownValuesForAutocomplete] only if a sensible number of suggestions is obvious.
*/
interface ICountable {
@ -30,11 +32,13 @@ interface ICountable {
*
* Expansion instructions:
* - A new simple "variable" needs to implement only [text] and [eval].
* - Not supplying [text] means the "variable" **must** implement either [matches] overload. If it parses placeholders, then it **must** override [noPlaceholders] to `false`.
* - A new "variable" _using placeholder(s)_ needs to implement [matches] and [eval].
* - Using [simpleName] inside [validate] as the examples do is only done for readability.
* - Implement [getErrorSeverity] in most cases, use [UniqueParameterType] to validate each placeholder content.
* - Implement [getKnownValuesForAutocomplete] only when a meaningful, not too large set of suggestions is obvious.
* - A new countable that draws from an existing enum or set of RulesetObjects should work along the lines of the [Stats] or [TileResources] examples.
* - **Do** heed the docs of [ICountable] - but be aware the [Countables] Enum class pre-implements some of the methods.
* - Run the unit tests! There's one checking implementation conventions.
* - When implementing a formula language for Countables, create a new object in a separate file with the actual
* implementation, then a new instance here that delegates all its methods to that object. And delete these lines.
*/
@ -98,7 +102,7 @@ enum class Countables(
return cities.count { it.matchesFilter(filter) }
}
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? =
UniqueParameterType.CityFilter.getErrorSeverity(parameterText.getPlaceholderParameters().first(), ruleset)
UniqueParameterType.CityFilter.getTranslatedErrorSeverity(parameterText, ruleset)
},
FilteredUnits("[mapUnitFilter] Units") {
@ -108,7 +112,7 @@ enum class Countables(
return unitManager.getCivUnits().count { it.matchesFilter(filter) }
}
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? =
UniqueParameterType.MapUnitFilter.getErrorSeverity(parameterText.getPlaceholderParameters().first(), ruleset)
UniqueParameterType.MapUnitFilter.getTranslatedErrorSeverity(parameterText, ruleset)
override fun getKnownValuesForAutocomplete(ruleset: Ruleset): Set<String> =
(ruleset.unitTypes.keys + ruleset.units.keys).mapTo(mutableSetOf()) { "[$it] Units" }
},
@ -122,7 +126,7 @@ enum class Countables(
}
}
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? =
UniqueParameterType.BuildingFilter.getErrorSeverity(parameterText.getPlaceholderParameters().first(), ruleset)
UniqueParameterType.BuildingFilter.getTranslatedErrorSeverity(parameterText, ruleset)
override fun getKnownValuesForAutocomplete(ruleset: Ruleset) = setOf<String>()
},
@ -133,7 +137,7 @@ enum class Countables(
return civilizations.count { it.isAlive() && it.matchesFilter(filter) }
}
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? =
UniqueParameterType.CivFilter.getErrorSeverity(parameterText.getPlaceholderParameters().first(), ruleset)
UniqueParameterType.CivFilter.getTranslatedErrorSeverity(parameterText, ruleset)
override fun getKnownValuesForAutocomplete(ruleset: Ruleset) = setOf<String>()
},
@ -143,6 +147,8 @@ enum class Countables(
val cities = stateForConditionals.civInfo?.cities ?: return null
return cities.sumOf { city -> city.getTiles().count { it.matchesFilter(filter) } }
}
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? =
UniqueParameterType.TileFilter.getTranslatedErrorSeverity(parameterText, ruleset)
override fun getKnownValuesForAutocomplete(ruleset: Ruleset) = setOf<String>()
},
@ -169,9 +175,11 @@ enum class Countables(
}
}
;
val placeholderText = text.getPlaceholderText()
val noPlaceholders = text.isNotEmpty() && !text.contains('[')
@VisibleForTesting
open val noPlaceholders = !text.contains('[')
// Leave these in place only for the really simple cases
override fun matches(parameterText: String) = if (noPlaceholders) parameterText == text
@ -181,37 +189,25 @@ enum class Countables(
open val documentationHeader get() =
"`$text`" + (if (shortDocumentation.isEmpty()) "" else " - $shortDocumentation")
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? {
/** Leave this only for Countables without any parameters - they can rely on [matches] having validated enough */
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? = null
override fun getDeprecationAnnotation(): Deprecated? = declaringJavaClass.getField(name).getAnnotation(Deprecated::class.java)
protected fun UniqueParameterType.getTranslatedErrorSeverity(parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? {
val severity = getErrorSeverity(parameterText.getPlaceholderParameters().first(), ruleset)
return when {
matches(parameterText) -> null
severity != UniqueType.UniqueParameterErrorSeverity.PossibleFilteringUnique -> severity
matchesWithRuleset -> UniqueType.UniqueParameterErrorSeverity.RulesetSpecific
else -> UniqueType.UniqueParameterErrorSeverity.RulesetInvariant
}
}
fun getParameterErrors(parameterText: String, ruleset: Ruleset): List<String> {
if (this.noPlaceholders) return emptyList()
val errors = mutableListOf<String>()
val placeholderParameters = text.getPlaceholderParameters()
for ((index, parameter) in placeholderParameters.withIndex()) {
val parameterType = UniqueParameterType.safeValueOf(parameter)
val actualParameterText = placeholderParameters[index]
val severity = parameterType.getErrorSeverity(actualParameterText, ruleset) ?: continue
errors.add("`$text` contains parameter `$actualParameterText` which is invalid for type `${parameterType.name}`")
}
return errors
}
override fun getDeprecationAnnotation(): Deprecated? = declaringJavaClass.getField(name).getAnnotation(Deprecated::class.java)
companion object {
fun getMatching(parameterText: String, ruleset: Ruleset?) = Countables.entries
.filter {
if (it.matchesWithRuleset) it.matches(parameterText, ruleset!!) else it.matches(parameterText)
}
}
fun getCountableAmount(parameterText: String, stateForConditionals: StateForConditionals): Int? {
val ruleset = stateForConditionals.gameInfo?.ruleset

4
core/src/com/unciv/models/translations/Translations.kt
View File

@ -501,8 +501,8 @@ fun String.getPlaceholderText(): String {
}
fun String.equalsPlaceholderText(str: String): Boolean {
if (isEmpty() && str.isEmpty()) return true // Empty strings have no .first()
if (isEmpty() != str.isEmpty()) return false
if (isEmpty()) return str.isEmpty()
if (str.isEmpty()) return false // Empty strings have no .first()
if (first() != str.first()) return false // for quick negative return 95% of the time
return this.getPlaceholderText() == str
}

145
tests/src/com/unciv/uniques/CountableTests.kt
View File

@ -3,7 +3,12 @@ package com.unciv.uniques
import com.unciv.models.metadata.BaseRuleset
import com.unciv.models.ruleset.Ruleset
import com.unciv.models.ruleset.RulesetCache
import com.unciv.models.ruleset.unique.*
import com.unciv.models.ruleset.unique.Countables
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.UniqueTriggerActivation
import com.unciv.models.ruleset.validation.RulesetErrorList
import com.unciv.models.ruleset.validation.RulesetValidator
import com.unciv.models.stats.Stat
import com.unciv.models.translations.getPlaceholderParameters
@ -24,13 +29,57 @@ class CountableTests {
private var civInfo = game.addCiv()
private var city = game.addCity(civInfo, game.tileMap[2,0])
@Test
fun testCountableConventions() {
fun Class<out Countables>.hasOverrideFor(name: String, vararg args: Class<out Any>): Boolean {
try {
getDeclaredMethod(name, *args)
} catch (ex: NoSuchMethodException) {
return false
}
return true
}
var fails = 0
println("Reflection check of the Countables class:")
for (instance in Countables::class.java.enumConstants) {
val instanceClazz = instance::class.java
val matchesRulesetOverridden = instanceClazz.hasOverrideFor("matches", String::class.java, Ruleset::class.java)
val matchesPlainOverridden = instanceClazz.hasOverrideFor("matches", String::class.java)
if (instance.matchesWithRuleset && !matchesRulesetOverridden) {
println("`$instance` is marked as working _with_ a `Ruleset` but fails to override `matches(String,Ruleset)`,")
fails++
} else if (instance.matchesWithRuleset && matchesPlainOverridden) {
println("`$instance` is marked as working _with_ a `Ruleset` but overrides `matches(String)` which is worthless.")
fails++
} else if (!instance.matchesWithRuleset && matchesRulesetOverridden) {
println("`$instance` is marked as working _without_ a `Ruleset` but overrides `matches(String,Ruleset)` which is worthless.")
fails++
}
if (instance.text.isEmpty() && !matchesPlainOverridden && !matchesRulesetOverridden) {
println("`$instance` has no `text` but fails to override either `matches` overload.")
fails++
}
val getErrOverridden = instanceClazz.hasOverrideFor("getErrorSeverity", String::class.java, Ruleset::class.java)
if (instance.noPlaceholders && getErrOverridden) {
println("`$instance` has no placeholders but overrides `getErrorSeverity` which is likely an error.")
fails++
} else if (!instance.noPlaceholders && !getErrOverridden) {
println("`$instance` has placeholders that must be treated and therefore **must** override `getErrorSeverity` but does not.")
fails++
}
}
assertEquals("failure count", 0, fails)
}
@Test
fun testAllCountableParametersAreUniqueParameterTypes() {
for (countable in Countables.entries) {
val parameters = countable.text.getPlaceholderParameters()
for (parameter in parameters) {
assertNotEquals("Countable ${countable.name} parameter ${parameter} is not a UniqueParameterType",
assertNotEquals("Countable ${countable.name} parameter $parameter is not a UniqueParameterType",
UniqueParameterType.safeValueOf(parameter), UniqueParameterType.Unknown)
}
}
@ -76,7 +125,7 @@ class CountableTests {
UniqueTriggerActivation.triggerUnique(Unique("Turn this tile into a [Coast] tile"), civInfo, tile = game.tileMap[-3,0])
UniqueTriggerActivation.triggerUnique(Unique("Turn this tile into a [Coast] tile"), civInfo, tile = game.tileMap[3,0])
val city2 = game.addCity(civInfo, game.tileMap[-2,0], initialPopulation = 9)
game.addCity(civInfo, game.tileMap[-2,0], initialPopulation = 9)
val tests = listOf(
"Owned [All] Tiles" to 14,
"Owned [worked] Tiles" to 8,
@ -111,39 +160,89 @@ class CountableTests {
@Test
fun testRulesetValidation() {
val mod = Ruleset().apply {
name = "Testing"
globalUniques.uniques.add("[+42 Faith] <when number of [turns] is less than [42]>")
globalUniques.uniques.add("[-1 Faith] <for every [turns]> <when number of [turns] is less than [42]>")
globalUniques.uniques.add("[+1 Happiness] <for every [City-States]>")
globalUniques.uniques.add("[+1 Happiness] <for every [[42] Monkeys]>")
}
game = TestGame {
RulesetCache[mod.name] = mod
RulesetCache.getComplexRuleset(RulesetCache[BaseRuleset.Civ_V_GnK.fullName]!!, listOf(mod))
}
val ruleset = game.ruleset
val ruleset = setupModdedGame(
"[+42 Faith] <when number of [turns] is less than [42]>",
"[-1 Faith] <for every [turns]> <when number of [turns] is between [0] and [41]>",
"[+1 Happiness] <for every [City-States]>", // 1 deprecation
"[+1 Happiness] <for every [Remaining [City-State] Civilizations]>",
"[+1 Happiness] <for every [[42] Monkeys]>", // 1 not a countable, 1 monkeys
"[+1 Gold] <when number of [year] is equal to [countable]>", // 1 not a countable
"[+1 Food] <when number of [-0] is different than [+0]>",
"[+1 Food] <when number of [5e1] is more than [0.5]>", // 2 not a countable
"[+1 Food] <when number of [0x12] is between [.99] and [99.]>", // 3 not a countable
"[+1 Food] <when number of [[~Nonexisting~] Cities] is between [[Annexed] Cities] and [Cities]>", // 1 not a countable
"[+1 Food] <when number of [[Paratrooper] Units] is between [[Air] Units] and [Units]>",
"[+1 Food] <when number of [[~Bogus~] Units] is between [[Land] Units] and [[Air] Units]>", // 1 not a countable
"[+1 Food] <when number of [[Barbarian] Units] is between [[Japanese] Units] and [[Embarked] Units]>", // 1 not a countable
"[+1 Food] <when number of [[Science] Buildings] is between [[Wonder] Buildings] and [[All] Buildings]>",
"[+1 Food] <when number of [[42] Buildings] is between [[Universe] Buildings] and [[Library] Buildings]>", // 2 not a countable
"[+1 Food] <when number of [Remaining [Human player] Civilizations] is between [Remaining [City-State] Civilizations] and [Remaining [Major] Civilizations]>",
"[+1 Food] <when number of [Remaining [city-state] Civilizations] is between [Remaining [la la la] Civilizations] and [Remaining [all] Civilizations]>", // 2 not a countable
"[+1 Food] <when number of [Owned [Land] Tiles] is between [Owned [Desert] Tiles] and [Owned [All] Tiles]>",
"[+1 Food] <when number of [Owned [worked] Tiles] is between [Owned [Camp] Tiles] and [Owned [Forest] Tiles]>",
// Attention: `Barbarians` is technically a valid TileFilter because it's a CivFilter. Validation can't know it's meaningless for the OwnedTiles countable.
// `Food` is currently not a valid TileFilter, but might be, and the last is just wrong case for a valid filter and should be flagged.
"[+1 Food] <when number of [Owned [Barbarians] Tiles] is between [Owned [Food] Tiles] and [Owned [strategic Resource] Tiles]>", // 2 not a countable
"[+1 Food] <when number of [Iron] is between [Whales] and [Crab]>",
"[+1 Food] <when number of [Cocoa] is between [Bison] and [Maryjane]>", // 3 not a countable
)
ruleset.modOptions.isBaseRuleset = true // To get ruleset-specific validation
val errors = RulesetValidator(ruleset).getErrorList()
val countDeprecations = errors.count {
it.text.matches(Regex("Countable.*deprecated.*"))
var fails = 0
val checks = sequenceOf(
"deprecated" to "Countable.*deprecated.*" to 1,
"monkeys" to ".*Monkeys.*not fit.*countable.*" to 1,
"not a countable" to ".*does not fit parameter type countable.*" to 19,
)
println("Countables validation over synthetic test input:")
for ((check, expected) in checks) {
val (label, pattern) = check
val actual = errors.count(pattern)
if (actual == expected) continue
fails++
println("\tTest '$label' should find $expected errors, found: $actual")
}
assertEquals("The test mod should flag one deprecated Countable", 1, countDeprecations)
val countInvalid = errors.count {
it.text.matches(Regex(".*Monkeys.*not fit.*countable.*"))
}
assertEquals("The test mod should flag one invalid Countable", 1, countInvalid)
assertEquals("failure count", 0, fails)
}
@Test
fun testForEveryWithInvalidCountable() {
setupModdedGame(
"[+42 Faith] <when number of [turns] is less than [42]>",
"[+1 Happiness] <for every [City-States]>",
"[+1 Happiness] <for every [[42] Monkeys]>",
)
game.makeHexagonalMap(3)
civInfo = game.addCiv()
city = game.addCity(civInfo, game.tileMap[2,0])
val cityState = game.addCiv(cityStateType = game.ruleset.cityStateTypes.keys.first())
val cityStateCity = game.addCity(cityState, game.tileMap[-2,0], true)
game.addCity(cityState, game.tileMap[-2,0], true)
civInfo.updateStatsForNextTurn()
val happiness = Countables.getCountableAmount("Happiness", StateForConditionals(civInfo, city))
// Base 9, -1 city, -3 population +1 deprecated countable should still work, but the bogus one should not
assertEquals("Testing Happiness", 6, happiness)
}
private fun setupModdedGame(vararg uniques: String): Ruleset {
val mod = Ruleset()
mod.name = "Testing"
for (unique in uniques)
mod.globalUniques.uniques.add(unique)
game = TestGame {
RulesetCache[mod.name] = mod
RulesetCache.getComplexRuleset(RulesetCache[BaseRuleset.Civ_V_GnK.fullName]!!, listOf(mod))
}
return game.ruleset
}
private fun RulesetErrorList.count(pattern: String): Int {
val regex = Regex(pattern)
return count { it.text.matches(regex) }
}
}