More improvements to internal Countables handling (#13187)

* TranslationFileWriter support for Countables

* Minor Countables Kdoc update

* Code deduplication in UniqueTriggerActivation, also for future reuse

* Sync UniqueParameterType.PolicyFilter checks and doc with implementation

* Improve testRulesetValidation
This commit is contained in:
SomeTroglodyte 2025-04-11 07:09:07 +02:00 committed by GitHub
parent 89a711d4de
commit b0c9295372
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 90 additions and 54 deletions

View File

@ -2009,11 +2009,6 @@ Cannot be built on [tileFilter] tiles =
Does not need removal of [feature] =
Gain a free [building] [cityFilter] =
# Countables
Remaining [civFilter] Civilizations =
Owned [tileFilter] Tiles =
Completed Policy branches =
# Unused Resources

View File

@ -171,6 +171,13 @@ class PolicyManager : IsPartOfGameInfoSerialization {
fun getAdoptedPolicies(): HashSet<String> = adoptedPolicies
/** Uncached, use carefully */
fun getAdoptedPoliciesMatching(policyFilter: String, stateForConditionals: StateForConditionals) =
adoptedPolicies.asSequence()
.mapNotNull { getRulesetPolicies()[it] }
.filter { it.matchesFilter(policyFilter, stateForConditionals) }
.toList()
fun isAdopted(policyName: String): Boolean = adoptedPolicies.contains(policyName)
/**
@ -197,7 +204,7 @@ class PolicyManager : IsPartOfGameInfoSerialization {
if (allPoliciesAdopted(true)) return false
return true
}
fun adopt(policy: Policy, branchCompletion: Boolean = false) {

View File

@ -41,11 +41,15 @@ open class Policy : RulesetObject() {
state != null && hasUnique(filter, state) ||
state == null && hasTagUnique(filter)
})
// Remember policy branches are duplicated in `policies` (as subclass carrying more information),
// so filtering by a policy branch name matches only the branch itself, filtering by "[name] branch"
// will match all policies in that branch plus the branch itself (since the loader sets a branch's branch to itself).
fun matchesSingleFilter(filter: String): Boolean {
return when(filter) {
in Constants.all -> true
name -> true
"[all] branch" -> branch == this
"[${branch.name}] branch" -> true
else -> false
}

View File

@ -34,13 +34,18 @@ interface ICountable {
* - 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].
* - Implement [getErrorSeverity] in most cases, use [UniqueParameterType] to validate each placeholder content.
* - Implement [getErrorSeverity] in most cases, typically using [UniqueParameterType] to validate each placeholder content.
* - If it uses exactly one UniqueParameterType placeholder, [getErrorSeverity] can use the [UniqueParameterType.getTranslatedErrorSeverity] extension provided below.
* - 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.
*
* @param text The "key" to recognize this countable. If not empty, it will be included in translations.
* Placeholders should match a `UniqueParameterType` by its `parameterType`.
* If the countable implements non-UniqueParameterType placeholders, it may be better to leave this empty.
*/
enum class Countables(
val text: String = "",

View File

@ -557,15 +557,24 @@ enum class UniqueParameterType(
},
/** Implemented by [com.unciv.models.ruleset.Policy.matchesFilter] */
PolicyFilter("policyFilter", "Oligarchy", "The name of any policy") {
override val staticKnownValues = Constants.all
PolicyFilter("policyFilter", "Oligarchy",
"The name of any policy, a filtering Unique, any branch (matching only the branch itself)," +
" a branch name with \" Completed\" appended (matches if the branch is completed)," +
" a policy branch as `[branchName] branch` (matching all policies in that branch)," +
" or `[all] branch` which matches all branch starter policies."
) {
override val staticKnownValues = Constants.all + "[all] branch"
override fun isKnownValue(parameterText: String, ruleset: Ruleset) = when {
parameterText in staticKnownValues -> true
parameterText in ruleset.policies -> true
parameterText.startsWith('[') && parameterText.endsWith("] branch") &&
parameterText.removeSurrounding("[", "] branch") in ruleset.policyBranches -> true
ruleset.policies.values.any { it.hasTagUnique(parameterText) } -> true
else -> false
}
override fun getKnownValuesForAutocomplete(ruleset: Ruleset) = staticKnownValues + ruleset.policies.keys
override fun getKnownValuesForAutocomplete(ruleset: Ruleset) =
staticKnownValues + ruleset.policies.keys +
ruleset.policyBranches.keys.map { "[$it] branch" }.toSet()
override fun getErrorSeverity(parameterText: String, ruleset: Ruleset) = getErrorSeverityForFilter(parameterText, ruleset)
},

View File

@ -325,9 +325,7 @@ object UniqueTriggerActivation {
}
UniqueType.OneTimeRemovePolicy -> {
val policyFilter = unique.params[0]
val policiesToRemove = civInfo.policies.adoptedPolicies
.mapNotNull { civInfo.gameInfo.ruleset.policies[it] }
.filter { it.matchesFilter(policyFilter, stateForConditionals) }
val policiesToRemove = civInfo.policies.getAdoptedPoliciesMatching(policyFilter, stateForConditionals)
if (policiesToRemove.isEmpty()) return null
return {
@ -348,9 +346,7 @@ object UniqueTriggerActivation {
UniqueType.OneTimeRemovePolicyRefund -> {
val policyFilter = unique.params[0]
val refundPercentage = unique.params[1].toInt()
val policiesToRemove = civInfo.policies.adoptedPolicies
.mapNotNull { civInfo.gameInfo.ruleset.policies[it] }
.filter { it.matchesFilter(policyFilter, stateForConditionals) }
val policiesToRemove = civInfo.policies.getAdoptedPoliciesMatching(policyFilter, stateForConditionals)
if (policiesToRemove.isEmpty()) return null
val policiesToRemoveMap = civInfo.policies.getCultureRefundMap(policiesToRemove, refundPercentage)

View File

@ -29,6 +29,7 @@ import com.unciv.models.ruleset.tech.TechColumn
import com.unciv.models.ruleset.tile.Terrain
import com.unciv.models.ruleset.tile.TileImprovement
import com.unciv.models.ruleset.tile.TileResource
import com.unciv.models.ruleset.unique.Countables
import com.unciv.models.ruleset.unique.Unique
import com.unciv.models.ruleset.unique.UniqueFlag
import com.unciv.models.ruleset.unique.UniqueParameterType
@ -127,6 +128,11 @@ object TranslationFileWriter {
for (uniqueTarget in UniqueTarget.entries)
linesToTranslate += "$uniqueTarget = "
linesToTranslate += "\n\n#################### Lines from Countables #######################\n"
for (countable in Countables.entries)
if (countable.text.isNotEmpty())
linesToTranslate += "${countable.text} = "
linesToTranslate += "\n\n#################### Lines from spy actions #######################\n"
for (spyAction in SpyAction.entries)
linesToTranslate += "${spyAction.displayString} = "

View File

@ -160,46 +160,65 @@ class CountableTests {
@Test
fun testRulesetValidation() {
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.
/** These are `Pair<String, Int>` with the second being the expected number of parameters to fail UniqueParameterType validation */
val testData = listOf(
"[+42 Faith] <when number of [turns] is less than [42]>" to 0,
"[-1 Faith] <for every [turns]> <when number of [turns] is between [0] and [41]>" to 0,
"[+1 Happiness] <for every [City-States]>" to 0, // +1 deprecation
"[+1 Happiness] <for every [Remaining [City-State] Civilizations]>" to 0,
"[+1 Happiness] <for every [[42] Monkeys]>" to 1, // +1 monkeys
"[+1 Gold] <when number of [year] is equal to [countable]>" to 1,
"[+1 Food] <when number of [-0] is different than [+0]>" to 0,
"[+1 Food] <when number of [5e1] is more than [0.5]>" to 2,
"[+1 Food] <when number of [0x12] is between [.99] and [99.]>" to 3,
"[+1 Food] <when number of [[~Nonexisting~] Cities] is between [[Annexed] Cities] and [Cities]>" to 1,
"[+1 Food] <when number of [[Paratrooper] Units] is between [[Air] Units] and [Units]>" to 0,
"[+1 Food] <when number of [[~Bogus~] Units] is between [[Land] Units] and [[Air] Units]>" to 1,
"[+1 Food] <when number of [[Barbarian] Units] is between [[Japanese] Units] and [[Embarked] Units]>" to 1,
"[+1 Food] <when number of [[Science] Buildings] is between [[Wonder] Buildings] and [[All] Buildings]>" to 0,
"[+1 Food] <when number of [[42] Buildings] is between [[Universe] Buildings] and [[Library] Buildings]>" to 2,
"[+1 Food] <when number of [Remaining [Human player] Civilizations] is between [Remaining [City-State] Civilizations] and [Remaining [Major] Civilizations]>" to 0,
"[+1 Food] <when number of [Remaining [city-state] Civilizations] is between [Remaining [la la la] Civilizations] and [Remaining [all] Civilizations]>" to 2,
"[+1 Food] <when number of [Owned [Land] Tiles] is between [Owned [Desert] Tiles] and [Owned [All] Tiles]>" to 0,
"[+1 Food] <when number of [Owned [worked] Tiles] is between [Owned [Camp] Tiles] and [Owned [Forest] Tiles]>" to 0,
// 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
"[+1 Food] <when number of [Owned [Barbarians] Tiles] is between [Owned [Food] Tiles] and [Owned [strategic Resource] Tiles]>" to 2,
"[+1 Food] <when number of [Iron] is between [Whales] and [Crab]>" to 0,
"[+1 Food] <when number of [Cocoa] is between [Bison] and [Maryjane]>" to 3,
)
val totalNotACountableExpected = testData.sumOf { it.second }
val notACountableRegex = Regex(""".*parameter "(.*)" which does not fit parameter type countable.*""")
val ruleset = setupModdedGame(*testData.map { it.first }.toTypedArray())
ruleset.modOptions.isBaseRuleset = true // To get ruleset-specific validation
val errors = RulesetValidator(ruleset).getErrorList()
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)
for ((uniqueText, expected) in testData) {
var actual = 0
val badOnes = mutableListOf<String>()
for (error in errors) {
if (uniqueText !in error.text) continue
val match = notACountableRegex.matchEntire(error.text) ?: continue
actual++
badOnes += match.groups[1]!!.value
}
if (actual == expected) continue
fails++
println("\tTest '$uniqueText' should find $expected errors, found: $actual, bad parameters: ${badOnes.joinToString()}")
}
val coarseChecks = sequenceOf(
"deprecated" to Regex("Countable.*deprecated.*") to 1,
"monkeys" to Regex(".*Monkeys.*not fit.*countable.*") to 1,
"not a countable" to notACountableRegex to totalNotACountableExpected,
)
for ((check, expected) in coarseChecks) {
val (label, regex) = check
val actual = errors.count { it.text.matches(regex) }
if (actual == expected) continue
fails++
println("\tTest '$label' should find $expected errors, found: $actual")
@ -240,9 +259,4 @@ class CountableTests {
}
return game.ruleset
}
private fun RulesetErrorList.count(pattern: String): Int {
val regex = Regex(pattern)
return count { it.text.matches(regex) }
}
}