From e2a1e44282d04cdfbc1f1d1bfd719ddd4acd0975 Mon Sep 17 00:00:00 2001 From: Xander Lenstra <71121390+xlenstra@users.noreply.github.com> Date: Wed, 8 Sep 2021 20:11:59 +0200 Subject: [PATCH] Fixed bugs with unit movement (#5126) * Fixed bugs with unit movement * Optimized function call * Cleaning up some code * Removed comments that are no longer applicable --- core/src/com/unciv/Constants.kt | 2 + .../automation/SpecificUnitAutomation.kt | 4 +- core/src/com/unciv/logic/map/MapUnit.kt | 19 ++++-- .../unciv/logic/map/UnitMovementAlgorithms.kt | 62 +++++++++---------- 4 files changed, 49 insertions(+), 38 deletions(-) diff --git a/core/src/com/unciv/Constants.kt b/core/src/com/unciv/Constants.kt index 07ca9658b6..725c289851 100644 --- a/core/src/com/unciv/Constants.kt +++ b/core/src/com/unciv/Constants.kt @@ -78,4 +78,6 @@ object Constants { const val rising = "Rising" const val lowering = "Lowering" + + const val minimumMovementEpsilon = 0.05 } diff --git a/core/src/com/unciv/logic/automation/SpecificUnitAutomation.kt b/core/src/com/unciv/logic/automation/SpecificUnitAutomation.kt index 41fe0fe6cc..951216c532 100644 --- a/core/src/com/unciv/logic/automation/SpecificUnitAutomation.kt +++ b/core/src/com/unciv/logic/automation/SpecificUnitAutomation.kt @@ -273,7 +273,7 @@ object SpecificUnitAutomation { val citiesByNearbyAirUnits = pathsToCities.keys .groupBy { key -> - key.getTilesInDistance(unit.getRange() * 2) + key.getTilesInDistance(unit.getMaxMovementForAirUnits()) .count { val firstAirUnit = it.airUnits.firstOrNull() firstAirUnit != null && firstAirUnit.civInfo.isAtWarWith(unit.civInfo) @@ -358,7 +358,7 @@ object SpecificUnitAutomation { private fun tryRelocateToCitiesWithEnemyNearBy(unit: MapUnit): Boolean { val immediatelyReachableCitiesAndCarriers = unit.currentTile - .getTilesInDistance(unit.getRange() * 2).filter { unit.movement.canMoveTo(it) } + .getTilesInDistance(unit.getMaxMovementForAirUnits()).filter { unit.movement.canMoveTo(it) } for (city in immediatelyReachableCitiesAndCarriers) { if (city.getTilesInDistance(unit.getRange()) diff --git a/core/src/com/unciv/logic/map/MapUnit.kt b/core/src/com/unciv/logic/map/MapUnit.kt index 6140aeed2e..2dbb17b818 100644 --- a/core/src/com/unciv/logic/map/MapUnit.kt +++ b/core/src/com/unciv/logic/map/MapUnit.kt @@ -359,6 +359,10 @@ class MapUnit { range += getMatchingUniques("[] Range").sumOf { it.params[0].toInt() } return range } + + fun getMaxMovementForAirUnits(): Int { + return getRange() * 2 + } fun isEmbarked(): Boolean { if (!baseUnit.isLandUnit()) return false @@ -815,15 +819,20 @@ class MapUnit { fun disband() { // evacuation of transported units before disbanding, if possible. toListed because we're modifying the unit list. - for (unit in currentTile.getUnits().filter { it.isTransported && isTransportTypeOf(it) } - .toList()) { + for (unit in currentTile.getUnits() + .filter { it.isTransported && isTransportTypeOf(it) } + .toList() + ) { // if we disbanded a unit carrying other units in a city, the carried units can still stay in the city - if (currentTile.isCityCenter() && unit.movement.canMoveTo(currentTile)) continue + if (currentTile.isCityCenter() && unit.movement.canMoveTo(currentTile)) { + unit.isTransported = false + continue + } // if no "fuel" to escape, should be disbanded as well - if (unit.currentMovement < 0.1) + if (unit.currentMovement < Constants.minimumMovementEpsilon) unit.disband() // let's find closest city or another carrier where it can be evacuated - val tileCanMoveTo = unit.currentTile.getTilesInDistance(unit.getRange() * 2) + val tileCanMoveTo = unit.currentTile.getTilesInDistance(unit.getMaxMovementForAirUnits()) .filterNot { it == currentTile }.firstOrNull { unit.movement.canMoveTo(it) } if (tileCanMoveTo != null) diff --git a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt index 24ea931a39..4aadd23c2a 100644 --- a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt +++ b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt @@ -274,7 +274,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) { fun canReachInCurrentTurn(destination: TileInfo): Boolean { if (unit.baseUnit.movesLikeAirUnits()) - return unit.currentTile.aerialDistanceTo(destination) <= unit.getRange()*2 + return unit.currentTile.aerialDistanceTo(destination) <= unit.getMaxMovementForAirUnits() if (unit.isPreparingParadrop()) return getDistance(unit.currentTile.position, destination.position) <= unit.paradropRange && canParadropOn(destination) return getDistanceToTiles().containsKey(destination) @@ -283,7 +283,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) { fun getReachableTilesInCurrentTurn(): Sequence { return when { unit.baseUnit.movesLikeAirUnits() -> - unit.getTile().getTilesInDistanceRange(IntRange(1, unit.getRange() * 2)) + unit.getTile().getTilesInDistanceRange(IntRange(1, unit.getMaxMovementForAirUnits())) unit.isPreparingParadrop() -> unit.getTile().getTilesInDistance(unit.paradropRange) .filter { unit.movement.canParadropOn(it) } @@ -407,16 +407,18 @@ class UnitMovementAlgorithms(val unit:MapUnit) { return val pathToLastReachableTile = distanceToTiles.getPathToTile(lastReachableTile) - if (!unit.civInfo.gameInfo.gameParameters.godMode) { - unit.currentMovement -= distanceToTiles[lastReachableTile]!!.totalDistance - if (unit.currentMovement < 0.1) unit.currentMovement = 0f // silly floats which are "almost zero" - } - if (unit.isFortified() || unit.isSetUpForSiege() || unit.isSleeping()) - unit.action = null // un-fortify/un-setup after moving + if (unit.isFortified() || unit.isSetUpForSiege() || unit.isSleeping() || unit.isAutomated()) + unit.action = null // un-fortify/un-setup/un-sleep/un-automate after moving // If this unit is a carrier, keep record of its air payload whereabouts. val origin = unit.getTile() var needToFindNewRoute = false + // Cache this in case something goes wrong + + var lastReachedEnterableTile = unit.getTile() + + unit.removeFromTile() + for (tile in pathToLastReachableTile) { if (!unit.movement.canPassThrough(tile)) { // AAAH something happened making our previous path invalid @@ -425,12 +427,28 @@ class UnitMovementAlgorithms(val unit:MapUnit) { // Anyway: PANIC!! We stop this route, and after leaving the game in a valid state, // we try again. needToFindNewRoute = true - break + break // If you ever remove this break, remove the `assumeCanPassThrough` param below } - unit.removeFromTile() - unit.putInTile(tile) + unit.moveThroughTile(tile) + + // In case something goes wrong, cache the last tile we were able to end on + // We can assume we can pass through this tile, as we would have broken earlier + if (unit.movement.canMoveTo(tile, assumeCanPassThrough = true)) { + lastReachedEnterableTile = tile + } + if (unit.isDestroyed) break } + + if (!unit.isDestroyed) + unit.putInTile(lastReachedEnterableTile) + + if (!unit.civInfo.gameInfo.gameParameters.godMode) { + unit.currentMovement -= distanceToTiles[lastReachedEnterableTile]!!.totalDistance + if (unit.currentMovement < Constants.minimumMovementEpsilon) + unit.currentMovement = 0f // silly floats which are "almost zero" + // const Epsilon, anyone? + } // The .toList() here is because we have a sequence that's running on the units in the tile, // then if we move one of the units we'll get a ConcurrentModificationException, se we save them all to a list @@ -445,24 +463,6 @@ class UnitMovementAlgorithms(val unit:MapUnit) { && (origin.isCityCenter() || lastReachableTile.isCityCenter()) && unit.civInfo.hasUnique("Units in cities cost no Maintenance") ) unit.civInfo.updateStatsForNextTurn() - - // Move through all intermediate tiles to get ancient ruins, barb encampments - // and to view tiles along the way - // We only activate the moveThroughTile AFTER the putInTile because of a really weird bug - - // If you're going to (or past) a ruin, and you activate the ruin bonus, and A UNIT spawns. - // That unit could now be blocking your entrance to the destination, so the putInTile would fail! =0 - // Instead, we move you to the destination directly, and only afterwards activate the various tiles on the way. - - // Actually, we will now stop doing that becasue of _another_ really weird bug (actually two) - // 1. Through some ancient ruins bonuses, we could upgrade our unit, effectively replacing it - // with another unit. However, doing so halfway through a movement would make it impossible - // to reach the last tile, as the new unit spawns with 0 movement points and not taking - // the old route again. Therefore, we might trigger barbarian encampments or ancient ruins - // at the destination field we in fact never reach - // 2. Which tile we reach might change during the route. Maybe our route used to go through - // fog of war, and it turns out there was an enemy city on the route we should take. - // As we can't go through cities, we need to find another route, and therefore this - // route should be impossible. if (needToFindNewRoute) moveToTile(destination, considerZoneOfControl) } @@ -494,11 +494,11 @@ class UnitMovementAlgorithms(val unit:MapUnit) { * Designates whether we can enter the tile - without attacking * DOES NOT designate whether we can reach that tile in the current turn */ - fun canMoveTo(tile: TileInfo): Boolean { + fun canMoveTo(tile: TileInfo, assumeCanPassThrough: Boolean = false): Boolean { if (unit.baseUnit.movesLikeAirUnits()) return canAirUnitMoveTo(tile, unit) - if (!canPassThrough(tile)) + if (!assumeCanPassThrough && !canPassThrough(tile)) return false // even if they'll let us pass through, we can't enter their city - unless we just captured it