Better barbarian automation (#1560)

core/src/com/unciv/logic/automation/BarbarianAutomation.kt

@@ -0,0 +1,200 @@
+package com.unciv.logic.automation
+
+import com.unciv.Constants
+import com.unciv.UncivGame
+import com.unciv.logic.battle.Battle
+import com.unciv.logic.battle.BattleDamage
+import com.unciv.logic.battle.MapUnitCombatant
+import com.unciv.logic.civilization.CivilizationInfo
+import com.unciv.logic.map.MapUnit
+import com.unciv.logic.map.PathsToTilesWithinTurn
+import com.unciv.logic.map.TileInfo
+import com.unciv.models.AttackableTile
+import com.unciv.models.UnitAction
+import com.unciv.models.UnitActionType
+import com.unciv.models.ruleset.unit.UnitType
+import com.unciv.ui.worldscreen.unit.UnitActions
+
+class BarbarianAutomation(val civInfo: CivilizationInfo) {
+
+    private val battleHelper = BattleHelper()
+    private val battleDamage = BattleDamage()
+
+    fun automate() {
+        // ranged go first, after melee and then everyone else
+        civInfo.getCivUnits().filter { it.type.isRanged() }.forEach(::automateUnit)
+        civInfo.getCivUnits().filter { it.type.isMelee() }.forEach(::automateUnit)
+        civInfo.getCivUnits().filter { !it.type.isRanged() && !it.type.isMelee() }.forEach(::automateUnit)
+    }
+
+    private fun automateUnit(unit: MapUnit) {
+        when {
+            unit.currentTile.improvement == Constants.barbarianEncampment -> automateEncampment(unit)
+            unit.type == UnitType.Scout -> automateScout(unit)
+            else -> automateCombatUnit(unit)
+        }
+    }
+
+    private fun automateEncampment(unit: MapUnit) {
+        val unitActions = UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen)
+
+        // 1 - trying to upgrade
+        if (tryUpgradeUnit(unit, unitActions)) return
+
+        // 2 - trying to attack somebody
+        if (battleHelper.tryAttackNearbyEnemy(unit)) return
+
+        // 3 - at least fortifying
+        unit.fortifyIfCan()
+    }
+
+    private fun automateCombatUnit(unit: MapUnit) {
+        val unitActions = UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen)
+        val unitDistanceToTiles = unit.movement.getDistanceToTiles()
+        val nearEnemyTiles = battleHelper.getAttackableEnemies(unit, unitDistanceToTiles)
+
+        // 1 - heal or fortifying if death is near
+        if (unit.health < 50) {
+            val possibleDamage = nearEnemyTiles
+                    .map {
+                        battleDamage.calculateDamageToAttacker(MapUnitCombatant(unit),
+                                Battle.getMapCombatantOfTile(it.tileToAttack)!!)
+                    }
+                    .sum()
+            val possibleHeal = unit.rankTileForHealing(unit.currentTile)
+            if (possibleDamage > possibleHeal) {
+                // run
+                val furthestTile = findFurthestTile(unit, unitDistanceToTiles, nearEnemyTiles)
+                unit.movement.moveToTile(furthestTile)
+            } else {
+                // heal
+                unit.fortifyIfCan()
+            }
+            return
+        }
+
+        // 2 - trying to upgrade
+        if (tryUpgradeUnit(unit, unitActions)) return
+
+        // 3 - trying to attack enemy
+        // if a embarked melee unit can land and attack next turn, do not attack from water.
+        if (battleHelper.tryDisembarkUnitToAttackPosition(unit, unitDistanceToTiles)) return
+        if (battleHelper.tryAttackNearbyEnemy(unit)) return
+
+        // 4 - trying to pillage tile or route
+        if (tryPillageImprovement(unit, unitDistanceToTiles, unitActions)) return
+
+        // 5 - heal the unit if needed
+        if (unit.health < 100) {
+            healUnit(unit, unitDistanceToTiles)
+            return
+        }
+
+        // 6 - wander
+        UnitAutomation().wander(unit, unitDistanceToTiles)
+    }
+
+    private fun automateScout(unit: MapUnit) {
+        val unitActions = UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen)
+        val unitDistanceToTiles = unit.movement.getDistanceToTiles()
+        val nearEnemyTiles = battleHelper.getAttackableEnemies(unit, unitDistanceToTiles)
+
+        // 1 - heal or run if death is near
+        if (unit.health < 50) {
+            if (nearEnemyTiles.isNotEmpty()) {
+                // run
+                val furthestTile = findFurthestTile(unit, unitDistanceToTiles, nearEnemyTiles)
+                unit.movement.moveToTile(furthestTile)
+            } else {
+                // heal
+                unit.fortifyIfCan()
+            }
+            return
+        }
+
+        // 2 - trying to capture someone
+        // TODO
+
+        // 3 - trying to pillage tile or trade route
+        if (tryPillageImprovement(unit, unitDistanceToTiles, unitActions)) return
+
+        // 4 - heal the unit if needed
+        if (unit.health < 100) {
+            healUnit(unit, unitDistanceToTiles)
+            return
+        }
+
+        // 5 - wander
+        UnitAutomation().wander(unit, unitDistanceToTiles)
+    }
+
+    private fun findFurthestTile(
+            unit: MapUnit,
+            unitDistanceToTiles: PathsToTilesWithinTurn,
+            nearEnemyTiles: List<AttackableTile>
+    ): TileInfo {
+        val possibleTiles = unitDistanceToTiles.keys.filter { unit.movement.canMoveTo(it) }
+        val enemies = nearEnemyTiles.mapNotNull { it.tileToAttack.militaryUnit }
+        var furthestTile: Pair<TileInfo, Float> = possibleTiles.random() to 0f
+        for (enemy in enemies) {
+            for (tile in possibleTiles) {
+                val distance = enemy.movement.getMovementCostBetweenAdjacentTiles(enemy.currentTile, tile, enemy.civInfo)
+                if (distance > furthestTile.second) {
+                    furthestTile = tile to distance
+                }
+            }
+        }
+        return furthestTile.first
+    }
+
+    private fun healUnit(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn) {
+        val currentUnitTile = unit.getTile()
+        val bestTilesForHealing = unitDistanceToTiles.keys
+                .filter { unit.movement.canMoveTo(it) }
+                .groupBy { unit.rankTileForHealing(it) }
+                .maxBy { it.key }
+
+        // within the tiles with best healing rate, we'll prefer one which has the highest defensive bonuses
+        val bestTileForHealing = bestTilesForHealing?.value?.maxBy { it.getDefensiveBonus() }
+        if (bestTileForHealing != null
+            && currentUnitTile != bestTileForHealing
+            && unit.rankTileForHealing(bestTileForHealing) > unit.rankTileForHealing(currentUnitTile)) {
+            unit.movement.moveToTile(bestTileForHealing)
+        }
+
+        unit.fortifyIfCan()
+    }
+
+    private fun tryUpgradeUnit(unit: MapUnit, unitActions: List<UnitAction>): Boolean {
+        if (unit.baseUnit().upgradesTo != null) {
+            val upgradedUnit = unit.civInfo.gameInfo.ruleSet.units[unit.baseUnit().upgradesTo!!]!!
+            if (upgradedUnit.isBuildable(unit.civInfo)) {
+                val upgradeAction = unitActions.firstOrNull { it.type == UnitActionType.Upgrade }
+                if (upgradeAction != null && upgradeAction.canAct) {
+                    upgradeAction.action?.invoke()
+                    return true
+                }
+            }
+        }
+        return false
+    }
+
+    private fun tryPillageImprovement(
+            unit: MapUnit,
+            unitDistanceToTiles: PathsToTilesWithinTurn,
+            unitActions: List<UnitAction>
+    ): Boolean {
+        val tilesThatCanWalkToAndThenPillage = unitDistanceToTiles
+                .filter { it.value.totalDistance < unit.currentMovement }.keys
+                .filter { unit.movement.canMoveTo(it) && UnitActions().canPillage(unit, it) }
+
+        if (tilesThatCanWalkToAndThenPillage.isEmpty()) return false
+        val tileToPillage = tilesThatCanWalkToAndThenPillage.maxBy { it.getDefensiveBonus() }!!
+        if (unit.getTile() != tileToPillage) {
+            unit.movement.moveToTile(tileToPillage)
+        }
+
+        unitActions.first { it.type == UnitActionType.Pillage }.action?.invoke()
+        return true
+    }
+}

