Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix units not entering cities upon capture #4839

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions core/src/com/unciv/logic/automation/NextTurnAutomation.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand Down Expand Up @@ -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) }
Expand Down
9 changes: 4 additions & 5 deletions core/src/com/unciv/logic/battle/Battle.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 1 addition & 2 deletions core/src/com/unciv/logic/city/CityExpansionManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down
18 changes: 15 additions & 3 deletions core/src/com/unciv/logic/civilization/CivilizationInfo.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -740,14 +741,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
}


Expand Down
2 changes: 1 addition & 1 deletion core/src/com/unciv/logic/map/MapUnit.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
12 changes: 7 additions & 5 deletions core/src/com/unciv/logic/map/TileInfo.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
25 changes: 17 additions & 8 deletions core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion core/src/com/unciv/logic/trade/TradeLogic.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down