From 0a4df1ca9ff9d9427885bc3f84b9fdcdd944fa1e Mon Sep 17 00:00:00 2001
From: Yair Morgenstern <yairm210@hotmail.com>
Date: Sat, 19 Feb 2022 23:05:47 +0200
Subject: [PATCH] Resolved #6188 - don't show 'missing cities that need to
 build X' for buildings already built in the city It gives the impression that
 the buildings stop providing their bonus if the condition is no longer
 satisfied

---
 core/src/com/unciv/models/ruleset/Building.kt | 21 ++++++++++---------
 .../com/unciv/ui/cityscreen/CityInfoTable.kt  |  2 +-
 .../ui/cityscreen/ConstructionInfoTable.kt    |  2 +-
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/core/src/com/unciv/models/ruleset/Building.kt b/core/src/com/unciv/models/ruleset/Building.kt
index 54ba568c03..e3f67fb607 100644
--- a/core/src/com/unciv/models/ruleset/Building.kt
+++ b/core/src/com/unciv/models/ruleset/Building.kt
@@ -99,21 +99,13 @@ class Building : RulesetStatsObject(), INonPerpetualConstruction {
         }
 
     /** used in CityScreen (CityInfoTable and ConstructionInfoTable) */
-    fun getDescription(cityInfo: CityInfo): String {
+    fun getDescription(cityInfo: CityInfo, showMissingRequiredCities:Boolean): String {
         val stats = getStats(cityInfo)
         val lines = ArrayList<String>()
         val isFree = name in cityInfo.civInfo.civConstructions.getFreeBuildings(cityInfo.id)
         if (uniqueTo != null) lines += if (replaces == null) "Unique to [$uniqueTo]"
             else "Unique to [$uniqueTo], replaces [$replaces]"
         val missingUnique = getMatchingUniques(UniqueType.RequiresBuildingInAllCities).firstOrNull()
-        // Inefficient in theory. In practice, buildings seem to have only a small handful of uniques.
-        val missingCities = if (missingUnique != null)
-            // TODO: Unify with rejection reasons?
-            cityInfo.civInfo.cities.filterNot {
-                it.isPuppet
-                || it.cityConstructions.containsBuildingOrEquivalent(missingUnique.params[0])
-            }
-            else listOf()
         if (isWonder) lines += "Wonder"
         if (isNationalWonder) lines += "National Wonder"
         if (!isFree) {
@@ -122,6 +114,15 @@ class Building : RulesetStatsObject(), INonPerpetualConstruction {
                 else "Consumes [$amount] [$resource]"
             }
         }
+
+        // Inefficient in theory. In practice, buildings seem to have only a small handful of uniques.
+        val missingCities = if (missingUnique != null)
+        // TODO: Unify with rejection reasons?
+            cityInfo.civInfo.cities.filterNot {
+                it.isPuppet
+                        || it.cityConstructions.containsBuildingOrEquivalent(missingUnique.params[0])
+            }
+        else listOf()
         if (uniques.isNotEmpty()) {
             if (replacementTextForUniques != "") lines += replacementTextForUniques
             else lines += getUniquesStringsWithoutDisablers(
@@ -148,7 +149,7 @@ class Building : RulesetStatsObject(), INonPerpetualConstruction {
         if (cityStrength != 0) lines += "{City strength} +$cityStrength"
         if (cityHealth != 0) lines += "{City health} +$cityHealth"
         if (maintenance != 0 && !isFree) lines += "{Maintenance cost}: $maintenance {Gold}"
-        if (missingCities.isNotEmpty()) {
+        if (showMissingRequiredCities && missingCities.isNotEmpty()) {
             // Could be red. But IMO that should be done by enabling GDX's ColorMarkupLanguage globally instead of adding a separate label.
             lines += "\n" + 
                 "[${cityInfo.civInfo.getEquivalentBuilding(missingUnique!!.params[0])}] required:".tr() +
diff --git a/core/src/com/unciv/ui/cityscreen/CityInfoTable.kt b/core/src/com/unciv/ui/cityscreen/CityInfoTable.kt
index db38088037..4773cefd0e 100644
--- a/core/src/com/unciv/ui/cityscreen/CityInfoTable.kt
+++ b/core/src/com/unciv/ui/cityscreen/CityInfoTable.kt
@@ -70,7 +70,7 @@ class CityInfoTable(private val cityScreen: CityScreen) : Table(BaseScreen.skin)
         val isFree = building.name in cityScreen.city.civInfo.civConstructions.getFreeBuildings(cityScreen.city.id)
         val displayName = if (isFree) "{${building.name}} ({Free})" else building.name
         val buildingNameAndIconTable = ExpanderTab(displayName, Constants.defaultFontSize, icon, false, 5f) {
-            val detailsString = building.getDescription(cityScreen.city)
+            val detailsString = building.getDescription(cityScreen.city, false)
             it.add(detailsString.toLabel().apply { wrap = true })
                 .width(cityScreen.stage.width / 4 - 2 * pad).row() // when you set wrap, then you need to manually set the size of the label
             if (building.isSellable() && !isFree) {
diff --git a/core/src/com/unciv/ui/cityscreen/ConstructionInfoTable.kt b/core/src/com/unciv/ui/cityscreen/ConstructionInfoTable.kt
index a2972259da..8e8eda0101 100644
--- a/core/src/com/unciv/ui/cityscreen/ConstructionInfoTable.kt
+++ b/core/src/com/unciv/ui/cityscreen/ConstructionInfoTable.kt
@@ -59,7 +59,7 @@ class ConstructionInfoTable(val cityScreen: CityScreen): Table() {
 
             val (description, link) = when (construction) {
                 is BaseUnit -> construction.getDescription() to construction.makeLink()
-                is Building -> construction.getDescription(city) to construction.makeLink()
+                is Building -> construction.getDescription(city, true) to construction.makeLink()
                 is PerpetualConstruction -> construction.description.replace("[rate]", "[${construction.getConversionRate(city)}]") to ""
                 else -> "" to "" // Should never happen
             }