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

@@ -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

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 ""

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 finalPossibleUniques = ArrayList<String>()
+        
+        for (possibleUnique in possibleUniques) {
+            for (parameter in possibleUnique.replace('<', ' ').getPlaceholderParameters()) {
                 val parameterNumberInDeprecatedUnique =
-                deprecatedUniquePlaceholders.indexOf(parameter.removePrefix("+").removePrefix("-"))
+                    deprecatedUniquePlaceholders.indexOf(
+                        parameter.removePrefix("+").removePrefix("-")
+                    )
                 if (parameterNumberInDeprecatedUnique == -1) continue
                 var replacementText = params[parameterNumberInDeprecatedUnique]
                 if (parameter.startsWith('+')) replacementText = "+$replacementText"
-            else if(parameter.startsWith('-')) replacementText = "-$replacementText"
+                else if (parameter.startsWith('-')) replacementText = "-$replacementText"
-            replacementUniqueText =
+                finalPossibleUniques +=
-                replacementUniqueText.replace("[$parameter]", "[$replacementText]")
+                    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])

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 <after adopting [buildingName/tech/resource/policy]>\"" +
             " OR \"Only available <with [buildingName/tech/resource/policy]>\"" +
+            " OR \"Only available <if [buildingName/tech/resource/policy] is constructed>\"" +
             " OR \"Only available <after discovering [buildingName/tech/resource/policy]>"))
     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),

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 <vs [unitFilter] units>, which themselves
          * can contain multiple filters, such as <vs [{Military} {Water}] units>.

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}>"

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: "<before adopting [Oligarchy]>"
 
 Applicable to: Conditional
 
+#### <if [buildingName] is constructed>
+Example: "<if [Library] is constructed>"
+
+Applicable to: Conditional
+
 #### <for [amount] turns>
 Example: "<for [20] turns>"
 
@@ -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 <with a garrison> <when attacking>"
  - "Incompatible with [policy/tech/promotion]" - Deprecated as of 3.19.8, replace with "Only available <before adopting [policy/tech/promotion]>" OR "Only available <before discovering [policy/tech/promotion]>" OR "Only available <for units without [policy/tech/promotion]>"
- - "Not displayed as an available construction without [buildingName/tech/resource/policy]" - Deprecated as of 3.19.8, replace with "Only available <after adopting [buildingName/tech/resource/policy]>" OR "Only available <with [buildingName/tech/resource/policy]>" OR "Only available <after discovering [buildingName/tech/resource/policy]>"
+ - "Not displayed as an available construction without [buildingName/tech/resource/policy]" - Deprecated as of 3.19.8, replace with "Only available <after adopting [buildingName/tech/resource/policy]>" OR "Only available <with [buildingName/tech/resource/policy]>" OR "Only available <if [buildingName/tech/resource/policy] is constructed>" OR "Only available <after discovering [buildingName/tech/resource/policy]>"
  - "Cannot be built with [buildingName]" - Deprecated as of 3.19.9, replace with "Only available <in cities without a [buildingName]>"
  - "Requires a [buildingName] in this city" - Deprecated as of 3.19.9, replace with "Only available <in cities with a [buildingName]>"
  - "[stats] with [resource]" - Deprecated as of 3.19.7, replace with "[stats] <with [resource]>"

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))
                 }
             }
         }