Some linting and readability helpers (#11215)

core/src/com/unciv/logic/map/mapunit/MapUnit.kt

@@ -612,18 +612,11 @@ class MapUnit : IsPartOfGameInfoSerialization {
     }
 
     fun updateUniques() {
-        val uniques = ArrayList<Unique>()
+        val uniqueSources =
-        uniques.addAll(baseUnit.uniqueObjects)
+            baseUnit.uniqueObjects.asSequence() +
-        uniques.addAll(type.uniqueObjects)
+                type.uniqueObjects +
-
+                promotions.getPromotions().flatMap { it.uniqueObjects }
-        for (promotion in promotions.getPromotions()) {
+        tempUniquesMap = UniqueMap(uniqueSources)
-            uniques.addAll(promotion.uniqueObjects)
-        }
-
-        tempUniquesMap = UniqueMap().apply {
-            addUniques(uniques)
-        }
-
         cache.updateUniques()
     }
 

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

@@ -53,7 +53,7 @@ open class Tile : IsPartOfGameInfoSerialization {
     var owningCity: City? = null
         private set
 
-    fun setOwningCity(city:City?) {
+    fun setOwningCity(city: City?) {
         if (city != null) {
             if (roadStatus != RoadStatus.None && roadOwner != "") {
                 // remove previous neutral tile owner
@@ -118,11 +118,11 @@ open class Tile : IsPartOfGameInfoSerialization {
 
     @Transient
     /** Between 0.0 and 1.0 - For map generation use only */
-    var humidity:Double? = null
+    var humidity: Double? = null
 
     @Transient
     /** Between -1.0 and 1.0 - For map generation use only */
-    var temperature:Double? = null
+    var temperature: Double? = null
 
     var naturalWonder: String? = null
     var resource: String? = null
@@ -457,8 +457,8 @@ open class Tile : IsPartOfGameInfoSerialization {
             // Per Civ V, resources under city tiles require the *possibility of extraction* -
             //  that is, there needs to be a tile improvement you have the tech for.
             // Does NOT take all GetImprovementBuildingProblems into account.
-            return possibleImprovements.any {
+            return possibleImprovements.any { improvement ->
-                ruleset.tileImprovements[it]?.let { it.techRequired==null || civInfo.tech.isResearched(it.techRequired!!) } == true
+                ruleset.tileImprovements[improvement]?.let { it.techRequired == null || civInfo.tech.isResearched(it.techRequired!!) } == true
             }
         }
         val improvement = getUnpillagedTileImprovement()
@@ -525,7 +525,7 @@ open class Tile : IsPartOfGameInfoSerialization {
 
             in terrainFeatures -> true
             else -> {
-                if (terrainUniqueMap.getUniques(filter).any()) return true
+                if (terrainUniqueMap.containsFilteringUnique(filter)) return true
                 if (getOwner()?.matchesFilter(filter) == true) return true
 
                 // Resource type check is last - cannot succeed if no resource here
@@ -597,16 +597,14 @@ open class Tile : IsPartOfGameInfoSerialization {
 
     fun canBeSettled(): Boolean {
         val modConstants = tileMap.gameInfo.ruleset.modOptions.constants
-        if (isWater || isImpassible())
+        return when {
-            return false
+            isWater || isImpassible() -> false
-        if (getTilesInDistance(modConstants.minimalCityDistanceOnDifferentContinents)
+            getTilesInDistance(modConstants.minimalCityDistanceOnDifferentContinents)
-                .any { it.isCityCenter() && it.getContinent() != getContinent() }
+                .any { it.isCityCenter() && it.getContinent() != getContinent() } -> false
-            || getTilesInDistance(modConstants.minimalCityDistance)
+            getTilesInDistance(modConstants.minimalCityDistance)
-                .any { it.isCityCenter() && it.getContinent() == getContinent() }
+                .any { it.isCityCenter() && it.getContinent() == getContinent() } -> false
-        ) {
+            else -> true
-            return false
         }
-        return true
     }
 
     /** Shows important properties of this tile for debugging _only_, it helps to see what you're doing */
@@ -664,18 +662,17 @@ open class Tile : IsPartOfGameInfoSerialization {
         if (tileOwner == null || tileOwner == civInfo) return true
         if (isCityCenter() && civInfo.isAtWarWith(tileOwner)
                 && !getCity()!!.hasJustBeenConquered) return false
-        if (!civInfo.diplomacyFunctions.canPassThroughTiles(tileOwner)) return false
+        return civInfo.diplomacyFunctions.canPassThroughTiles(tileOwner)
-        return true
     }
 
     fun hasEnemyInvisibleUnit(viewingCiv: Civilization): Boolean {
         val unitsInTile = getUnits()
-        if (unitsInTile.none()) return false
+        return when {
-        if (unitsInTile.first().civ != viewingCiv &&
+            unitsInTile.none() -> false
-                unitsInTile.firstOrNull { it.isInvisible(viewingCiv) } != null) {
+            unitsInTile.first().civ == viewingCiv -> false
-            return true
+            unitsInTile.none { it.isInvisible(viewingCiv) } -> false
+            else -> true
         }
-        return false
     }
 
     fun hasConnection(civInfo: Civilization) =
@@ -769,7 +766,7 @@ open class Tile : IsPartOfGameInfoSerialization {
      *
      * [resourceAmount] is determined by [MapParameters.mapResources] and [majorDeposit], and
      * if the latter is `null` a random choice between major and minor deposit is made, approximating
-     * the frequency [MapRegions][com.unciv.logic.map.mapgenerator.MapRegions] would use.
+     * the frequency [MapRegions][com.unciv.logic.map.mapgenerator.mapregions.MapRegions] would use.
      * A randomness source ([rng]) can optionally be provided for that step (not used otherwise).
      */
     fun setTileResource(newResource: TileResource, majorDeposit: Boolean? = null, rng: Random = Random.Default) {
@@ -847,15 +844,8 @@ open class Tile : IsPartOfGameInfoSerialization {
         val terrainNameList = allTerrains.map { it.name }.toList()
 
         // List hash is function of all its items, so the same items in the same order will always give the same hash
-        val cachedUniqueMap = tileMap.tileUniqueMapCache[terrainNameList]
+        terrainUniqueMap = tileMap.tileUniqueMapCache.getOrPut(terrainNameList) {
-        terrainUniqueMap = if (cachedUniqueMap != null)
+            UniqueMap(allTerrains.flatMap { it.uniqueObjects })
-            cachedUniqueMap
-        else {
-            val newUniqueMap = UniqueMap()
-            for (terrain in allTerrains)
-                newUniqueMap.addUniques(terrain.uniqueObjects)
-            tileMap.tileUniqueMapCache[terrainNameList] = newUniqueMap
-            newUniqueMap
         }
     }
 

core/src/com/unciv/models/ruleset/nation/CityStateType.kt

@@ -8,10 +8,13 @@ import com.unciv.ui.components.extensions.colorFromRGB
 
 class CityStateType: INamed {
     override var name = ""
+
     var friendBonusUniques = ArrayList<String>()
-    val friendBonusUniqueMap by lazy { UniqueMap().apply { addUniques(friendBonusUniques.map { Unique(it, sourceObjectType = UniqueTarget.CityState) }) } }
+    val friendBonusUniqueMap by lazy { friendBonusUniques.toUniqueMap() }
     var allyBonusUniques = ArrayList<String>()
-    val allyBonusUniqueMap by lazy { UniqueMap().apply { addUniques(allyBonusUniques.map { Unique(it, sourceObjectType = UniqueTarget.CityState) }) } }
+    val allyBonusUniqueMap by lazy { allyBonusUniques.toUniqueMap() }
+    private fun ArrayList<String>.toUniqueMap() =
+        UniqueMap(asSequence().map { Unique(it, sourceObjectType = UniqueTarget.CityState) })
 
     var color:List<Int> = listOf(255,255,255)
     private val colorObject by lazy { colorFromRGB(color) }

core/src/com/unciv/models/ruleset/unique/Unique.kt

@@ -204,11 +204,15 @@ class LocalUniqueCache(val cache:Boolean = true) {
     }
 }
 
-class UniqueMap: HashMap<String, ArrayList<Unique>>() {
+class UniqueMap() : HashMap<String, ArrayList<Unique>>() {
     //todo Once all untyped Uniques are converted, this should be  HashMap<UniqueType, *>
     // For now, we can have both map types "side by side" each serving their own purpose,
     // and gradually this one will be deprecated in favor of the other
 
+    constructor(uniques: Sequence<Unique>) : this() {
+        addUniques(uniques.asIterable())
+    }
+
     /** Adds one [unique] unless it has a ConditionalTimedUnique conditional */
     fun addUnique(unique: Unique) {
         val existingArrayList = get(unique.placeholderText)
@@ -221,11 +225,8 @@ class UniqueMap: HashMap<String, ArrayList<Unique>>() {
         for (unique in uniques) addUnique(unique)
     }
 
-    fun getUniques(placeholderText: String): Sequence<Unique> {
+    fun getUniques(uniqueType: UniqueType) =
-        return this[placeholderText]?.asSequence() ?: emptySequence()
+        this[uniqueType.placeholderText]?.asSequence() ?: emptySequence()
-    }
-
-    fun getUniques(uniqueType: UniqueType) = getUniques(uniqueType.placeholderText)
 
     fun getMatchingUniques(uniqueType: UniqueType, state: StateForConditionals) = getUniques(uniqueType)
         .filter { it.conditionalsApply(state) && !it.isTimedTriggerable }
@@ -238,6 +239,9 @@ class UniqueMap: HashMap<String, ArrayList<Unique>>() {
             && unique.conditionalsApply(stateForConditionals)
         }
     }
+
+    /** This is an alias for [containsKey] to clarify when a pure string-based check is legitimate. */
+    fun containsFilteringUnique(filter: String) = containsKey(filter)
 }
 
 
@@ -253,8 +257,8 @@ class TemporaryUnique() : IsPartOfGameInfoSerialization {
 
     var unique: String = ""
 
-    var sourceObjectType: UniqueTarget? = null
+    private var sourceObjectType: UniqueTarget? = null
-    var sourceObjectName: String? = null
+    private var sourceObjectName: String? = null
 
     @delegate:Transient
     val uniqueObject: Unique by lazy { Unique(unique, sourceObjectType, sourceObjectName) }