From f2365568d4a2267d8365e0b57fbec8b797a04bda Mon Sep 17 00:00:00 2001 From: Yair Morgenstern Date: Sun, 10 Jul 2022 02:12:59 +0300 Subject: [PATCH] Fix multiple capture uniques resulting in double-capture (#7403) --- .../jsons/translations/template.properties | 5 +- core/src/com/unciv/logic/battle/Battle.kt | 157 +++++++++--------- .../unciv/models/ruleset/unique/UniqueType.kt | 9 +- 3 files changed, 84 insertions(+), 87 deletions(-) diff --git a/android/assets/jsons/translations/template.properties b/android/assets/jsons/translations/template.properties index ec16a988d2..c0517e4b40 100644 --- a/android/assets/jsons/translations/template.properties +++ b/android/assets/jsons/translations/template.properties @@ -790,8 +790,11 @@ The City-State of [name] has been destroyed! = Your [ourUnit] captured an enemy [theirUnit]! = Your [ourUnit] plundered [amount] [Stat] from [theirUnit] = We have captured a barbarian encampment and recovered [goldAmount] gold! = +An enemy [unitType] has joined us! = + +# This might be needed for a rewrite of Germany's unique - see #7376 A barbarian [unitType] has joined us! = -We have captured an enemy [unitType]! = + We have found survivors in the ruins - population added to [cityName] = We have discovered cultural artifacts in the ruins! (+20 Culture) = We have discovered the lost technology of [techName] in the ruins! = diff --git a/core/src/com/unciv/logic/battle/Battle.kt b/core/src/com/unciv/logic/battle/Battle.kt index 208c912213..30fc6ec707 100644 --- a/core/src/com/unciv/logic/battle/Battle.kt +++ b/core/src/com/unciv/logic/battle/Battle.kt @@ -118,9 +118,7 @@ object Battle { // check if unit is captured by the attacker (prize ships unique) // As ravignir clarified in issue #4374, this only works for aggressor - val captureMilitaryUnitSuccess = defender is MapUnitCombatant && attacker is MapUnitCombatant - && defender.isDefeated() && !defender.unit.isCivilian() - && tryCaptureUnit(attacker, defender) + val captureMilitaryUnitSuccess = tryCaptureUnit(attacker, defender, attackedTile) if (!captureMilitaryUnitSuccess) // capture creates a new unit, but `defender` still is the original, so this function would still show a kill message postBattleNotifications(attacker, defender, attackedTile, attacker.getTile()) @@ -128,8 +126,6 @@ object Battle { if (defender.getCivInfo().isBarbarian() && attackedTile.improvement == Constants.barbarianEncampment) defender.getCivInfo().gameInfo.barbarians.campAttacked(attackedTile.position) - postBattleNationUniques(defender, attackedTile, attacker) - // This needs to come BEFORE the move-to-tile, because if we haven't conquered it we can't move there =) if (defender.isDefeated() && defender is CityCombatant && attacker is MapUnitCombatant && attacker.isMelee() && !attacker.unit.hasUnique(UniqueType.CannotCaptureCities)) { @@ -226,21 +222,87 @@ object Battle { } } - private fun tryCaptureUnit(attacker: MapUnitCombatant, defender: MapUnitCombatant): Boolean { + private fun tryCaptureUnit(attacker: ICombatant, defender: ICombatant, attackedTile: TileInfo): Boolean { // https://forums.civfanatics.com/threads/prize-ships-for-land-units.650196/ - // https://civilization.fandom.com/wiki/Module:Data/Civ5/GK/Defines + // https://civilization.fandom.com/wiki/Module:Data/Civ5/GK/Defines\ + // There are 3 ways of capturing a unit, we separate them for cleaner code but we also need to ensure a unit isn't captured twice - if (attacker.unit.getMatchingUniques(UniqueType.KillUnitCapture) - .none { defender.matchesCategory(it.params[0]) } - ) return false + if (defender !is MapUnitCombatant || attacker !is MapUnitCombatant) return false - val captureChance = min(0.8f, 0.1f + attacker.getAttackingStrength().toFloat() / defender.getDefendingStrength().toFloat() * 0.4f) - if (Random().nextFloat() > captureChance) return false + if (!defender.isDefeated() || defender.unit.isCivilian()) return false + + fun unitCapturedPrizeShipsUnique(): Boolean { + if (attacker.unit.getMatchingUniques(UniqueType.KillUnitCapture) + .none { defender.matchesCategory(it.params[0]) } + ) return false + + val captureChance = min( + 0.8f, + 0.1f + attacker.getAttackingStrength().toFloat() / defender.getDefendingStrength() + .toFloat() * 0.4f + ) + return Random().nextFloat() <= captureChance + } + + fun unitGainFromEncampment(): Boolean { + if (!defender.getCivInfo().isBarbarian()) return false + if (attackedTile.improvement != Constants.barbarianEncampment) return false + + var unitCaptured = false + // German unique - needs to be checked before we try to move to the enemy tile, since the encampment disappears after we move in + + for (unique in attacker.getCivInfo() + .getMatchingUniques(UniqueType.GainFromEncampment)) { + attacker.getCivInfo().addGold(unique.params[0].toInt()) + unitCaptured = true + } + return unitCaptured + } + + + fun unitGainFromDefeatingUnit(): Boolean { + if (!attacker.isMelee()) return false + var unitCaptured = false + for (unique in attacker.getCivInfo() + .getMatchingUniques(UniqueType.GainFromDefeatingUnit)) { + if (defender.unit.matchesFilter(unique.params[0])) { + attacker.getCivInfo().addGold(unique.params[1].toInt()) + unitCaptured = true + } + } + return unitCaptured + } + + // Due to the way OR operators short-circuit, calling just A() || B() means B isn't called if A is true. + // Therefore we run all functions before checking if one is true. + val wasUnitCaptured = listOf( + unitCapturedPrizeShipsUnique(), + unitGainFromEncampment(), + unitGainFromDefeatingUnit() + ).any() + + if (!wasUnitCaptured) return false // This is called after takeDamage and so the defeated defender is already destroyed and // thus removed from the tile - but MapUnit.destroy() will not clear the unit's currentTile. // Therefore placeUnitNearTile _will_ place the new unit exactly where the defender was - return spawnCapturedUnit(defender.getName(), attacker, defender.getTile(), "Your [${attacker.getName()}] captured an enemy [${defender.getName()}]!") + return spawnCapturedUnit(defender.getName(), attacker, defender.getTile()) + } + + /** Places a [unitName] unit near [tile] after being attacked by [attacker]. + * Adds a notification to [attacker]'s civInfo and returns whether the captured unit could be placed */ + private fun spawnCapturedUnit(unitName: String, attacker: ICombatant, tile: TileInfo): Boolean { + val addedUnit = attacker.getCivInfo().placeUnitNearTile(tile.position, unitName) ?: return false + addedUnit.currentMovement = 0f + addedUnit.health = 50 + attacker.getCivInfo().addNotification("An enemy [${unitName}] has joined us!", addedUnit.getTile().position, unitName) + + val civilianUnit = tile.civilianUnit + // placeUnitNearTile might not have spawned the unit in exactly this tile, in which case no capture would have happened on this tile. So we need to do that here. + if (addedUnit.getTile() != tile && civilianUnit != null) { + captureCivilianUnit(attacker, MapUnitCombatant(civilianUnit)) + } + return true } private fun takeDamage(attacker: ICombatant, defender: ICombatant) { @@ -346,75 +408,6 @@ object Battle { } } - /** Places a [unitName] unit near [tile] after being attacked by [attacker]. - * Adds a notification to [attacker]'s civInfo and returns whether the captured unit could be placed */ - private fun spawnCapturedUnit(unitName: String, attacker: ICombatant, tile: TileInfo, notification: String): Boolean { - val addedUnit = attacker.getCivInfo().placeUnitNearTile(tile.position, unitName) ?: return false - addedUnit.currentMovement = 0f - addedUnit.health = 50 - attacker.getCivInfo().addNotification(notification, addedUnit.getTile().position, attacker.getName(), unitName) - - val civilianUnit = tile.civilianUnit - // placeUnitNearTile might not have spawned the unit in exactly this tile, in which case no capture would have happened on this tile. So we need to do that here. - if (addedUnit.getTile() != tile && civilianUnit != null) { - captureCivilianUnit(attacker, MapUnitCombatant(civilianUnit)) - } - return true - } - - private fun postBattleNationUniques(defender: ICombatant, attackedTile: TileInfo, attacker: ICombatant) { - if (!defender.isDefeated()) return - - // Barbarians reduce spawn countdown after their camp was attacked "kicking the hornet's nest" - if (defender.getCivInfo().isBarbarian() && attackedTile.improvement == Constants.barbarianEncampment) { - var unitPlaced = false - // German unique - needs to be checked before we try to move to the enemy tile, since the encampment disappears after we move in - // Deprecated as of 4.0.3 - if (attacker.getCivInfo().hasUnique(UniqueType.ChanceToRecruitBarbarianFromEncampment) - && Random().nextDouble() < 0.67 - ) { - attacker.getCivInfo().addGold(25) - unitPlaced = spawnCapturedUnit(defender.getName(), attacker, attackedTile,"A barbarian [${defender.getName()}] has joined us!") - } - - // New version of unique - // - for (unique in attacker.getCivInfo().getMatchingUniques(UniqueType.GainFromEncampment)) { - attacker.getCivInfo().addGold(unique.params[0].toInt()) - if (unitPlaced) continue - unitPlaced = spawnCapturedUnit(defender.getName(), attacker, attackedTile,"A barbarian [${defender.getName()}] has joined us!") - } - } - - // Similarly, Ottoman unique - // Deprecated as of 4.0.3 - if (attacker.getCivInfo().hasUnique(UniqueType.ChanceToRecruitNavalBarbarian) - && defender.isDefeated() - && defender is MapUnitCombatant - && defender.unit.baseUnit.isWaterUnit() - && defender.getCivInfo().isBarbarian() - && attacker.isMelee() - && attacker is MapUnitCombatant - && attacker.unit.baseUnit.isWaterUnit() - && Random().nextDouble() < 0.5) { - attacker.getCivInfo().addGold(25) - spawnCapturedUnit(defender.getName(), attacker, attackedTile, "We have captured an enemy [${defender.getName()}]!") - } - // - if (defender.isDefeated() && defender is MapUnitCombatant) { - var unitPlaced = false - for (unique in attacker.getCivInfo().getMatchingUniques(UniqueType.GainFromDefeatingUnit)) { - if (defender.unit.matchesFilter(unique.params[0]) - && attacker.isMelee() - ) { - attacker.getCivInfo().addGold(unique.params[1].toInt()) - if (unitPlaced) continue - unitPlaced = spawnCapturedUnit(defender.getName(), attacker, attackedTile, "We have captured an enemy [${defender.getName()}]!") - - } - } - } - } private fun postBattleMoveToAttackedTile(attacker: ICombatant, defender: ICombatant, attackedTile: TileInfo) { if (!attacker.isMelee()) return diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt index 4954260b35..1dc14fd6b2 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt @@ -230,11 +230,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags: GreatGeneralProvidesDoubleCombatBonus("Great General provides double combat bonus", UniqueTarget.Unit, UniqueTarget.Global), TechBoostWhenScientificBuildingsBuiltInCapital("Receive a tech boost when scientific buildings/wonders are built in capital", UniqueTarget.Global), MayNotGenerateGreatProphet("May not generate great prophet equivalents naturally", UniqueTarget.Global), - @Deprecated("as of 4.0.3", ReplaceWith("When conquering an encampment, earn [25] Gold and recruit a Barbarian unit ")) - ChanceToRecruitBarbarianFromEncampment("67% chance to earn 25 Gold and recruit a Barbarian unit from a conquered encampment", UniqueTarget.Global), GainFromEncampment("When conquering an encampment, earn [amount] Gold and recruit a Barbarian unit", UniqueTarget.Global), - @Deprecated("as of 4.0.3", ReplaceWith("When defeating a [{Barbarian} {Water}] unit, earn [25] Gold and recruit it ")) - ChanceToRecruitNavalBarbarian("50% chance of capturing defeated Barbarian naval units and earning 25 Gold", UniqueTarget.Global), GainFromDefeatingUnit("When defeating a [mapUnitFilter] unit, earn [amount] Gold and recruit it", UniqueTarget.Global), TripleGoldFromEncampmentsAndCities("Receive triple Gold from Barbarian encampments and pillaging Cities", UniqueTarget.Global), CitiesAreRazedXTimesFaster("Cities are razed [amount] times as fast", UniqueTarget.Global), @@ -717,6 +713,11 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags: // endregion // region DEPRECATED AND REMOVED + @Deprecated("as of 4.0.3", ReplaceWith("When conquering an encampment, earn [25] Gold and recruit a Barbarian unit "), DeprecationLevel.ERROR) + ChanceToRecruitBarbarianFromEncampment("67% chance to earn 25 Gold and recruit a Barbarian unit from a conquered encampment", UniqueTarget.Global), + @Deprecated("as of 4.0.3", ReplaceWith("When defeating a [{Barbarian} {Water}] unit, earn [25] Gold and recruit it "), DeprecationLevel.ERROR) + ChanceToRecruitNavalBarbarian("50% chance of capturing defeated Barbarian naval units and earning 25 Gold", UniqueTarget.Global), + @Deprecated("as of 3.19.8", ReplaceWith("Eliminates combat penalty for attacking across a coast"), DeprecationLevel.ERROR) AttackFromSea("Eliminates combat penalty for attacking from the sea", UniqueTarget.Unit), @Deprecated("as of 3.19.19", ReplaceWith("[+4] Sight\", \"Can see over obstacles"), DeprecationLevel.ERROR)