Double movement unique parameterized (#5319)

* Double movement unique parameterized

* Double movement unique - all filters

android/assets/jsons/Civ V - Vanilla/UnitPromotions.json

@@ -132,9 +132,7 @@
 	{
 		"name": "Woodsman",
 		"prerequisites": ["Shock III","Drill III"],
-		"uniques": ["Double movement rate through Forest and Jungle"],
+		"uniques": ["Double movement in [Forest]","Double movement in [Jungle]"],
-		// This could be generalized: ["-[50]% movement costs through [Forest] tiles", "-[50]% movement costs through [Jungle] tiles"],
-		// but with how getMovementCostBetweenAdjacentTiles() is optimized, that's difficult to implement.
 		"unitTypes": ["Sword","Gunpowder"]
 	},
 	{

android/assets/jsons/Civ V - Vanilla/Units.json

@@ -1011,7 +1011,12 @@
 		"requiredTech": "Rifling",
 		"obsoleteTech": "Replaceable Parts",
 		"upgradesTo": "Great War Infantry",
-		"uniques": ["+[25]% Strength in [Snow]", "+[25]% Strength in [Tundra]", "+[25]% Strength in [Hill]", "Double movement in Snow, Tundra and Hills"],
+		"uniques": ["+[25]% Strength in [Snow]",
+			"+[25]% Strength in [Tundra]",
+			"+[25]% Strength in [Hill]",
+			"Double movement in [Snow]",
+			"Double movement in [Tundra]",
+			"Double movement in [Hill]"],
 		"attackSound": "shot"
 	},
 	{
@@ -1083,7 +1088,7 @@
 		"requiredResource": "Coal",
 		"upgradesTo": "Destroyer",
 		"obsoleteTech": "Combustion",
-		"uniques": ["+[33]% Strength vs [City]","Double movement in coast"],
+		"uniques": ["+[33]% Strength vs [City]","Double movement in [Coast]"],
 		"attackSound": "shipguns"
 	},
 	{

core/src/com/unciv/logic/civilization/CivilizationInfo.kt

@@ -335,6 +335,7 @@ class CivilizationInfo {
                 else city.getAllUniquesWithNonLocalEffects()
         }
 
+    fun hasUnique(uniqueType: UniqueType) = getMatchingUniques(uniqueType).any()
     fun hasUnique(unique: String) = getMatchingUniques(unique).any()
 
     /** Destined to replace getMatchingUniques, gradually, as we fill the enum */

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

@@ -11,6 +11,7 @@ import com.unciv.logic.civilization.LocationAction
 import com.unciv.logic.civilization.NotificationIcon
 import com.unciv.models.UnitActionType
 import com.unciv.models.ruleset.Ruleset
+import com.unciv.models.ruleset.tile.TerrainType
 import com.unciv.models.ruleset.unique.Unique
 import com.unciv.models.ruleset.tile.TileImprovement
 import com.unciv.models.ruleset.unique.UniqueType
@@ -51,27 +52,45 @@ class MapUnit {
     // which in turn is a component of getShortestPath and canReach
     @Transient
     var ignoresTerrainCost = false
+        private set
 
     @Transient
     var ignoresZoneOfControl = false
+        private set
 
     @Transient
     var allTilesCosts1 = false
+        private set
 
     @Transient
     var canPassThroughImpassableTiles = false
+        private set
 
     @Transient
     var roughTerrainPenalty = false
+        private set
 
+    /** If set causes an early exit in getMovementCostBetweenAdjacentTiles
+     *  - means no double movement uniques, roughTerrainPenalty or ignoreHillMovementCost */
     @Transient
-    var doubleMovementInCoast = false
+    var noTerrainMovementUniques = false
+        private set
 
+    /** If set causes a second early exit in getMovementCostBetweenAdjacentTiles */
     @Transient
-    var doubleMovementInForestAndJungle = false
+    var noBaseTerrainOrHillDoubleMovementUniques = false
+        private set
 
+    /** If set skips tile.matchesFilter tests for double movement in getMovementCostBetweenAdjacentTiles */
     @Transient
-    var doubleMovementInSnowTundraAndHills = false
+    var noFilteredDoubleMovementUniques = false
+        private set
+
+    /** Used for getMovementCostBetweenAdjacentTiles only, based on order of testing */
+    enum class DoubleMovementTerrainTarget { Feature, Base, Hill, Filter }
+    /** Mod-friendly cache of double-movement terrains */
+    @Transient
+    val doubleMovementInTerrain = HashMap<String, DoubleMovementTerrainTarget>()
 
     @Transient
     var canEnterIceTiles = false
@@ -91,6 +110,7 @@ class MapUnit {
     @Transient
     var hasUniqueToBuildImprovements = false    // not canBuildImprovements to avoid confusion
 
+    /** civName owning the unit */
     lateinit var owner: String
 
     /**
@@ -208,35 +228,75 @@ class MapUnit {
         tempUniques.asSequence().filter { it.placeholderText == placeholderText } + 
             civInfo.getMatchingUniques(placeholderText)
 
+    fun getMatchingUniques(uniqueType: UniqueType): Sequence<Unique> =
+        tempUniques.asSequence().filter { it.type == uniqueType } +
+            civInfo.getMatchingUniques(uniqueType)
+
     fun hasUnique(unique: String): Boolean {
-        return getUniques().any { it.placeholderText == unique } || civInfo.hasUnique(unique)
+        return tempUniques.any { it.placeholderText == unique } || civInfo.hasUnique(unique)
     }
 
-    fun updateUniques() {
+    fun hasUnique(uniqueType: UniqueType): Boolean {
+        return tempUniques.any { it.type == uniqueType } || civInfo.hasUnique(uniqueType)
+    }
+
+    fun updateUniques(ruleset: Ruleset) {
         val uniques = ArrayList<Unique>()
         val baseUnit = baseUnit()
         uniques.addAll(baseUnit.uniqueObjects)
         uniques.addAll(type.uniqueObjects)
 
-        for (promotion in promotions.promotions) {
+        for (promotion in promotions.getPromotions()) {
-            uniques.addAll(currentTile.tileMap.gameInfo.ruleSet.unitPromotions[promotion]!!.uniqueObjects)
+            uniques.addAll(promotion.uniqueObjects)
         }
 
         tempUniques = uniques
 
-        //todo: parameterize [terrainFilter] in 5 to 7 of the following:
+        allTilesCosts1 = hasUnique(UniqueType.AllTilesCost1Move)
+        canPassThroughImpassableTiles = hasUnique(UniqueType.CanPassImpassable)
+        ignoresTerrainCost = hasUnique(UniqueType.IgnoresTerrainCost)
+        ignoresZoneOfControl = hasUnique(UniqueType.IgnoresZOC)
+        roughTerrainPenalty = hasUnique(UniqueType.RoughTerrainPenalty)
+
+        doubleMovementInTerrain.clear()
+        // Cache the deprecated uniques
+        if (hasUnique(UniqueType.DoubleMovementCoast)) {
+            doubleMovementInTerrain[Constants.coast] = DoubleMovementTerrainTarget.Base
+        }
+        if (hasUnique(UniqueType.DoubleMovementForestJungle)) {
+            doubleMovementInTerrain[Constants.forest] = DoubleMovementTerrainTarget.Feature
+            doubleMovementInTerrain[Constants.jungle] = DoubleMovementTerrainTarget.Feature
+        }
+        if (hasUnique(UniqueType.DoubleMovementSnowTundraHill)) {
+            doubleMovementInTerrain[Constants.snow] = DoubleMovementTerrainTarget.Base
+            doubleMovementInTerrain[Constants.tundra] = DoubleMovementTerrainTarget.Base
+            doubleMovementInTerrain[Constants.hill] = DoubleMovementTerrainTarget.Feature
+        }
+        // Now the current unique
+        for (unique in getMatchingUniques(UniqueType.DoubleMovementOnTerrain)) {
+            val param = unique.params[0]
+            val terrain = ruleset.terrains[param]
+            doubleMovementInTerrain[param] = when {
+                terrain == null -> DoubleMovementTerrainTarget.Filter
+                terrain.name == Constants.hill -> DoubleMovementTerrainTarget.Hill
+                terrain.type == TerrainType.TerrainFeature -> DoubleMovementTerrainTarget.Feature
+                terrain.type.isBaseTerrain -> DoubleMovementTerrainTarget.Base
+                else -> DoubleMovementTerrainTarget.Filter
+            }
+        }
+        // Init shortcut flags
+        noTerrainMovementUniques = doubleMovementInTerrain.isEmpty() &&
+            !roughTerrainPenalty && !civInfo.nation.ignoreHillMovementCost
+        noBaseTerrainOrHillDoubleMovementUniques = doubleMovementInTerrain
+            .none { it.value != DoubleMovementTerrainTarget.Feature }
+        noFilteredDoubleMovementUniques = doubleMovementInTerrain
+            .none { it.value == DoubleMovementTerrainTarget.Filter }
+
+        //todo: consider parameterizing [terrainFilter] in some of the following:
+        canEnterIceTiles = hasUnique(UniqueType.CanEnterIceTiles)
+        cannotEnterOceanTiles = hasUnique(UniqueType.CannotEnterOcean)
+        cannotEnterOceanTilesUntilAstronomy = hasUnique(UniqueType.CannotEnterOceanUntilAstronomy)
 
-        allTilesCosts1 = hasUnique("All tiles cost 1 movement")
-        canPassThroughImpassableTiles = hasUnique("Can pass through impassable tiles")
-        ignoresTerrainCost = hasUnique("Ignores terrain cost")
-        ignoresZoneOfControl = hasUnique("Ignores Zone of Control")
-        roughTerrainPenalty = hasUnique("Rough terrain penalty")
-        doubleMovementInCoast = hasUnique("Double movement in coast")
-        doubleMovementInForestAndJungle = hasUnique("Double movement rate through Forest and Jungle")
-        doubleMovementInSnowTundraAndHills = hasUnique("Double movement in Snow, Tundra and Hills")
-        canEnterIceTiles = hasUnique("Can enter ice tiles")
-        cannotEnterOceanTiles = hasUnique("Cannot enter ocean tiles")
-        cannotEnterOceanTilesUntilAstronomy = hasUnique("Cannot enter ocean tiles until Astronomy")
         hasUniqueToBuildImprovements = hasUnique(Constants.canBuildImprovements)
         canEnterForeignTerrain =
             hasUnique("May enter foreign tiles without open borders, but loses [] religious strength each turn it ends there")
@@ -255,7 +315,7 @@ class MapUnit {
 
         newUnit.promotions = promotions.clone()
 
-        newUnit.updateUniques()
+        newUnit.updateUniques(civInfo.gameInfo.ruleSet)
         newUnit.updateVisibleTiles()
     }
 
@@ -454,7 +514,7 @@ class MapUnit {
         baseUnit = ruleset.units[name]
             ?: throw java.lang.Exception("Unit $name is not found!")
 
-        updateUniques()
+        updateUniques(ruleset)
     }
 
     fun useMovementPoints(amount: Float) {
@@ -988,7 +1048,7 @@ class MapUnit {
         return getMatchingUniques("Can [] [] times").any { it.params[0] == action }
     }
 
-    /** For the actual value, check the member variable `maxAbilityUses`
+    /** For the actual value, check the member variable [maxAbilityUses]
      */
     fun getBaseMaxActionUses(action: String): Int {
         return getMatchingUniques("Can [] [] times")

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

@@ -8,11 +8,16 @@ import com.unciv.logic.civilization.CivilizationInfo
 class UnitMovementAlgorithms(val unit:MapUnit) {
 
     // This function is called ALL THE TIME and should be as time-optimal as possible!
-    fun getMovementCostBetweenAdjacentTiles(from: TileInfo, to: TileInfo, civInfo: CivilizationInfo, considerZoneOfControl: Boolean = true): Float {
+    private fun getMovementCostBetweenAdjacentTiles(
+        from: TileInfo,
+        to: TileInfo,
+        civInfo: CivilizationInfo,
+        considerZoneOfControl: Boolean = true
+    ): Float {
 
         if (from.isLand != to.isLand && unit.baseUnit.isLandUnit())
-            if (unit.civInfo.nation.disembarkCosts1 && from.isWater && to.isLand) return 1f
+            return if (unit.civInfo.nation.disembarkCosts1 && from.isWater && to.isLand) 1f
-            else return 100f // this is embarkment or disembarkment, and will take the entire turn
+            else 100f // this is embarkment or disembarkment, and will take the entire turn
 
         // If the movement is affected by a Zone of Control, all movement points are expended
         if (considerZoneOfControl && isMovementAffectedByZoneOfControl(from, to, civInfo))
@@ -22,11 +27,13 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
         if (unit.allTilesCosts1)
             return 1f
 
-        var extraCost = 0f
-
         val toOwner = to.getOwner()
-        if (toOwner != null && to.isLand && toOwner.hasActiveGreatWall && civInfo.isAtWarWith(toOwner))
+        val extraCost = if (
-            extraCost += 1
+            toOwner != null &&
+            to.isLand &&
+            toOwner.hasActiveGreatWall &&
+            civInfo.isAtWarWith(toOwner)
+        ) 1f else 0f
 
         if (from.roadStatus == RoadStatus.Railroad && to.roadStatus == RoadStatus.Railroad)
             return RoadStatus.Railroad.movement + extraCost
@@ -40,26 +47,38 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
         if (unit.ignoresTerrainCost) return 1f + extraCost
         if (areConnectedByRiver) return 100f  // Rivers take the entire turn to cross
 
-        if (unit.doubleMovementInForestAndJungle &&
+        val terrainCost = to.getLastTerrain().movementCost.toFloat()
-                (to.terrainFeatures.contains(Constants.forest) || to.terrainFeatures.contains(Constants.jungle)))
+
-            return 1f + extraCost // usually forest and jungle take 2 movements, so here it is 1
+        if (unit.noTerrainMovementUniques)
+            return terrainCost + extraCost
+
+        if (to.terrainFeatures.any { unit.doubleMovementInTerrain[it] == MapUnit.DoubleMovementTerrainTarget.Feature })
+            return terrainCost * 0.5f + extraCost
 
         if (unit.roughTerrainPenalty && to.isRoughTerrain())
-            return 100f // units that have to sped all movement in rough terrain, have to spend all movement in rough terrain
+            return 100f // units that have to spend all movement in rough terrain, have to spend all movement in rough terrain
         // Placement of this 'if' based on testing, see #4232
 
         if (civInfo.nation.ignoreHillMovementCost && to.isHill())
             return 1f + extraCost // usually hills take 2 movements, so here it is 1
 
-        if (unit.doubleMovementInCoast && to.baseTerrain == Constants.coast)
+        if (unit.noBaseTerrainOrHillDoubleMovementUniques)
-            return 1 / 2f + extraCost
+            return terrainCost + extraCost
 
-        if (unit.doubleMovementInSnowTundraAndHills && to.isHill())
+        if (unit.doubleMovementInTerrain[to.baseTerrain] == MapUnit.DoubleMovementTerrainTarget.Base)
-            return 1f + extraCost // usually hills take 2
+            return terrainCost * 0.5f + extraCost
-        if (unit.doubleMovementInSnowTundraAndHills && (to.baseTerrain == Constants.snow || to.baseTerrain == Constants.tundra))
+        if (unit.doubleMovementInTerrain[Constants.hill] == MapUnit.DoubleMovementTerrainTarget.Hill && to.isHill())
-            return 1 / 2f + extraCost
+            return terrainCost * 0.5f + extraCost
 
-        return to.getLastTerrain().movementCost.toFloat() + extraCost // no road
+        if (unit.noFilteredDoubleMovementUniques)
+            return terrainCost + extraCost
+        if (unit.doubleMovementInTerrain.any { 
+            it.value == MapUnit.DoubleMovementTerrainTarget.Filter &&
+                to.matchesFilter(it.key)
+        })
+            return terrainCost * 0.5f + extraCost
+
+        return terrainCost + extraCost // no road or other movement cost reduction
     }
 
     /** Returns whether the movement between the adjacent tiles [from] and [to] is affected by Zone of Control */
@@ -130,20 +149,18 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
             val updatedTiles = ArrayList<TileInfo>()
             for (tileToCheck in tilesToCheck)
                 for (neighbor in tileToCheck.neighbors) {
-                    var totalDistanceToTile: Float
+                    var totalDistanceToTile: Float = if (unit.civInfo.exploredTiles.contains(neighbor.position)) {
-
-                    if (unit.civInfo.exploredTiles.contains(neighbor.position)) {
                         if (!canPassThrough(neighbor))
-                            totalDistanceToTile = unitMovement // Can't go here.
+                            unitMovement // Can't go here.
                         // The reason that we don't just "return" is so that when calculating how to reach an enemy,
                         // You need to assume his tile is reachable, otherwise all movement algorithms on reaching enemy
                         // cities and units goes kaput.
 
                         else {
                             val distanceBetweenTiles = getMovementCostBetweenAdjacentTiles(tileToCheck, neighbor, unit.civInfo, considerZoneOfControl)
-                            totalDistanceToTile = distanceToTiles[tileToCheck]!!.totalDistance + distanceBetweenTiles
+                            distanceToTiles[tileToCheck]!!.totalDistance + distanceBetweenTiles
                         }
-                    } else totalDistanceToTile = distanceToTiles[tileToCheck]!!.totalDistance + 1f // If we don't know then we just guess it to be 1.
+                    } else distanceToTiles[tileToCheck]!!.totalDistance + 1f // If we don't know then we just guess it to be 1.
 
                     if (!distanceToTiles.containsKey(neighbor) || distanceToTiles[neighbor]!!.totalDistance > totalDistanceToTile) { // this is the new best path
                         if (totalDistanceToTile < unitMovement)  // We can still keep moving from here!
@@ -281,7 +298,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
         return getShortestPath(destination).any()
     }
 
-    fun canReachInCurrentTurn(destination: TileInfo): Boolean {
+    private fun canReachInCurrentTurn(destination: TileInfo): Boolean {
         if (unit.baseUnit.movesLikeAirUnits())
             return unit.currentTile.aerialDistanceTo(destination) <= unit.getMaxMovementForAirUnits()
         if (unit.isPreparingParadrop())
@@ -412,8 +429,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
         val pathToDestination = distanceToTiles.getPathToTile(destination)
         val movableTiles = pathToDestination.takeWhile { canPassThrough(it) }
         val lastReachableTile = movableTiles.lastOrNull { canMoveTo(it) }
-        if (lastReachableTile == null) // no tiles can pass though/can move to
+            ?: return  // no tiles can pass though/can move to
-            return
         val pathToLastReachableTile = distanceToTiles.getPathToTile(lastReachableTile)
 
         if (unit.isFortified() || unit.isSetUpForSiege() || unit.isSleeping())

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

@@ -11,20 +11,45 @@ class UnitPromotions {
     @Transient 
     private lateinit var unit: MapUnit
 
+    /** Experience this unit has accumulated on top of the last promotion */
     @Suppress("PropertyName")
     var XP = 0
 
+    /** The _names_ of the promotions this unit has acquired - see [getPromotions] for object access */
     var promotions = HashSet<String>()
-    // The number of times this unit has been promoted
+        private set
+
     // some promotions don't come from being promoted but from other things,
     // like from being constructed in a specific city etc.
+    /** The number of times this unit has been promoted using experience, not counting free promotions */
     var numberOfPromotions = 0
 
+    /** Gets this unit's promotions as objects.
+     *  @param sorted if `true` return the promotions in json order (`false` gives hashset order) for display.
+     *  @return a Sequence of this unit's promotions
+     */
+    fun getPromotions(sorted: Boolean = false): Sequence<Promotion> = sequence {
+        if (promotions.isEmpty()) return@sequence
+        val unitPromotions = unit.civInfo.gameInfo.ruleSet.unitPromotions
+        if (sorted && promotions.size > 1) {
+            for (promotion in unitPromotions.values)
+                if (promotion.name in promotions) yield(promotion)
+        } else {
+            for (name in promotions)
+                yield(unitPromotions[name] ?: continue)
+        }
+    }
+
     fun setTransients(unit: MapUnit) {
         this.unit = unit
     }
 
-    fun xpForNextPromotion() = (numberOfPromotions+1)*10
+    /** @return the XP points needed to "buy" the next promotion. 10, 30, 60, 100, 150,... */
+    fun xpForNextPromotion() = (numberOfPromotions + 1) * 10
+
+    /** @return Total XP including that already "spent" on promotions */
+    fun totalXpProduced() = XP + (numberOfPromotions * (numberOfPromotions + 1)) * 5
+
     fun canBePromoted(): Boolean {
         if (XP < xpForNextPromotion()) return false
         if (getAvailablePromotions().none()) return false
@@ -37,13 +62,14 @@ class UnitPromotions {
             numberOfPromotions++
         }
 
-        val promotion = unit.civInfo.gameInfo.ruleSet.unitPromotions[promotionName]!!
+        val ruleset = unit.civInfo.gameInfo.ruleSet
+        val promotion = ruleset.unitPromotions[promotionName]!!
         doDirectPromotionEffects(promotion)
 
         if (promotion.uniqueObjects.none { it.placeholderText == "Doing so will consume this opportunity to choose a Promotion" })
             promotions.add(promotionName)
 
-        unit.updateUniques()
+        unit.updateUniques(ruleset)
 
         // Since some units get promotions upon construction, they will get the addPromotion from the unit.postBuildEvent
         // upon creation, BEFORE they are assigned to a tile, so the updateVisibleTiles() would crash.
@@ -51,12 +77,15 @@ class UnitPromotions {
         unit.updateVisibleTiles()  // some promotions/uniques give the unit bonus sight
     }
 
-    fun doDirectPromotionEffects(promotion: Promotion) {
+    private fun doDirectPromotionEffects(promotion: Promotion) {
         for (unique in promotion.uniqueObjects) {
             UniqueTriggerActivation.triggerUnitwideUnique(unique, unit)
         }
     }
 
+    /** Gets all promotions this unit could currently "buy" with enough [XP]
+     *  Checks unit type, already acquired promotions, prerequisites and incompatibility uniques.
+     */
     fun getAvailablePromotions(): Sequence<Promotion> {
         return unit.civInfo.gameInfo.ruleSet.unitPromotions.values
             .asSequence()
@@ -78,10 +107,4 @@ class UnitPromotions {
         toReturn.unit = unit
         return toReturn
     }
-
-    fun totalXpProduced(): Int {
-        var sum = XP
-        for(i in 1..numberOfPromotions) sum += 10*i
-        return sum
-    }
 }

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

@@ -22,5 +22,3 @@ interface IHasUniques {
     fun hasUnique(uniqueTemplate: String) = uniqueObjects.any { it.placeholderText == uniqueTemplate }
     fun hasUnique(uniqueType: UniqueType) = uniqueObjects.any { it.isOfType(uniqueType) }
 }
-
-

core/src/com/unciv/models/ruleset/unique/UniqueType.kt

@@ -113,6 +113,23 @@ enum class UniqueType(val text:String, vararg targets: UniqueTarget) {
 
     TerrainGrantsPromotion("Grants [promotion] ([comment]) to adjacent [mapUnitFilter] units for the rest of the game", UniqueTarget.Terrain),
 
+    // The following block gets cached in MapUnit for faster getMovementCostBetweenAdjacentTiles
+    DoubleMovementOnTerrain("Double movement in [terrainFilter]", UniqueTarget.Unit),
+    @Deprecated("As of 3.17.1", ReplaceWith("Double movement in [terrainFilter]"), DeprecationLevel.WARNING)
+    DoubleMovementCoast("Double movement in coast", UniqueTarget.Unit),
+    @Deprecated("As of 3.17.1", ReplaceWith("Double movement in [terrainFilter]"), DeprecationLevel.WARNING)
+    DoubleMovementForestJungle("Double movement rate through Forest and Jungle", UniqueTarget.Unit),
+    @Deprecated("As of 3.17.1", ReplaceWith("Double movement in [terrainFilter]"), DeprecationLevel.WARNING)
+    DoubleMovementSnowTundraHill("Double movement in Snow, Tundra and Hills", UniqueTarget.Unit),
+    AllTilesCost1Move("All tiles cost 1 movement", UniqueTarget.Unit),
+    CanPassImpassable("Can pass through impassable tiles", UniqueTarget.Unit),
+    IgnoresTerrainCost("Ignores terrain cost", UniqueTarget.Unit),
+    IgnoresZOC("Ignores Zone of Control", UniqueTarget.Unit),
+    RoughTerrainPenalty("Rough terrain penalty", UniqueTarget.Unit),
+    CanEnterIceTiles("Can enter ice tiles", UniqueTarget.Unit),
+    CannotEnterOcean("Cannot enter ocean tiles", UniqueTarget.Unit),
+    CannotEnterOceanUntilAstronomy("Cannot enter ocean tiles until Astronomy", UniqueTarget.Unit),
+
 
     ///// CONDITIONALS
 

core/src/com/unciv/ui/mapeditor/MapEditorOptionsTable.kt

@@ -201,7 +201,7 @@ class MapEditorOptionsTable(val mapEditorScreen: MapEditorScreen): Table(CameraS
                 unit.name = currentUnit.name
                 unit.owner = currentNation.name
                 unit.civInfo = CivilizationInfo(currentNation.name).apply { nation = currentNation } // needed for the unit icon to render correctly
-                unit.updateUniques()
+                unit.updateUniques(ruleset)
                 if (unit.movement.canMoveTo(it)) {
                     when {
                         unit.baseUnit.movesLikeAirUnits() -> {

core/src/com/unciv/ui/overviewscreen/UnitOverviewTable.kt

@@ -101,10 +101,8 @@ class UnitOverviewTable(
                     unit.getTile().getTilesInDistance(3).firstOrNull { it.isCityCenter() }
                 if (closestCity != null) add(closestCity.getCity()!!.name.tr()) else add()
                 val promotionsTable = Table()
-                val promotionsForUnit = unit.civInfo.gameInfo.ruleSet.unitPromotions.values.filter {
+                // getPromotions goes by json order on demand, so this is same sorting as on picker
-                    unit.promotions.promotions.contains(it.name)
+                for (promotion in unit.promotions.getPromotions(true))
-                }     // force same sorting as on picker (.sorted() would be simpler code, but...)
-                for (promotion in promotionsForUnit)
                     promotionsTable.add(ImageGetter.getPromotionIcon(promotion.name))
                 if (unit.promotions.canBePromoted()) promotionsTable.add(
                     ImageGetter.getImage("OtherIcons/Star").apply { color = Color.GOLDENROD })

core/src/com/unciv/ui/worldscreen/unit/UnitTable.kt

@@ -216,8 +216,8 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
             if (selectedUnits.size == 1) { // single selected unit
                 unitIconHolder.add(UnitGroup(selectedUnit!!, 30f)).pad(5f)
 
-                for (promotion in selectedUnit!!.promotions.promotions.sorted())
+                for (promotion in selectedUnit!!.promotions.getPromotions(true))
-                    promotionsTable.add(ImageGetter.getPromotionIcon(promotion))
+                    promotionsTable.add(ImageGetter.getPromotionIcon(promotion.name))
 
                 // Since Clear also clears the listeners, we need to re-add it every time
                 promotionsTable.onClick {

tests/src/com/unciv/logic/map/UnitMovementAlgorithmsTests.kt

@@ -118,7 +118,7 @@ class UnitMovementAlgorithmsTests {
 
         for (type in ruleSet.unitTypes) {
             unit.baseUnit = BaseUnit().apply { unitType = type.key; ruleset = ruleSet }
-            unit.updateUniques()
+            unit.updateUniques(ruleSet)
 
             Assert.assertTrue(
                 "$type cannot be in Ice",
@@ -190,7 +190,7 @@ class UnitMovementAlgorithmsTests {
                 if (this.isRanged())
                     uniques.add("Cannot enter ocean tiles until Astronomy")
             }
-            unit.updateUniques()
+            unit.updateUniques(ruleSet)
 
             Assert.assertTrue("$type cannot be in Ocean",
                     (unit.baseUnit.isMelee()) != unit.movement.canPassThrough(tile))