From 8203557633568adde2f4c4fc3d6f5c49f1c39824 Mon Sep 17 00:00:00 2001
From: OptimizedForDensity
 <105244635+OptimizedForDensity@users.noreply.github.com>
Date: Mon, 4 Jul 2022 09:34:33 -0400
Subject: [PATCH] Better AI targeting (#7359)

---
 .../unciv/logic/automation/BattleHelper.kt    | 30 +++++++++++++++++--
 .../unciv/logic/automation/UnitAutomation.kt  | 13 ++++++--
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/core/src/com/unciv/logic/automation/BattleHelper.kt b/core/src/com/unciv/logic/automation/BattleHelper.kt
index 4b6c0cf101..aac9596f62 100644
--- a/core/src/com/unciv/logic/automation/BattleHelper.kt
+++ b/core/src/com/unciv/logic/automation/BattleHelper.kt
@@ -166,11 +166,35 @@ object BattleHelper {
             enemyTileToAttack = capturableCity // enter it quickly, top priority!
 
         else if (nonCityTilesToAttack.isNotEmpty()) // second priority, units
-            enemyTileToAttack = nonCityTilesToAttack.minByOrNull {
-                Battle.getMapCombatantOfTile(it.tileToAttack)!!.getHealth()
-            }
+            enemyTileToAttack = chooseUnitToAttack(unit, nonCityTilesToAttack)
         else if (cityWithHealthLeft != null) enemyTileToAttack = cityWithHealthLeft // third priority, city
 
         return enemyTileToAttack
     }
+
+    private fun chooseUnitToAttack(unit: MapUnit, attackableUnits: List<AttackableTile>): AttackableTile {
+        val militaryUnits = attackableUnits.filter { it.tileToAttack.militaryUnit != null }
+
+        // prioritize attacking military
+        if (militaryUnits.isNotEmpty()) {
+            // associate enemy units with number of hits from this unit to kill them
+            val attacksToKill = militaryUnits
+                .associateWith { it.tileToAttack.militaryUnit!!.health.toFloat() / BattleDamage.calculateDamageToDefender(
+                        MapUnitCombatant(unit),
+                        MapUnitCombatant(it.tileToAttack.militaryUnit!!)
+                    ).toFloat().coerceAtLeast(1f) }
+
+            // kill a unit if possible, prioritizing by attack strength
+            val canKill = attacksToKill.filter { it.value <= 1 }.maxByOrNull { MapUnitCombatant(it.key.tileToAttack.militaryUnit!!).getAttackingStrength() }?.key
+            if (canKill != null) return canKill
+
+            // otherwise pick the unit we can kill the fastest
+            return attacksToKill.minByOrNull { it.value }!!.key
+        }
+
+        // only civilians in attacking range
+        return attackableUnits.minByOrNull {
+            Battle.getMapCombatantOfTile(it.tileToAttack)!!.getHealth()
+        }!!
+    }
 }
diff --git a/core/src/com/unciv/logic/automation/UnitAutomation.kt b/core/src/com/unciv/logic/automation/UnitAutomation.kt
index 18c28f3ff4..21028cea2e 100644
--- a/core/src/com/unciv/logic/automation/UnitAutomation.kt
+++ b/core/src/com/unciv/logic/automation/UnitAutomation.kt
@@ -528,13 +528,22 @@ object UnitAutomation {
 
         val siegeUnits = targets
                 .filter { it is MapUnitCombatant && it.unit.baseUnit.isProbablySiegeUnit() }
-        if (siegeUnits.any()) targets = siegeUnits
+        val nonEmbarkedSiege = siegeUnits.filter { it is MapUnitCombatant && !it.unit.isEmbarked() }
+        if (nonEmbarkedSiege.any()) targets = nonEmbarkedSiege
+        else if (siegeUnits.any()) targets = siegeUnits
         else {
             val rangedUnits = targets
                     .filter { it.isRanged() }
             if (rangedUnits.any()) targets = rangedUnits
         }
-        return targets.minByOrNull { it.getHealth() }
+
+        val hitsToKill = targets.associateWith { it.getHealth().toFloat() / BattleDamage.calculateDamageToDefender(
+            CityCombatant(city),
+            it
+        ).toFloat().coerceAtLeast(1f) }
+        val target = hitsToKill.filter { it.value <= 1 }.maxByOrNull { it.key.getAttackingStrength() }?.key
+        if (target != null) return target
+        return hitsToKill.minByOrNull { it.value }?.key
     }
 
     private fun tryTakeBackCapturedCity(unit: MapUnit): Boolean {