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

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 {
+            .firstOrNull {
-                    (
+                (
-                        (it.improvement != null && it.getTileImprovement()!!.isAncientRuinsEquivalent()) 
+                    (it.improvement != null && it.getTileImprovement()!!.isAncientRuinsEquivalent()) 
-                        || it.improvement == Constants.barbarianEncampment
+                    || it.improvement == Constants.barbarianEncampment
-                    )
+                )
-                    && unit.movement.canMoveTo(it)
+                && unit.movement.canMoveTo(it)
-                } ?: return false
+            } ?: 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")

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)) {

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]) }

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()
+        var needToFindNewRoute = false
-        unit.putInTile(lastReachableTile)
+        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())
+            && (origin.isCityCenter() || lastReachableTile.isCityCenter())
-                && unit.civInfo.hasUnique("Units in cities cost no Maintenance")
+            && 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)
     }
 
     /**