From 6a18a33674aa8edfd71f5d9fef31646e35d8d975 Mon Sep 17 00:00:00 2001 From: Arthur van der Staaij <32672293+avdstaaij@users.noreply.github.com> Date: Fri, 13 Aug 2021 10:30:45 +0200 Subject: [PATCH] Fix units not entering cities upon capture (#4839) * Renamed and added doc comments for some some can-pass-through functions * Moved a city-state pass-through exception to a lower-level function Moved the exception that players (but not AI) can always pass through city-states from TileInfo.canCivPassThrough to CivilizationInfo.canPassThroughTiles (which is called by the former). This makes the behavior of the latter function accurate to its name, and makes the code more understandable overall. A side-effect of this change is that functions that used the lower-level CivilizationInfo.canPassThroughTiles function directly now consider the aforementioned exception. For example, if a city-state expands its border, it should now no longer push out units of a player civilization. It may make even more sense to move the exception to the "canMoveTo" logic, since AI should probably be able to pass through city-state tiles even when they don't want to end on them. That might however affect the AI more significantly. * Fixed bug where units would not enter a city upon capture As listed in #4697. * Fixed a bug where captured units were pushed out of captured cities Before, capturing a city with a civilian unit in it would cause the capturing military unit to enter the city but also cause the civilian unit to leave the city. Now, the civilian unit stays in the city as well. --- .../logic/automation/NextTurnAutomation.kt | 3 +-- core/src/com/unciv/logic/battle/Battle.kt | 9 +++---- .../unciv/logic/city/CityExpansionManager.kt | 3 +-- .../logic/civilization/CivilizationInfo.kt | 18 ++++++++++--- core/src/com/unciv/logic/map/MapUnit.kt | 2 +- core/src/com/unciv/logic/map/TileInfo.kt | 12 +++++---- .../unciv/logic/map/UnitMovementAlgorithms.kt | 25 +++++++++++++------ core/src/com/unciv/logic/trade/TradeLogic.kt | 2 +- 8 files changed, 47 insertions(+), 27 deletions(-) diff --git a/core/src/com/unciv/logic/automation/NextTurnAutomation.kt b/core/src/com/unciv/logic/automation/NextTurnAutomation.kt index 54b8313e9ed97..f4e1df6234b14 100644 --- a/core/src/com/unciv/logic/automation/NextTurnAutomation.kt +++ b/core/src/com/unciv/logic/automation/NextTurnAutomation.kt @@ -2,7 +2,6 @@ package com.unciv.logic.automation import com.unciv.Constants import com.unciv.logic.city.CityInfo -import com.unciv.logic.city.IConstruction import com.unciv.logic.city.INonPerpetualConstruction import com.unciv.logic.city.PerpetualConstruction import com.unciv.logic.civilization.* @@ -399,7 +398,7 @@ object NextTurnAutomation { fun isTileCanMoveThrough(tileInfo: TileInfo): Boolean { val owner = tileInfo.getOwner() return !tileInfo.isImpassible() - && (owner == otherCiv || owner == null || civInfo.canEnterTiles(owner)) + && (owner == otherCiv || owner == null || civInfo.canPassThroughTiles(owner)) } val reachableEnemyCitiesBfs = BFS(civInfo.getCapital().getCenterTile()) { isTileCanMoveThrough(it) } diff --git a/core/src/com/unciv/logic/battle/Battle.kt b/core/src/com/unciv/logic/battle/Battle.kt index 32008f48fd91e..e853eeec4906b 100644 --- a/core/src/com/unciv/logic/battle/Battle.kt +++ b/core/src/com/unciv/logic/battle/Battle.kt @@ -69,11 +69,11 @@ 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 captureSuccess = defender is MapUnitCombatant && attacker is MapUnitCombatant + val captureMilitaryUnitSuccess = defender is MapUnitCombatant && attacker is MapUnitCombatant && defender.isDefeated() && !defender.unit.isCivilian() && tryCaptureUnit(attacker, defender) - if (!captureSuccess) // capture creates a new unit, but `defender` still is the original, so this function would still show a kill message + 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()) postBattleNationUniques(defender, attackedTile, attacker) @@ -104,9 +104,8 @@ object Battle { attacker.unit.action = null } - // we're a melee unit and we destroyed\captured an enemy unit // Should be called after tryCaptureUnit(), as that might spawn a unit on the tile we go to - if (!captureSuccess) + if (!captureMilitaryUnitSuccess) postBattleMoveToAttackedTile(attacker, defender, attackedTile) reduceAttackerMovementPointsAndAttacks(attacker, defender) @@ -401,12 +400,12 @@ object Battle { attackerCiv.addNotification("We have conquered the city of [${city.name}]!", city.location, NotificationIcon.War) + city.hasJustBeenConquered = true city.getCenterTile().apply { if (militaryUnit != null) militaryUnit!!.destroy() if (civilianUnit != null) captureCivilianUnit(attacker, MapUnitCombatant(civilianUnit!!)) for (airUnit in airUnits.toList()) airUnit.destroy() } - city.hasJustBeenConquered = true for (unique in attackerCiv.getMatchingUniques("Upon capturing a city, receive [] times its [] production as [] immediately")) { attackerCiv.addStat( diff --git a/core/src/com/unciv/logic/city/CityExpansionManager.kt b/core/src/com/unciv/logic/city/CityExpansionManager.kt index fb33610b8f1db..5aae756f30a16 100644 --- a/core/src/com/unciv/logic/city/CityExpansionManager.kt +++ b/core/src/com/unciv/logic/city/CityExpansionManager.kt @@ -8,7 +8,6 @@ import com.unciv.logic.map.TileInfo import com.unciv.ui.utils.withItem import com.unciv.ui.utils.withoutItem import kotlin.math.max -import kotlin.math.min import kotlin.math.pow import kotlin.math.roundToInt @@ -158,7 +157,7 @@ class CityExpansionManager { cityInfo.cityStats.update() for (unit in tileInfo.getUnits().toList()) // toListed because we're modifying - if (!unit.civInfo.canEnterTiles(cityInfo.civInfo)) + if (!unit.civInfo.canPassThroughTiles(cityInfo.civInfo)) unit.movement.teleportToClosestMoveableTile() cityInfo.civInfo.updateViewableTiles() diff --git a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt index c9aa0ad7541fe..af9821d2e251e 100644 --- a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt +++ b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt @@ -12,6 +12,7 @@ import com.unciv.logic.civilization.diplomacy.DiplomacyManager import com.unciv.logic.civilization.diplomacy.DiplomaticStatus import com.unciv.logic.map.MapUnit import com.unciv.logic.map.TileInfo +import com.unciv.logic.map.UnitMovementAlgorithms import com.unciv.logic.trade.TradeEvaluation import com.unciv.logic.trade.TradeRequest import com.unciv.models.Counter @@ -730,14 +731,25 @@ class CivilizationInfo { return greatPersonPoints } - fun canEnterTiles(otherCiv: CivilizationInfo): Boolean { + /** + * @returns whether units of this civilization can pass through the tiles owned by [otherCiv], + * considering only civ-wide filters. + * Use [TileInfo.canCivPassThrough] to check whether units of a civilization can pass through + * a specific tile, considering only civ-wide filters. + * Use [UnitMovementAlgorithms.canPassThrough] to check whether a specific unit can pass through + * a specific tile. + */ + fun canPassThroughTiles(otherCiv: CivilizationInfo): Boolean { if (otherCiv == this) return true if (otherCiv.isBarbarian()) return true if (nation.isBarbarian() && gameInfo.turns >= gameInfo.difficultyObject.turnBarbariansCanEnterPlayerTiles) return true val diplomacyManager = diplomacy[otherCiv.civName] - ?: return false // not encountered yet - return (diplomacyManager.hasOpenBorders || diplomacyManager.diplomaticStatus == DiplomaticStatus.War) + if (diplomacyManager != null && (diplomacyManager.hasOpenBorders || diplomacyManager.diplomaticStatus == DiplomaticStatus.War)) + return true + // Players can always pass through city-state tiles + if (isPlayerCivilization() && otherCiv.isCityState()) return true + return false } diff --git a/core/src/com/unciv/logic/map/MapUnit.kt b/core/src/com/unciv/logic/map/MapUnit.kt index 787bf89c9f7fa..8601c59e35f72 100644 --- a/core/src/com/unciv/logic/map/MapUnit.kt +++ b/core/src/com/unciv/logic/map/MapUnit.kt @@ -653,7 +653,7 @@ class MapUnit { action = null val tileOwner = getTile().getOwner() - if (tileOwner != null && !civInfo.canEnterTiles(tileOwner) && !tileOwner.isCityState()) // if an enemy city expanded onto this tile while I was in it + if (tileOwner != null && !civInfo.canPassThroughTiles(tileOwner) && !tileOwner.isCityState()) // if an enemy city expanded onto this tile while I was in it movement.teleportToClosestMoveableTile() } diff --git a/core/src/com/unciv/logic/map/TileInfo.kt b/core/src/com/unciv/logic/map/TileInfo.kt index ebd2431ec016e..4a06b9669baa4 100644 --- a/core/src/com/unciv/logic/map/TileInfo.kt +++ b/core/src/com/unciv/logic/map/TileInfo.kt @@ -565,15 +565,17 @@ open class TileInfo { fun isAdjacentToRiver() = neighbors.any { isConnectedByRiver(it) } - fun canCivEnter(civInfo: CivilizationInfo): Boolean { + /** + * @returns whether units of [civInfo] can pass through this tile, considering only civ-wide filters. + * Use [UnitMovementAlgorithms.canPassThrough] to check whether a specific unit can pass through a tile. + */ + fun canCivPassThrough(civInfo: CivilizationInfo): Boolean { val tileOwner = getOwner() - if (tileOwner == null || tileOwner == civInfo) return true // comparing the CivInfo objects is cheaper than comparing strings + if (tileOwner == null || tileOwner == civInfo) return true if (isCityCenter() && civInfo.isAtWarWith(tileOwner) && !getCity()!!.hasJustBeenConquered) return false - if (!civInfo.canEnterTiles(tileOwner) - && !(civInfo.isPlayerCivilization() && tileOwner.isCityState())) return false - // AIs won't enter city-state's border. + if (!civInfo.canPassThroughTiles(tileOwner)) return false return true } diff --git a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt index 5a939e0f3b422..4a2dda3447573 100644 --- a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt +++ b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt @@ -431,11 +431,14 @@ class UnitMovementAlgorithms(val unit:MapUnit) { if (!canPassThrough(tile)) return false - if (tile.isCityCenter() && tile.getOwner() != unit.civInfo) return false // even if they'll let us pass through, we can't enter their city + // even if they'll let us pass through, we can't enter their city - unless we just captured it + if (tile.isCityCenter() && tile.getOwner() != unit.civInfo && !tile.getCity()!!.hasJustBeenConquered) + return false - if (unit.isCivilian()) - return tile.civilianUnit == null && (tile.militaryUnit == null || tile.militaryUnit!!.owner == unit.owner) - else return tile.militaryUnit == null && (tile.civilianUnit == null || tile.civilianUnit!!.owner == unit.owner) + return if (unit.isCivilian()) + tile.civilianUnit == null && (tile.militaryUnit == null || tile.militaryUnit!!.owner == unit.owner) + else + tile.militaryUnit == null && (tile.civilianUnit == null || tile.civilianUnit!!.owner == unit.owner) } private fun canAirUnitMoveTo(tile: TileInfo, unit: MapUnit): Boolean { @@ -460,9 +463,15 @@ class UnitMovementAlgorithms(val unit:MapUnit) { return true } - // This is the most called function in the entire game, - // so multiple callees of this function have been optimized, - // because optimization on this function results in massive benefits! + /** + * @returns whether this unit can pass through [tile]. + * Note that sometimes, a tile can be passed through but not entered. Use [canMoveTo] to + * determine whether a unit can enter a tile. + * + * This is the most called function in the entire game, + * so multiple callees of this function have been optimized, + * because optimization on this function results in massive benefits! + */ fun canPassThrough(tile: TileInfo): Boolean { if (tile.isImpassible()) { // special exception - ice tiles are technically impassible, but some units can move through them anyway @@ -490,7 +499,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) { } if (tile.naturalWonder != null) return false - if (!tile.canCivEnter(unit.civInfo)) return false + if (!tile.canCivPassThrough(unit.civInfo)) return false val firstUnit = tile.getFirstUnit() if (firstUnit != null && firstUnit.civInfo != unit.civInfo && unit.civInfo.isAtWarWith(firstUnit.civInfo)) diff --git a/core/src/com/unciv/logic/trade/TradeLogic.kt b/core/src/com/unciv/logic/trade/TradeLogic.kt index 04183d50023e6..bb4aa8959a1cd 100644 --- a/core/src/com/unciv/logic/trade/TradeLogic.kt +++ b/core/src/com/unciv/logic/trade/TradeLogic.kt @@ -91,7 +91,7 @@ class TradeLogic(val ourCivilization:CivilizationInfo, val otherCivilization: Ci city.getCenterTile().getUnits().toList().forEach { it.movement.teleportToClosestMoveableTile() } for (tile in city.getTiles()) { for (unit in tile.getUnits().toList()) { - if (!unit.civInfo.canEnterTiles(to)) unit.movement.teleportToClosestMoveableTile() + if (!unit.civInfo.canPassThroughTiles(to)) unit.movement.teleportToClosestMoveableTile() } } to.updateViewableTiles()