Skip to content

Commit

Permalink
Fix units not entering cities upon capture (#4839)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
avdstaaij authored Aug 13, 2021
1 parent 2161790 commit 6a18a33
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 27 deletions.
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 @@ -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
}


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

0 comments on commit 6a18a33

Please sign in to comment.