diff --git a/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt b/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt index 3ebaf4cb2e..362bc1e211 100644 --- a/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt +++ b/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt @@ -142,6 +142,7 @@ object UnitAutomation { val upgradeActions = UnitActionsUpgrade.getUpgradeActions(unit) upgradeActions.firstOrNull{ (it as UpgradeUnitAction).unitToUpgradeTo == upgradedUnit }?.action?.invoke() ?: return false + //todo Incorrect - an _unsuccessful_ upgrade might have _resurrected_ the original in which case it's a new clone, and unit.isDestroyed is still true return unit.isDestroyed // a successful upgrade action will destroy this unit } diff --git a/core/src/com/unciv/logic/map/mapunit/MapUnit.kt b/core/src/com/unciv/logic/map/mapunit/MapUnit.kt index c07783aed8..9f7005b619 100644 --- a/core/src/com/unciv/logic/map/mapunit/MapUnit.kt +++ b/core/src/com/unciv/logic/map/mapunit/MapUnit.kt @@ -377,14 +377,15 @@ class MapUnit : IsPartOfGameInfoSerialization { } - fun canFortify(): Boolean { - if (baseUnit.isWaterUnit()) return false - if (isCivilian()) return false - if (baseUnit.movesLikeAirUnits()) return false - if (isEmbarked()) return false - if (hasUnique(UniqueType.NoDefensiveTerrainBonus)) return false - if (isFortified()) return false - return true + fun canFortify(ignoreAlreadyFortified: Boolean = false) = when { + baseUnit.isWaterUnit() -> false + isCivilian() -> false + baseUnit.movesLikeAirUnits() -> false + isEmbarked() -> false + hasUnique(UniqueType.NoDefensiveTerrainBonus) -> false + ignoreAlreadyFortified -> true + isFortified() -> false + else -> true } private fun adjacentHealingBonus(): Int { diff --git a/core/src/com/unciv/logic/map/mapunit/UnitUpgradeManager.kt b/core/src/com/unciv/logic/map/mapunit/UnitUpgradeManager.kt index 3af0a71ec0..08432b754a 100644 --- a/core/src/com/unciv/logic/map/mapunit/UnitUpgradeManager.kt +++ b/core/src/com/unciv/logic/map/mapunit/UnitUpgradeManager.kt @@ -1,11 +1,11 @@ package com.unciv.logic.map.mapunit -import com.unciv.logic.UncivShowableException import com.unciv.models.ruleset.RejectionReasonType import com.unciv.models.ruleset.unique.StateForConditionals import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.ruleset.unit.BaseUnit import com.unciv.ui.components.extensions.toPercent +import com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsUpgrade import kotlin.math.pow class UnitUpgradeManager(val unit:MapUnit) { @@ -26,7 +26,7 @@ class UnitUpgradeManager(val unit:MapUnit) { val rejectionReasons = unitToUpgradeTo.getRejectionReasons(unit.civ, additionalResources = unit.getResourceRequirementsPerTurn()) - var relevantRejectionReasons = rejectionReasons.filterNot { + var relevantRejectionReasons = rejectionReasons.filterNot { it.isConstructionRejection() || it.type == RejectionReasonType.Obsoleted } if (ignoreRequirements) @@ -68,9 +68,42 @@ class UnitUpgradeManager(val unit:MapUnit) { cost = (cost * civModifier).pow(constants.exponent) cost *= unit.civ.gameInfo.speed.modifier goldCostOfUpgrade += (cost / constants.roundTo).toInt() * constants.roundTo - + return goldCostOfUpgrade } + /** _Perform_ an upgrade, assuming validity checks were already passed. + * + * Continuing to use a reference to this manager or its unit after this call is invalid! + * + * Please use [UnitActionsUpgrade.getUpgradeActions] instead if at all possible. + * + * Note - the upgraded unit is a new instance, and it's possible this method will need to place it on a different tile. + * It is also possible the placement fails and the original is resurrected - in which case it is a **new instance** as well. + * It might be desirable to return `newUnit` (or `resurrectedUnit`) if needed - + * but then the lambda in UnitActionsUpgrade will complain and need to be forced back to Unit type. + */ + fun performUpgrade(upgradedUnit: BaseUnit, isFree: Boolean, goldCostOfUpgrade: Int? = null) { + unit.destroy(destroyTransportedUnit = false) + val civ = unit.civ + val position = unit.currentTile.position + val newUnit = civ.units.placeUnitNearTile(position, upgradedUnit) + /** We were UNABLE to place the new unit, which means that the unit failed to upgrade! + * The only known cause of this currently is "land units upgrading to water units" which fail to be placed. + */ + if (newUnit == null) { + val resurrectedUnit = civ.units.placeUnitNearTile(position, unit.baseUnit)!! + unit.copyStatisticsTo(resurrectedUnit) + return + } + + // Managed to upgrade + if (!isFree) civ.addGold(-(goldCostOfUpgrade ?: getCostOfUpgrade(upgradedUnit))) + unit.copyStatisticsTo(newUnit) + newUnit.currentMovement = 0f + // wake up if lost ability to fortify + if (newUnit.isFortified() && !newUnit.canFortify(ignoreAlreadyFortified = true)) + newUnit.action = null + } } diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt index a96f6caa9d..cd8ddd62ad 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt +++ b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt @@ -60,20 +60,7 @@ object UnitActionsUpgrade { goldCostOfUpgrade = goldCostOfUpgrade, newResourceRequirements = resourceRequirementsDelta, action = { - unit.destroy(destroyTransportedUnit = false) - val newUnit = civInfo.units.placeUnitNearTile(unitTile.position, upgradedUnit) - - /** We were UNABLE to place the new unit, which means that the unit failed to upgrade! - * The only known cause of this currently is "land units upgrading to water units" which fail to be placed. - */ - if (newUnit == null) { - val resurrectedUnit = civInfo.units.placeUnitNearTile(unitTile.position, unit.baseUnit)!! - unit.copyStatisticsTo(resurrectedUnit) - } else { // Managed to upgrade - if (!isFree) civInfo.addGold(-goldCostOfUpgrade) - unit.copyStatisticsTo(newUnit) - newUnit.currentMovement = 0f - } + unit.upgrade.performUpgrade(upgradedUnit, isFree, goldCostOfUpgrade) }.takeIf { isFree || ( unit.civ.gold >= goldCostOfUpgrade