core/src/com/unciv/logic/automation/BattleHelper.kt

@@ -0,0 +1,146 @@
+package com.unciv.logic.automation
+
+import com.unciv.Constants
+import com.unciv.logic.battle.Battle
+import com.unciv.logic.battle.BattleDamage
+import com.unciv.logic.battle.ICombatant
+import com.unciv.logic.battle.MapUnitCombatant
+import com.unciv.logic.map.MapUnit
+import com.unciv.logic.map.PathsToTilesWithinTurn
+import com.unciv.logic.map.TileInfo
+import com.unciv.models.AttackableTile
+
+class BattleHelper {
+
+    fun tryAttackNearbyEnemy(unit: MapUnit): Boolean {
+        val attackableEnemies = getAttackableEnemies(unit, unit.movement.getDistanceToTiles())
+                // Only take enemies we can fight without dying
+                .filter {
+                    BattleDamage().calculateDamageToAttacker(MapUnitCombatant(unit),
+                            Battle.getMapCombatantOfTile(it.tileToAttack)!!) < unit.health
+                }
+
+        val enemyTileToAttack = chooseAttackTarget(unit, attackableEnemies)
+
+        if (enemyTileToAttack != null) {
+            Battle.moveAndAttack(MapUnitCombatant(unit), enemyTileToAttack)
+            return true
+        }
+        return false
+    }
+
+    fun getAttackableEnemies(
+            unit: MapUnit,
+            unitDistanceToTiles: PathsToTilesWithinTurn,
+            tilesToCheck: List<TileInfo>? = null
+    ): ArrayList<AttackableTile> {
+        val tilesWithEnemies = (tilesToCheck ?: unit.civInfo.viewableTiles)
+                .filter { containsAttackableEnemy(it, MapUnitCombatant(unit)) }
+
+        val rangeOfAttack = unit.getRange()
+
+        val attackableTiles = ArrayList<AttackableTile>()
+        // The >0.1 (instead of >0) solves a bug where you've moved 2/3 road tiles,
+        // you come to move a third (distance is less that remaining movements),
+        // and then later we round it off to a whole.
+        // So the poor unit thought it could attack from the tile, but when it comes to do so it has no movement points!
+        // Silly floats, basically
+
+        val unitMustBeSetUp = unit.hasUnique("Must set up to ranged attack")
+        val tilesToAttackFrom = if (unit.type.isAirUnit()) sequenceOf(unit.currentTile)
+        else
+            unitDistanceToTiles.asSequence()
+                    .filter {
+                        val movementPointsToExpendAfterMovement = if (unitMustBeSetUp) 1 else 0
+                        val movementPointsToExpendHere = if (unitMustBeSetUp && unit.action != Constants.unitActionSetUp) 1 else 0
+                        val movementPointsToExpendBeforeAttack = if (it.key == unit.currentTile) movementPointsToExpendHere else movementPointsToExpendAfterMovement
+                        unit.currentMovement - it.value.totalDistance - movementPointsToExpendBeforeAttack > 0.1
+                    } // still got leftover movement points after all that, to attack (0.1 is because of Float nonsense, see MapUnit.moveToTile(...)
+                    .map { it.key }
+                    .filter { unit.movement.canMoveTo(it) || it == unit.getTile() }
+
+        for (reachableTile in tilesToAttackFrom) {  // tiles we'll still have energy after we reach there
+            val tilesInAttackRange =
+                    if (unit.hasUnique("Ranged attacks may be performed over obstacles") || unit.type.isAirUnit())
+                        reachableTile.getTilesInDistance(rangeOfAttack)
+                    else reachableTile.getViewableTiles(rangeOfAttack, unit.type.isWaterUnit())
+
+            attackableTiles += tilesInAttackRange.asSequence().filter { it in tilesWithEnemies }
+                    .map { AttackableTile(reachableTile, it) }
+        }
+        return attackableTiles
+    }
+
+    fun containsAttackableEnemy(tile: TileInfo, combatant: ICombatant): Boolean {
+        if (combatant is MapUnitCombatant) {
+            if (combatant.unit.isEmbarked()) {
+                if (tile.isWater) return false // can't attack water units while embarked, only land
+                if (combatant.isRanged()) return false
+            }
+            if (combatant.unit.hasUnique("Can only attack water")) {
+                if (tile.isLand) return false
+
+                // trying to attack lake-to-coast or vice versa
+                if ((tile.baseTerrain == Constants.lakes) != (combatant.getTile().baseTerrain == Constants.lakes))
+                    return false
+            }
+        }
+
+        val tileCombatant = Battle.getMapCombatantOfTile(tile) ?: return false
+        if (tileCombatant.getCivInfo() == combatant.getCivInfo()) return false
+        if (!combatant.getCivInfo().isAtWarWith(tileCombatant.getCivInfo())) return false
+
+        //only submarine and destroyer can attack submarine
+        //garrisoned submarine can be attacked by anyone, or the city will be in invincible
+        if (tileCombatant.isInvisible() && !tile.isCityCenter()) {
+            if (combatant is MapUnitCombatant
+                && combatant.unit.hasUnique("Can attack submarines")
+                && combatant.getCivInfo().viewableInvisibleUnitsTiles.map { it.position }.contains(tile.position)) {
+                return true
+            }
+            return false
+        }
+        return true
+    }
+
+    fun tryDisembarkUnitToAttackPosition(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn): Boolean {
+        if (!unit.type.isMelee() || !unit.type.isLandUnit() || !unit.isEmbarked()) return false
+
+        val attackableEnemiesNextTurn = getAttackableEnemies(unit, unitDistanceToTiles)
+                // Only take enemies we can fight without dying
+                .filter {
+                    BattleDamage().calculateDamageToAttacker(MapUnitCombatant(unit),
+                            Battle.getMapCombatantOfTile(it.tileToAttack)!!) < unit.health
+                }
+                .filter { it.tileToAttackFrom.isLand }
+
+        val enemyTileToAttackNextTurn = chooseAttackTarget(unit, attackableEnemiesNextTurn)
+
+        if (enemyTileToAttackNextTurn != null) {
+            unit.movement.moveToTile(enemyTileToAttackNextTurn.tileToAttackFrom)
+            return true
+        }
+        return false
+    }
+
+    private fun chooseAttackTarget(unit: MapUnit, attackableEnemies: List<AttackableTile>): AttackableTile? {
+        val cityTilesToAttack = attackableEnemies.filter { it.tileToAttack.isCityCenter() }
+        val nonCityTilesToAttack = attackableEnemies.filter { !it.tileToAttack.isCityCenter() }
+
+        // todo For air units, prefer to attack tiles with lower intercept chance
+
+        var enemyTileToAttack: AttackableTile? = null
+        val capturableCity = cityTilesToAttack.firstOrNull { it.tileToAttack.getCity()!!.health == 1 }
+        val cityWithHealthLeft = cityTilesToAttack.filter { it.tileToAttack.getCity()!!.health != 1 } // don't want ranged units to attack defeated cities
+                .minBy { it.tileToAttack.getCity()!!.health }
+
+        if (unit.type.isMelee() && capturableCity != null)
+            enemyTileToAttack = capturableCity // enter it quickly, top priority!
+
+        else if (nonCityTilesToAttack.isNotEmpty()) // second priority, units
+            enemyTileToAttack = nonCityTilesToAttack.minBy { Battle.getMapCombatantOfTile(it.tileToAttack)!!.getHealth() }
+        else if (cityWithHealthLeft != null) enemyTileToAttack = cityWithHealthLeft // third priority, city
+
+        return enemyTileToAttack
+    }
+}

