From dc707382f3f8a9f2538132604f8ecff8860e0820 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Sat, 3 Jun 2023 21:44:51 +0200 Subject: [PATCH] More reapply CityFocus on yield changes (#9459) --- core/src/com/unciv/logic/city/City.kt | 16 +++++++ .../city/managers/CityExpansionManager.kt | 3 ++ .../logic/city/managers/CityTurnManager.kt | 1 - core/src/com/unciv/logic/map/tile/Tile.kt | 10 +++- core/src/com/unciv/models/ruleset/Building.kt | 19 ++++++-- .../models/ruleset/tile/TileImprovement.kt | 5 ++ .../unciv/ui/screens/cityscreen/CityScreen.kt | 47 ++++++++++++------- .../screens/cityscreen/CityScreenTileTable.kt | 45 +++--------------- .../unit/actions/UnitActionsPillage.kt | 3 +- 9 files changed, 87 insertions(+), 62 deletions(-) diff --git a/core/src/com/unciv/logic/city/City.kt b/core/src/com/unciv/logic/city/City.kt index eccbca52d5..096d1379a6 100644 --- a/core/src/com/unciv/logic/city/City.kt +++ b/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, diff --git a/core/src/com/unciv/logic/city/managers/CityExpansionManager.kt b/core/src/com/unciv/logic/city/managers/CityExpansionManager.kt index 2867d3ab1a..f7cc8e9ef1 100644 --- a/core/src/com/unciv/logic/city/managers/CityExpansionManager.kt +++ b/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 { diff --git a/core/src/com/unciv/logic/city/managers/CityTurnManager.kt b/core/src/com/unciv/logic/city/managers/CityTurnManager.kt index 2ad6803c6e..864c1c58c9 100644 --- a/core/src/com/unciv/logic/city/managers/CityTurnManager.kt +++ b/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() diff --git a/core/src/com/unciv/logic/map/tile/Tile.kt b/core/src/com/unciv/logic/map/tile/Tile.kt index 6cda58a9ff..fdfc5b6533 100644 --- a/core/src/com/unciv/logic/map/tile/Tile.kt +++ b/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() } diff --git a/core/src/com/unciv/models/ruleset/Building.kt b/core/src/com/unciv/models/ruleset/Building.kt index 43f8e1c798..d7556491f7 100644 --- a/core/src/com/unciv/models/ruleset/Building.kt +++ b/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() - cityConstructions.city.updateCitizens = false + // Happiness is global, so it could affect all cities + 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 diff --git a/core/src/com/unciv/models/ruleset/tile/TileImprovement.kt b/core/src/com/unciv/models/ruleset/tile/TileImprovement.kt index 86cc797355..54ee0003e3 100644 --- a/core/src/com/unciv/models/ruleset/tile/TileImprovement.kt +++ b/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() } /** diff --git a/core/src/com/unciv/ui/screens/cityscreen/CityScreen.kt b/core/src/com/unciv/ui/screens/cityscreen/CityScreen.kt index dfca9e4a1d..65aef724ad 100644 --- a/core/src/com/unciv/ui/screens/cityscreen/CityScreen.kt +++ b/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) { - - 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() + askToBuyTile(tile) } } + /** 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 diff --git a/core/src/com/unciv/ui/screens/cityscreen/CityScreenTileTable.kt b/core/src/com/unciv/ui/screens/cityscreen/CityScreenTileTable.kt index 5df9e8a641..b89fb4026b 100644 --- a/core/src/com/unciv/ui/screens/cityscreen/CityScreenTileTable.kt +++ b/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() diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsPillage.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsPillage.kt index bff086e426..37621cfbd2 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsPillage.kt +++ b/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)