Prevent selling free buildings (#10094)

* A few yield extensions - use in existing code to do later

* Refactor getFreeBuildings to allow hasFreeBuilding not enumerating all

* Prevent selling free buildings - with a little easter egg

* Test translatability

* Shift "Free Building" methods towards preferring object parameters

* Remove easter egg

* Linting and improving Kdoc precision

* Linting and improving Kdoc precision: CityConstructions

core/src/com/unciv/logic/automation/civilization/NextTurnAutomation.kt

@@ -622,7 +622,7 @@ object NextTurnAutomation {
                         city.cityConstructions.isBuilt(it.name)
                         && it.requiresResource(resource)
                         && it.isSellable()
-                        && it.name !in civInfo.civConstructions.getFreeBuildings(city.id) }
+                        && !civInfo.civConstructions.hasFreeBuilding(city, it) }
                     .randomOrNull()
                 if (buildingToSell != null) {
                     city.sellBuilding(buildingToSell)

core/src/com/unciv/logic/city/City.kt

@@ -255,7 +255,7 @@ class City : IsPartOfGameInfoSerialization {
     }
 
     private fun manageCityResourcesRequiredByBuildings(cityResources: ResourceSupplyList) {
-        val freeBuildings = civ.civConstructions.getFreeBuildings(id)
+        val freeBuildings = civ.civConstructions.getFreeBuildingNames(this)
         for (building in cityConstructions.getBuiltBuildings()) {
             // Free buildings cost no resources
             if (building.name in freeBuildings) continue

core/src/com/unciv/logic/city/CityConstructions.kt

@@ -79,7 +79,9 @@ class CityConstructions : IsPartOfGameInfoSerialization {
     var productionOverflow = 0
     private val queueMaxSize = 10
 
-    // Maps cities to the buildings they received
+    /** Maps cities by id to a set of the buildings they received (by nation equivalent name)
+     *  Source: [UniqueType.GainFreeBuildings]
+     */
     val freeBuildingsProvidedFromThisCity: HashMap<String, HashSet<String>> = hashMapOf()
 
     //endregion
@@ -122,7 +124,7 @@ class CityConstructions : IsPartOfGameInfoSerialization {
      */
     fun getMaintenanceCosts(): Int {
         var maintenanceCost = 0
-        val freeBuildings = city.civ.civConstructions.getFreeBuildings(city.id)
+        val freeBuildings = city.civ.civConstructions.getFreeBuildingNames(city)
 
         for (building in getBuiltBuildings())
             if (building.name !in freeBuildings)
@@ -602,10 +604,7 @@ class CityConstructions : IsPartOfGameInfoSerialization {
             for (city in citiesThatApply) {
                 if (city.cityConstructions.containsBuildingOrEquivalent(freeBuilding.name)) continue
                 city.cityConstructions.addBuilding(freeBuilding)
-                if (city.id !in freeBuildingsProvidedFromThisCity)
+                freeBuildingsProvidedFromThisCity.getOrPut(city.id) { hashSetOf() }.add(freeBuilding.name)
-                    freeBuildingsProvidedFromThisCity[city.id] = hashSetOf()
-
-                freeBuildingsProvidedFromThisCity[city.id]!!.add(freeBuilding.name)
             }
         }
 
@@ -613,9 +612,7 @@ class CityConstructions : IsPartOfGameInfoSerialization {
         for (unique in city.civ.getMatchingUniques(UniqueType.GainFreeBuildings, stateForConditionals = StateForConditionals(city.civ, city))) {
             val freeBuilding = city.civ.getEquivalentBuilding(unique.params[0])
             if (city.matchesFilter(unique.params[1])) {
-                if (city.id !in freeBuildingsProvidedFromThisCity)
+                freeBuildingsProvidedFromThisCity.getOrPut(city.id) { hashSetOf() }.add(freeBuilding.name)
-                    freeBuildingsProvidedFromThisCity[city.id] = hashSetOf()
-                freeBuildingsProvidedFromThisCity[city.id]!!.add(freeBuilding.name)
                 if (!isBuilt(freeBuilding.name))
                     addBuilding(freeBuilding)
             }

core/src/com/unciv/logic/city/managers/CityConquestFunctions.kt

@@ -50,7 +50,7 @@ class CityConquestFunctions(val city: City){
 
     private fun removeBuildingsOnMoveToCiv(oldCiv: Civilization) {
         // Remove all buildings provided for free to this city
-        for (building in city.civ.civConstructions.getFreeBuildings(city.id)) {
+        for (building in city.civ.civConstructions.getFreeBuildingNames(city)) {
             city.cityConstructions.removeBuilding(building)
         }
 

core/src/com/unciv/logic/civilization/CivConstructions.kt

@@ -1,46 +1,53 @@
 package com.unciv.logic.civilization
 
 import com.unciv.logic.IsPartOfGameInfoSerialization
+import com.unciv.logic.city.City
 import com.unciv.models.Counter
 import com.unciv.models.ruleset.Building
 import com.unciv.models.ruleset.INonPerpetualConstruction
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.models.stats.Stat
+import com.unciv.ui.components.extensions.yieldAllNotNull
 
 class CivConstructions : IsPartOfGameInfoSerialization {
 
     @Transient
     lateinit var civInfo: Civilization
 
-    // Maps objects to the amount of times bought
+    /** Maps construction names to the amount of times bought */
     val boughtItemsWithIncreasingPrice: Counter<String> = Counter()
 
-    // Maps to cities to all free buildings they contain
+    /** Maps cities by id to a set of all free buildings by name they contain.
+     *  The building name is the Nation-specific equivalent if available.
+     *  Sources: [UniqueType.FreeStatBuildings] **and** [UniqueType.FreeSpecificBuildings]
+     *  This is persisted and _never_ cleared or elements removed (per civ and game).
+     */
     private val freeBuildings: HashMap<String, HashSet<String>> = hashMapOf()
 
-    // Maps stats to the cities that have received a building of that stat
+    /** Maps stat names to a set of cities by id that have received a building of that stat.
-    // We can't use an enum instead of a string, due to the inability of the JSON-parser
+     *  Source: [UniqueType.FreeStatBuildings]
+     *  This is persisted and _never_ cleared or elements removed (per civ and game).
+     */
+    // We can't use the Stat enum instead of a string, due to the inability of the JSON-parser
     // to function properly and forcing this to be an `HashMap<String, HashSet<String>>`
     // when loading, even if this wasn't the original type, leading to run-time errors.
     private val freeStatBuildingsProvided: HashMap<String, HashSet<String>> = hashMapOf()
 
-    // Maps buildings to the cities that have received that building
+    /** Maps buildings by name to a set of cities by id that have received that building.
+     *  The building name is the Nation-specific equivalent if available.
+     *  Source: [UniqueType.FreeSpecificBuildings]
+     *  This is persisted and _never_ cleared or elements removed (per civ and game).
+     */
     private val freeSpecificBuildingsProvided: HashMap<String, HashSet<String>> = hashMapOf()
 
-    init {
-        for (stat in Stat.values()) {
-            freeStatBuildingsProvided[stat.name] = hashSetOf()
-        }
-    }
-
     fun clone(): CivConstructions {
         val toReturn = CivConstructions()
         toReturn.civInfo = civInfo
         toReturn.freeBuildings.putAll(freeBuildings)
         toReturn.freeStatBuildingsProvided.putAll(freeStatBuildingsProvided)
         toReturn.freeSpecificBuildingsProvided.putAll(freeSpecificBuildingsProvided)
-        toReturn.boughtItemsWithIncreasingPrice.add(boughtItemsWithIncreasingPrice.clone())
+        toReturn.boughtItemsWithIncreasingPrice.add(boughtItemsWithIncreasingPrice)  // add copies
         return toReturn
     }
 
@@ -57,21 +64,32 @@ class CivConstructions : IsPartOfGameInfoSerialization {
         addFreeSpecificBuildings()
     }
 
-    fun getFreeBuildings(cityId: String): HashSet<String> {
+    /** Common to [hasFreeBuildingByName] and [getFreeBuildingNames] - 'has' doesn't need the whole set, one enumeration is enough.
-        val toReturn = freeBuildings[cityId] ?: hashSetOf()
+     *  Note: Operates on String city.id and String building name, close to the serialized and stored form.
+     *  When/if we do a transient cache for these using our objects, please rewrite this.
+     */
+    private fun getFreeBuildingNamesSequence(cityId: String) = sequence {
+        yieldAllNotNull(freeBuildings[cityId])
         for (city in civInfo.cities) {
-            val freeBuildingsProvided =
+            yieldAllNotNull(city.cityConstructions.freeBuildingsProvidedFromThisCity[cityId])
-                city.cityConstructions.freeBuildingsProvidedFromThisCity[cityId]
-            if (freeBuildingsProvided != null)
-                toReturn.addAll(freeBuildingsProvided)
         }
-        return toReturn
     }
 
+    /** Gets a Set of all building names the [city] has for free, from nationwide sources or buildings in other cities */
+    fun getFreeBuildingNames(city: City) =
+        getFreeBuildingNamesSequence(city.id).toSet()
+
+    /** Tests whether the [city] has [building] for free, from nationwide sources or buildings in other cities */
+    fun hasFreeBuilding(city: City, building: Building) =
+        hasFreeBuildingByName(city.id, building.name)
+
+    /** Tests whether a city by [cityId] has a building named [buildingName] for free, from nationwide sources or buildings in other cities */
+    private fun hasFreeBuildingByName(cityId: String, buildingName: String) =
+        getFreeBuildingNamesSequence(cityId).contains(buildingName)
+
     private fun addFreeBuilding(cityId: String, building: String) {
-        if (!freeBuildings.containsKey(cityId))
+        freeBuildings.getOrPut(cityId) { hashSetOf() }
-            freeBuildings[cityId] = hashSetOf()
+            .add(civInfo.getEquivalentBuilding(building).name)
-        freeBuildings[cityId]!!.add(civInfo.getEquivalentBuilding(building).name)
     }
 
     private fun addFreeStatsBuildings() {
@@ -79,7 +97,6 @@ class CivConstructions : IsPartOfGameInfoSerialization {
             .groupBy { it.params[0] }
             .mapKeys { Stat.valueOf(it.key) }
             .mapValues { unique -> unique.value.sumOf { it.params[1].toInt() } }
-            .toMutableMap()
 
         for ((stat, amount) in statUniquesData) {
             addFreeStatBuildings(stat, amount)
@@ -88,11 +105,12 @@ class CivConstructions : IsPartOfGameInfoSerialization {
 
     private fun addFreeStatBuildings(stat: Stat, amount: Int) {
         for (city in civInfo.cities.take(amount)) {
-            if (freeStatBuildingsProvided[stat.name]!!.contains(city.id) || !city.cityConstructions.hasBuildableStatBuildings(stat)) continue
+            if (freeStatBuildingsProvided[stat.name]?.contains(city.id) == true) continue
+            if (!city.cityConstructions.hasBuildableStatBuildings(stat)) continue
 
             val builtBuilding = city.cityConstructions.addCheapestBuildableStatBuilding(stat)
             if (builtBuilding != null) {
-                freeStatBuildingsProvided[stat.name]!!.add(city.id)
+                freeStatBuildingsProvided.getOrPut(stat.name) { hashSetOf() }.add(city.id)
                 addFreeBuilding(city.id, builtBuilding)
             }
         }
@@ -117,14 +135,18 @@ class CivConstructions : IsPartOfGameInfoSerialization {
 
             building.postBuildEvent(city.cityConstructions)
 
-            if (!freeSpecificBuildingsProvided.containsKey(building.name))
+            freeSpecificBuildingsProvided.getOrPut(building.name) { hashSetOf() }.add(city.id)
-                freeSpecificBuildingsProvided[building.name] = hashSetOf()
-            freeSpecificBuildingsProvided[building.name]!!.add(city.id)
-
             addFreeBuilding(city.id, building.name)
         }
     }
 
+    /** Calculates a civ-wide total for [objectToCount].
+     *
+     *  It counts:
+     *  * "Spaceship part" units added to "spaceship" in capital
+     *  * Built buildings or those in a construction queue
+     *  * Units on the map or being constructed
+     */
     fun countConstructedObjects(objectToCount: INonPerpetualConstruction): Int {
         val amountInSpaceShip = civInfo.victoryManager.currentsSpaceshipParts[objectToCount.name]
 

core/src/com/unciv/models/ruleset/Building.kt

@@ -109,7 +109,7 @@ class Building : RulesetStatsObject(), INonPerpetualConstruction {
     fun getDescription(city: City, showAdditionalInfo: Boolean): String {
         val stats = getStats(city)
         val translatedLines = ArrayList<String>() // Some translations require special handling
-        val isFree = name in city.civ.civConstructions.getFreeBuildings(city.id)
+        val isFree = city.civ.civConstructions.hasFreeBuilding(city, this)
         if (uniqueTo != null) translatedLines += if (replaces == null) "Unique to [$uniqueTo]".tr()
             else "Unique to [$uniqueTo], replaces [$replaces]".tr()
         val missingUnique = getMatchingUniques(UniqueType.RequiresBuildingInAllCities).firstOrNull()
@@ -467,7 +467,7 @@ class Building : RulesetStatsObject(), INonPerpetualConstruction {
         for (unique in uniqueObjects) {
             if (unique.type != UniqueType.OnlyAvailableWhen &&
                 !unique.conditionalsApply(StateForConditionals(civ, cityConstructions.city))) continue
-                
+
             @Suppress("NON_EXHAUSTIVE_WHEN")
             when (unique.type) {
                 // for buildings that are created as side effects of other things, and not directly built,

core/src/com/unciv/ui/components/extensions/CollectionExtensions.kt

@@ -68,3 +68,18 @@ fun <T> Iterable<T>.toGdxArray(): Array<T> {
     for (it in this) arr.add(it)
     return arr
 }
+
+/** [yield][SequenceScope.yield]s [element] if it's not null */
+suspend fun <T> SequenceScope<T>.yieldIfNotNull(element: T?) {
+    if (element != null) yield(element)
+}
+/** [yield][SequenceScope.yield]s all elements of [elements] if it's not null */
+suspend fun <T> SequenceScope<T>.yieldAllNotNull(elements: Iterable<T>?) {
+    if (elements != null) yieldAll(elements)
+}
+@JvmName("yieldAllNotNullNotNull")
+/** [yield][SequenceScope.yield]s all non-null elements of [elements] if it's not null */
+suspend fun <T> SequenceScope<T>.yieldAllNotNull(elements: Iterable<T?>?) {
+    if (elements == null) return
+    for (element in elements) yieldIfNotNull(element)
+}

core/src/com/unciv/ui/screens/cityscreen/CityScreen.kt

@@ -440,6 +440,10 @@ class CityScreen(
         update()
     }
 
+    /** Convenience shortcut to [CivConstructions.hasFreeBuilding][com.unciv.logic.civilization.CivConstructions.hasFreeBuilding], nothing more */
+    internal fun hasFreeBuilding(building: Building) =
+        city.civ.civConstructions.hasFreeBuilding(city, building)
+
     fun selectConstruction(name: String) {
         selectConstruction(city.cityConstructions.getConstruction(name))
     }

core/src/com/unciv/ui/screens/cityscreen/CityStatsTable.kt

@@ -285,7 +285,7 @@ class CityStatsTable(private val cityScreen: CityScreen): Table() {
         val statsAndSpecialists = Table()
 
         val icon = ImageGetter.getConstructionPortrait(building.name, 50f)
-        val isFree = building.name in cityScreen.city.civ.civConstructions.getFreeBuildings(cityScreen.city.id)
+        val isFree = cityScreen.hasFreeBuilding(building)
         val displayName = if (isFree) "{${building.name}} ({Free})" else building.name
 
         info.add(displayName.toLabel(fontSize = Constants.defaultFontSize, hideIcons = true)).padBottom(5f).right().row()

core/src/com/unciv/ui/screens/cityscreen/ConstructionInfoTable.kt

@@ -16,6 +16,7 @@ import com.unciv.models.translations.tr
 import com.unciv.ui.components.Fonts
 import com.unciv.ui.components.extensions.darken
 import com.unciv.ui.components.extensions.disable
+import com.unciv.ui.components.extensions.isEnabled
 import com.unciv.ui.components.extensions.toTextButton
 import com.unciv.ui.components.input.onClick
 import com.unciv.ui.images.ImageGetter
@@ -99,22 +100,15 @@ class ConstructionInfoTable(val cityScreen: CityScreen): Table() {
                 row()
                 add(sellBuildingButton).padTop(5f).colspan(2).center()
 
-                sellBuildingButton.onClick(UncivSound.Coin) {
+                val isFree = cityScreen.hasFreeBuilding(construction)
+                val enableSell = !isFree &&
+                    !cityScreen.city.isPuppet &&
+                    cityScreen.canChangeState &&
+                    (!cityScreen.city.hasSoldBuildingThisTurn || cityScreen.city.civ.gameInfo.gameParameters.godMode)
+                sellBuildingButton.isEnabled = enableSell
+                if (sellBuildingButton.isEnabled) sellBuildingButton.onClick(UncivSound.Coin) {
                     sellBuildingButton.disable()
-                    cityScreen.closeAllPopups()
+                    sellBuildingClicked(construction, isFree, sellText)
-
-                    ConfirmPopup(
-                        cityScreen,
-                        "Are you sure you want to sell this [${construction.name}]?",
-                        sellText,
-                        restoreDefault = {
-                            cityScreen.update()
-                        }
-                    ) {
-                        cityScreen.city.sellBuilding(construction)
-                        cityScreen.clearSelection()
-                        cityScreen.update()
-                    }.open()
                 }
 
                 if (cityScreen.city.hasSoldBuildingThisTurn && !cityScreen.city.civ.gameInfo.gameParameters.godMode
@@ -125,4 +119,24 @@ class ConstructionInfoTable(val cityScreen: CityScreen): Table() {
         }
     }
 
+    private fun sellBuildingClicked(construction: Building, isFree: Boolean, sellText: String) {
+        cityScreen.closeAllPopups()
+
+        ConfirmPopup(
+            cityScreen,
+            "Are you sure you want to sell this [${construction.name}]?",
+            sellText,
+            restoreDefault = {
+                cityScreen.update()
+            }
+        ) {
+            sellBuildingConfirmed(construction, isFree)
+        }.open()
+    }
+
+    private fun sellBuildingConfirmed(construction: Building, isFree: Boolean) {
+        cityScreen.city.sellBuilding(construction)
+        cityScreen.clearSelection()
+        cityScreen.update()
+    }
 }