core/src/com/unciv/logic/automation/NextTurnAutomation.kt

@@ -19,6 +19,9 @@ class NextTurnAutomation{
 
     /** Top-level AI turn tasklist */
     fun automateCivMoves(civInfo: CivilizationInfo) {
+        if (civInfo.isBarbarian()) {
+            BarbarianAutomation(civInfo).automate()
+        } else {
             respondToDemands(civInfo)
             respondToTradeRequests(civInfo)
 
@@ -40,6 +43,8 @@ class NextTurnAutomation{
             automateUnits(civInfo)
             reassignWorkedTiles(civInfo)
             trainSettler(civInfo)
+        }
+
         civInfo.popupAlerts.clear() // AIs don't care about popups.
     }
 

core/src/com/unciv/logic/automation/SpecificUnitAutomation.kt

@@ -15,6 +15,8 @@ import com.unciv.ui.worldscreen.unit.UnitActions
 
 class SpecificUnitAutomation{
 
+    private val battleHelper = BattleHelper()
+
     private fun hasWorkableSeaResource(tileInfo: TileInfo, civInfo: CivilizationInfo): Boolean {
         return tileInfo.hasViewableResource(civInfo) && tileInfo.isWater && tileInfo.improvement==null
     }
@@ -186,14 +188,14 @@ class SpecificUnitAutomation{
                 .flatMap { it.airUnits }.filter { it.civInfo.isAtWarWith(unit.civInfo) }
 
         if(enemyAirUnitsInRange.isNotEmpty()) return // we need to be on standby in case they attack
-        if(UnitAutomation().tryAttackNearbyEnemy(unit)) return
+        if(battleHelper.tryAttackNearbyEnemy(unit)) return
 
         val immediatelyReachableCities = tilesInRange
                 .filter { it.isCityCenter() && it.getOwner()==unit.civInfo && unit.movement.canMoveTo(it)}
 
         for(city in immediatelyReachableCities){
             if(city.getTilesInDistance(unit.getRange())
-                            .any { UnitAutomation().containsAttackableEnemy(it,MapUnitCombatant(unit)) }) {
+                            .any { battleHelper.containsAttackableEnemy(it,MapUnitCombatant(unit)) }) {
                 unit.movement.moveToTile(city)
                 return
             }
@@ -220,7 +222,7 @@ class SpecificUnitAutomation{
     }
 
     fun automateBomber(unit: MapUnit) {
-        if (UnitAutomation().tryAttackNearbyEnemy(unit)) return
+        if (battleHelper.tryAttackNearbyEnemy(unit)) return
 
         val tilesInRange = unit.currentTile.getTilesInDistance(unit.getRange())
 
@@ -229,7 +231,7 @@ class SpecificUnitAutomation{
 
         for (city in immediatelyReachableCities) {
             if (city.getTilesInDistance(unit.getRange())
-                            .any { UnitAutomation().containsAttackableEnemy(it, MapUnitCombatant(unit)) }) {
+                            .any { battleHelper.containsAttackableEnemy(it, MapUnitCombatant(unit)) }) {
                 unit.movement.moveToTile(city)
                 return
             }
@@ -245,7 +247,7 @@ class SpecificUnitAutomation{
                 .filter {
                     it != airUnit.currentTile
                             && it.getTilesInDistance(airUnit.getRange())
-                            .any { UnitAutomation().containsAttackableEnemy(it, MapUnitCombatant(airUnit)) }
+                            .any { battleHelper.containsAttackableEnemy(it, MapUnitCombatant(airUnit)) }
                 }
         if (citiesThatCanAttackFrom.isEmpty()) return
 
@@ -258,7 +260,7 @@ class SpecificUnitAutomation{
 
     // This really needs to be changed, to have better targetting for missiles
     fun automateMissile(unit: MapUnit) {
-        if (UnitAutomation().tryAttackNearbyEnemy(unit)) return
+        if (battleHelper.tryAttackNearbyEnemy(unit)) return
 
         val tilesInRange = unit.currentTile.getTilesInDistance(unit.getRange())
 
@@ -267,7 +269,7 @@ class SpecificUnitAutomation{
 
         for (city in immediatelyReachableCities) {
             if (city.getTilesInDistance(unit.getRange())
-                            .any { UnitAutomation().containsAttackableEnemy(it, MapUnitCombatant(unit)) }) {
+                            .any { battleHelper.containsAttackableEnemy(it, MapUnitCombatant(unit)) }) {
                 unit.movement.moveToTile(city)
                 return
             }

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

@@ -3,7 +3,10 @@ package com.unciv.logic.automation
 import com.badlogic.gdx.graphics.Color
 import com.unciv.Constants
 import com.unciv.UncivGame
-import com.unciv.logic.battle.*
+import com.unciv.logic.battle.Battle
+import com.unciv.logic.battle.BattleDamage
+import com.unciv.logic.battle.CityCombatant
+import com.unciv.logic.battle.MapUnitCombatant
 import com.unciv.logic.city.CityInfo
 import com.unciv.logic.civilization.GreatPersonManager
 import com.unciv.logic.civilization.diplomacy.DiplomaticStatus
@@ -15,7 +18,6 @@ import com.unciv.models.UnitActionType
 import com.unciv.models.ruleset.unit.UnitType
 import com.unciv.ui.worldscreen.unit.UnitActions
 
-
 class UnitAutomation {
 
     companion object {
@@ -23,7 +25,12 @@ class UnitAutomation{
         const val CLOSE_ENEMY_TURNS_AWAY_LIMIT = 3f
     }
 
+    private val battleHelper = BattleHelper()
+
     fun automateUnitMoves(unit: MapUnit) {
+        if (unit.civInfo.isBarbarian()) {
+            throw IllegalStateException("Barbarians is not allowed here.")
+        }
 
         if (unit.name == Constants.settler) {
             return SpecificUnitAutomation().automateSettlerActions(unit)
@@ -57,12 +64,6 @@ class UnitAutomation{
         val unitActions = UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen)
         var unitDistanceToTiles = unit.movement.getDistanceToTiles()
 
-        if(unit.civInfo.isBarbarian() &&
-                unit.currentTile.improvement==Constants.barbarianEncampment && unit.type.isLandUnit()) {
-            if(unit.canFortify()) unit.fortify()
-            return // stay in the encampment
-        }
-
         if (tryGoToRuin(unit, unitDistanceToTiles)) {
             if (unit.currentMovement == 0f) return
             unitDistanceToTiles = unit.movement.getDistanceToTiles()
@@ -76,15 +77,10 @@ class UnitAutomation{
         if (unit.health < 50 && tryHealUnit(unit, unitDistanceToTiles)) return // do nothing but heal
 
         // if a embarked melee unit can land and attack next turn, do not attack from water.
-        if (unit.type.isLandUnit() && unit.type.isMelee() && unit.isEmbarked()) {
-            if (tryDisembarkUnitToAttackPosition(unit,unitDistanceToTiles)) return
-        }
+        if (battleHelper.tryDisembarkUnitToAttackPosition(unit, unitDistanceToTiles)) return
 
         // if there is an attackable unit in the vicinity, attack!
-        if (tryAttackNearbyEnemy(unit)) return
-
-        // Barbarians try to pillage improvements if no targets reachable
-        if (unit.civInfo.isBarbarian() && tryPillageImprovement(unit, unitDistanceToTiles)) return
+        if (battleHelper.tryAttackNearbyEnemy(unit)) return
 
         if (tryGarrisoningUnit(unit)) return
 
@@ -102,29 +98,24 @@ class UnitAutomation{
 
         // else, try to go o unreached tiles
         if (tryExplore(unit, unitDistanceToTiles)) return
-
-        // Barbarians just wander all over the place
-        if(unit.civInfo.isBarbarian())
-            wander(unit,unitDistanceToTiles)
     }
 
     private fun tryHeadTowardsEncampment(unit: MapUnit): Boolean {
-        if(unit.civInfo.isBarbarian()) return false
         if (unit.type == UnitType.Missile) return false // don't use missiles against barbarians...
         val knownEncampments = unit.civInfo.gameInfo.tileMap.values.asSequence()
                 .filter { it.improvement == Constants.barbarianEncampment && unit.civInfo.exploredTiles.contains(it.position) }
         val cities = unit.civInfo.cities
-        val encampmentsCloseToCities
-                = knownEncampments.filter { cities.any { city -> city.getCenterTile().arialDistanceTo(it) < 6 } }
+        val encampmentsCloseToCities = knownEncampments.filter { cities.any { city -> city.getCenterTile().arialDistanceTo(it) < 6 } }
                 .sortedBy { it.arialDistanceTo(unit.currentTile) }
         val encampmentToHeadTowards = encampmentsCloseToCities.firstOrNull { unit.movement.canReach(it) }
-        if(encampmentToHeadTowards==null) return false
+        if (encampmentToHeadTowards == null) {
+            return false
+        }
         unit.movement.headTowards(encampmentToHeadTowards)
         return true
     }
 
-
-    fun tryHealUnit(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn):Boolean {
+    private fun tryHealUnit(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn): Boolean {
         val tilesInDistance = unitDistanceToTiles.keys.filter { unit.movement.canMoveTo(it) }
         if (unitDistanceToTiles.isEmpty()) return true // can't move, so...
         val currentUnitTile = unit.getTile()
@@ -155,7 +146,7 @@ class UnitAutomation{
         return true
     }
 
-    fun tryPillageImprovement(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn) : Boolean {
+    private fun tryPillageImprovement(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn): Boolean {
         if (unit.type.isCivilian()) return false
         val tilesThatCanWalkToAndThenPillage = unitDistanceToTiles
                 .filter { it.value.totalDistance < unit.currentMovement }.keys
@@ -171,86 +162,9 @@ class UnitAutomation{
         return true
     }
 
-    fun containsAttackableEnemy(tile: TileInfo, combatant: ICombatant): Boolean {
-        if(combatant is MapUnitCombatant) {
-            if (combatant.unit.isEmbarked()) {
-                if (tile.isWater) return false // can't attack water units while embarked, only land
-                if (combatant.isRanged()) return false
-            }
-            if (combatant.unit.hasUnique("Can only attack water")) {
-                if (tile.isLand) return false
-
-                // trying to attack lake-to-coast or vice versa
-                if ((tile.baseTerrain == Constants.lakes) != (combatant.getTile().baseTerrain == Constants.lakes))
-                    return false
-            }
-        }
-
-        val tileCombatant = Battle.getMapCombatantOfTile(tile)
-        if(tileCombatant==null) return false
-        if(tileCombatant.getCivInfo()==combatant.getCivInfo() ) return false
-        if(!combatant.getCivInfo().isAtWarWith(tileCombatant.getCivInfo())) return false
-
-        //only submarine and destroyer can attack submarine
-        //garrisoned submarine can be attacked by anyone, or the city will be in invincible
-        if (tileCombatant.isInvisible() && !tile.isCityCenter()) {
-            if (combatant is MapUnitCombatant
-                    && combatant.unit.hasUnique("Can attack submarines")
-                    && combatant.getCivInfo().viewableInvisibleUnitsTiles.map { it.position }.contains(tile.position)) {
-                return true
-            }
-            return false
-        }
-        return true
-    }
-
-    class AttackableTile(val tileToAttackFrom:TileInfo, val tileToAttack:TileInfo)
-
-    fun getAttackableEnemies(
-            unit: MapUnit,
-            unitDistanceToTiles: PathsToTilesWithinTurn,
-            tilesToCheck: List<TileInfo>? = null
-    ): ArrayList<AttackableTile> {
-        val tilesWithEnemies = (tilesToCheck ?: unit.civInfo.viewableTiles)
-            .filter { containsAttackableEnemy(it, MapUnitCombatant(unit)) }
-
-        val rangeOfAttack = unit.getRange()
-
-        val attackableTiles = ArrayList<AttackableTile>()
-        // The >0.1 (instead of >0) solves a bug where you've moved 2/3 road tiles,
-        // you come to move a third (distance is less that remaining movements),
-        // and then later we round it off to a whole.
-        // So the poor unit thought it could attack from the tile, but when it comes to do so it has no movement points!
-        // Silly floats, basically
-
-        val unitMustBeSetUp = unit.hasUnique("Must set up to ranged attack")
-        val tilesToAttackFrom = if (unit.type.isAirUnit()) sequenceOf(unit.currentTile)
-        else
-            unitDistanceToTiles.asSequence()
-                    .filter {
-                        val movementPointsToExpendAfterMovement = if (unitMustBeSetUp) 1 else 0
-                        val movementPointsToExpendHere = if (unitMustBeSetUp && unit.action != Constants.unitActionSetUp) 1 else 0
-                        val movementPointsToExpendBeforeAttack = if (it.key == unit.currentTile) movementPointsToExpendHere else movementPointsToExpendAfterMovement
-                        unit.currentMovement - it.value.totalDistance - movementPointsToExpendBeforeAttack > 0.1
-                    } // still got leftover movement points after all that, to attack (0.1 is because of Float nonsense, see MapUnit.moveToTile(...)
-                    .map { it.key }
-                    .filter { unit.movement.canMoveTo(it) || it == unit.getTile() }
-
-        for (reachableTile in tilesToAttackFrom) {  // tiles we'll still have energy after we reach there
-            val tilesInAttackRange =
-                    if (unit.hasUnique("Ranged attacks may be performed over obstacles") || unit.type.isAirUnit())
-                        reachableTile.getTilesInDistance(rangeOfAttack)
-                    else reachableTile.getViewableTiles(rangeOfAttack, unit.type.isWaterUnit())
-
-            attackableTiles += tilesInAttackRange.asSequence().filter { it in tilesWithEnemies }
-                    .map { AttackableTile(reachableTile, it) }
-        }
-        return attackableTiles
-    }
-
     fun getBombardTargets(city: CityInfo): List<TileInfo> {
         return city.getCenterTile().getViewableTiles(city.range, true)
-                .filter { containsAttackableEnemy(it, CityCombatant(city)) }
+                .filter { battleHelper.containsAttackableEnemy(it, CityCombatant(city)) }
     }
 
     /** Move towards the closest attackable enemy of the [unit].
@@ -263,11 +177,12 @@ class UnitAutomation{
                 unit.getTile().position,
                 unit.getMaxMovement() * CLOSE_ENEMY_TURNS_AWAY_LIMIT
         )
-        var closeEnemies = getAttackableEnemies(
+        var closeEnemies = battleHelper.getAttackableEnemies(
                 unit,
                 unitDistanceToTiles,
                 tilesToCheck = unit.getTile().getTilesInDistance(CLOSE_ENEMY_TILES_AWAY_LIMIT)
-        ).filter {  // Ignore units that would 1-shot you if you attacked
+        ).filter {
+            // Ignore units that would 1-shot you if you attacked
             BattleDamage().calculateDamageToAttacker(MapUnitCombatant(unit),
                 Battle.getMapCombatantOfTile(it.tileToAttack)!!) < unit.health
         }
@@ -286,9 +201,11 @@ class UnitAutomation{
 
     private fun tryAccompanySettlerOrGreatPerson(unit: MapUnit): Boolean {
         val settlerOrGreatPersonToAccompany = unit.civInfo.getCivUnits()
-                .firstOrNull { val tile = it.currentTile
+                .firstOrNull {
+                    val tile = it.currentTile
                     (it.name == Constants.settler || it.name in GreatPersonManager().statToGreatPersonMapping.values)
-                            && tile.militaryUnit==null && unit.movement.canMoveTo(tile) && unit.movement.canReach(tile) }
+                    && tile.militaryUnit == null && unit.movement.canMoveTo(tile) && unit.movement.canReach(tile)
+                }
         if (settlerOrGreatPersonToAccompany == null) return false
         unit.movement.headTowards(settlerOrGreatPersonToAccompany.currentTile)
         return true
@@ -321,7 +238,8 @@ class UnitAutomation{
 
         val closestReachableEnemyCity = enemyCities
                 .asSequence().map { it.getCenterTile() }
-                .sortedBy { cityCenterTile -> // sort enemy cities by closeness to our cities, and only then choose the first reachable - checking canReach is comparatively very time-intensive!
+                .sortedBy { cityCenterTile ->
+                    // sort enemy cities by closeness to our cities, and only then choose the first reachable - checking canReach is comparatively very time-intensive!
                     unit.civInfo.cities.asSequence().map { cityCenterTile.arialDistanceTo(it.getCenterTile()) }.min()!!
                 }
                 .firstOrNull { unit.movement.canReach(it) }
@@ -330,24 +248,19 @@ class UnitAutomation{
             val unitDistanceToTiles = unit.movement.getDistanceToTiles()
             val tilesInBombardRange = closestReachableEnemyCity.getTilesInDistance(2)
             val reachableTilesNotInBombardRange = unitDistanceToTiles.keys.filter { it !in tilesInBombardRange }
-            val canMoveIntoBombardRange = tilesInBombardRange.any { unitDistanceToTiles.containsKey(it)}
 
             val suitableGatheringGroundTiles = closestReachableEnemyCity.getTilesAtDistance(4)
                     .union(closestReachableEnemyCity.getTilesAtDistance(3))
                     .filter { it.isLand }
-            val closestReachableLandingGroundTile = suitableGatheringGroundTiles
-                    .sortedBy { it.arialDistanceTo(unit.currentTile) }
-                    .firstOrNull { unit.movement.canReach(it) }
 
             // don't head straight to the city, try to head to landing grounds -
             // this is against tha AI's brilliant plan of having everyone embarked and attacking via sea when unnecessary.
-            val tileToHeadTo = if(closestReachableLandingGroundTile!=null) closestReachableLandingGroundTile
-            else closestReachableEnemyCity
-
+            val tileToHeadTo = suitableGatheringGroundTiles
+                                       .sortedBy { it.arialDistanceTo(unit.currentTile) }
+                                       .firstOrNull { unit.movement.canReach(it) } ?: closestReachableEnemyCity
 
             if (tileToHeadTo !in tilesInBombardRange) // no need to worry, keep going as the movement alg. says
                 unit.movement.headTowards(tileToHeadTo)
-
             else {
                 if (unit.getRange() > 2) { // should never be in a bombardable position
                     val tilesCanAttackFromButNotInBombardRange =
@@ -356,8 +269,7 @@ class UnitAutomation{
                     // move into position far away enough that the bombard doesn't hurt
                     if (tilesCanAttackFromButNotInBombardRange.any())
                         unit.movement.headTowards(tilesCanAttackFromButNotInBombardRange.minBy { unitDistanceToTiles[it]!!.totalDistance }!!)
-                }
-                else {
+                } else {
                     // calculate total damage of units in surrounding 4-spaces from enemy city (so we can attack a city from 2 directions at once)
                     val militaryUnitsAroundEnemyCity = closestReachableEnemyCity.getTilesInDistance(3)
                             .filter { it.militaryUnit != null && it.militaryUnit!!.civInfo == unit.civInfo }
@@ -377,42 +289,6 @@ class UnitAutomation{
         return false
     }
 
-    private fun tryDisembarkUnitToAttackPosition(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn): Boolean {
-        if (!unit.type.isMelee() || !unit.type.isLandUnit() || !unit.isEmbarked()) return false
-        val attackableEnemiesNextTurn = getAttackableEnemies(unit, unitDistanceToTiles)
-                // Only take enemies we can fight without dying
-                .filter {
-                    BattleDamage().calculateDamageToAttacker(MapUnitCombatant(unit),
-                            Battle.getMapCombatantOfTile(it.tileToAttack)!!) < unit.health
-                }
-                .filter {it.tileToAttackFrom.isLand}
-
-        val enemyTileToAttackNextTurn = chooseAttackTarget(unit, attackableEnemiesNextTurn)
-
-        if (enemyTileToAttackNextTurn != null) {
-            unit.movement.moveToTile(enemyTileToAttackNextTurn.tileToAttackFrom)
-            return true
-        }
-        return false
-    }
-
-    fun tryAttackNearbyEnemy(unit: MapUnit): Boolean {
-        val attackableEnemies = getAttackableEnemies(unit, unit.movement.getDistanceToTiles())
-                // Only take enemies we can fight without dying
-                .filter {
-                    BattleDamage().calculateDamageToAttacker(MapUnitCombatant(unit),
-                            Battle.getMapCombatantOfTile(it.tileToAttack)!!) < unit.health
-                }
-
-        val enemyTileToAttack = chooseAttackTarget(unit, attackableEnemies)
-
-        if (enemyTileToAttack != null) {
-            Battle.moveAndAttack(MapUnitCombatant(unit), enemyTileToAttack)
-            return true
-        }
-        return false
-    }
-
     fun tryBombardEnemy(city: CityInfo): Boolean {
         if (!city.attackedThisTurn) {
             val target = chooseBombardTarget(city)
@@ -424,28 +300,6 @@ class UnitAutomation{
         return false
     }
 
-    private fun chooseAttackTarget(unit: MapUnit, attackableEnemies: List<AttackableTile>): AttackableTile? {
-        val cityTilesToAttack = attackableEnemies.filter { it.tileToAttack.isCityCenter() }
-        val nonCityTilesToAttack = attackableEnemies.filter { !it.tileToAttack.isCityCenter() }
-
-        // todo For air units, prefer to attack tiles with lower intercept chance
-
-        var enemyTileToAttack: AttackableTile? = null
-        val capturableCity = cityTilesToAttack.firstOrNull{it.tileToAttack.getCity()!!.health == 1}
-        val cityWithHealthLeft = cityTilesToAttack.filter { it.tileToAttack.getCity()!!.health != 1 } // don't want ranged units to attack defeated cities
-                .minBy { it.tileToAttack.getCity()!!.health  }
-
-        if (unit.type.isMelee() && capturableCity!=null)
-            enemyTileToAttack = capturableCity // enter it quickly, top priority!
-
-        else if (nonCityTilesToAttack.isNotEmpty()) // second priority, units
-            enemyTileToAttack = nonCityTilesToAttack.minBy { Battle.getMapCombatantOfTile(it.tileToAttack)!!.getHealth() }
-
-        else if (cityWithHealthLeft!=null) enemyTileToAttack = cityWithHealthLeft// third priority, city
-
-        return enemyTileToAttack
-    }
-
     private fun chooseBombardTarget(city: CityInfo): TileInfo? {
         var targets = getBombardTargets(city)
         if (targets.isEmpty()) return null
@@ -498,7 +352,7 @@ class UnitAutomation{
         return true
     }
 
-    fun tryGoToRuin(unit:MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn): Boolean {
+    private fun tryGoToRuin(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn): Boolean {
         if (!unit.civInfo.isMajorCiv()) return false // barbs don't have anything to do in ruins
         val tileWithRuin = unitDistanceToTiles.keys
                 .firstOrNull { it.improvement == Constants.ancientRuins && unit.movement.canMoveTo(it) }
@@ -508,8 +362,7 @@ class UnitAutomation{
     }
 
     internal fun tryExplore(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn): Boolean {
-        if(tryGoToRuin(unit,unitDistanceToTiles))
-        {
+        if (tryGoToRuin(unit, unitDistanceToTiles)) {
             if (unit.currentMovement == 0f) return true
         }
 
@@ -533,8 +386,10 @@ class UnitAutomation{
 
         for (i in 1..10) {
             val unexploredTilesAtDistance = unit.getTile().getTilesAtDistance(i)
-                    .filter { unit.movement.canMoveTo(it)  && it.position !in unit.civInfo.exploredTiles
-                            && unit.movement.canReach(it) }
+                    .filter {
+                        unit.movement.canMoveTo(it) && it.position !in unit.civInfo.exploredTiles
+                        && unit.movement.canReach(it)
+                    }
             if (unexploredTilesAtDistance.isNotEmpty()) {
                 unit.movement.headTowards(unexploredTilesAtDistance.random())
                 return
@@ -543,7 +398,6 @@ class UnitAutomation{
         unit.civInfo.addNotification("[${unit.name}] finished exploring.", unit.currentTile.position, Color.GRAY)
     }
 
-
     fun wander(unit: MapUnit, unitDistanceToTiles: PathsToTilesWithinTurn) {
         val reachableTiles = unitDistanceToTiles
                 .filter { unit.movement.canMoveTo(it.key) && unit.movement.canReach(it.key) }
@@ -551,7 +405,5 @@ class UnitAutomation{
         val reachableTilesMaxWalkingDistance = reachableTiles.filter { it.value.totalDistance == unit.currentMovement }
         if (reachableTilesMaxWalkingDistance.any()) unit.movement.moveToTile(reachableTilesMaxWalkingDistance.toList().random().first)
         else if (reachableTiles.any()) unit.movement.moveToTile(reachableTiles.toList().random().first)
-
     }
-
 }

core/src/com/unciv/logic/battle/Battle.kt

@@ -3,13 +3,13 @@ package com.unciv.logic.battle
 import com.badlogic.gdx.graphics.Color
 import com.unciv.Constants
 import com.unciv.UncivGame
-import com.unciv.logic.automation.UnitAutomation
 import com.unciv.logic.city.CityInfo
 import com.unciv.logic.civilization.AlertType
 import com.unciv.logic.civilization.PopupAlert
 import com.unciv.logic.civilization.diplomacy.DiplomaticModifiers
 import com.unciv.logic.map.RoadStatus
 import com.unciv.logic.map.TileInfo
+import com.unciv.models.AttackableTile
 import com.unciv.models.ruleset.unit.UnitType
 import java.util.*
 import kotlin.math.max
@@ -19,7 +19,7 @@ import kotlin.math.max
  */
 object Battle {
 
-    fun moveAndAttack(attacker: ICombatant, attackableTile: UnitAutomation.AttackableTile){
+    fun moveAndAttack(attacker: ICombatant, attackableTile: AttackableTile){
         if (attacker is MapUnitCombatant) {
             attacker.unit.movement.moveToTile(attackableTile.tileToAttackFrom)
             if (attacker.unit.hasUnique("Must set up to ranged attack") && attacker.unit.action != Constants.unitActionSetUp) {

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

@@ -262,7 +262,15 @@ class MapUnit {
         return true
     }
 
-    fun fortify(){ action = "Fortify 0"}
+    fun fortify() {
+        action = "Fortify 0"
+    }
+
+    fun fortifyIfCan() {
+        if (canFortify()) {
+            fortify()
+        }
+    }
 
     fun adjacentHealingBonus():Int{
         var healingBonus = 0

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

@@ -7,7 +7,7 @@ import com.unciv.logic.civilization.CivilizationInfo
 class UnitMovementAlgorithms(val unit:MapUnit) {
 
     // This function is called ALL THE TIME and should be as time-optimal as possible!
-    private fun getMovementCostBetweenAdjacentTiles(from: TileInfo, to: TileInfo, civInfo: CivilizationInfo): Float {
+     fun getMovementCostBetweenAdjacentTiles(from: TileInfo, to: TileInfo, civInfo: CivilizationInfo): Float {
 
         if ((from.isLand != to.isLand) && unit.type.isLandUnit())
             return 100f // this is embarkment or disembarkment, and will take the entire turn

core/src/com/unciv/models/AttackableTile.kt

@@ -0,0 +1,5 @@
+package com.unciv.models
+
+import com.unciv.logic.map.TileInfo
+
+class AttackableTile(val tileToAttackFrom: TileInfo, val tileToAttack: TileInfo)

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

@@ -9,6 +9,7 @@ import com.badlogic.gdx.scenes.scene2d.actions.FloatAction
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.Constants
 import com.unciv.UncivGame
+import com.unciv.logic.automation.BattleHelper
 import com.unciv.logic.automation.UnitAutomation
 import com.unciv.logic.city.CityInfo
 import com.unciv.logic.civilization.CivilizationInfo
@@ -244,7 +245,7 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
         val unitType = unit.type
         val attackableTiles: List<TileInfo> = if (unitType.isCivilian()) listOf()
         else {
-            val tiles = UnitAutomation().getAttackableEnemies(unit, unit.movement.getDistanceToTiles()).map { it.tileToAttack }
+            val tiles = BattleHelper().getAttackableEnemies(unit, unit.movement.getDistanceToTiles()).map { it.tileToAttack }
             tiles.filter { (UncivGame.Current.viewEntireMapForDebug || playerViewableTilePositions.contains(it.position)) }
         }
 

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

@@ -9,8 +9,10 @@ import com.badlogic.gdx.scenes.scene2d.ui.Label
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.scenes.scene2d.ui.TextButton
 import com.unciv.UncivGame
+import com.unciv.logic.automation.BattleHelper
 import com.unciv.logic.automation.UnitAutomation
 import com.unciv.logic.battle.*
+import com.unciv.models.AttackableTile
 import com.unciv.models.translations.tr
 import com.unciv.models.ruleset.unit.UnitType
 import com.unciv.ui.utils.*
@@ -20,6 +22,8 @@ import kotlin.math.max
 
 class BattleTable(val worldScreen: WorldScreen): Table() {
 
+    private val battleHelper = BattleHelper()
+
     init {
         isVisible = false
         skin = CameraStageBaseScreen.skin
@@ -170,11 +174,11 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
         }
         val attackButton = TextButton(attackText.tr(), skin).apply { color= Color.RED }
 
-        var attackableEnemy : UnitAutomation.AttackableTile? = null
+        var attackableEnemy : AttackableTile? = null
 
         if (attacker.canAttack()) {
             if (attacker is MapUnitCombatant) {
-                attackableEnemy = UnitAutomation()
+                attackableEnemy = battleHelper
                         .getAttackableEnemies(attacker.unit, attacker.unit.movement.getDistanceToTiles())
                         .firstOrNull{ it.tileToAttack == defender.getTile()}
             }
@@ -182,7 +186,7 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
             {
                 val canBombard = UnitAutomation().getBombardTargets(attacker.city).contains(defender.getTile())
                 if (canBombard) {
-                    attackableEnemy = UnitAutomation.AttackableTile(attacker.getTile(), defender.getTile())
+                    attackableEnemy = AttackableTile(attacker.getTile(), defender.getTile())
                 }    
             }
         }