From 8079a8dc7bbab3ff18509dd75c7104442e0c8f37 Mon Sep 17 00:00:00 2001 From: Xander Lenstra <71121390+xlenstra@users.noreply.github.com> Date: Mon, 6 Sep 2021 13:35:31 +0200 Subject: [PATCH] Fix bugs (#5103) * Fixed a bug where moving a unit through ancient ruins spawning another unit would duplicate units * Fixed a bug where automatically exploring units upgrading from ancient ruins would not upgrade * Fixed a bug where city state influence could go lower than the minimum --- .../unciv/logic/automation/UnitAutomation.kt | 15 ++++---- .../diplomacy/DiplomacyManager.kt | 2 +- core/src/com/unciv/logic/map/MapUnit.kt | 6 ++- .../unciv/logic/map/UnitMovementAlgorithms.kt | 37 +++++++++++++++---- 4 files changed, 43 insertions(+), 17 deletions(-) diff --git a/core/src/com/unciv/logic/automation/UnitAutomation.kt b/core/src/com/unciv/logic/automation/UnitAutomation.kt index cd7e0e2638..36a277f4b5 100644 --- a/core/src/com/unciv/logic/automation/UnitAutomation.kt +++ b/core/src/com/unciv/logic/automation/UnitAutomation.kt @@ -48,13 +48,13 @@ object UnitAutomation { if (!unit.civInfo.isMajorCiv()) return false // barbs don't have anything to do in ruins val unitDistanceToTiles = unit.movement.getDistanceToTiles() val tileWithRuinOrEncampment = unitDistanceToTiles.keys - .firstOrNull { - ( - (it.improvement != null && it.getTileImprovement()!!.isAncientRuinsEquivalent()) - || it.improvement == Constants.barbarianEncampment - ) - && unit.movement.canMoveTo(it) - } ?: return false + .firstOrNull { + ( + (it.improvement != null && it.getTileImprovement()!!.isAncientRuinsEquivalent()) + || it.improvement == Constants.barbarianEncampment + ) + && unit.movement.canMoveTo(it) + } ?: return false unit.movement.moveToTile(tileWithRuinOrEncampment) return true } @@ -492,6 +492,7 @@ object UnitAutomation { It also explores, but also has other functions, like healing if necessary. */ fun automatedExplore(unit: MapUnit) { if (tryGoToRuinAndEncampment(unit) && unit.currentMovement == 0f) return + if (unit.isDestroyed) return // Opening ruins _might_ have upgraded us to another unit if (unit.health < 80 && tryHealUnit(unit)) return if (tryExplore(unit)) return unit.civInfo.addNotification("[${unit.displayName()}] finished exploring.", unit.currentTile.position, unit.name, "OtherIcons/Sleep") diff --git a/core/src/com/unciv/logic/civilization/diplomacy/DiplomacyManager.kt b/core/src/com/unciv/logic/civilization/diplomacy/DiplomacyManager.kt index 43d6f284c0..5b2f88b389 100644 --- a/core/src/com/unciv/logic/civilization/diplomacy/DiplomacyManager.kt +++ b/core/src/com/unciv/logic/civilization/diplomacy/DiplomacyManager.kt @@ -597,7 +597,7 @@ class DiplomacyManager() { } otherCivDiplomacy.setModifier(DiplomaticModifiers.DeclaredWarOnUs, -20f) - if (otherCiv.isCityState()) otherCivDiplomacy.influence -= 60 + if (otherCiv.isCityState()) otherCivDiplomacy.setInfluence(-60f) for (thirdCiv in civInfo.getKnownCivs()) { if (thirdCiv.isAtWarWith(otherCiv)) { diff --git a/core/src/com/unciv/logic/map/MapUnit.kt b/core/src/com/unciv/logic/map/MapUnit.kt index 38423ced68..9988425f1e 100644 --- a/core/src/com/unciv/logic/map/MapUnit.kt +++ b/core/src/com/unciv/logic/map/MapUnit.kt @@ -35,6 +35,9 @@ class MapUnit { @Transient val movement = UnitMovementAlgorithms(this) + + @Transient + var isDestroyed = false // This is saved per each unit because if we need to recalculate viewable tiles every time a unit moves, // and we need to go over ALL the units, that's a lot of time spent on updating information we should already know! @@ -732,6 +735,7 @@ class MapUnit { currentTile.getUnits().filter { it.isTransported && isTransportTypeOf(it) } .toList() // because we're changing the list .forEach { unit -> unit.destroy() } + isDestroyed = true } fun gift(recipient: CivilizationInfo) { @@ -864,7 +868,7 @@ class MapUnit { return getMatchingUniques("Can carry [] [] units").any { mapUnit.matchesFilter(it.params[1]) } } - fun carryCapacity(unit: MapUnit): Int { + private fun carryCapacity(unit: MapUnit): Int { var capacity = getMatchingUniques("Can carry [] [] units").filter { unit.matchesFilter(it.params[1]) } .sumBy { it.params[0].toInt() } capacity += getMatchingUniques("Can carry [] extra [] units").filter { unit.matchesFilter(it.params[1]) } diff --git a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt index f678dadeb0..24ea931a39 100644 --- a/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt +++ b/core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt @@ -416,8 +416,21 @@ class UnitMovementAlgorithms(val unit:MapUnit) { // If this unit is a carrier, keep record of its air payload whereabouts. val origin = unit.getTile() - unit.removeFromTile() - unit.putInTile(lastReachableTile) + var needToFindNewRoute = false + for (tile in pathToLastReachableTile) { + if (!unit.movement.canPassThrough(tile)) { + // AAAH something happened making our previous path invalid + // Maybe we spawned a unit using ancient ruins, or our old route went through + // fog of war, and we found an obstacle halfway? + // Anyway: PANIC!! We stop this route, and after leaving the game in a valid state, + // we try again. + needToFindNewRoute = true + break + } + unit.removeFromTile() + unit.putInTile(tile) + if (unit.isDestroyed) break + } // 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 @@ -429,8 +442,8 @@ class UnitMovementAlgorithms(val unit:MapUnit) { // Unit maintenance changed if (unit.canGarrison() - && (origin.isCityCenter() || lastReachableTile.isCityCenter()) - && unit.civInfo.hasUnique("Units in cities cost no Maintenance") + && (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 @@ -439,10 +452,18 @@ class UnitMovementAlgorithms(val unit:MapUnit) { // 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. - for (tile in pathToLastReachableTile) { - unit.moveThroughTile(tile) - } - + + // 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) } /**