Unit action constants and worker unique cleanup (#4815)

* Unit action constants and worker unique cleanup

* Replace unit action constants with enum properties - patch1

* Replace unit action constants with enum properties - patch2

core/src/com/unciv/Constants.kt

@@ -3,9 +3,9 @@ package com.unciv
 object Constants {
     const val worker = "Worker"
     const val canBuildImprovements = "Can build [] improvements on tiles"
-    // Deprecated as of 3.15.5
+    @Deprecated("as of 3.15.5")
         const val workerUnique = "Can build improvements on tiles"
-    //
+    const val workBoatsUnique = "May create improvements on water resources"
     const val settler = "Settler"
     const val settlerUnique = "Founds a new city"
     const val eraSpecificUnit = "Era Starting Unit"
@@ -55,13 +55,6 @@ object Constants {
     const val citadel = "Citadel"
     const val tradingPost = "Trading post"
 
-    const val unitActionSetUp = "Set Up"
-    const val unitActionSleep = "Sleep"
-    const val unitActionSleepUntilHealed = "Sleep until healed"
-    const val unitActionAutomation = "Automate"
-    const val unitActionExplore = "Explore"
-    const val unitActionParadrop = "Paradrop"
-
     const val futureTech = "Future Tech"
     // Easter egg name. Hopefully is to hopefully avoid conflicts when later players can name their own religions.
     // This religion name should never be displayed.

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

@@ -1,6 +1,5 @@
 package com.unciv.logic.automation
 
-import com.unciv.Constants
 import com.unciv.logic.battle.Battle
 import com.unciv.logic.battle.BattleDamage
 import com.unciv.logic.battle.ICombatant
@@ -54,7 +53,7 @@ object BattleHelper {
             unitDistanceToTiles.asSequence()
                     .filter {
                         val movementPointsToExpendAfterMovement = if (unitMustBeSetUp) 1 else 0
-                        val movementPointsToExpendHere = if (unitMustBeSetUp && unit.action != Constants.unitActionSetUp) 1 else 0
+                        val movementPointsToExpendHere = if (unitMustBeSetUp && !unit.isSetUpForSiege()) 1 else 0
                         val movementPointsToExpendBeforeAttack = if (it.key == unit.currentTile) movementPointsToExpendHere else movementPointsToExpendAfterMovement
                         unit.currentMovement - it.value.totalDistance - movementPointsToExpendBeforeAttack > 0.1
                     } // still got leftover movement points after all that, to attack (0.1 is because of Float nonsense, see MapUnit.moveToTile(...)

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

@@ -27,8 +27,7 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
 
     val civUnits = civInfo.getCivUnits()
     val militaryUnits = civUnits.count { it.baseUnit.isMilitary() }
-    // Constants.workerUnique deprecated since 3.15.5
+    val workers = civUnits.count { it.hasUniqueToBuildImprovements && it.isCivilian() }.toFloat()
-    val workers = civUnits.count { (it.hasUnique(Constants.canBuildImprovements) || it.hasUnique(Constants.workerUnique)) && it.isCivilian() }.toFloat()
     val cities = civInfo.cities.size
     val allTechsAreResearched = civInfo.tech.getNumberOfTechsResearched() >= civInfo.gameInfo.ruleSet.technologies.size
 
@@ -116,9 +115,9 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
 
     private fun addWorkBoatChoice() {
         val buildableWorkboatUnits = cityInfo.cityConstructions.getConstructableUnits()
-                .filter { it.uniques.contains("May create improvements on water resources") }
+                .filter { it.uniques.contains(Constants.workBoatsUnique) }
         val canBuildWorkboat = buildableWorkboatUnits.any()
-                && !cityInfo.getTiles().any { it.civilianUnit?.hasUnique("May create improvements on water resources") == true }
+                && !cityInfo.getTiles().any { it.civilianUnit?.hasUnique(Constants.workBoatsUnique) == true }
         if (!canBuildWorkboat) return
         val tilesThatNeedWorkboat = cityInfo.getTiles()
                 .filter { it.isWater && it.hasViewableResource(civInfo) && it.improvement == null }.toList()
@@ -144,13 +143,11 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
                         unique -> unique.equalsPlaceholderText(Constants.canBuildImprovements) || unique.equalsPlaceholderText(Constants.workerUnique) 
                 } && it.isBuildable(cityConstructions) }
         if (workerEquivalents.isEmpty()) return // for mods with no worker units
-        // Constants.workerUnique deprecated since 3.15.5
+        if (civInfo.getIdleUnits().any { it.isAutomated() && it.hasUniqueToBuildImprovements })
-        if (civInfo.getIdleUnits().any { it.action == Constants.unitActionAutomation && (it.hasUnique(Constants.canBuildImprovements) || it.hasUnique(Constants.workerUnique)) })
             return // If we have automated workers who have no work to do then it's silly to construct new workers.
 
         val citiesCountedTowardsWorkers = min(5, cities) // above 5 cities, extra cities won't make us want more workers
-        // Constants.workerUnique deprecated since 3.15.5
+        if (workers < citiesCountedTowardsWorkers * 0.6f && civUnits.none { it.hasUniqueToBuildImprovements && it.isIdle() }) {
-        if (workers < citiesCountedTowardsWorkers * 0.6f && civUnits.none { (it.hasUnique(Constants.canBuildImprovements) || it.hasUnique(Constants.workerUnique)) && it.isIdle() }) {
             var modifier = citiesCountedTowardsWorkers / (workers + 0.1f)
             if (!cityIsOverAverageProduction) modifier /= 5 // higher production cities will deal with this
             addChoice(relativeCostEffectiveness, workerEquivalents.minByOrNull { it.cost }!!.name, modifier)

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

@@ -92,11 +92,10 @@ object UnitAutomation {
             if (unit.hasUnique(Constants.settlerUnique))
                 return SpecificUnitAutomation.automateSettlerActions(unit)
 
-            // Constants.workerUnique deprecated since 3.15.5
+            if (unit.hasUniqueToBuildImprovements)
-            if (unit.hasUnique(Constants.canBuildImprovements) || unit.hasUnique(Constants.workerUnique))
                 return WorkerAutomation(unit).automateWorkerAction()
 
-            if (unit.name == "Work Boats") // This is really not modular
+            if (unit.hasUnique(Constants.workBoatsUnique))
                 return SpecificUnitAutomation.automateWorkBoats(unit)
 
             if (unit.hasUnique("Bonus for units in 2 tile radius 15%"))

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

@@ -9,6 +9,7 @@ import com.unciv.logic.civilization.diplomacy.DiplomaticStatus
 import com.unciv.logic.map.RoadStatus
 import com.unciv.logic.map.TileInfo
 import com.unciv.models.AttackableTile
+import com.unciv.models.UnitActionType
 import com.unciv.models.ruleset.Unique
 import com.unciv.models.stats.Stat
 import com.unciv.models.stats.Stats
@@ -33,8 +34,8 @@ object Battle {
              * but the hidden tile is actually IMPASSIBLE so you stop halfway!
              */
             if (attacker.getTile() != attackableTile.tileToAttackFrom) return
-            if (attacker.unit.hasUnique("Must set up to ranged attack") && attacker.unit.action != Constants.unitActionSetUp) {
+            if (attacker.unit.hasUnique("Must set up to ranged attack") && !attacker.unit.isSetUpForSiege()) {
-                attacker.unit.action = Constants.unitActionSetUp
+                attacker.unit.action = UnitActionType.SetUp.value
                 attacker.unit.useMovementPoints(1f)
             }
         }
@@ -84,7 +85,7 @@ object Battle {
             conquerCity(defender.city, attacker)
 
         // Exploring units surviving an attack should "wake up"
-        if (!defender.isDefeated() && defender is MapUnitCombatant && defender.unit.action == Constants.unitActionExplore)
+        if (!defender.isDefeated() && defender is MapUnitCombatant && defender.unit.isExploring())
             defender.unit.action = null
 
         // Add culture when defeating a barbarian when Honor policy is adopted, gold from enemy killed when honor is complete
@@ -407,7 +408,7 @@ object Battle {
         city.hasJustBeenConquered = true
         city.getCenterTile().apply {
             if (militaryUnit != null) militaryUnit!!.destroy()
-            if (civilianUnit != null) captureCivilianUnit(attacker, MapUnitCombatant(civilianUnit!!))
+            if (civilianUnit != null) captureCivilianUnit(attacker, MapUnitCombatant(civilianUnit!!), checkDefeat = false)
             for (airUnit in airUnits.toList()) airUnit.destroy()
         }
 
@@ -442,7 +443,7 @@ object Battle {
         return null
     }
 
-    private fun captureCivilianUnit(attacker: ICombatant, defender: MapUnitCombatant) {
+    private fun captureCivilianUnit(attacker: ICombatant, defender: MapUnitCombatant, checkDefeat: Boolean = true) {
         // barbarians don't capture civilians
         if (attacker.getCivInfo().isBarbarian()
                 || defender.unit.hasUnique("Uncapturable")) {
@@ -477,7 +478,8 @@ object Battle {
             }
         }
 
-        destroyIfDefeated(defenderCiv, attacker.getCivInfo())
+        if (checkDefeat)
+            destroyIfDefeated(defenderCiv, attacker.getCivInfo())
         capturedUnit.updateVisibleTiles()
     }
 

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

@@ -8,6 +8,7 @@ import com.unciv.logic.automation.WorkerAutomation
 import com.unciv.logic.civilization.CivilizationInfo
 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.Unique
 import com.unciv.models.ruleset.tile.TileImprovement
@@ -76,10 +77,13 @@ class MapUnit {
 
     @Transient
     var canEnterForeignTerrain: Boolean = false
-    
+
     @Transient
     var paradropRange = 0
 
+    @Transient
+    var hasUniqueToBuildImprovements = false    // not canBuildImprovements to avoid confusion
+
     lateinit var owner: String
 
     /**
@@ -120,7 +124,7 @@ class MapUnit {
     var abilityUsedCount: HashMap<String, Int> = hashMapOf()
     var religion: String? = null
     var religiousStrengthLost = 0
-    
+
     //region pure functions
     fun clone(): MapUnit {
         val toReturn = MapUnit()
@@ -171,16 +175,21 @@ class MapUnit {
         return movement
     }
 
-    // This SHOULD NOT be a hashset, because if it is, then promotions with the same text (e.g. barrage I, barrage II)
+
+    // This SHOULD NOT be a HashSet, because if it is, then promotions with the same text (e.g. barrage I, barrage II)
     //  will not get counted twice!
     @Transient
-    var tempUniques = ArrayList<Unique>()
+    private var tempUniques = ArrayList<Unique>()
 
     fun getUniques(): ArrayList<Unique> = tempUniques
 
     fun getMatchingUniques(placeholderText: String): Sequence<Unique> =
         tempUniques.asSequence().filter { it.placeholderText == placeholderText }
 
+    fun hasUnique(unique: String): Boolean {
+        return getUniques().any { it.placeholderText == unique }
+    }
+
     fun updateUniques() {
         val uniques = ArrayList<Unique>()
         val baseUnit = baseUnit()
@@ -195,7 +204,7 @@ class MapUnit {
 
         //todo: parameterize [terrainFilter] in 5 to 7 of the following:
 
-        // "All tiles costs 1" obsoleted in 3.11.18
+        // "All tiles costs 1" obsoleted in 3.11.18 - keyword: Deprecate
         allTilesCosts1 = hasUnique("All tiles cost 1 movement") || hasUnique("All tiles costs 1")
         canPassThroughImpassableTiles = hasUnique("Can pass through impassable tiles")
         ignoresTerrainCost = hasUnique("Ignores terrain cost")
@@ -207,13 +216,11 @@ class MapUnit {
         canEnterIceTiles = hasUnique("Can enter ice tiles")
         cannotEnterOceanTiles = hasUnique("Cannot enter ocean tiles")
         cannotEnterOceanTilesUntilAstronomy = hasUnique("Cannot enter ocean tiles until Astronomy")
-        canEnterForeignTerrain = 
+        // Constants.workerUnique deprecated since 3.15.5
+        hasUniqueToBuildImprovements = hasUnique(Constants.canBuildImprovements) || hasUnique(Constants.workerUnique)
+        canEnterForeignTerrain =
             hasUnique("May enter foreign tiles without open borders, but loses [] religious strength each turn it ends there")
-            || hasUnique("May enter foreign tiles without open borders")
+                    || hasUnique("May enter foreign tiles without open borders")
-    }
-
-    fun hasUnique(unique: String): Boolean {
-        return getUniques().any { it.placeholderText == unique }
     }
 
     fun copyStatisticsTo(newUnit: MapUnit) {
@@ -250,9 +257,9 @@ class MapUnit {
         for (unique in getTile().getAllTerrains().flatMap { it.uniqueObjects })
             if (unique.placeholderText == "[] Sight for [] units" && matchesFilter(unique.params[1]))
                 visibilityRange += unique.params[0].toInt()
-        
+
         if (visibilityRange < 1) visibilityRange = 1
-        
+
         return visibilityRange
     }
 
@@ -271,16 +278,24 @@ class MapUnit {
         civInfo.updateViewableTiles() // for the civ
     }
 
-    fun isFortified() = action?.startsWith("Fortify") == true
+    fun isActionUntilHealed() = action?.endsWith("until healed") == true
 
-    fun isFortifyingUntilHealed() = isFortified() && action?.endsWith("until healed") == true
+    fun isFortified() = action?.startsWith(UnitActionType.Fortify.value) == true
+    fun isFortifyingUntilHealed() = isFortified() && isActionUntilHealed()
 
-    fun isSleeping() = action?.startsWith("Sleep") == true
+    fun isSleeping() = action?.startsWith(UnitActionType.Sleep.value) == true
-
+    fun isSleepingUntilHealed() = isSleeping() && isActionUntilHealed()
-    fun isSleepingUntilHealed() = isSleeping() && action?.endsWith("until healed") == true
 
     fun isMoving() = action?.startsWith("moveTo") == true
     
+    fun isAutomated() = action == UnitActionType.Automate.value
+    fun isExploring() = action == UnitActionType.Explore.value
+    fun isPreparingParadrop() = action == UnitActionType.Paradrop.value
+    fun isSetUpForSiege() = action == UnitActionType.SetUp.value
+
+    /** For display in Unit Overview */
+    fun getActionLabel() = if (action == null) "" else if (isFortified()) UnitActionType.Fortify.value else action!!
+
     fun isCivilian() = baseUnit.isCivilian()
 
     fun getFortificationTurns(): Int {
@@ -288,6 +303,7 @@ class MapUnit {
         return action!!.split(" ")[1].toInt()
     }
 
+    // debug helper (please update comment if you see some "$unit" using this)
     override fun toString() = "$name - $owner"
 
 
@@ -300,11 +316,7 @@ class MapUnit {
         // unique "Can construct roads" deprecated since 3.15.5
             if (hasUnique("Can construct roads") && currentTile.improvementInProgress == RoadStatus.Road.name) return false
         //
-        if (isFortified()) return false
+        return !(isFortified() || isExploring() || isSleeping() || isAutomated() || isMoving())
-        if (action == Constants.unitActionExplore || isSleeping()
-            || action == Constants.unitActionAutomation || isMoving()
-        ) return false
-        return true
     }
 
     fun maxAttacksPerTurn(): Int {
@@ -472,9 +484,9 @@ class MapUnit {
             return
         }
 
-        if (action == Constants.unitActionAutomation) WorkerAutomation(this).automateWorkerAction()
+        if (isAutomated()) WorkerAutomation(this).automateWorkerAction()
 
-        if (action == Constants.unitActionExplore) UnitAutomation.automatedExplore(this)
+        if (isExploring()) UnitAutomation.automatedExplore(this)
     }
 
 
@@ -626,16 +638,16 @@ class MapUnit {
         ) heal()
 
         if (action != null && health > 99)
-            if (action!!.endsWith(" until healed")) {
+            if (isActionUntilHealed()) {
                 action = null // wake up when healed
             }
 
-        if (action == Constants.unitActionParadrop)
+        if (isPreparingParadrop())
             action = null
-        
+
-        if (hasUnique("Religious Unit") 
+        if (hasUnique("Religious Unit")
-            && getTile().getOwner() != null 
+            && getTile().getOwner() != null
-            && !getTile().getOwner()!!.isCityState() 
+            && !getTile().getOwner()!!.isCityState()
             && !civInfo.canPassThroughTiles(getTile().getOwner()!!)
         ) {
             val lostReligiousStrength =
@@ -947,11 +959,11 @@ class MapUnit {
     fun canSpreadReligion(): Boolean {
         return hasUnique("Can spread religion [] times")
     }
-    
+
     fun getPressureAddedFromSpread(): Int {
         return baseUnit.religiousStrength
     }
-    
+
     fun getReligionString(): String {
         val maxSpreads = maxReligionSpreads()
         if (abilityUsedCount["Religion Spread"] == null) return "" // That is, either the key doesn't exist, or it does exist and the value is null.
@@ -960,7 +972,7 @@ class MapUnit {
 
     fun actionsOnDeselect() {
         showAdditionalActions = false
-        if (action == Constants.unitActionParadrop) action = null
+        if (isPreparingParadrop()) action = null
     }
 
     //endregion

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

@@ -136,7 +136,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
                         if (!canPassThrough(neighbor))
                             totalDistanceToTile = 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 algs on reaching enemy
+                        // You need to assume his tile is reachable, otherwise all movement algorithms on reaching enemy
                         // cities and units goes kaput.
 
                         else {
@@ -229,7 +229,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
         if (currentTile == finalDestination) return currentTile
 
         // If we can fly, head there directly
-        if (unit.baseUnit.movesLikeAirUnits() || unit.action == Constants.unitActionParadrop) return finalDestination
+        if (unit.baseUnit.movesLikeAirUnits() || unit.isPreparingParadrop()) return finalDestination
 
         val distanceToTiles = getDistanceToTiles()
 
@@ -267,7 +267,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
     /** This is performance-heavy - use as last resort, only after checking everything else!
      * Also note that REACHABLE tiles are not necessarily tiles that the unit CAN ENTER */
     fun canReach(destination: TileInfo): Boolean {
-        if (unit.baseUnit.movesLikeAirUnits() || unit.action == Constants.unitActionParadrop)
+        if (unit.baseUnit.movesLikeAirUnits() || unit.isPreparingParadrop())
             return canReachInCurrentTurn(destination)
         return getShortestPath(destination).any()
     }
@@ -275,7 +275,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
     fun canReachInCurrentTurn(destination: TileInfo): Boolean {
         if (unit.baseUnit.movesLikeAirUnits())
             return unit.currentTile.aerialDistanceTo(destination) <= unit.getRange()*2
-        if (unit.action == Constants.unitActionParadrop)
+        if (unit.isPreparingParadrop())
             return getDistance(unit.currentTile.position, destination.position) <= unit.paradropRange && canParadropOn(destination)
         return getDistanceToTiles().containsKey(destination)
     }
@@ -284,7 +284,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
         return when {
             unit.baseUnit.movesLikeAirUnits() ->
                 unit.getTile().getTilesInDistanceRange(IntRange(1, unit.getRange() * 2))
-            unit.action == Constants.unitActionParadrop ->
+            unit.isPreparingParadrop() ->
                 unit.getTile().getTilesInDistance(unit.paradropRange)
                     .filter { unit.movement.canParadropOn(it) }
             else ->
@@ -383,7 +383,7 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
             unit.putInTile(destination)
             unit.currentMovement = 0f
             return
-        } else if (unit.action == Constants.unitActionParadrop) { // paradropping units move differently
+        } else if (unit.isPreparingParadrop()) { // paradropping units move differently
             unit.action = null
             unit.removeFromTile()
             unit.putInTile(destination)
@@ -412,8 +412,8 @@ class UnitMovementAlgorithms(val unit:MapUnit) {
             unit.currentMovement -= distanceToTiles[lastReachableTile]!!.totalDistance
             if (unit.currentMovement < 0.1) unit.currentMovement = 0f // silly floats which are "almost zero"
         }
-        if (unit.isFortified() || unit.action == Constants.unitActionSetUp || unit.isSleeping())
+        if (unit.isFortified() || unit.isSetUpForSiege() || unit.isSleeping())
-            unit.action = null // unfortify/setup after moving
+            unit.action = null // un-fortify/un-setup after moving
 
         // If this unit is a carrier, keep record of its air payload whereabouts.
         val origin = unit.getTile()

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

@@ -45,9 +45,8 @@ class UnitOverviewTable(
                 game.worldScreen.mapHolder.setCenterPosition(unit.currentTile.position)
             }
             add(button).left()
-            val mapUnitAction = unit.action
+            if (unit.action == null) add()
-            if (mapUnitAction == null) add()
+            else add(unit.getActionLabel().tr())
-            else add(if (mapUnitAction.startsWith("Fortify")) "Fortify".tr() else mapUnitAction.tr())
             if (baseUnit.strength > 0) add(baseUnit.strength.toString()) else add()
             if (baseUnit.rangedStrength > 0) add(baseUnit.rangedStrength.toString()) else add()
             add(DecimalFormat("0.#").format(unit.currentMovement) + "/" + unit.getMaxMovement())

core/src/com/unciv/ui/utils/UnitGroup.kt

@@ -5,7 +5,6 @@ import com.badlogic.gdx.scenes.scene2d.Group
 import com.badlogic.gdx.scenes.scene2d.actions.Actions
 import com.badlogic.gdx.scenes.scene2d.actions.RepeatAction
 import com.badlogic.gdx.scenes.scene2d.ui.Image
-import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.logic.map.MapUnit
 
@@ -56,9 +55,10 @@ class UnitGroup(val unit: MapUnit, val size: Float): Group() {
             unit.isFortified() -> ImageGetter.getImage("OtherIcons/Shield")
             unit.isSleeping() -> ImageGetter.getImage("OtherIcons/Sleep")
             unit.isMoving() -> ImageGetter.getStatIcon("Movement")
-            unit.action == Constants.unitActionExplore -> ImageGetter.getUnitIcon("Scout")
+            //todo: Less hardcoding, or move to Constants with explanation (should icon change with mods?)
-            unit.action == Constants.unitActionAutomation -> ImageGetter.getUnitIcon("Great Engineer")
+            unit.isExploring() -> ImageGetter.getUnitIcon("Scout")
-            unit.action == Constants.unitActionSetUp -> ImageGetter.getUnitIcon("Catapult")
+            unit.isAutomated() -> ImageGetter.getUnitIcon("Great Engineer")
+            unit.isSetUpForSiege() -> ImageGetter.getUnitIcon("Catapult")
             else -> null
         }
     }

core/src/com/unciv/ui/worldscreen/WorldMapHolder.kt

@@ -214,7 +214,7 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
                     // I can't think of any way to avoid this,
                     // but it's so rare and edge-case-y that ignoring its failure is actually acceptable, hence the empty catch
                     selectedUnit.movement.moveToTile(tileToMoveTo)
-                    if (selectedUnit.action == Constants.unitActionExplore || selectedUnit.isMoving())
+                    if (selectedUnit.isExploring() || selectedUnit.isMoving())
                         selectedUnit.action = null // remove explore on manual move
                     Sounds.play(UncivSound.Whoosh)
                     if (selectedUnit.currentTile != targetTile)
@@ -236,7 +236,7 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
     private fun swapMoveUnitToTargetTile(selectedUnit: MapUnit, targetTile: TileInfo) {
         selectedUnit.movement.swapMoveToTile(targetTile)
 
-        if (selectedUnit.action == Constants.unitActionExplore || selectedUnit.isMoving())
+        if (selectedUnit.isExploring() || selectedUnit.isMoving())
             selectedUnit.action = null // remove explore on manual swap-move
 
         // Play something like a swish-swoosh
@@ -262,7 +262,7 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
                 val turnsToGetThere = if (unit.baseUnit.movesLikeAirUnits()) {
                     if (unit.movement.canReach(tileInfo)) 1
                     else 0
-                } else if (unit.action == Constants.unitActionParadrop) {
+                } else if (unit.isPreparingParadrop()) {
                     if (unit.movement.canReach(tileInfo)) 1
                     else 0
                 } else {
@@ -481,7 +481,7 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
     private fun updateTilegroupsForSelectedUnit(unit: MapUnit, playerViewableTilePositions: HashSet<Vector2>) {
         val tileGroup = tileGroups[unit.getTile()] ?: return
         // Entirely unclear when this happens, but this seems to happen since version 520 (3.12.9)
-        // so maybe has to do with the construction list being asyc?
+        // so maybe has to do with the construction list being async?
         for (group in tileGroup) {
             group.selectUnit(unit)
         }
@@ -510,7 +510,7 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
         }
 
         val isAirUnit = unit.baseUnit.movesLikeAirUnits()
-        val moveTileOverlayColor = if (unit.action == Constants.unitActionParadrop) Color.BLUE else Color.WHITE
+        val moveTileOverlayColor = if (unit.isPreparingParadrop()) Color.BLUE else Color.WHITE
         val tilesInMoveRange = unit.movement.getReachableTilesInCurrentTurn()
 
         for (tile in tilesInMoveRange) {
@@ -538,10 +538,10 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
             }
         }
 
-        if(unit.isMoving()) {
+        if (unit.isMoving()) {
             val destinationTileGroups = tileGroups[unit.getMovementDestination()]!!
-            for (tileGroup in destinationTileGroups)
+            for (destinationTileGroup in destinationTileGroups)
-                tileGroup.showCircle(Color.WHITE, 0.7f)
+                destinationTileGroup.showCircle(Color.WHITE, 0.7f)
         }
 
         val attackableTiles: List<AttackableTile> = if (unit.isCivilian()) listOf()
@@ -629,7 +629,7 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
                 Actions.run { tileGroup.circleImage.isVisible = true },
                 Actions.delay(.3f)
         ))
-        addAction(blinkAction) // Don't set it on the group because it's an actionlss group
+        addAction(blinkAction) // Don't set it on the group because it's an actionless group
 
         worldScreen.shouldUpdate = true
         return true
@@ -643,7 +643,7 @@ class WorldMapHolder(internal val worldScreen: WorldScreen, internal val tileMap
                 tileGroup.cityButtonLayerGroup.isTransform = false // to save on rendering time to improve framerate
         if (scale < 1 && scale > 0.5f)
             for (tileGroup in allWorldTileGroups) {
-                // ONLY set those groups that have active citybuttons as transformable!
+                // ONLY set those groups that have active city buttons as transformable!
                 // This is massively framerate-improving!
                 if (tileGroup.cityButtonLayerGroup.hasChildren())
                     tileGroup.cityButtonLayerGroup.isTransform = true

core/src/com/unciv/ui/worldscreen/WorldScreen.kt

@@ -482,8 +482,7 @@ class WorldScreen(val gameInfo: GameInfo, val viewingCiv:CivilizationInfo) : Cam
 
         displayTutorial(Tutorial.Workers) {
             gameInfo.getCurrentPlayerCivilization().getCivUnits().any {
-                (it.hasUnique(Constants.canBuildImprovements) || it.hasUnique(Constants.workerUnique))
+                it.hasUniqueToBuildImprovements && it.isCivilian()
-                    && it.isCivilian()
             }
         }
     }

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

@@ -35,9 +35,9 @@ object UnitActions {
         val actionList = ArrayList<UnitAction>()
 
         if (unit.isMoving()) actionList += UnitAction(UnitActionType.StopMovement) { unit.action = null }
-        if (unit.action == Constants.unitActionExplore)
+        if (unit.isExploring())
             actionList += UnitAction(UnitActionType.StopExploration) { unit.action = null }
-        if (unit.action == Constants.unitActionAutomation)
+        if (unit.isAutomated())
             actionList += UnitAction(UnitActionType.StopAutomation) { unit.action = null }
 
         addSleepActions(actionList, unit, false)
@@ -56,8 +56,7 @@ object UnitActions {
         actionList += getImprovementConstructionActions(unit, tile)
         addSpreadReligionActions(unit, actionList, tile)
 
-        
+
-        
         addToggleActionsAction(unit, actionList, unitTable)
 
         return actionList
@@ -123,7 +122,7 @@ object UnitActions {
 
     fun getWaterImprovementAction(unit: MapUnit): UnitAction? {
         val tile = unit.currentTile
-        if (!tile.isWater || !unit.hasUnique("May create improvements on water resources") || tile.resource == null) return null
+        if (!tile.isWater || !unit.hasUnique(Constants.workBoatsUnique) || tile.resource == null) return null
 
         val improvementName = tile.getTileResource().improvement ?: return null
         val improvement = tile.ruleset.tileImprovements[improvementName] ?: return null
@@ -221,11 +220,11 @@ object UnitActions {
 
     private fun addSetupAction(unit: MapUnit, actionList: ArrayList<UnitAction>) {
         if (!unit.hasUnique("Must set up to ranged attack") || unit.isEmbarked()) return
-        val isSetUp = unit.action == "Set Up"
+        val isSetUp = unit.isSetUpForSiege()
         actionList += UnitAction(UnitActionType.SetUp,
                 isCurrentAction = isSetUp,
                 action = {
-                    unit.action = Constants.unitActionSetUp
+                    unit.action = UnitActionType.SetUp.value
                     unit.useMovementPoints(1f)
                 }.takeIf { unit.currentMovement > 0 && !isSetUp })
     }
@@ -235,12 +234,12 @@ object UnitActions {
         if (!paradropUniques.any() || unit.isEmbarked()) return
         unit.paradropRange = paradropUniques.maxOfOrNull { it.params[0] }!!.toInt()
         actionList += UnitAction(UnitActionType.Paradrop,
-                isCurrentAction = unit.action == Constants.unitActionParadrop,
+                isCurrentAction = unit.isPreparingParadrop(),
                 action = {
-                    if (unit.action != Constants.unitActionParadrop) {
+                    if (unit.isPreparingParadrop()) {
-                        unit.action = Constants.unitActionParadrop
-                    } else {
                         unit.action = null
+                    } else {
+                        unit.action = UnitActionType.Paradrop.value
                     }
                 }.takeIf {
                     unit.currentMovement == unit.getMaxMovement().toFloat() &&
@@ -288,11 +287,10 @@ object UnitActions {
 
     private fun addExplorationActions(unit: MapUnit, actionList: ArrayList<UnitAction>) {
         if (unit.baseUnit.movesLikeAirUnits()) return
-        if (unit.action != Constants.unitActionExplore) {
+        if (unit.isExploring()) return
-            actionList += UnitAction(UnitActionType.Explore) {
+        actionList += UnitAction(UnitActionType.Explore) {
-                unit.action = Constants.unitActionExplore
+            unit.action = UnitActionType.Explore.value
-                if (unit.currentMovement > 0) UnitAutomation.automatedExplore(unit)
+            if (unit.currentMovement > 0) UnitAutomation.automatedExplore(unit)
-            }
         }
     }
 
@@ -357,8 +355,7 @@ object UnitActions {
     }
 
     private fun addBuildingImprovementsAction(unit: MapUnit, actionList: ArrayList<UnitAction>, tile: TileInfo, worldScreen: WorldScreen, unitTable: UnitTable) {
-        // Constants.workerUnique deprecated since 3.15.5
+        if (!unit.hasUniqueToBuildImprovements) return
-        if (!unit.hasUnique(Constants.canBuildImprovements) && !unit.hasUnique(Constants.workerUnique)) return
         if (unit.isEmbarked()) return
 
         val canConstruct = unit.currentMovement > 0
@@ -374,12 +371,12 @@ object UnitActions {
     }
 
     private fun addAutomateBuildingImprovementsAction(unit: MapUnit, actionList: ArrayList<UnitAction>) {
-        // Constants.workerUnique deprecated since 3.15.5
+        if (!unit.hasUniqueToBuildImprovements) return
-        if (!unit.hasUnique(Constants.canBuildImprovements) && !unit.hasUnique(Constants.workerUnique)) return
 
         actionList += UnitAction(UnitActionType.Automate,
+            isCurrentAction = unit.isAutomated(),
             action = {
-                unit.action = Constants.unitActionAutomation
+                unit.action = UnitActionType.Automate.value
                 WorkerAutomation(unit).automateWorkerAction()
             }.takeIf { unit.currentMovement > 0 }
         )
@@ -575,7 +572,7 @@ object UnitActions {
     private fun addFortifyActions(actionList: ArrayList<UnitAction>, unit: MapUnit, showingAdditionalActions: Boolean) {
         if (unit.isFortified() && !showingAdditionalActions) {
             actionList += UnitAction(
-                type = if (unit.action!!.endsWith(" until healed"))
+                type = if (unit.isActionUntilHealed())
                     UnitActionType.FortifyUntilHealed else
                     UnitActionType.Fortify,
                 isCurrentAction = true,
@@ -614,13 +611,13 @@ object UnitActions {
         if (isDamaged && !showingAdditionalActions) {
             actionList += UnitAction(UnitActionType.SleepUntilHealed,
                 action = {
-                    unit.action = Constants.unitActionSleepUntilHealed
+                    unit.action = UnitActionType.SleepUntilHealed.value
                 }.takeIf { !unit.isSleepingUntilHealed() }
             )
         } else if (isDamaged || !showingAdditionalActions) {
             actionList += UnitAction(UnitActionType.Sleep,
                 action = {
-                    unit.action = Constants.unitActionSleep
+                    unit.action = UnitActionType.Sleep.value
                 }.takeIf { !isSleeping }
             )
         }