diff --git a/android/assets/jsons/translations/template.properties b/android/assets/jsons/translations/template.properties index c1f9571fca..258c420ac0 100644 --- a/android/assets/jsons/translations/template.properties +++ b/android/assets/jsons/translations/template.properties @@ -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 diff --git a/core/src/com/unciv/logic/civilization/managers/PolicyManager.kt b/core/src/com/unciv/logic/civilization/managers/PolicyManager.kt index 5a6c39b89a..3509d0edf3 100644 --- a/core/src/com/unciv/logic/civilization/managers/PolicyManager.kt +++ b/core/src/com/unciv/logic/civilization/managers/PolicyManager.kt @@ -171,6 +171,13 @@ class PolicyManager : IsPartOfGameInfoSerialization { fun getAdoptedPolicies(): HashSet = 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) { diff --git a/core/src/com/unciv/models/ruleset/Policy.kt b/core/src/com/unciv/models/ruleset/Policy.kt index 9e952ddef7..bc970802d1 100644 --- a/core/src/com/unciv/models/ruleset/Policy.kt +++ b/core/src/com/unciv/models/ruleset/Policy.kt @@ -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 } diff --git a/core/src/com/unciv/models/ruleset/unique/Countables.kt b/core/src/com/unciv/models/ruleset/unique/Countables.kt index 302a123d17..f62be6b94a 100644 --- a/core/src/com/unciv/models/ruleset/unique/Countables.kt +++ b/core/src/com/unciv/models/ruleset/unique/Countables.kt @@ -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 = "", diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt index ef3cdf3c6e..e64f71d979 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt @@ -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) }, diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueTriggerActivation.kt b/core/src/com/unciv/models/ruleset/unique/UniqueTriggerActivation.kt index 7b71daf6d5..ec54c0f848 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueTriggerActivation.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueTriggerActivation.kt @@ -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) diff --git a/core/src/com/unciv/models/translations/TranslationFileWriter.kt b/core/src/com/unciv/models/translations/TranslationFileWriter.kt index 60c49e7367..0acc8a9bd6 100644 --- a/core/src/com/unciv/models/translations/TranslationFileWriter.kt +++ b/core/src/com/unciv/models/translations/TranslationFileWriter.kt @@ -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} = " diff --git a/tests/src/com/unciv/uniques/CountableTests.kt b/tests/src/com/unciv/uniques/CountableTests.kt index 1c620035c0..8b723f9ae3 100644 --- a/tests/src/com/unciv/uniques/CountableTests.kt +++ b/tests/src/com/unciv/uniques/CountableTests.kt @@ -160,46 +160,65 @@ class CountableTests { @Test fun testRulesetValidation() { - val ruleset = setupModdedGame( - "[+42 Faith] ", - "[-1 Faith] ", - "[+1 Happiness] ", // 1 deprecation - "[+1 Happiness] ", - "[+1 Happiness] ", // 1 not a countable, 1 monkeys - "[+1 Gold] ", // 1 not a countable - "[+1 Food] ", - "[+1 Food] ", // 2 not a countable - "[+1 Food] ", // 3 not a countable - "[+1 Food] ", // 1 not a countable - "[+1 Food] ", - "[+1 Food] ", // 1 not a countable - "[+1 Food] ", // 1 not a countable - "[+1 Food] ", - "[+1 Food] ", // 2 not a countable - "[+1 Food] ", - "[+1 Food] ", // 2 not a countable - "[+1 Food] ", - "[+1 Food] ", - // 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` with the second being the expected number of parameters to fail UniqueParameterType validation */ + val testData = listOf( + "[+42 Faith] " to 0, + "[-1 Faith] " to 0, + "[+1 Happiness] " to 0, // +1 deprecation + "[+1 Happiness] " to 0, + "[+1 Happiness] " to 1, // +1 monkeys + "[+1 Gold] " to 1, + "[+1 Food] " to 0, + "[+1 Food] " to 2, + "[+1 Food] " to 3, + "[+1 Food] " to 1, + "[+1 Food] " to 0, + "[+1 Food] " to 1, + "[+1 Food] " to 1, + "[+1 Food] " to 0, + "[+1 Food] " to 2, + "[+1 Food] " to 0, + "[+1 Food] " to 2, + "[+1 Food] " to 0, + "[+1 Food] " 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] ", // 2 not a countable - "[+1 Food] ", - "[+1 Food] ", // 3 not a countable + "[+1 Food] " to 2, + "[+1 Food] " to 0, + "[+1 Food] " 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() + 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) } - } }