Resource supply list reorg (#6881)

* ResourceSupplyList reorg

* ResourceSupplyList reorg - instrumentation

* ResourceSupplyList reorg - map of maps

* ResourceSupplyList reorg - revert to ArrayList

* ResourceSupplyList reorg - review

* ResourceSupplyList reorg - review

* ResourceSupplyList reorg - almost-immutability

core/src/com/unciv/Constants.kt

@@ -41,6 +41,11 @@ object Constants {
     const val peaceTreaty = "Peace Treaty"
     const val researchAgreement = "Research Agreement"
     const val openBorders = "Open Borders"
+    /** Used as origin in StatMap or ResourceSupplyList, or the toggle button in DiplomacyOverviewTab */
+    const val cityStates = "City-States"
+    /** Used as origin in ResourceSupplyList */
+    const val tradable = "Tradable"
+
     const val random = "Random"
     const val unknownNationName = "???"
     const val unknownCityName = "???"
@@ -79,6 +84,7 @@ object Constants {
      * _Most_ checks do compare to 0!
      */
     const val minimumMovementEpsilon = 0.05f  // 0.1f was used previously, too - here for global searches
+
     const val defaultFontSize = 18
     const val headingFontSize = 24
 }

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

@@ -386,8 +386,10 @@ class CityInfo {
     fun isWeLoveTheKingDayActive() = hasFlag(CityFlags.WeLoveTheKing)
     fun isInResistance() = hasFlag(CityFlags.Resistance)
 
-    /** @return the number of tiles 4 out from this city that could hold a city, ie how lonely this city is */
+    /** @return the number of tiles 4 (un-modded) out from this city that could hold a city, ie how lonely this city is */
-    fun getFrontierScore() = getCenterTile().getTilesAtDistance(4).count { it.canBeSettled() && (it.getOwner() == null || it.getOwner() == civInfo ) }
+    fun getFrontierScore() = getCenterTile()
+        .getTilesAtDistance(civInfo.gameInfo.ruleSet.modOptions.constants.minimalCityDistance + 1)
+        .count { it.canBeSettled() && (it.getOwner() == null || it.getOwner() == civInfo ) }
 
     fun getRuleset() = civInfo.gameInfo.ruleSet
 
@@ -397,11 +399,9 @@ class CityInfo {
         for (tileInfo in getTiles().filter { it.resource != null }) {
             val resource = tileInfo.tileResource
             val amount = getTileResourceAmount(tileInfo) * civInfo.getResourceModifier(resource)
-            if (amount > 0) cityResources.add(resource, amount, "Tiles")
+            if (amount > 0) cityResources.add(resource, "Tiles", amount)
         }
 
-
-
         for (tileInfo in getTiles()) {
             val stateForConditionals = StateForConditionals(civInfo, this, tile = tileInfo)
             if (tileInfo.improvement == null) continue
@@ -409,17 +409,15 @@ class CityInfo {
             for (unique in tileImprovement!!.getMatchingUniques(UniqueType.ProvidesResources, stateForConditionals)) {
                 val resource = getRuleset().tileResources[unique.params[1]] ?: continue
                 cityResources.add(
-                    resource,
+                    resource, "Improvements",
-                    unique.params[0].toInt() * civInfo.getResourceModifier(resource),
+                    unique.params[0].toInt() * civInfo.getResourceModifier(resource)
-                    "Improvements"
                 )
             }
             for (unique in tileImprovement.getMatchingUniques(UniqueType.ConsumesResources, stateForConditionals)) {
                 val resource = getRuleset().tileResources[unique.params[1]] ?: continue
                 cityResources.add(
-                    resource,
+                    resource, "Improvements",
-                    -1 * unique.params[0].toInt(),
+                    -1 * unique.params[0].toInt()
-                    "Improvements"
                 )
             }
         }
@@ -427,28 +425,22 @@ class CityInfo {
         val freeBuildings = civInfo.civConstructions.getFreeBuildings(id)
         for (building in cityConstructions.getBuiltBuildings()) {
             // Free buildings cost no resources
-            if (building.name in freeBuildings)
+            if (building.name in freeBuildings) continue
-                continue
+            cityResources.subtractResourceRequirements(building.getResourceRequirements(), getRuleset(), "Buildings")
-            for ((resourceName, amount) in building.getResourceRequirements()) {
-                val resource = getRuleset().tileResources[resourceName]!!
-                cityResources.add(resource, -amount, "Buildings")
-            }
         }
 
         for (unique in getLocalMatchingUniques(UniqueType.ProvidesResources, StateForConditionals(civInfo, this))) { // E.G "Provides [1] [Iron]"
             val resource = getRuleset().tileResources[unique.params[1]]
-            if (resource != null) {
+                ?: continue
             cityResources.add(
-                    resource,
+                resource, "Buildings+",
-                    unique.params[0].toInt() * civInfo.getResourceModifier(resource),
+                unique.params[0].toInt() * civInfo.getResourceModifier(resource)
-                    "Buildings+"
             )
         }
-        }
+
         if (civInfo.isCityState() && isCapital() && civInfo.cityStateResource != null) {
             cityResources.add(
                 getRuleset().tileResources[civInfo.cityStateResource]!!,
-                1,
                 "Mercantile City-State"
             )
         }

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

@@ -1,5 +1,6 @@
 package com.unciv.logic.city
 
+import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.logic.civilization.CityStateType
 import com.unciv.logic.civilization.diplomacy.RelationshipLevel
@@ -465,7 +466,7 @@ class CityStats(val cityInfo: CityInfo) {
             getStatsFromSpecialists(cityInfo.population.getNewSpecialists())
         newBaseStatList["Trade routes"] = getStatsFromTradeRoute()
         newBaseStatTree.children["Buildings"] = statsFromBuildings
-        newBaseStatList["City-States"] = getStatsFromCityStates()
+        newBaseStatList[Constants.cityStates] = getStatsFromCityStates()
 
         for ((source, stats) in newBaseStatList)
             newBaseStatTree.addStats(stats, source)

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

@@ -1,5 +1,6 @@
 package com.unciv.logic.civilization
 
+import com.unciv.Constants
 import com.unciv.logic.automation.NextTurnAutomation
 import com.unciv.logic.civilization.diplomacy.*
 import com.unciv.models.metadata.GameSpeed
@@ -667,13 +668,10 @@ class CityStateFunctions(val civInfo: CivilizationInfo) {
         }
     }
 
-    fun getCityStateResourcesForAlly(): ResourceSupplyList {
+    fun getCityStateResourcesForAlly() = ResourceSupplyList().apply {
-        val newDetailedCivResources = ResourceSupplyList()
         for (city in civInfo.cities) {
-            for (resourceSupply in city.getCityResources())
+            // IGNORE the fact that they consume their own resources - #4769
-                if (resourceSupply.amount > 0) // IGNORE the fact that they consume their own resources - #4769
+            addPositiveByResource(city.getCityResources(), Constants.cityStates)
-                    newDetailedCivResources.add(resourceSupply.resource, resourceSupply.amount, "City-State")
+        }
-        }
-        return newDetailedCivResources
     }
 }

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

@@ -1,5 +1,6 @@
 package com.unciv.logic.civilization
 
+import com.unciv.Constants
 import com.unciv.logic.civilization.diplomacy.RelationshipLevel
 import com.unciv.logic.map.RoadStatus
 import com.unciv.models.metadata.BASE_GAME_DURATION_TURNS
@@ -169,7 +170,7 @@ class CivInfoStats(val civInfo: CivilizationInfo) {
                     cityStateBonus[Stat.valueOf(unique.params[1])] *= unique.params[0].toPercent()
                 }
 
-                statMap.add("City-States", cityStateBonus)
+                statMap.add(Constants.cityStates, cityStateBonus)
             }
 
             if (otherCiv.isCityState())
@@ -178,7 +179,7 @@ class CivInfoStats(val civInfo: CivilizationInfo) {
                             .relationshipLevel() != RelationshipLevel.Ally
                     ) continue
                     statMap.add(
-                        "City-States",
+                        Constants.cityStates,
                         Stats().add(
                             Stat.valueOf(unique.params[0]),
                             otherCiv.statsForNextTurn[Stat.valueOf(unique.params[0])] * unique.params[1].toFloat() / 100f
@@ -346,7 +347,7 @@ class CivInfoStats(val civInfo: CivilizationInfo) {
             }
         }
 
-        if (cityStatesHappiness > 0) statMap["City-States"] = cityStatesHappiness
+        if (cityStatesHappiness > 0) statMap[Constants.cityStates] = cityStatesHappiness
 
         return statMap
     }

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

@@ -1,8 +1,8 @@
 package com.unciv.logic.civilization
 
+import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.logic.map.TileInfo
-import com.unciv.models.ruleset.tile.ResourceSupply
 import com.unciv.models.ruleset.tile.ResourceSupplyList
 import com.unciv.models.ruleset.unique.UniqueType
 
@@ -174,35 +174,27 @@ class CivInfoTransientUpdater(val civInfo: CivilizationInfo) {
             for (unique in civInfo.getMatchingUniques(UniqueType.CityStateResources))
                 resourceBonusPercentage += unique.params[0].toFloat() / 100
             for (cityStateAlly in civInfo.getKnownCivs().filter { it.getAllyCiv() == civInfo.civName }) {
-                for (resource in cityStateAlly.cityStateFunctions.getCityStateResourcesForAlly()) {
+                for (resourceSupply in cityStateAlly.cityStateFunctions.getCityStateResourcesForAlly()) {
-                    cityStateProvidedResources.add(
+                    val newAmount = (resourceSupply.amount * resourceBonusPercentage).toInt()
-                        resource.apply { amount = (amount * resourceBonusPercentage).toInt() }
+                    cityStateProvidedResources.add(resourceSupply.copy(amount = newAmount))
-                    )
                 }
             }
             // Then we combine these into one
-            for (resourceSupply in cityStateProvidedResources.groupBy { it.resource }) {
+            newDetailedCivResources.addByResource(cityStateProvidedResources, Constants.cityStates)
-                newDetailedCivResources.add(ResourceSupply(resourceSupply.key, resourceSupply.value.sumOf { it.amount }, "City-States"))
         }
 
-        }
+        for (diplomacyManager in civInfo.diplomacy.values)
+            newDetailedCivResources.add(diplomacyManager.resourcesFromTrade())
 
-
-        for (diplomacyManager in civInfo.diplomacy.values) newDetailedCivResources.add(diplomacyManager.resourcesFromTrade())
         for (unit in civInfo.getCivUnits())
-            for ((resource, amount) in unit.baseUnit.getResourceRequirements())
+            newDetailedCivResources.subtractResourceRequirements(
-                newDetailedCivResources.add(civInfo.gameInfo.ruleSet.tileResources[resource]!!, -amount, "Units")
+                unit.baseUnit.getResourceRequirements(), civInfo.gameInfo.ruleSet, "Units")
 
         // Check if anything has actually changed so we don't update stats for no reason - this uses List equality which means it checks the elements
         if (civInfo.detailedCivResources == newDetailedCivResources) return
 
         civInfo.detailedCivResources = newDetailedCivResources
-
+        civInfo.summarizedCivResources = newDetailedCivResources.sumByResource("All")
-        val newSummarizedCivResources = ResourceSupplyList()
-        for (resourceSupply in newDetailedCivResources) {
-            newSummarizedCivResources.add(resourceSupply.resource, resourceSupply.amount, "All")
-        }
-        civInfo.summarizedCivResources = newSummarizedCivResources
 
         civInfo.updateStatsForNextTurn() // More or less resources = more or less happiness, with potential domino effects
     }

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

@@ -375,15 +375,15 @@ class CivilizationInfo {
 
     // Preserves some origins for resources so we can separate them for trades
     fun getCivResourcesWithOriginsForTrade(): ResourceSupplyList {
-        val newResourceSupplyList = ResourceSupplyList()
+        val newResourceSupplyList = ResourceSupplyList(keepZeroAmounts = true)
         for (resourceSupply in detailedCivResources) {
             // If we got it from another trade or from a CS, preserve the origin
-            if ((resourceSupply.origin == "City-States" || resourceSupply.origin == "Trade") && resourceSupply.amount > 0) {
+            if (resourceSupply.isCityStateOrTradeOrigin()) {
-                newResourceSupplyList.add(resourceSupply.resource, resourceSupply.amount, resourceSupply.origin)
+                newResourceSupplyList.add(resourceSupply.copy())
-                newResourceSupplyList.add(resourceSupply.resource, 0, "Tradable") // Still add an empty "tradable" entry so it shows up in the list
+                newResourceSupplyList.add(resourceSupply.resource, Constants.tradable, 0) // Still add an empty "tradable" entry so it shows up in the list
             }
             else
-                newResourceSupplyList.add(resourceSupply.resource, resourceSupply.amount, "Tradable")
+                newResourceSupplyList.add(resourceSupply.resource, Constants.tradable, resourceSupply.amount)
         }
         return newResourceSupplyList
     }

core/src/com/unciv/logic/civilization/diplomacy/DiplomacyManager.kt

@@ -325,25 +325,25 @@ class DiplomacyManager() {
     }
 
     fun resourcesFromTrade(): ResourceSupplyList {
-        val counter = ResourceSupplyList()
+        val newResourceSupplyList = ResourceSupplyList()
         val resourcesMap = civInfo.gameInfo.ruleSet.tileResources
         val isResourceFilter: (TradeOffer) -> Boolean = {
             (it.type == TradeType.Strategic_Resource || it.type == TradeType.Luxury_Resource)
-                    && civInfo.gameInfo.ruleSet.tileResources.containsKey(it.name)
+                    && resourcesMap.containsKey(it.name)
         }
         for (trade in trades) {
             for (offer in trade.ourOffers.filter(isResourceFilter))
-                counter.add(resourcesMap[offer.name]!!, -offer.amount, "Trade")
+                newResourceSupplyList.add(resourcesMap[offer.name]!!, "Trade", -offer.amount)
             for (offer in trade.theirOffers.filter(isResourceFilter))
-                counter.add(resourcesMap[offer.name]!!, offer.amount, "Trade")
+                newResourceSupplyList.add(resourcesMap[offer.name]!!, "Trade", offer.amount)
         }
 
         for (trade in otherCiv().tradeRequests.filter { it.requestingCiv == civInfo.civName }) {
             for (offer in trade.trade.theirOffers.filter(isResourceFilter))
-                counter.add(resourcesMap[offer.name]!!, -offer.amount, "Trade request")
+                newResourceSupplyList.add(resourcesMap[offer.name]!!, "Trade request", -offer.amount)
         }
 
-        return counter
+        return newResourceSupplyList
     }
 
     /** Returns the [civilizations][CivilizationInfo] that know about both sides ([civInfo] and [otherCiv]) */

core/src/com/unciv/logic/trade/TradeLogic.kt

@@ -31,7 +31,7 @@ class TradeLogic(val ourCivilization:CivilizationInfo, val otherCivilization: Ci
 
         for (entry in civInfo.getCivResourcesWithOriginsForTrade()
             .filterNot { it.resource.resourceType == ResourceType.Bonus }
-            .filter { it.origin == "Tradable" }
+            .filter { it.origin == Constants.tradable }
         ) {
             val resourceTradeType = if (entry.resource.resourceType == ResourceType.Luxury) TradeType.Luxury_Resource
             else TradeType.Strategic_Resource

core/src/com/unciv/models/ruleset/tile/ResourceSupplyList.kt

@@ -0,0 +1,108 @@
+package com.unciv.models.ruleset.tile
+
+import com.unciv.Constants
+import com.unciv.models.ruleset.Ruleset
+import com.unciv.logic.city.IConstruction  // Kdoc only
+
+/** Container helps aggregating supply and demand of [resources][ResourceSupply.resource], categorized by [origin][ResourceSupply.origin].
+ *
+ *  @param keepZeroAmounts If `false`, entries with [amount][ResourceSupply.amount] 0 are eliminated
+ */
+class ResourceSupplyList(
+    private val keepZeroAmounts: Boolean = false
+) : ArrayList<ResourceSupplyList.ResourceSupply>(24) {
+    // initialCapacity 24: Allows all resources in G&K with just _one_ Array growth step (which is 50%)
+
+    /**
+     * Holds one "data row", [resource] and [origin] function as keys while [amount] is the 'value'
+     * This is not technically immutable, but **no** code outside [ResourceSupplyList] should update the value.
+     * [ResourceSupplyList.add] will update the value in existing instances, and should remain the only place.
+     */
+    data class ResourceSupply(val resource: TileResource, val origin: String, var amount: Int) {
+        fun isCityStateOrTradeOrigin() = (origin == Constants.cityStates || origin == "Trade") && amount > 0
+        override fun toString() = "$amount ${resource.name} from $origin"
+    }
+
+    /** Fetch a [ResourceSupply] entry or `null` if no match found */
+    fun get(resource: TileResource, origin: String) =
+        firstOrNull { it.resource == resource && it.origin == origin }
+
+    /** Get the total amount for a resource by [resourceName] */
+    fun sumBy(resourceName: String) =
+        asSequence().filter { it.resource.name == resourceName }.sumOf { it.amount }
+
+    /**
+     *  Add [element] unless one for [resource][ResourceSupply.resource]/[origin][ResourceSupply.origin] already exists,
+     *  in which case the amounts are added up. Ensures the list contains no entries with [amount][ResourceSupply.amount] 0 unless [keepZeroAmounts] is on.
+     *  @return `true` if the length of the list changed.
+     */
+    override fun add(element: ResourceSupply): Boolean {
+        val existingResourceSupply = get(element.resource, element.origin)
+        if (existingResourceSupply != null) {
+            // This is at the time of writing the _only_ place updating the field.
+            // To check: Change to val, comment out this line, compile, revert.
+            existingResourceSupply.amount += element.amount
+            if (keepZeroAmounts || existingResourceSupply.amount != 0) return false
+            remove(existingResourceSupply)
+        } else {
+            if (!keepZeroAmounts && element.amount == 0) return false
+            super.add(element)
+        }
+        return true
+    }
+
+    /** Add [amount] to the [entry][ResourceSupply] for [resource]/[origin] or create a new one. */
+    fun add(resource: TileResource, origin: String, amount: Int = 1) {
+        add(ResourceSupply(resource, origin, amount))
+    }
+
+    /** Add all [entries][ResourceSupply] from [resourceSupplyList] to this one. */
+    fun add(resourceSupplyList: ResourceSupplyList) {
+        for (resourceSupply in resourceSupplyList)
+            add(resourceSupply)
+    }
+
+    /** Add entries from a requirements list (as produced by [IConstruction.getResourceRequirements]), expressing requirement as negative supply. */
+    fun subtractResourceRequirements(resourceRequirements: HashMap<String, Int>, ruleset: Ruleset, origin: String) {
+        for ((resourceName, amount) in resourceRequirements) {
+            val resource = ruleset.tileResources[resourceName] ?: continue
+            add(resource, origin, -amount)
+        }
+    }
+
+    /**
+     *  Aggregate [fromList] by resource into this (by adding all entries replacing their origin with [newOrigin])
+     *  @return `this`, allowing chaining
+     */
+    fun addByResource(fromList: ResourceSupplyList, newOrigin: String): ResourceSupplyList {
+        for (resourceSupply in fromList)
+            add(resourceSupply.resource, newOrigin, resourceSupply.amount)
+        return this
+    }
+
+    /** Same as [addByResource] but ignores negative amounts */
+    fun addPositiveByResource(fromList: ResourceSupplyList, newOrigin: String) {
+        for (resourceSupply in fromList)
+            if (resourceSupply.amount > 0)
+                add(resourceSupply.resource, newOrigin, resourceSupply.amount)
+    }
+
+    /** Create a new [ResourceSupplyList] aggregating resources over all origins */
+    fun sumByResource(newOrigin: String) = ResourceSupplyList(keepZeroAmounts).addByResource(this, newOrigin)
+
+    /**
+     *  Remove all entries from a specific [origin]
+     *  @return `this`, allowing chaining
+     */
+    fun removeAll(origin: String): ResourceSupplyList {
+        // The filter creates a separate list so the iteration does not modify concurrently
+        filter { it.origin == origin }.forEach {
+            remove(it)
+        }
+        return this
+    }
+
+    companion object {
+        val emptyList = ResourceSupplyList()
+    }
+}

core/src/com/unciv/models/ruleset/tile/TileResource.kt

@@ -142,21 +142,3 @@ class TileResource : RulesetStatsObject() {
     }
 
 }
-
-
-data class ResourceSupply(val resource:TileResource, var amount:Int, val origin:String)
-
-class ResourceSupplyList:ArrayList<ResourceSupply>() {
-    fun add(resource: TileResource, amount: Int, origin: String) {
-        val existingResourceSupply = firstOrNull { it.resource == resource && it.origin == origin }
-        if (existingResourceSupply != null) {
-            existingResourceSupply.amount += amount
-            if (existingResourceSupply.amount == 0) remove(existingResourceSupply)
-        } else add(ResourceSupply(resource, amount, origin))
-    }
-
-    fun add(resourceSupplyList: ResourceSupplyList) {
-        for (resourceSupply in resourceSupplyList)
-            add(resourceSupply.resource, resourceSupply.amount, resourceSupply.origin)
-    }
-}

core/src/com/unciv/ui/overviewscreen/DiplomacyOverviewTable.kt

@@ -7,6 +7,7 @@ import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.scenes.scene2d.ui.TextButton
 import com.badlogic.gdx.utils.Align
+import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.logic.HexMath
 import com.unciv.logic.civilization.CivilizationInfo
@@ -35,7 +36,7 @@ class DiplomacyOverviewTab (
         defaults().pad(5f)
         background = ImageGetter.getBackground(Color.BLACK)
     }
-    val toggleCityStatesButton: TextButton = "City-States".toTextButton().apply {
+    val toggleCityStatesButton: TextButton = Constants.cityStates.toTextButton().apply {
         onClick {
             persistableData.includeCityStates = !persistableData.includeCityStates
             update()

core/src/com/unciv/ui/overviewscreen/ResourcesOverviewTable.kt

@@ -67,7 +67,7 @@ class ResourcesOverviewTab(
         .mapNotNull { ExtraInfoOrigin.safeValueOf(it.origin) }.distinct().toList()
 
     private fun ResourceSupplyList.getLabel(resource: TileResource, origin: String): Label? =
-        firstOrNull { it.resource == resource && it.origin == origin }?.amount?.toLabel()
+        get(resource, origin)?.amount?.toLabel()
     private fun ResourceSupplyList.getTotalLabel(resource: TileResource): Label =
         filter { it.resource == resource }.sumOf { it.amount }.toLabel()
     private fun getResourceImage(name: String) =
@@ -215,14 +215,14 @@ class ResourcesOverviewTab(
     }
 
     private fun getExtraDrilldown(): ResourceSupplyList {
-        val resourceSupplyList = ResourceSupplyList()
+        val newResourceSupplyList = ResourceSupplyList()
         for (city in viewingPlayer.cities) {
             if (city.demandedResource.isEmpty()) continue
             val wltkResource = gameInfo.ruleSet.tileResources[city.demandedResource] ?: continue
             if (city.isWeLoveTheKingDayActive()) {
-                resourceSupplyList.add(wltkResource, 1, ExtraInfoOrigin.CelebratingWLKT.name)
+                newResourceSupplyList.add(wltkResource, ExtraInfoOrigin.CelebratingWLKT.name)
             } else {
-                resourceSupplyList.add(wltkResource, 1, ExtraInfoOrigin.DemandingWLTK.name)
+                newResourceSupplyList.add(wltkResource, ExtraInfoOrigin.DemandingWLTK.name)
             }
             for (tile in city.getTiles()) {
                 if (tile.isCityCenter()) continue
@@ -231,9 +231,9 @@ class ResourcesOverviewTab(
                 if (tileResource.resourceType == ResourceType.Bonus) continue
                 if (tile.improvement != null && tileResource.isImprovedBy(tile.improvement!!)) continue
                 if (tileResource.resourceType == ResourceType.Strategic && tile.getTileImprovement()?.isGreatImprovement() == true) continue
-                resourceSupplyList.add(tileResource, 1, ExtraInfoOrigin.Unimproved.name)
+                newResourceSupplyList.add(tileResource, ExtraInfoOrigin.Unimproved.name)
             }
         }
-        return resourceSupplyList
+        return newResourceSupplyList
     }
 }

core/src/com/unciv/ui/trade/OfferColumnsTable.kt

@@ -1,6 +1,7 @@
 package com.unciv.ui.trade
 
 import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.unciv.Constants
 import com.unciv.logic.trade.TradeLogic
 import com.unciv.logic.trade.TradeOffer
 import com.unciv.logic.trade.TradeOffersList
@@ -72,8 +73,10 @@ class OfferColumnsTable(private val tradeLogic: TradeLogic, val screen: Diplomac
     fun update() {
         val ourFilteredOffers = tradeLogic.ourAvailableOffers.without(tradeLogic.currentTrade.ourOffers)
         val theirFilteredOffers = tradeLogic.theirAvailableOffers.without(tradeLogic.currentTrade.theirOffers)
-        val ourUntradables = tradeLogic.ourCivilization.getCivResourcesWithOriginsForTrade().filterNot { it.origin == "Tradable" }
+        val ourUntradables = tradeLogic.ourCivilization.getCivResourcesWithOriginsForTrade()
-        val theirUntradables = tradeLogic.otherCivilization.getCivResourcesWithOriginsForTrade().filterNot { it.origin == "Tradable" }
+            .removeAll(Constants.tradable)
+        val theirUntradables = tradeLogic.otherCivilization.getCivResourcesWithOriginsForTrade()
+            .removeAll(Constants.tradable)
         ourAvailableOffersTable.update(ourFilteredOffers, tradeLogic.theirAvailableOffers, ourUntradables)
         ourOffersTable.update(tradeLogic.currentTrade.ourOffers, tradeLogic.theirAvailableOffers)
         theirOffersTable.update(tradeLogic.currentTrade.theirOffers, tradeLogic.ourAvailableOffers)

core/src/com/unciv/ui/trade/OffersListScroll.kt

@@ -9,7 +9,7 @@ import com.unciv.logic.trade.TradeOffer
 import com.unciv.logic.trade.TradeOffersList
 import com.unciv.logic.trade.TradeType
 import com.unciv.logic.trade.TradeType.*
-import com.unciv.models.ruleset.tile.ResourceSupply
+import com.unciv.models.ruleset.tile.ResourceSupplyList
 import com.unciv.models.translations.tr
 import com.unciv.ui.images.IconTextButton
 import com.unciv.ui.images.ImageGetter
@@ -34,9 +34,13 @@ class OffersListScroll(
     /**
      * @param offersToDisplay The offers which should be displayed as buttons
      * @param otherOffers The list of other side's offers to compare with whether these offers are unique
-     * @param untradableOffers Things we got from sources that we can't trade on, displayed for completeness
+     * @param untradableOffers Things we got from sources that we can't trade on, displayed for completeness - should be aggregated per resource to "All" origin
      */
-    fun update(offersToDisplay:TradeOffersList, otherOffers: TradeOffersList, untradableOffers: List<ResourceSupply> = emptyList()) {
+    fun update(
+        offersToDisplay: TradeOffersList,
+        otherOffers: TradeOffersList,
+        untradableOffers: ResourceSupplyList = ResourceSupplyList.emptyList
+    ) {
         table.clear()
         expanderTabs.clear()
 
@@ -70,7 +74,7 @@ class OffersListScroll(
             }
 
             for (offer in offersOfType) {
-                val tradeLabel = offer.getOfferText(untradableOffers.filter { it.resource.name == offer.name }.sumOf { it.amount })
+                val tradeLabel = offer.getOfferText(untradableOffers.sumBy(offer.name))
                 val tradeIcon = when (offer.type) {
                     Luxury_Resource, Strategic_Resource ->
                         ImageGetter.getResourceImage(offer.name, 30f)

core/src/com/unciv/ui/worldscreen/WorldScreenTopBar.kt

@@ -231,8 +231,8 @@ class WorldScreenTopBar(val worldScreen: WorldScreen) : Table() {
             val isRevealed = resource.revealedBy == null || civInfo.tech.isResearched(resource.revealedBy!!)
             resourceLabels[resource.name]!!.isVisible = isRevealed
             resourceImages[resource.name]!!.isVisible = isRevealed
-            if (!civResources.any { it.resource == resource }) resourceLabels[resource.name]!!.setText("0")
+            val amountText = (civResources.get(resource, "All")?.amount ?: 0).toString()
-            else resourceLabels[resource.name]!!.setText(civResources.first { it.resource == resource }.amount.toString())
+            resourceLabels[resource.name]!!.setText(amountText)
         }
 
         val year = civInfo.gameInfo.getYear()