More reapply CityFocus on yield changes (#9459)

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

@@ -1,6 +1,7 @@
 package com.unciv.logic.city
 
 import com.badlogic.gdx.math.Vector2
+import com.unciv.GUI
 import com.unciv.logic.IsPartOfGameInfoSerialization
 import com.unciv.logic.city.managers.CityEspionageManager
 import com.unciv.logic.city.managers.CityExpansionManager
@@ -403,6 +404,10 @@ class City : IsPartOfGameInfoSerialization {
         reassignPopulation(resetLocked = true)
     }
 
+    /** Apply worked tiles optimization (aka CityFocus) - Expensive!
+     *
+     *  If the next City.startTurn is soon enough, then use [reassignPopulationDeferred] instead.
+     */
     fun reassignPopulation(resetLocked: Boolean = false) {
         if (resetLocked) {
             workedTiles = hashSetOf()
@@ -412,9 +417,20 @@ class City : IsPartOfGameInfoSerialization {
         }
         if (!manualSpecialists)
             population.specialistAllocations.clear()
+        updateCitizens = false
         population.autoAssignPopulation()
     }
 
+    /** Apply worked tiles optimization (aka CityFocus) -
+     *  immediately for a human player whoes turn it is (interactive),
+     *  or deferred to the next startTurn while nextTurn is running (for AI)
+     *  @see reassignPopulation
+     */
+    fun reassignPopulationDeferred() {
+        // TODO - is this the best (or even correct) way to detect "interactive" UI calls?
+        if (GUI.isMyTurn() && GUI.getViewingPlayer() == civ) reassignPopulation()
+        else updateCitizens = true
+    }
 
     fun destroyCity(overrideSafeties: Boolean = false) {
         // Original capitals and holy cities cannot be destroyed,

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

@@ -73,6 +73,9 @@ class CityExpansionManager : IsPartOfGameInfoSerialization {
             throw NotEnoughGoldToBuyTileException()
         city.civ.addGold(-goldCost)
         takeOwnership(tile)
+
+        // Reapply worked tiles optimization (aka CityFocus) - doing it here means AI profits too
+        city.reassignPopulationDeferred()
     }
 
     fun getGoldCostOfTile(tile: Tile): Int {

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

@@ -33,7 +33,6 @@ class CityTurnManager(val city: City) {
             city.reassignAllPopulation()
         } else if (city.updateCitizens) {
             city.reassignPopulation()  // includes cityStats.update
-            city.updateCitizens = false
         } else
             city.cityStats.update()
 

core/src/com/unciv/logic/map/tile/Tile.kt

@@ -883,7 +883,11 @@ open class Tile : IsPartOfGameInfoSerialization {
             return
         // http://well-of-souls.com/civ/civ5_improvements.html says that naval improvements are destroyed upon pillage
         //    and I can't find any other sources so I'll go with that
-        if (!isLand) { changeImprovement(null); return }
+        if (!isLand) {
+            changeImprovement(null)
+            owningCity?.reassignPopulationDeferred()
+            return
+        }
 
         // Setting turnsToImprovement might interfere with UniqueType.CreatesOneImprovement
         improvementFunctions.removeCreatesOneImprovementMarker()
@@ -902,6 +906,8 @@ open class Tile : IsPartOfGameInfoSerialization {
             else
                 roadIsPillaged = true
         }
+
+        owningCity?.reassignPopulationDeferred()
     }
 
     fun isPillaged(): Boolean {
@@ -915,6 +921,8 @@ open class Tile : IsPartOfGameInfoSerialization {
             improvementIsPillaged = false
         else
             roadIsPillaged = false
+
+        owningCity?.reassignPopulationDeferred()
     }
 
 

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

@@ -23,6 +23,7 @@ import com.unciv.ui.components.extensions.getConsumesAmountString
 import com.unciv.ui.components.extensions.getNeedMoreAmountString
 import com.unciv.ui.components.extensions.toPercent
 import com.unciv.ui.screens.civilopediascreen.FormattedLine
+import com.unciv.utils.Concurrency
 import kotlin.math.pow
 
 
@@ -690,10 +691,22 @@ class Building : RulesetStatsObject(), INonPerpetualConstruction {
             civInfo.tech.addScience(civInfo.tech.scienceOfLast8Turns.sum() / 8)
 
         // Happiness change _may_ invalidate best worked tiles (#9238), but if the building
-        // isn't bought then reassignPopulation will run later in startTurn anyway
+        // isn't bought (or the AI bought it) then reassignPopulation will run later in startTurn anyway
         if (boughtWith != null && isStatRelated(Stat.Happiness)) {
-            cityConstructions.city.reassignPopulation()
+            // Happiness is global, so it could affect all cities
-            cityConstructions.city.updateCitizens = false
+            Concurrency.runOnNonDaemonThreadPool("reassignPopulationAllCities") {
+                for (city in civInfo.cities)
+                    city.reassignPopulationDeferred()
+            }
+        }
+
+        // Buying a building influencing tile yield may change CityFocus decisions
+        val uniqueTypesModifyingYields = listOf(
+            UniqueType.StatsFromTiles, UniqueType.StatsFromTilesWithout, UniqueType.StatsFromObject,
+            UniqueType.StatPercentFromObject, UniqueType.AllStatsPercentFromObject
+        )
+        if (boughtWith != null && uniqueTypesModifyingYields.any { hasUnique(it) }) {
+            cityConstructions.city.reassignPopulationDeferred()
         }
 
         cityConstructions.city.cityStats.update() // new building, new stats

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

@@ -75,8 +75,10 @@ class TileImprovement : RulesetStatsObject() {
 
     fun handleImprovementCompletion(builder: MapUnit) {
         val tile = builder.getTile()
+
         if (hasUnique(UniqueType.TakesOverAdjacentTiles))
             UnitActions.takeOverTilesAround(builder)
+
         if (tile.resource != null) {
             val city = builder.getTile().getCity()
             if (city != null) {
@@ -85,6 +87,7 @@ class TileImprovement : RulesetStatsObject() {
                 city.civ.cache.updateCivResources()
             }
         }
+
         if (hasUnique(UniqueType.RemovesFeaturesIfBuilt)) {
             // Remove terrainFeatures that a Worker can remove
             // and that aren't explicitly allowed under the improvement
@@ -100,6 +103,8 @@ class TileImprovement : RulesetStatsObject() {
 
             tile.setTerrainFeatures(tile.terrainFeatures.filterNot { it in removableTerrainFeatures })
         }
+
+        tile.owningCity?.reassignPopulationDeferred()
     }
 
     /**

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

@@ -39,6 +39,7 @@ import com.unciv.ui.popups.ConfirmPopup
 import com.unciv.ui.popups.ToastPopup
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.basescreen.RecreateOnResize
+import com.unciv.ui.popups.closeAllPopups
 import com.unciv.ui.screens.worldscreen.WorldScreen
 
 class CityScreen(
@@ -358,25 +359,39 @@ class CityScreen(
             update()
 
         } else if (tileGroup.tileState == CityTileState.PURCHASABLE) {
-
+            askToBuyTile(tile)
-            val price = city.expansion.getGoldCostOfTile(tile)
-            val purchasePrompt = "Currently you have [${city.civ.gold}] [Gold].".tr() + "\n\n" +
-                "Would you like to purchase [Tile] for [$price] [${Stat.Gold.character}]?".tr()
-            ConfirmPopup(
-                this,
-                purchasePrompt,
-                "Purchase",
-                true,
-                restoreDefault = { update() }
-            ) {
-                SoundPlayer.play(UncivSound.Coin)
-                city.expansion.buyTile(tile)
-                // preselect the next tile on city screen rebuild so bulk buying can go faster
-                UncivGame.Current.replaceCurrentScreen(CityScreen(city, initSelectedTile = city.expansion.chooseNewTileToOwn()))
-            }.open()
         }
     }
 
+    /** Ask whether user wants to buy [selectedTile] for gold.
+     *
+     * Used from onClick and keyboard dispatch, thus only minimal parameters are passed,
+     * and it needs to do all checks and the sound as appropriate.
+     */
+    internal fun askToBuyTile(selectedTile: Tile) {
+        // These checks are redundant for the onClick action, but not for the keyboard binding
+        if (!canChangeState || !city.expansion.canBuyTile(selectedTile)) return
+        val goldCostOfTile = city.expansion.getGoldCostOfTile(selectedTile)
+        if (!city.civ.hasStatToBuy(Stat.Gold, goldCostOfTile)) return
+
+        closeAllPopups()
+
+        val purchasePrompt = "Currently you have [${city.civ.gold}] [Gold].".tr() + "\n\n" +
+            "Would you like to purchase [Tile] for [$goldCostOfTile] [${Stat.Gold.character}]?".tr()
+        ConfirmPopup(
+            this,
+            purchasePrompt,
+            "Purchase",
+            true,
+            restoreDefault = { update() }
+        ) {
+            SoundPlayer.play(UncivSound.Coin)
+            city.expansion.buyTile(selectedTile)
+            // preselect the next tile on city screen rebuild so bulk buying can go faster
+            UncivGame.Current.replaceCurrentScreen(CityScreen(city, initSelectedTile = city.expansion.chooseNewTileToOwn()))
+        }.open()
+    }
+
 
     private fun tileWorkedIconDoubleClick(tileGroup: CityTileGroup, city: City) {
         if (!canChangeState || city.isPuppet || tileGroup.tileState != CityTileState.WORKABLE) return

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

@@ -5,18 +5,8 @@ import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.UncivGame
 import com.unciv.logic.map.tile.Tile
 import com.unciv.logic.map.tile.TileDescription
-import com.unciv.models.UncivSound
 import com.unciv.models.stats.Stat
 import com.unciv.models.stats.Stats
-import com.unciv.models.translations.tr
-import com.unciv.ui.audio.SoundPlayer
-import com.unciv.ui.screens.civilopediascreen.CivilopediaScreen
-import com.unciv.ui.screens.civilopediascreen.FormattedLine.IconDisplay
-import com.unciv.ui.screens.civilopediascreen.MarkupRenderer
-import com.unciv.ui.images.ImageGetter
-import com.unciv.ui.popups.ConfirmPopup
-import com.unciv.ui.popups.closeAllPopups
-import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.components.UncivTooltip.Companion.addTooltip
 import com.unciv.ui.components.extensions.darken
 import com.unciv.ui.components.extensions.disable
@@ -26,6 +16,11 @@ import com.unciv.ui.components.extensions.onActivation
 import com.unciv.ui.components.extensions.onClick
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.screens.basescreen.BaseScreen
+import com.unciv.ui.screens.civilopediascreen.CivilopediaScreen
+import com.unciv.ui.screens.civilopediascreen.FormattedLine.IconDisplay
+import com.unciv.ui.screens.civilopediascreen.MarkupRenderer
 import kotlin.math.roundToInt
 
 class CityScreenTileTable(private val cityScreen: CityScreen): Table() {
@@ -64,7 +59,7 @@ class CityScreenTileTable(private val cityScreen: CityScreen): Table() {
             val buyTileButton = "Buy for [$goldCostOfTile] gold".toTextButton()
             buyTileButton.onActivation {
                 buyTileButton.disable()
-                askToBuyTile(selectedTile)
+                cityScreen.askToBuyTile(selectedTile)
             }
             buyTileButton.keyShortcuts.add('T')
             buyTileButton.isEnabled =  cityScreen.canChangeState && city.civ.hasStatToBuy(Stat.Gold, goldCostOfTile)
@@ -108,34 +103,6 @@ class CityScreenTileTable(private val cityScreen: CityScreen): Table() {
         pack()
     }
 
-    /** Ask whether user wants to buy [selectedTile] for gold.
-     *
-     * Used from onClick and keyboard dispatch, thus only minimal parameters are passed,
-     * and it needs to do all checks and the sound as appropriate.
-     */
-    private fun askToBuyTile(selectedTile: Tile) {
-        // These checks are redundant for the onClick action, but not for the keyboard binding
-        if (!cityScreen.canChangeState || !city.expansion.canBuyTile(selectedTile)) return
-        val goldCostOfTile = city.expansion.getGoldCostOfTile(selectedTile)
-        if (!city.civ.hasStatToBuy(Stat.Gold, goldCostOfTile)) return
-
-        cityScreen.closeAllPopups()
-
-        val purchasePrompt = "Currently you have [${city.civ.gold}] [Gold].".tr() + "\n\n" +
-                "Would you like to purchase [Tile] for [$goldCostOfTile] [${Stat.Gold.character}]?".tr()
-        ConfirmPopup(
-            cityScreen,
-            purchasePrompt,
-            "Purchase",
-            true,
-            restoreDefault = { cityScreen.update() }
-        ) {
-            SoundPlayer.play(UncivSound.Coin)
-            city.expansion.buyTile(selectedTile)
-            // preselect the next tile on city screen rebuild so bulk buying can go faster
-            UncivGame.Current.replaceCurrentScreen(CityScreen(city, initSelectedTile = city.expansion.chooseNewTileToOwn()))
-        }.open()
-    }
 
     private fun getTileStatsTable(stats: Stats): Table {
         val statsTable = Table()

core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsPillage.kt

@@ -59,9 +59,8 @@ object UnitActionsPillage {
                 )
 
                 pillageLooting(tile, unit)
-                tile.setPillaged()
+                tile.setPillaged()  // Also triggers reassignPopulation
                 if (tile.resource != null) tile.getOwner()?.cache?.updateCivResources()    // this might take away a resource
-                tile.getCity()?.updateCitizens = true
 
                 val freePillage = unit.hasUnique(UniqueType.NoMovementToPillage, checkCivInfoUniques = true)
                 if (!freePillage) unit.useMovementPoints(1f)