Fix double-count of civ-wide resources, while allowing propagation of city-wide resources via uniques (#11236)

* Fix double-count of civ-wide resources, while allowing propagation of city-wide resources via uniques

* Added test to ensure civ-wide resources propagate between cities (they do)

GetResourceAmount -> GetAvailableResourceAmount

* Extra clarity in function names! Better than possible confusion.
This commit is contained in:
Yair Morgenstern
2024-03-03 20:02:21 +02:00
committed by GitHub
parent 2ec4d3be8e
commit b577f5ee9a
12 changed files with 126 additions and 36 deletions

View File

@ -187,8 +187,8 @@ class City : IsPartOfGameInfoSerialization {
fun getRuleset() = civ.gameInfo.ruleset
fun getCityResources() = CityResources.getCityResources(this)
fun getResourceAmount(resourceName: String) = CityResources.getResourceAmount(this, resourceName)
fun getResourcesGeneratedByCity() = CityResources.getResourcesGeneratedByCity(this)
fun getAvailableResourceAmount(resourceName: String) = CityResources.getAvailableResourceAmount(this, resourceName)
fun isGrowing() = foodForNextTurn() > 0
fun isStarving() = foodForNextTurn() < 0

View File

@ -8,20 +8,43 @@ import com.unciv.models.ruleset.unique.UniqueType
object CityResources {
fun getCityResources(city: City): ResourceSupplyList {
/** Returns ALL resources, city-wide and civ-wide */
fun getResourcesGeneratedByCity(city: City): ResourceSupplyList {
val resourceModifers = HashMap<String, Float>()
for (resource in city.civ.gameInfo.ruleset.tileResources.values)
resourceModifers[resource.name] = city.civ.getResourceModifier(resource)
val cityResources = getResourcesGeneratedByCityNotIncludingBuildings(city, resourceModifers)
addCityResourcesGeneratedFromUniqueBuildings(city, cityResources, resourceModifers)
return cityResources
}
/** Only for *city-wide* resources - civ-wide resources should use civ-level resources */
fun getCityResourcesAvailableToCity(city: City): ResourceSupplyList {
val resourceModifers = HashMap<String, Float>()
for (resource in city.civ.gameInfo.ruleset.tileResources.values)
resourceModifers[resource.name] = city.civ.getResourceModifier(resource)
val cityResources = getResourcesGeneratedByCityNotIncludingBuildings(city, resourceModifers)
// We can't use getResourcesGeneratedByCity directly, because that would include the resources generated by buildings -
// which are part of the civ-wide uniques, so we'd be getting them twice!
// This way we get them once, but it is ugly, I welcome other ideas :/
getCityResourcesFromCiv(city, cityResources, resourceModifers)
return cityResources
}
private fun getResourcesGeneratedByCityNotIncludingBuildings(city: City, resourceModifers: HashMap<String, Float>): ResourceSupplyList {
val cityResources = ResourceSupplyList()
val resourceModifer = HashMap<String, Float>()
for (resource in city.civ.gameInfo.ruleset.tileResources.values)
resourceModifer[resource.name] = city.civ.getResourceModifier(resource)
addResourcesFromTiles(city, resourceModifers, cityResources)
getResourcesFromTiles(city, resourceModifer, cityResources)
addResourceFromUniqueImprovedTiles(city, cityResources, resourceModifers)
getResourceFromUniqueImprovedTiles(city, cityResources, resourceModifer)
manageCityResourcesRequiredByBuildings(city, cityResources)
getCityResourcesFromCiv(city, cityResources, resourceModifer)
removeCityResourcesRequiredByBuildings(city, cityResources)
if (city.civ.isCityState() && city.isCapital() && city.civ.cityStateResource != null) {
cityResources.add(
@ -33,17 +56,28 @@ object CityResources {
return cityResources
}
private fun addCityResourcesGeneratedFromUniqueBuildings(city: City, cityResources: ResourceSupplyList, resourceModifer: HashMap<String, Float>) {
for (unique in city.getMatchingUniquesWithNonLocalEffects(UniqueType.ProvidesResources, StateForConditionals(city.civ, city))) { // E.G "Provides [1] [Iron]"
val resource = city.getRuleset().tileResources[unique.params[1]]
?: continue
cityResources.add(
resource, unique.getSourceNameForUser(),
(unique.params[0].toFloat() * resourceModifer[resource.name]!!).toInt()
)
}
}
/** Gets the number of resources available to this city
* Accommodates both city-wide and civ-wide resources */
fun getResourceAmount(city: City, resourceName: String): Int {
fun getAvailableResourceAmount(city: City, resourceName: String): Int {
val resource = city.getRuleset().tileResources[resourceName] ?: return 0
if (resource.hasUnique(UniqueType.CityResource))
return getCityResources(city).asSequence().filter { it.resource == resource }.sumOf { it.amount }
return getCityResourcesAvailableToCity(city).asSequence().filter { it.resource == resource }.sumOf { it.amount }
return city.civ.getResourceAmount(resourceName)
}
private fun getResourcesFromTiles(city: City, resourceModifer: HashMap<String, Float>, cityResources: ResourceSupplyList) {
private fun addResourcesFromTiles(city: City, resourceModifer: HashMap<String, Float>, cityResources: ResourceSupplyList) {
for (tileInfo in city.getTiles().filter { it.resource != null }) {
val resource = tileInfo.tileResource
val amount = getTileResourceAmount(city, tileInfo) * resourceModifer[resource.name]!!
@ -51,7 +85,7 @@ object CityResources {
}
}
private fun getResourceFromUniqueImprovedTiles(city: City, cityResources: ResourceSupplyList, resourceModifer: HashMap<String, Float>) {
private fun addResourceFromUniqueImprovedTiles(city: City, cityResources: ResourceSupplyList, resourceModifer: HashMap<String, Float>) {
for (tileInfo in city.getTiles().filter { it.getUnpillagedImprovement() != null }) {
val stateForConditionals = StateForConditionals(city.civ, city, tile = tileInfo)
val tileImprovement = tileInfo.getUnpillagedTileImprovement()
@ -72,7 +106,7 @@ object CityResources {
}
}
private fun manageCityResourcesRequiredByBuildings(city: City, cityResources: ResourceSupplyList) {
private fun removeCityResourcesRequiredByBuildings(city: City, cityResources: ResourceSupplyList) {
val freeBuildings = city.civ.civConstructions.getFreeBuildingNames(city)
for (building in city.cityConstructions.getBuiltBuildings()) {
// Free buildings cost no resources
@ -81,14 +115,14 @@ object CityResources {
}
}
private fun getCityResourcesFromCiv(city: City, cityResources: ResourceSupplyList, resourceModifer: HashMap<String, Float>) {
private fun getCityResourcesFromCiv(city: City, cityResources: ResourceSupplyList, resourceModifers: HashMap<String, Float>) {
// This includes the uniques from buildings, from this and all other cities
for (unique in city.civ.getMatchingUniques(UniqueType.ProvidesResources, StateForConditionals(city.civ, city))) { // E.G "Provides [1] [Iron]"
for (unique in city.getMatchingUniques(UniqueType.ProvidesResources, StateForConditionals(city.civ, city))) { // E.G "Provides [1] [Iron]"
val resource = city.getRuleset().tileResources[unique.params[1]]
?: continue
cityResources.add(
resource, "Buildings",
(unique.params[0].toFloat() * resourceModifer[resource.name]!!).toInt()
resource, unique.getSourceNameForUser(),
(unique.params[0].toFloat() * resourceModifers[resource.name]!!).toInt()
)
}
}

View File

@ -49,7 +49,7 @@ class CityTurnManager(val city: City) {
private fun tryWeLoveTheKing() {
if (city.demandedResource == "") return
if (city.getResourceAmount(city.demandedResource) > 0) {
if (city.getAvailableResourceAmount(city.demandedResource) > 0) {
city.setFlag(CityFlags.WeLoveTheKing, 20 + 1) // +1 because it will be decremented by 1 in the same startTurn()
city.civ.addNotification(
"Because they have [${city.demandedResource}], the citizens of [${city.name}] are celebrating We Love The King Day!",

View File

@ -821,9 +821,10 @@ class Civilization : IsPartOfGameInfoSerialization {
}
// endregion
fun addCity(location: Vector2, unit: MapUnit? = null) {
fun addCity(location: Vector2, unit: MapUnit? = null): City {
val newCity = CityFounder().foundCity(this, location, unit)
newCity.cityConstructions.chooseNextConstruction()
return newCity
}
/** Destroy what's left of a Civilization

View File

@ -675,9 +675,10 @@ class CityStateFunctions(val civInfo: Civilization) {
}
fun getCityStateResourcesForAlly() = ResourceSupplyList().apply {
// TODO: City-states don't give allies resources from civ-wide uniques!
for (city in civInfo.cities) {
// IGNORE the fact that they consume their own resources - #4769
addPositiveByResource(city.getCityResources(), Constants.cityStates)
addPositiveByResource(city.getResourcesGeneratedByCity(), Constants.cityStates)
}
}

View File

@ -282,7 +282,7 @@ class CivInfoTransientCache(val civInfo: Civilization) {
fun updateCivResources() {
val newDetailedCivResources = ResourceSupplyList()
for (city in civInfo.cities) newDetailedCivResources.add(city.getCityResources())
for (city in civInfo.cities) newDetailedCivResources.add(city.getResourcesGeneratedByCity())
if (!civInfo.isCityState()) {
// First we get all these resources of each city state separately

View File

@ -412,7 +412,7 @@ class Building : RulesetStatsObject(), INonPerpetualConstruction {
for ((resourceName, requiredAmount) in getResourceRequirementsPerTurn(
StateForConditionals(cityConstructions.city.civ, cityConstructions.city))
) {
val availableAmount = cityConstructions.city.getResourceAmount(resourceName)
val availableAmount = cityConstructions.city.getAvailableResourceAmount(resourceName)
if (availableAmount < requiredAmount) {
yield(RejectionReasonType.ConsumesResources.toInstance(resourceName.getNeedMoreAmountString(requiredAmount - availableAmount)))
}

View File

@ -48,7 +48,7 @@ object Conditionals {
val stateBasedRandom by lazy { Random(state.hashCode()) }
fun getResourceAmount(resourceName: String): Int {
if (relevantCity != null) return relevantCity!!.getResourceAmount(resourceName)
if (relevantCity != null) return relevantCity!!.getAvailableResourceAmount(resourceName)
if (relevantCiv != null) return relevantCiv!!.getResourceAmount(resourceName)
return 0
}

View File

@ -49,7 +49,7 @@ object BuildingDescriptions {
if (isNationalWonder) translatedLines += "National Wonder".tr()
if (!isFree) {
for ((resourceName, amount) in getResourceRequirementsPerTurn(StateForConditionals(city.civ, city))) {
val available = city.getResourceAmount(resourceName)
val available = city.getAvailableResourceAmount(resourceName)
val resource = city.getRuleset().tileResources[resourceName] ?: continue
val consumesString = resourceName.getConsumesAmountString(amount, resource.isStockpiled())

View File

@ -11,6 +11,7 @@ import com.unciv.UncivGame
import com.unciv.logic.city.City
import com.unciv.logic.city.CityFlags
import com.unciv.logic.city.CityFocus
import com.unciv.logic.city.CityResources
import com.unciv.logic.city.GreatPersonPointsBreakdown
import com.unciv.models.Counter
import com.unciv.models.ruleset.Building
@ -183,7 +184,8 @@ class CityStatsTable(private val cityScreen: CityScreen) : Table() {
val resourceTable = Table()
val resourceCounter = Counter<TileResource>()
for (resourceSupply in city.getCityResources()) resourceCounter.add(resourceSupply.resource, resourceSupply.amount)
for (resourceSupply in CityResources.getCityResourcesAvailableToCity(city))
resourceCounter.add(resourceSupply.resource, resourceSupply.amount)
for ((resource, amount) in resourceCounter)
if (resource.hasUnique(UniqueType.CityResource)) {
resourceTable.add(amount.toLabel())

View File

@ -2,6 +2,7 @@ package com.unciv.logic.city
import com.badlogic.gdx.math.Vector2
import com.unciv.logic.civilization.Civilization
import com.unciv.models.ruleset.unique.UniqueType
import com.unciv.testing.GdxTestRunner
import com.unciv.testing.TestGame
import org.junit.Assert.assertEquals
@ -108,7 +109,7 @@ class CityTest {
tile.improvement = "Mine"
// when
val cityResources = capitalCity.getCityResources()
val cityResources = capitalCity.getResourcesGeneratedByCity()
// then
assertEquals(1, cityResources.size)
@ -122,11 +123,11 @@ class CityTest {
capitalCity.cityConstructions.addBuilding(building)
// when
val cityResources = capitalCity.getCityResources()
val resources = testCiv.detailedCivResources
// then
assertEquals(1, cityResources.size)
assertEquals("4 Iron from Buildings", cityResources[0].toString())
assertEquals(1, resources.size)
assertEquals("4 Iron from Buildings", resources[0].toString())
}
@Test
@ -135,10 +136,58 @@ class CityTest {
capitalCity.cityConstructions.addBuilding("Factory")
// when
val cityResources = capitalCity.getCityResources()
val resources = testCiv.detailedCivResources
// then
assertEquals(1, cityResources.size)
assertEquals("-1 Coal from Buildings", cityResources[0].toString())
assertEquals(1, resources.size)
assertEquals("-1 Coal from Buildings", resources[0].toString())
}
@Test
fun `Civ-wide resources from building uniques propagate between cities`() {
// given
val building = testGame.createBuilding("Provides [4] [Coal]")
capitalCity.cityConstructions.addBuilding(building)
val otherCity = testCiv.addCity(Vector2(2f,2f))
// when
val resourceAmountInOtherCity = otherCity.getAvailableResourceAmount("Coal")
// then
assertEquals(4, resourceAmountInOtherCity)
}
@Test
fun `City-wide resources from building uniques propagate between cities`() {
// given
val resource = testGame.createResource(UniqueType.CityResource.text)
val building = testGame.createBuilding("Provides [4] [${resource.name}]")
capitalCity.cityConstructions.addBuilding(building)
val otherCity = testCiv.addCity(Vector2(2f,2f))
// when
val resourceAmountInOtherCity = otherCity.getAvailableResourceAmount(resource.name)
// then
assertEquals(4, resourceAmountInOtherCity)
}
@Test
fun `City-wide resources not double-counted in same city`() {
// given
val resource = testGame.createResource(UniqueType.CityResource.text)
val building = testGame.createBuilding("Provides [4] [${resource.name}]")
capitalCity.cityConstructions.addBuilding(building)
// when
val resourceAmountInCapital = capitalCity.getAvailableResourceAmount(resource.name)
// then
assertEquals(4, resourceAmountInCapital)
}
}

View File

@ -26,6 +26,7 @@ import com.unciv.models.ruleset.Specialist
import com.unciv.models.ruleset.Speed
import com.unciv.models.ruleset.nation.Nation
import com.unciv.models.ruleset.tile.TileImprovement
import com.unciv.models.ruleset.tile.TileResource
import com.unciv.models.ruleset.unique.UniqueType
import com.unciv.models.ruleset.unit.BaseUnit
import com.unciv.models.ruleset.unit.Promotion
@ -243,6 +244,8 @@ class TestGame {
createRulesetObject(ruleset.beliefs, *uniques) { Belief(type) }
fun createBuilding(vararg uniques: String) =
createRulesetObject(ruleset.buildings, *uniques) { Building() }
fun createResource(vararg uniques: String) =
createRulesetObject(ruleset.tileResources, *uniques) { TileResource() }
fun createWonder(vararg uniques: String): Building {
val createdBuilding = createBuilding(*uniques)