From c92afe0f041e5cc7a369dd0967848b4cd9fd6d96 Mon Sep 17 00:00:00 2001 From: Yair Morgenstern Date: Thu, 3 Aug 2023 11:39:04 +0300 Subject: [PATCH] Compare improvements by tile yield calculation (#9873) * Compare improvements by tile yield calculation * Docstring * Explicit documentation for which functions in Stats are and are not mutating functions --- .../com/unciv/logic/automation/Automation.kt | 10 ------ .../logic/automation/unit/WorkerAutomation.kt | 10 +++--- .../unciv/logic/map/tile/TileStatFunctions.kt | 30 ++++++++++++------ core/src/com/unciv/models/stats/Stats.kt | 31 ++++++++++++++----- .../unciv/ui/screens/cityscreen/CityScreen.kt | 13 +++----- .../pickerscreens/ImprovementPickerScreen.kt | 14 +-------- 6 files changed, 55 insertions(+), 53 deletions(-) diff --git a/core/src/com/unciv/logic/automation/Automation.kt b/core/src/com/unciv/logic/automation/Automation.kt index 656e2f3809..0a3de95314 100644 --- a/core/src/com/unciv/logic/automation/Automation.kt +++ b/core/src/com/unciv/logic/automation/Automation.kt @@ -388,16 +388,6 @@ object Automation { if (distance > 3) score += 100 } - // Improvements are good: less points - if (tile.improvement != null && - tile.stats.getImprovementStats( - tile.getTileImprovement()!!, - city.civ, - city, - localUniqueCache - ).values.sum() > 0f - ) score -= 5 - if (tile.naturalWonder != null) score -= 105 // Straight up take the sum of all yields diff --git a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt b/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt index b3d3a5c5db..57ea33be25 100644 --- a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt +++ b/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt @@ -391,18 +391,18 @@ class WorkerAutomation( if (potentialTileImprovements.isEmpty()) return null val cityUniqueCaches = HashMap() - fun getRankingWithImprovement(improvementName: String): Float { + fun getImprovementRanking(improvementName: String): Float { val improvement = ruleSet.tileImprovements[improvementName]!! val city = tile.getCity() val cache = if (city == null) LocalUniqueCache(false) else cityUniqueCaches.getOrPut(city) { LocalUniqueCache() } - val stats = tile.stats.getImprovementStats(improvement, civInfo, tile.getCity(), cache) + val stats = tile.stats.getStatDiffForImprovement(improvement, civInfo, tile.getCity(), cache) return Automation.rankStatsValue(stats, unit.civ) } val bestBuildableImprovement = potentialTileImprovements.values.asSequence() - .map { Pair(it, getRankingWithImprovement(it.name)) } + .map { Pair(it, getImprovementRanking(it.name)) } .filter { it.second > 0f } .maxByOrNull { it.second }?.first @@ -418,7 +418,7 @@ class WorkerAutomation( && !tile.providesResources(civInfo) && !isResourceImprovementAllowedOnFeature(tile, potentialTileImprovements) -> Constants.remove + lastTerrain.name else -> tile.tileResource.getImprovements().filter { it in potentialTileImprovements || it==tile.improvement } - .maxByOrNull { getRankingWithImprovement(it) } + .maxByOrNull { getImprovementRanking(it) } } // After gathering all the data, we conduct the hierarchy in one place @@ -430,7 +430,7 @@ class WorkerAutomation( bestBuildableImprovement == null -> null tile.improvement != null && - getRankingWithImprovement(tile.improvement!!) > getRankingWithImprovement(bestBuildableImprovement.name) + getImprovementRanking(tile.improvement!!) > getImprovementRanking(bestBuildableImprovement.name) -> null // What we have is better, even if it's pillaged we should repair it lastTerrain.let { diff --git a/core/src/com/unciv/logic/map/tile/TileStatFunctions.kt b/core/src/com/unciv/logic/map/tile/TileStatFunctions.kt index 1de681845d..e08ed95303 100644 --- a/core/src/com/unciv/logic/map/tile/TileStatFunctions.kt +++ b/core/src/com/unciv/logic/map/tile/TileStatFunctions.kt @@ -193,22 +193,34 @@ class TileStatFunctions(val tile: Tile) { food + production + gold } + /** Returns the extra stats that we would get if we switched to this improvement + * Can be negative if we're switching to a worse improvement */ + fun getStatDiffForImprovement( + improvement: TileImprovement, + observingCiv: Civilization, + city: City?, + cityUniqueCache: LocalUniqueCache = LocalUniqueCache(false)): Stats { + + val currentStats = getTileStats(city, observingCiv, cityUniqueCache) + + val tileClone = tile.clone() + tileClone.setTransients() + + if (improvement.name.startsWith(Constants.remove)) + tileClone.removeTerrainFeature(improvement.name.removePrefix(Constants.remove)) + else tileClone.changeImprovement(improvement.name) + val futureStats = tileClone.stats.getTileStats(city, observingCiv, cityUniqueCache) + + return futureStats.minus(currentStats) + } // Also multiplies the stats by the percentage bonus for improvements (but not for tiles) - fun getImprovementStats( + private fun getImprovementStats( improvement: TileImprovement, observingCiv: Civilization, city: City?, cityUniqueCache: LocalUniqueCache = LocalUniqueCache(false) ): Stats { - if (improvement.name.startsWith(Constants.remove)){ - val currentTileStats = getTileStats(city, observingCiv, cityUniqueCache) - val tileClone = tile.clone() - tileClone.removeTerrainFeature(improvement.name.removePrefix(Constants.remove)) - val tileStatsAfterRemoval = tileClone.stats.getTileStats(city, observingCiv, cityUniqueCache) - return tileStatsAfterRemoval.minus(currentTileStats) - } - val stats = improvement.cloneStats() if (tile.hasViewableResource(observingCiv) && tile.tileResource.isImprovedBy(improvement.name) && tile.tileResource.improvementStats != null diff --git a/core/src/com/unciv/models/stats/Stats.kt b/core/src/com/unciv/models/stats/Stats.kt index 16bcf64d69..c049342c7d 100644 --- a/core/src/com/unciv/models/stats/Stats.kt +++ b/core/src/com/unciv/models/stats/Stats.kt @@ -56,7 +56,8 @@ open class Stats( && faith == otherStats.faith } - /** @return a new instance containing the same values as `this` */ + /** **Non-Mutating function** + * @return a new instance containing the same values as `this` */ fun clone() = Stats(production, food, gold, science, culture, happiness, faith) /** @return `true` if all values are zero */ @@ -80,7 +81,9 @@ open class Stats( faith = 0f } - /** Adds each value of another [Stats] instance to this one in place */ + /** **Mutating function** + * Adds each value of another [Stats] instance to this one in place + * @return this for chaining */ fun add(other: Stats): Stats { production += other.production food += other.food @@ -92,20 +95,28 @@ open class Stats( return this } - /** @return a new [Stats] instance containing the sum of its operands value by value */ + /** **Non-mutating function** + * @return a new [Stats] instance */ operator fun plus(stats: Stats) = clone().apply { add(stats) } + + /** **Non-mutating function** + * @return a new [Stats] instance */ operator fun minus(stats: Stats) = clone().apply { add(stats.times(-1)) } - /** Adds the [value] parameter to the instance value specified by [stat] in place + /** **Mutating function** + * Adds the [value] parameter to the instance value specified by [stat] in place * @return `this` to allow chaining */ fun add(stat: Stat, value: Float): Stats { set(stat, value + get(stat)) return this } - /** @return The result of multiplying each value of this instance by [number] as a new instance */ + /** **Non-Mutating function** + * @return a new [Stats] instance with the result of multiplying each value of this instance by [number] as a new instance */ operator fun times(number: Int) = times(number.toFloat()) - /** @return The result of multiplying each value of this instance by [number] as a new instance */ + + /** **Non-Mutating function** + * @return a new [Stats] instance with the result of multiplying each value of this instance by [number] as a new instance */ operator fun times(number: Float) = Stats( production * number, food * number, @@ -116,7 +127,8 @@ open class Stats( faith * number ) - /** Multiplies each value of this instance by [number] in place */ + /** **Mutating function** + * Multiplies each value of this instance by [number] in place */ fun timesInPlace(number: Float) { production *= number food *= number @@ -127,9 +139,12 @@ open class Stats( faith *= number } + /** **Non-Mutating function** + * @return a new [Stats] instance */ operator fun div(number: Float) = times(1/number) - /** Apply weighting for Production Ranking */ + /** **Mutating function** + * Apply weighting for Production Ranking */ fun applyRankingWeights(){ food *= 14 production *= 12 diff --git a/core/src/com/unciv/ui/screens/cityscreen/CityScreen.kt b/core/src/com/unciv/ui/screens/cityscreen/CityScreen.kt index 5d59e1fbca..51db6c5020 100644 --- a/core/src/com/unciv/ui/screens/cityscreen/CityScreen.kt +++ b/core/src/com/unciv/ui/screens/cityscreen/CityScreen.kt @@ -209,19 +209,16 @@ class CityScreen( fun isExistingImprovementValuable(tile: Tile, improvementToPlace: TileImprovement): Boolean { if (tile.improvement == null) return false val civInfo = city.civ - val existingStats = tile.stats.getImprovementStats( + + val statDiffForNewImprovement = tile.stats.getStatDiffForImprovement( tile.getTileImprovement()!!, civInfo, city, cityUniqueCache ) - val replacingStats = tile.stats.getImprovementStats( - improvementToPlace, - civInfo, - city, - cityUniqueCache - ) - return Automation.rankStatsValue(existingStats, civInfo) > Automation.rankStatsValue(replacingStats, civInfo) + + // If stat diff for new improvement is negative/zero utility, current improvement is valuable + return Automation.rankStatsValue(statDiffForNewImprovement, civInfo) <= 0 } fun getPickImprovementColor(tile: Tile): Pair { diff --git a/core/src/com/unciv/ui/screens/pickerscreens/ImprovementPickerScreen.kt b/core/src/com/unciv/ui/screens/pickerscreens/ImprovementPickerScreen.kt index 7d6f6e250d..8c85d71d30 100644 --- a/core/src/com/unciv/ui/screens/pickerscreens/ImprovementPickerScreen.kt +++ b/core/src/com/unciv/ui/screens/pickerscreens/ImprovementPickerScreen.kt @@ -157,24 +157,12 @@ class ImprovementPickerScreen( val statIcons = getStatIconsTable(provideResource, removeImprovement) // get benefits of the new improvement - val stats = tile.stats.getImprovementStats( + val stats = tile.stats.getStatDiffForImprovement( improvement, currentPlayerCiv, tile.getCity(), cityUniqueCache ) - // subtract the benefits of the replaced improvement, if any - val existingImprovement = tile.getTileImprovement() - if (existingImprovement != null && removeImprovement) { - val existingStats = tile.stats.getImprovementStats( - existingImprovement, - currentPlayerCiv, - tile.getCity(), - cityUniqueCache - ) - stats.add(existingStats.times(-1.0f)) - } - val statsTable = getStatsTable(stats) statIcons.add(statsTable).padLeft(13f)