From 13af71e4273e175de35bef218d6ee517bee21310 Mon Sep 17 00:00:00 2001 From: Yair Morgenstern Date: Tue, 22 Feb 2022 11:33:07 +0200 Subject: [PATCH] Autoupdates to uniques will multiple solutions now replace to the correct one (#6208) * Autoupdates to uniques will multiple solutions now replace to the correct one Problem - some deprecated uniques can lead to multiple possibilities of replacements, depending on the parameter type This lead to replacements in the jsons that were either unparseable entirely or were causing errors, both of which needed to be corrected by hand We now separate such deprecations into their constituent potential replacement uniques, and try and take only the unique that doesn't cause any errors Works like a charmander :) * Conditional name change * Resolved #6179 - when changing units production due to deprecation, notification no longer counts the same city multiple times if it appears multiple times in its queue --- core/src/com/unciv/Constants.kt | 2 + core/src/com/unciv/models/ruleset/Ruleset.kt | 2 +- .../com/unciv/models/ruleset/unique/Unique.kt | 47 ++++++++++++++----- .../unciv/models/ruleset/unique/UniqueType.kt | 2 + .../unciv/models/translations/Translations.kt | 2 +- .../ui/worldscreen/mainmenu/OptionsPopup.kt | 4 +- docs/uniques.md | 31 +++++++++++- tests/src/com/unciv/testing/BasicTests.kt | 5 +- 8 files changed, 76 insertions(+), 19 deletions(-) diff --git a/core/src/com/unciv/Constants.kt b/core/src/com/unciv/Constants.kt index 9e69e9ae28..ba9a014b7d 100644 --- a/core/src/com/unciv/Constants.kt +++ b/core/src/com/unciv/Constants.kt @@ -59,6 +59,8 @@ object Constants { const val lowering = "Lowering" const val remove = "Remove " + const val uniqueOrDelimiter = "\" OR \"" + const val minimumMovementEpsilon = 0.05 const val defaultFontSize = 18 const val headingFontSize = 24 diff --git a/core/src/com/unciv/models/ruleset/Ruleset.kt b/core/src/com/unciv/models/ruleset/Ruleset.kt index 8b7f05c709..8a56530e7c 100644 --- a/core/src/com/unciv/models/ruleset/Ruleset.kt +++ b/core/src/com/unciv/models/ruleset/Ruleset.kt @@ -406,7 +406,7 @@ class Ruleset { val deprecationAnnotation = unique.getDeprecationAnnotation() if (deprecationAnnotation != null) { - val replacementUniqueText = unique.getReplacementText() + val replacementUniqueText = unique.getReplacementText(this) val deprecationText = "$name's unique \"${unique.text}\" is deprecated ${deprecationAnnotation.message}," + if (deprecationAnnotation.replaceWith.expression != "") " replace with \"${replacementUniqueText}\"" else "" diff --git a/core/src/com/unciv/models/ruleset/unique/Unique.kt b/core/src/com/unciv/models/ruleset/unique/Unique.kt index f48d349d9d..7e17a355a8 100644 --- a/core/src/com/unciv/models/ruleset/unique/Unique.kt +++ b/core/src/com/unciv/models/ruleset/unique/Unique.kt @@ -1,11 +1,13 @@ package com.unciv.models.ruleset.unique +import com.unciv.Constants import com.unciv.logic.battle.CombatAction import com.unciv.logic.battle.MapUnitCombatant import com.unciv.logic.city.CityInfo import com.unciv.models.stats.Stats import com.unciv.models.translations.* import com.unciv.logic.civilization.CivilizationInfo +import com.unciv.models.ruleset.Ruleset import java.util.* import kotlin.collections.ArrayList import kotlin.collections.HashMap @@ -52,24 +54,43 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s fun getDeprecationAnnotation(): Deprecated? = type?.getDeprecationAnnotation() - fun getReplacementText(): String { + fun getReplacementText(ruleset: Ruleset): String { val deprecationAnnotation = getDeprecationAnnotation() ?: return "" - var replacementUniqueText = deprecationAnnotation.replaceWith.expression + val replacementUniqueText = deprecationAnnotation.replaceWith.expression val deprecatedUniquePlaceholders = type!!.text.getPlaceholderParameters() + val possibleUniques = replacementUniqueText.split(Constants.uniqueOrDelimiter) // Here, for once, we DO want the conditional placeholder parameters together with the regular ones, // so we cheat the conditional detector by removing the '<' - for (parameter in replacementUniqueText.replace('<',' ').getPlaceholderParameters()) { - val parameterNumberInDeprecatedUnique = - deprecatedUniquePlaceholders.indexOf(parameter.removePrefix("+").removePrefix("-")) - if (parameterNumberInDeprecatedUnique == -1) continue - var replacementText = params[parameterNumberInDeprecatedUnique] - if (parameter.startsWith('+')) replacementText = "+$replacementText" - else if(parameter.startsWith('-')) replacementText = "-$replacementText" - replacementUniqueText = - replacementUniqueText.replace("[$parameter]", "[$replacementText]") + val finalPossibleUniques = ArrayList() + + for (possibleUnique in possibleUniques) { + for (parameter in possibleUnique.replace('<', ' ').getPlaceholderParameters()) { + val parameterNumberInDeprecatedUnique = + deprecatedUniquePlaceholders.indexOf( + parameter.removePrefix("+").removePrefix("-") + ) + if (parameterNumberInDeprecatedUnique == -1) continue + var replacementText = params[parameterNumberInDeprecatedUnique] + if (parameter.startsWith('+')) replacementText = "+$replacementText" + else if (parameter.startsWith('-')) replacementText = "-$replacementText" + finalPossibleUniques += + possibleUnique.replace("[$parameter]", "[$replacementText]") + } } - return replacementUniqueText + if (finalPossibleUniques.size == 1) return finalPossibleUniques.first() + + // filter out possible replacements that are obviously wrong + val uniquesWithNoErrors = finalPossibleUniques.filter { + val unique = Unique(it) + val errors = ruleset.checkUnique(unique, true, "", + UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific, unique.type!!.targetTypes.first()) + errors.isEmpty() + } + if (uniquesWithNoErrors.size == 1) return uniquesWithNoErrors.first() + + val uniquesToUnify = if (uniquesWithNoErrors.isNotEmpty()) uniquesWithNoErrors else possibleUniques + return uniquesToUnify.joinToString("\", \"") } private fun conditionalApplies( @@ -120,6 +141,8 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s state.civInfo != null && state.civInfo.policies.isAdopted(condition.params[0]) UniqueType.ConditionalNoPolicy -> state.civInfo != null && !state.civInfo.policies.isAdopted(condition.params[0]) + UniqueType.ConditionalBuildingBuilt -> + state.civInfo != null && state.civInfo.cities.any { it.cityConstructions.containsBuildingOrEquivalent(condition.params[0]) } UniqueType.ConditionalCityWithBuilding -> state.cityInfo != null && state.cityInfo.cityConstructions.containsBuildingOrEquivalent(condition.params[0]) diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt index 3d8ac08815..8a7a561131 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt @@ -314,6 +314,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags: UniqueTarget.Policy, UniqueTarget.Tech, UniqueTarget.Promotion), @Deprecated("as of 3.19.8", ReplaceWith("Only available \"" + " OR \"Only available \"" + + " OR \"Only available \"" + " OR \"Only available ")) NotDisplayedWithout("Not displayed as an available construction without [buildingName/tech/resource/policy]", UniqueTarget.Building, UniqueTarget.Unit), ConvertFoodToProductionWhenConstructed("Excess Food converted to Production when under construction", UniqueTarget.Building, UniqueTarget.Unit), @@ -571,6 +572,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags: ConditionalNoTech("before discovering [tech]", UniqueTarget.Conditional), ConditionalPolicy("after adopting [policy]", UniqueTarget.Conditional), ConditionalNoPolicy("before adopting [policy]", UniqueTarget.Conditional), + ConditionalBuildingBuilt("if [buildingName] is constructed", UniqueTarget.Conditional), ConditionalTimedUnique("for [amount] turns", UniqueTarget.Conditional), ConditionalConsumeUnit("by consuming this unit", UniqueTarget.Conditional), diff --git a/core/src/com/unciv/models/translations/Translations.kt b/core/src/com/unciv/models/translations/Translations.kt index 46e47bd5cc..664ea45984 100644 --- a/core/src/com/unciv/models/translations/Translations.kt +++ b/core/src/com/unciv/models/translations/Translations.kt @@ -262,7 +262,7 @@ fun String.tr(): String { }.toHashSet() val language = UncivGame.Current.settings.language - if (contains('<')) { // Conditionals! + if (contains('<') && contains('>')) { // Conditionals! /** * So conditionals can contain placeholders, such as , which themselves * can contain multiple filters, such as . diff --git a/core/src/com/unciv/ui/worldscreen/mainmenu/OptionsPopup.kt b/core/src/com/unciv/ui/worldscreen/mainmenu/OptionsPopup.kt index 5187bb25f3..12d5de8291 100644 --- a/core/src/com/unciv/ui/worldscreen/mainmenu/OptionsPopup.kt +++ b/core/src/com/unciv/ui/worldscreen/mainmenu/OptionsPopup.kt @@ -388,9 +388,9 @@ class OptionsPopup(val previousScreen: BaseScreen) : Popup(previousScreen) { // note that this replacement does not contain conditionals attached to the original! - var uniqueReplacementText = deprecatedUnique.getReplacementText() + var uniqueReplacementText = deprecatedUnique.getReplacementText(mod) while (Unique(uniqueReplacementText).getDeprecationAnnotation() != null) - uniqueReplacementText = Unique(uniqueReplacementText).getReplacementText() + uniqueReplacementText = Unique(uniqueReplacementText).getReplacementText(mod) for (conditional in deprecatedUnique.conditionals) uniqueReplacementText += " <${conditional.text}>" diff --git a/docs/uniques.md b/docs/uniques.md index 425acef4fc..481985acd7 100644 --- a/docs/uniques.md +++ b/docs/uniques.md @@ -169,6 +169,9 @@ Example: "[20]% [Culture] from City-States" Applicable to: Global +#### Gold from all trade routes +25% +Applicable to: Global + #### Nullifies [stat] [cityFilter] Example: "Nullifies [Culture] [in all cities]" @@ -316,6 +319,9 @@ Example: "[20]% Food consumption by specialists [in all cities]" Applicable to: Global, FollowerBelief +#### Provides 1 happiness per 2 additional social policies adopted +Applicable to: Global + #### [amount]% of excess happiness converted to [stat] Example: "[20]% of excess happiness converted to [Culture]" @@ -452,6 +458,12 @@ Applicable to: Global #### Enables construction of Spaceship parts Applicable to: Global +#### Enemy land units must spend 1 extra movement point when inside your territory (obsolete upon Dynamite) +Applicable to: Global + +#### Production to science conversion in cities increased by 33% +Applicable to: Global + #### Notified of new Barbarian encampments Applicable to: Global @@ -473,6 +485,18 @@ Applicable to: Global #### Receive a tech boost when scientific buildings/wonders are built in capital Applicable to: Global +#### May not generate great prophet equivalents naturally +Applicable to: Global + +#### 67% chance to earn 25 Gold and recruit a Barbarian unit from a conquered encampment +Applicable to: Global + +#### 50% chance of capturing defeated Barbarian naval units and earning 25 Gold +Applicable to: Global + +#### Receive triple Gold from Barbarian encampments and pillaging Cities +Applicable to: Global + #### Enables Open Borders agreements Applicable to: Global @@ -1404,6 +1428,11 @@ Example: "" Applicable to: Conditional +#### +Example: "" + +Applicable to: Conditional + #### Example: "" @@ -1604,7 +1633,7 @@ Applicable to: Conditional - "-50% food consumption by specialists" - Deprecated Extremely old - used for auto-updates only, replace with "[-50]% Food consumption by specialists [in all cities]" - "+50% attacking strength for cities with garrisoned units" - Deprecated Extremely old - used for auto-updates only, replace with "[+50]% Strength for cities " - "Incompatible with [policy/tech/promotion]" - Deprecated as of 3.19.8, replace with "Only available " OR "Only available " OR "Only available " - - "Not displayed as an available construction without [buildingName/tech/resource/policy]" - Deprecated as of 3.19.8, replace with "Only available " OR "Only available " OR "Only available " + - "Not displayed as an available construction without [buildingName/tech/resource/policy]" - Deprecated as of 3.19.8, replace with "Only available " OR "Only available " OR "Only available " OR "Only available " - "Cannot be built with [buildingName]" - Deprecated as of 3.19.9, replace with "Only available " - "Requires a [buildingName] in this city" - Deprecated as of 3.19.9, replace with "Only available " - "[stats] with [resource]" - Deprecated as of 3.19.7, replace with "[stats] " diff --git a/tests/src/com/unciv/testing/BasicTests.kt b/tests/src/com/unciv/testing/BasicTests.kt index 2027a3f408..3d394728b7 100644 --- a/tests/src/com/unciv/testing/BasicTests.kt +++ b/tests/src/com/unciv/testing/BasicTests.kt @@ -2,6 +2,7 @@ package com.unciv.testing import com.badlogic.gdx.Gdx +import com.unciv.Constants import com.unciv.UncivGame import com.unciv.UncivGameParameters import com.unciv.models.metadata.BaseRuleset @@ -130,7 +131,7 @@ class BasicTests { for (uniqueType in UniqueType.values()) { val deprecationAnnotation = uniqueType.getDeprecationAnnotation() ?: continue - val uniquesToCheck = deprecationAnnotation.replaceWith.expression.split("\", \"", "\" OR \"") + val uniquesToCheck = deprecationAnnotation.replaceWith.expression.split("\", \"", Constants.uniqueOrDelimiter) for (uniqueText in uniquesToCheck) { val replacementTextUnique = Unique(uniqueText) @@ -160,7 +161,7 @@ class BasicTests { break } iteration++ - replacementUnique = Unique(replacementUnique.getReplacementText()) + replacementUnique = Unique(replacementUnique.getReplacementText(ruleset)) } } }