From f8ccefd10cd8161f59f972a7017639d407f7e291 Mon Sep 17 00:00:00 2001 From: Yair Morgenstern Date: Thu, 31 Aug 2023 14:27:57 +0300 Subject: [PATCH] performance: Use the same trick of 'save unfiltered, return filtered' for cached city uniques, so devs don't need to worry about cache state when improving performance! Big DUH moment, we've been applying this same trick everywhere but haven't generalized it yet... This will both make performance improvements easier, AND improve readability! --- .../unciv/logic/map/tile/TileStatFunctions.kt | 49 +++++++------------ .../com/unciv/models/ruleset/unique/Unique.kt | 11 ++--- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/core/src/com/unciv/logic/map/tile/TileStatFunctions.kt b/core/src/com/unciv/logic/map/tile/TileStatFunctions.kt index 11ed0ccd04..339681c147 100644 --- a/core/src/com/unciv/logic/map/tile/TileStatFunctions.kt +++ b/core/src/com/unciv/logic/map/tile/TileStatFunctions.kt @@ -27,23 +27,24 @@ class TileStatFunctions(val tile: Tile) { val stateForConditionals = StateForConditionals(civInfo = observingCiv, city = city, tile = tile) if (city != null) { - var tileUniques = + val statsFromTilesUniques = localUniqueCache.forCityGetMatchingUniques( - city, UniqueType.StatsFromTiles, true) + city, UniqueType.StatsFromTiles, + stateForConditionals) .filter { city.matchesFilter(it.params[2]) } - tileUniques += localUniqueCache.forCityGetMatchingUniques( - city, UniqueType.StatsFromObject, true) - for (unique in tileUniques) { - if (!unique.conditionalsApply(stateForConditionals)) continue + + val statsFromObjectsUniques = localUniqueCache.forCityGetMatchingUniques( + city, UniqueType.StatsFromObject, stateForConditionals) + + for (unique in statsFromTilesUniques + statsFromObjectsUniques) { val tileType = unique.params[1] if (!tile.matchesTerrainFilter(tileType, observingCiv)) continue stats.add(unique.stats) } for (unique in localUniqueCache.forCityGetMatchingUniques( - city, UniqueType.StatsFromTilesWithout, true)) { + city, UniqueType.StatsFromTilesWithout, stateForConditionals)) { if ( - unique.conditionalsApply(stateForConditionals) && tile.matchesTerrainFilter(unique.params[1]) && !tile.matchesTerrainFilter(unique.params[2]) && city.matchesFilter(unique.params[3]) @@ -116,21 +117,18 @@ class TileStatFunctions(val tile: Tile) { val stateForConditionals = StateForConditionals(civInfo = observingCiv, city = city, tile = tile) if (city != null) { - // Since the tile changes every time, we cache all uniques, and filter by conditional state only when iterating val cachedStatPercentFromObjectCityUniques = uniqueCache.forCityGetMatchingUniques( - city, UniqueType.StatPercentFromObject, true) + city, UniqueType.StatPercentFromObject, stateForConditionals) for (unique in cachedStatPercentFromObjectCityUniques) { - if (!unique.conditionalsApply(stateForConditionals)) continue val tileFilter = unique.params[2] if (tile.matchesTerrainFilter(tileFilter, observingCiv)) stats[Stat.valueOf(unique.params[1])] += unique.params[0].toFloat() } val cachedAllStatPercentFromObjectCityUniques = uniqueCache.forCityGetMatchingUniques( - city, UniqueType.AllStatsPercentFromObject, true) + city, UniqueType.AllStatsPercentFromObject, stateForConditionals) for (unique in cachedAllStatPercentFromObjectCityUniques) { - if (!unique.conditionalsApply(stateForConditionals)) continue val tileFilter = unique.params[1] if (!tile.matchesTerrainFilter(tileFilter, observingCiv)) continue val statPercentage = unique.params[0].toFloat() @@ -257,13 +255,8 @@ class TileStatFunctions(val tile: Tile) { val stats = Stats() fun statsFromTiles(){ - // Since the conditionalState contains the current tile, it is different for each tile, - // therefore if we want the cache to be useful it needs to hold the pre-filtered uniques, - // and then for each improvement we'll filter the uniques locally. - // This is still a MASSIVE save of RAM! - val tileUniques = uniqueCache.forCityGetMatchingUniques(city, UniqueType.StatsFromTiles, true) - .filter { city.matchesFilter(it.params[2]) } // These are the uniques for all improvements for this city, - .filter { it.conditionalsApply(conditionalState) } // ...and this is those with applicable conditions + val tileUniques = uniqueCache.forCityGetMatchingUniques(city, UniqueType.StatsFromTiles, conditionalState) + .filter { city.matchesFilter(it.params[2]) } val improvementUniques = improvement.getMatchingUniques(UniqueType.ImprovementStatsOnTile, conditionalState) @@ -278,12 +271,11 @@ class TileStatFunctions(val tile: Tile) { statsFromTiles() fun statsFromObject() { - // Same as above - cache holds unfiltered uniques for the city, while we use only the filtered ones val uniques = uniqueCache.forCityGetMatchingUniques( city, UniqueType.StatsFromObject, - true - ).filter { it.conditionalsApply(conditionalState) } + conditionalState + ) for (unique in uniques) { if (improvement.matchesFilter(unique.params[1])) { stats.add(unique.stats) @@ -305,13 +297,11 @@ class TileStatFunctions(val tile: Tile) { val conditionalState = StateForConditionals(civInfo = observingCiv, city = city, tile = tile) if (city != null) { - // As above, since the conditional is tile-dependant, - // we save uniques in the cache without conditional filtering, and use only filtered ones val allStatPercentUniques = cityUniqueCache.forCityGetMatchingUniques( city, UniqueType.AllStatsPercentFromObject, - true - ).filter { it.conditionalsApply(conditionalState) } + conditionalState + ) for (unique in allStatPercentUniques) { if (!improvement.matchesFilter(unique.params[1])) continue for (stat in Stat.values()) { @@ -319,12 +309,11 @@ class TileStatFunctions(val tile: Tile) { } } - // Same trick different unique - not sure if worth generalizing this 'late apply' of conditions? val statPercentUniques = cityUniqueCache.forCityGetMatchingUniques( city, UniqueType.StatPercentFromObject, - true - ).filter { it.conditionalsApply(conditionalState) } + conditionalState + ) for (unique in statPercentUniques) { if (!improvement.matchesFilter(unique.params[2])) continue diff --git a/core/src/com/unciv/models/ruleset/unique/Unique.kt b/core/src/com/unciv/models/ruleset/unique/Unique.kt index 19f6dbb572..bed8598c6a 100644 --- a/core/src/com/unciv/models/ruleset/unique/Unique.kt +++ b/core/src/com/unciv/models/ruleset/unique/Unique.kt @@ -323,21 +323,20 @@ class LocalUniqueCache(val cache:Boolean = true) { fun forCityGetMatchingUniques( city: City, uniqueType: UniqueType, - ignoreConditionals: Boolean = false + stateForConditionals: StateForConditionals = StateForConditionals(city.civ, city) ): Sequence { // City uniques are a combination of *global civ* uniques plus *city relevant* uniques (see City.getMatchingUniques()) // We can cache the civ uniques separately, so if we have several cities using the same cache, // we can cache the list of *civ uniques* to reuse between cities. // This is assuming that we're ignoring conditionals, because otherwise - // the conditionals will render the the *filtered uniques* different anyway, so there's no reason to cache... - val uniques = if (!ignoreConditionals) city.getMatchingUniques(uniqueType, StateForConditionals(city.civ, city)) - else forCivGetMatchingUniques(city.civ, uniqueType, StateForConditionals.IgnoreConditionals) + + val unfilteredUniques = forCivGetMatchingUniques(city.civ, uniqueType, StateForConditionals.IgnoreConditionals) + city.getLocalMatchingUniques(uniqueType, StateForConditionals.IgnoreConditionals) return get( - "city-${city.id}-${uniqueType.name}-${ignoreConditionals}", - uniques - ) + "city-${city.id}-${uniqueType.name}", + unfilteredUniques + ).filter { it.conditionalsApply(stateForConditionals) } } fun forCivGetMatchingUniques(