Fix AI being able to bombard non-visible tiles + optimizations (#8353)

Co-authored-by: tunerzinc@gmail.com <vfylfhby>

core/src/com/unciv/logic/automation/unit/UnitAutomation.kt

@@ -375,9 +375,10 @@ object UnitAutomation {
         return unit.currentMovement == 0f
     }
 
-    fun getBombardTargets(city: CityInfo): Sequence<TileInfo> =
+    /** Get a list of visible tiles which have something attackable */
+    fun getBombardableTiles(city: CityInfo): Sequence<TileInfo> =
             city.getCenterTile().getTilesInDistance(city.range)
-                    .filter { BattleHelper.containsAttackableEnemy(it, CityCombatant(city)) }
+                    .filter { it.isVisible(city.civInfo) && BattleHelper.containsAttackableEnemy(it, CityCombatant(city)) }
 
     /** Move towards the closest attackable enemy of the [unit].
      *
@@ -547,7 +548,7 @@ object UnitAutomation {
     }
 
     private fun chooseBombardTarget(city: CityInfo): ICombatant? {
-        var targets = getBombardTargets(city).map { Battle.getMapCombatantOfTile(it)!! }
+        var targets = getBombardableTiles(city).map { Battle.getMapCombatantOfTile(it)!! }
         if (targets.none()) return null
 
         val siegeUnits = targets

core/src/com/unciv/logic/map/TileInfo.kt

@@ -228,6 +228,12 @@ open class TileInfo : IsPartOfGameInfoSerialization {
             if (naturalWonder == null) throw Exception("No natural wonder exists for this tile!")
             else ruleset.terrains[naturalWonder!!]!!
 
+    fun isVisible(player: CivilizationInfo): Boolean {
+        if (UncivGame.Current.viewEntireMapForDebug)
+            return true
+        return player.viewableTiles.contains(this)
+    }
+
     fun isCityCenter(): Boolean = isCityCenterInternal
     fun isNaturalWonder(): Boolean = naturalWonder != null
     fun isImpassible() = getLastTerrain().impassable

core/src/com/unciv/ui/worldscreen/WorldMapHolder.kt

@@ -576,8 +576,6 @@ class WorldMapHolder(
             allWorldTileGroups.forEach { it.showEntireMap = true } // So we can see all resources, regardless of tech
         }
 
-        val playerViewableTilePositions = viewingCiv.viewableTiles.map { it.position }.toHashSet()
-
         for (tileGroup in allWorldTileGroups) {
             tileGroup.update(viewingCiv)
 
@@ -597,11 +595,11 @@ class WorldMapHolder(
         when {
             unitTable.selectedCity != null -> {
                 val city = unitTable.selectedCity!!
-                updateTilegroupsForSelectedCity(city, playerViewableTilePositions)
+                updateBombardableTilesForSelectedCity(city)
             }
             unitTable.selectedUnit != null -> {
                 for (unit in unitTable.selectedUnits) {
-                    updateTilegroupsForSelectedUnit(unit, playerViewableTilePositions)
+                    updateTilegroupsForSelectedUnit(unit)
                 }
             }
             unitActionOverlays.isNotEmpty() -> {
@@ -618,7 +616,7 @@ class WorldMapHolder(
         zoom(scaleX) // zoom to current scale, to set the size of the city buttons after "next turn"
     }
 
-    private fun updateTilegroupsForSelectedUnit(unit: MapUnit, playerViewableTilePositions: HashSet<Vector2>) {
+    private fun updateTilegroupsForSelectedUnit(unit: MapUnit) {
         val tileGroup = tileGroups[unit.getTile()] ?: return
         // Entirely unclear when this happens, but this seems to happen since version 520 (3.12.9)
         // so maybe has to do with the construction list being async?
@@ -691,10 +689,7 @@ class WorldMapHolder(
         val attackableTiles: List<AttackableTile> = if (unit.isCivilian()) listOf()
         else {
             BattleHelper.getAttackableEnemies(unit, unit.movement.getDistanceToTiles())
-                    .filter {
+                    .filter { it.tileToAttack.isVisible(unit.civInfo) }
-                        (UncivGame.Current.viewEntireMapForDebug ||
-                                playerViewableTilePositions.contains(it.tileToAttack.position))
-                    }
                     .distinctBy { it.tileToAttack }
         }
 
@@ -711,12 +706,9 @@ class WorldMapHolder(
         }
     }
 
-    private fun updateTilegroupsForSelectedCity(city: CityInfo, playerViewableTilePositions: HashSet<Vector2>) {
+    private fun updateBombardableTilesForSelectedCity(city: CityInfo) {
         if (!city.canBombard()) return
-
+        for (attackableTile in UnitAutomation.getBombardableTiles(city)) {
-        val attackableTiles = UnitAutomation.getBombardTargets(city)
-                .filter { (UncivGame.Current.viewEntireMapForDebug || playerViewableTilePositions.contains(it.position)) }
-        for (attackableTile in attackableTiles) {
             for (group in tileGroups[attackableTile]!!) {
                 group.showHighlight(colorFromRGB(237, 41, 57))
                 group.showCrosshair()

core/src/com/unciv/ui/worldscreen/bottombar/BattleTable.kt

@@ -235,7 +235,7 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
                         .getAttackableEnemies(attacker.unit, attacker.unit.movement.getDistanceToTiles())
                         .firstOrNull{ it.tileToAttack == defender.getTile()}
             } else if (attacker is CityCombatant) {
-                val canBombard = UnitAutomation.getBombardTargets(attacker.city).contains(defender.getTile())
+                val canBombard = UnitAutomation.getBombardableTiles(attacker.city).contains(defender.getTile())
                 if (canBombard) {
                     attackableTile = AttackableTile(attacker.getTile(), defender.getTile(), 0f)
                 }