Solved movement bug where extra tiles you could move through, but not *to*, would end up *adding* movememnt.

Movement bugs are hard. So hard, in fact, that most of this commit is just adding the "amount of movement left after attacking" so I could ensure that there wasn't a problem in the "calculating attackable tiles" part. The actual fix is in UnitMovementAlgorithms.

core/src/com/unciv/logic/automation/BattleHelper.kt

@@ -38,7 +38,7 @@ object BattleHelper {
             stayOnTile: Boolean = false
     ): ArrayList<AttackableTile> {
         val tilesWithEnemies = (tilesToCheck ?: unit.civInfo.viewableTiles)
-                .filter { containsAttackableEnemy(it, MapUnitCombatant(unit)) }
+            .filter { containsAttackableEnemy(it, MapUnitCombatant(unit)) }
 
         val rangeOfAttack = unit.getRange()
 
@@ -50,27 +50,36 @@ object BattleHelper {
         // Silly floats, basically
 
         val unitMustBeSetUp = unit.hasUnique("Must set up to ranged attack")
-        val tilesToAttackFrom = if (stayOnTile || unit.baseUnit.movesLikeAirUnits()) sequenceOf(unit.currentTile)
+        val tilesToAttackFrom = if (stayOnTile || unit.baseUnit.movesLikeAirUnits())
+            sequenceOf(Pair(unit.currentTile, unit.currentMovement))
         else
             unitDistanceToTiles.asSequence()
-                    .filter {
+                .map {
-                        val movementPointsToExpendAfterMovement = if (unitMustBeSetUp) 1 else 0
+                    val tile = it.key
-                        val movementPointsToExpendHere = if (unitMustBeSetUp && !unit.isSetUpForSiege()) 1 else 0
+                    val movementPointsToExpendAfterMovement = if (unitMustBeSetUp) 1 else 0
-                        val movementPointsToExpendBeforeAttack = if (it.key == unit.currentTile) movementPointsToExpendHere else movementPointsToExpendAfterMovement
+                    val movementPointsToExpendHere =
-                        unit.currentMovement - it.value.totalDistance - movementPointsToExpendBeforeAttack > 0.1
+                        if (unitMustBeSetUp && !unit.isSetUpForSiege()) 1 else 0
-                    } // still got leftover movement points after all that, to attack (0.1 is because of Float nonsense, see MapUnit.moveToTile(...)
+                    val movementPointsToExpendBeforeAttack =
-                    .map { it.key }
+                        if (it.key == unit.currentTile) movementPointsToExpendHere else movementPointsToExpendAfterMovement
-                    .filter { unit.movement.canMoveTo(it) || it == unit.getTile() }
+                    val movementLeft =
+                        unit.currentMovement - it.value.totalDistance - movementPointsToExpendBeforeAttack
+                    Pair(tile, movementLeft)
+                }
+                // still got leftover movement points after all that, to attack (0.1 is because of Float nonsense, see MapUnit.moveToTile(...)
+                .filter { it.second > 0.1f }
+                .filter {
+                    it.first == unit.getTile() || unit.movement.canMoveTo(it.first)
+                }
 
-        for (reachableTile in tilesToAttackFrom) {  // tiles we'll still have energy after we reach there
+        for ((reachableTile, movementLeft) in tilesToAttackFrom) {  // tiles we'll still have energy after we reach there
             val tilesInAttackRange =
-                    if (unit.hasUnique("Ranged attacks may be performed over obstacles") || unit.baseUnit.movesLikeAirUnits())
+                if (unit.hasUnique("Ranged attacks may be performed over obstacles") || unit.baseUnit.movesLikeAirUnits())
-                        reachableTile.getTilesInDistance(rangeOfAttack)
+                    reachableTile.getTilesInDistance(rangeOfAttack)
-                    else reachableTile.getViewableTilesList(rangeOfAttack)
+                else reachableTile.getViewableTilesList(rangeOfAttack)
-                            .asSequence()
+                    .asSequence()
 
             attackableTiles += tilesInAttackRange.filter { it in tilesWithEnemies }
-                    .map { AttackableTile(reachableTile, it) }
+                .map { AttackableTile(reachableTile, it, movementLeft) }
         }
         return attackableTiles
     }

core/src/com/unciv/logic/battle/Battle.kt

@@ -372,15 +372,13 @@ object Battle {
     }
 
     private fun postBattleAddXp(attacker: ICombatant, defender: ICombatant) {
-        if (attacker.isMelee()) {
+        if (!attacker.isMelee()) { // ranged attack
-            if (!defender.isCivilian()) // unit was not captured but actually attacked
-            {
-                addXp(attacker, 5, defender)
-                addXp(defender, 4, attacker)
-            }
-        } else { // ranged attack
             addXp(attacker, 2, defender)
             addXp(defender, 2, attacker)
+        } else if (!defender.isCivilian()) // unit was not captured but actually attacked
+        {
+            addXp(attacker, 5, defender)
+            addXp(defender, 4, attacker)
         }
     }
 

core/src/com/unciv/logic/map/UnitMovementAlgorithms.kt

@@ -470,15 +470,19 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
                 passingMovementSpent = 0f
             }
 
-
             previousTile = tile
-            
+
+            // We can't continue, stop here.
             if (unit.isDestroyed || unit.currentMovement - passingMovementSpent < Constants.minimumMovementEpsilon) {
-                unit.currentMovement = passingMovementSpent // silly floats which are "almost zero"
                 break
             }
         }
 
+        // Silly floats which are almost zero
+        if (unit.currentMovement < Constants.minimumMovementEpsilon)
+            unit.currentMovement = 0f
+
+
         if (!unit.isDestroyed)
             unit.putInTile(lastReachedEnterableTile)
 

core/src/com/unciv/models/AttackableTile.kt

@@ -2,4 +2,5 @@ package com.unciv.models
 
 import com.unciv.logic.map.TileInfo
 
-class AttackableTile(val tileToAttackFrom: TileInfo, val tileToAttack: TileInfo)
+class AttackableTile(val tileToAttackFrom: TileInfo, val tileToAttack: TileInfo,
+                     val movementLeftAfterMovingToAttackTile:Float)

core/src/com/unciv/models/ruleset/Ruleset.kt

@@ -588,7 +588,7 @@ class Ruleset {
                     lines.add("${promotion.name} requires promotion $prereq which does not exist!", RulesetErrorSeverity.Warning)
             for (unitType in promotion.unitTypes)
                 if (!unitTypes.containsKey(unitType) && (unitTypes.isNotEmpty() || !baseRuleset.unitTypes.containsKey(unitType)))
-                    lines.add("${promotion.name} references unit type ${unitType}, which does not exist!", RulesetErrorSeverity.Warning)
+                    lines.add("${promotion.name} references unit type $unitType, which does not exist!", RulesetErrorSeverity.Warning)
             checkUniques(promotion, lines, UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific)
         }
         for (unitType in unitTypes.values) {

core/src/com/unciv/ui/worldscreen/bottombar/BattleTable.kt

@@ -196,7 +196,7 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
             {
                 val canBombard = UnitAutomation.getBombardTargets(attacker.city).contains(defender.getTile())
                 if (canBombard) {
-                    attackableTile = AttackableTile(attacker.getTile(), defender.getTile())
+                    attackableTile = AttackableTile(attacker.getTile(), defender.getTile(), 0f)
                 }
             }
         }