Resolved #5641 - unit upgrade check ONLY removes/adds unit if absolutely necessary

This previously was done every time the function was called, leading to updating the civ resources twice per unit check, which is kind of heavy

buildSrc/src/main/kotlin/BuildConfig.kt

@@ -4,7 +4,7 @@ object BuildConfig {
     const val kotlinVersion = "1.5.30"
     const val appName = "Unciv"
     const val appCodeNumber = 644
-    const val appVersion = "3.17.15"
+    const val appVersion = "3.18.0"
 
     const val gdxVersion = "1.10.0"
     const val roboVMVersion = "2.3.1"

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

@@ -517,6 +517,7 @@ object Battle {
                 capturedUnit.capturedBy(attacker.getCivInfo())
                 attacker.getCivInfo().popupAlerts.add(PopupAlert(AlertType.RecapturedCivilian, capturedUnitTile.position.toString()))
             }
+
             // Captured settlers are converted to workers unless captured by barbarians (so they can be returned later).
             capturedUnit.hasUnique("Founds a new city") && !attacker.getCivInfo().isBarbarian() -> {
                 capturedUnit.destroy()
@@ -525,9 +526,7 @@ object Battle {
                 capturedUnit.civInfo = attacker.getCivInfo()
                 attacker.getCivInfo().placeUnitNearTile(capturedUnitTile.position, Constants.worker)
             }
-            else -> {
+            else -> capturedUnit.capturedBy(attacker.getCivInfo())
-                capturedUnit.capturedBy(attacker.getCivInfo())
-            }
         }
 
         if (checkDefeat)

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

@@ -6,6 +6,7 @@ import com.unciv.UncivGame
 import com.unciv.logic.automation.UnitAutomation
 import com.unciv.logic.automation.WorkerAutomation
 import com.unciv.logic.city.CityInfo
+import com.unciv.logic.city.RejectionReason
 import com.unciv.logic.civilization.CivilizationInfo
 import com.unciv.logic.civilization.LocationAction
 import com.unciv.logic.civilization.NotificationIcon
@@ -470,12 +471,15 @@ class MapUnit {
      * Used for upgrading units via ancient ruins.
      */
     fun canUpgrade(unitToUpgradeTo: BaseUnit = getUnitToUpgradeTo(), ignoreRequired: Boolean = false): Boolean {
+        if (name == unitToUpgradeTo.name) return false
+        val rejectionReasons = unitToUpgradeTo.getRejectionReasons(civInfo)
+        if (rejectionReasons.isEmpty()) return true
+
+        if (rejectionReasons.size == 1 && rejectionReasons.contains(RejectionReason.ConsumesResources)) {
             // We need to remove the unit from the civ for this check,
             // because if the unit requires, say, horses, and so does its upgrade,
-        // and the civ currently has 0 horses,
+            // and the civ currently has 0 horses, we need to see if the upgrade will be buildable
-        // if we don't remove the unit before the check it's return false!
+            // WHEN THE CURRENT UNIT IS NOT HERE
-
-        if (name == unitToUpgradeTo.name) return false
             civInfo.removeUnit(this)
             val canUpgrade =
                 if (ignoreRequired) unitToUpgradeTo.isBuildableIgnoringTechs(civInfo)
@@ -483,6 +487,8 @@ class MapUnit {
             civInfo.addUnit(this)
             return canUpgrade
         }
+        return false
+    }
 
     fun getCostOfUpgrade(): Int {
         val unitToUpgradeTo = getUnitToUpgradeTo()