Hide lesser used action buttons to free up space (#4580)

* You can now use 'h' to fortify/sleep until healed

* Hide lesser used unit actions by default

* Refactored worker actions into automate & build improvements

* Added missing translation lines

* Fixed translations

android/assets/jsons/translations/Dutch.properties

@@ -607,6 +607,7 @@ Do you really want to disband this unit? = Wil je echt deze eenheid opheffen
 Disband this unit for [goldAmount] gold? = Deze eenheid ontbinden voor [goldAmount] goud?
 Create [improvement] = Maak [improvement]
 Start Golden Age = Start Gouden Eeuw
+Show more = Toon meer
 Yes = Ja
 No = Nee
 Acquire = Verkrijg

android/assets/jsons/translations/template.properties

@@ -552,15 +552,16 @@ Found city =
 Promote = 
 Health = 
 Disband unit = 
+Do you really want to disband this unit? = 
+Disband this unit for [goldAmount] gold? = 
 Gift unit = 
 Explore = 
 Stop exploration = 
 Pillage = 
 Are you sure you want to pillage this [improvement]? = 
-Do you really want to disband this unit? = 
-Disband this unit for [goldAmount] gold? = 
 Create [improvement] = 
 Start Golden Age = 
+Show more = 
 Yes = 
 No = 
 Acquire = 

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

@@ -102,6 +102,8 @@ class MapUnit {
     var health: Int = 100
 
     var action: String? = null // work, automation, fortifying, I dunno what.
+    @Transient
+    var showAdditionalActions: Boolean = false
 
     var attacksThisTurn = 0
     var promotions = UnitPromotions()
@@ -247,10 +249,16 @@ class MapUnit {
     }
 
     fun isFortified() = action?.startsWith("Fortify") == true
+    
+    fun isFortifyingUntilHealed() = isFortified() && action?.endsWith("until healed") == true
 
     fun isSleeping() = action?.startsWith("Sleep") == true
+    
+    fun isSleepingUntilHealed() = isSleeping() && action?.endsWith("until healed") == true
 
     fun isMoving() = action?.startsWith("moveTo") == true
+    
+    fun isAutomaticallyBuildingImprovements() = action != null && action == Constants.unitActionAutomation
 
     fun getFortificationTurns(): Int {
         if (!isFortified()) return 0
@@ -996,6 +1004,11 @@ class MapUnit {
         if (abilityUsedCount["Religion Spread"] == null) return "" // That is, either the key doesn't exist, or it does exist and the value is null.
         return "${maxSpreads - abilityUsedCount["Religion Spread"]!!}/${maxSpreads}"
     }
-    
+
+    fun actionsOnDeselect() {
+        showAdditionalActions = false
+        if (action == Constants.unitActionParadrop) action = null
+    }
+
     //endregion
 }

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

@@ -37,5 +37,6 @@ enum class UnitActionType(val value: String) {
     ConductTradeMission("Conduct Trade Mission"),
     FoundReligion("Found a Religion"),
     DisbandUnit("Disband unit"),
-    GiftUnit("Gift unit")
+    GiftUnit("Gift unit"),
+    ShowAdditionalActions("Show more"),
 }

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

@@ -25,46 +25,60 @@ import com.unciv.ui.worldscreen.WorldScreen
 object UnitActions {
 
     fun getUnitActions(unit: MapUnit, worldScreen: WorldScreen): List<UnitAction> {
+        return if (unit.showAdditionalActions) getAdditionalActions(unit, worldScreen)
+        else getNormalActions(unit, worldScreen)
+    }
+
+    private fun getNormalActions(unit: MapUnit, worldScreen: WorldScreen): List<UnitAction> {
         val tile = unit.getTile()
         val unitTable = worldScreen.bottomUnitTable
         val actionList = ArrayList<UnitAction>()
 
         if (unit.isMoving()) actionList += UnitAction(UnitActionType.StopMovement) { unit.action = null }
+        if (unit.isAutomaticallyBuildingImprovements())
+            actionList += UnitAction(UnitActionType.StopAutomation) { unit.action = null }
 
-        // Constants.workerUnique deprecated since 3.15.5
+        addSleepActions(actionList, unit, false)
-        val workingOnImprovement = unit.currentTile.hasImprovementInProgress() && unit.canBuildImprovement(unit.currentTile.getTileImprovementInProgress()!!)
+        addFortifyActions(actionList, unit, false)
-        if (!unit.isFortified() && !unit.canFortify() && unit.currentMovement > 0 && !workingOnImprovement) {
+        
-            addSleepActions(actionList, unit, unitTable)
-        }
-
-        if (unit.canFortify()) addFortifyActions(actionList, unit, unitTable)
-        else if (unit.isFortified()) {
-            actionList += UnitAction(
-                    type = if (unit.action!!.endsWith(" until healed"))
-                        UnitActionType.FortifyUntilHealed else
-                        UnitActionType.Fortify,
-                    isCurrentAction = true,
-                    title = "${"Fortification".tr()} ${unit.getFortificationTurns() * 20}%"
-            )
-        }
-
-        addSwapAction(unit, actionList, worldScreen)
-        addExplorationActions(unit, actionList)
         addPromoteAction(unit, actionList)
         addUnitUpgradeAction(unit, actionList)
         addPillageAction(unit, actionList, worldScreen)
         addParadropAction(unit, actionList, worldScreen)
         addSetupAction(unit, actionList)
         addFoundCityAction(unit, actionList, tile)
-        addWorkerActions(unit, actionList, tile, worldScreen, unitTable)
+        addBuildingImprovementsAction(unit, actionList, tile, worldScreen, unitTable)
+        addAutomateBuildingImprovementsAction(unit, actionList)
         addCreateWaterImprovements(unit, actionList)
         addGreatPersonActions(unit, actionList, tile)
         addFoundReligionAction(unit, actionList, tile)
-        addSpreadReligionActions(unit, actionList, tile)
         actionList += getImprovementConstructionActions(unit, tile)
+        addSpreadReligionActions(unit, actionList, tile)
+
+
+
+        addToggleActionsAction(unit, actionList, unitTable)
+
+        return actionList
+    }
+
+    private fun getAdditionalActions(unit: MapUnit, worldScreen: WorldScreen): List<UnitAction> {
+        val tile = unit.getTile()
+        val unitTable = worldScreen.bottomUnitTable
+        val actionList = ArrayList<UnitAction>()
+
+        addSleepActions(actionList, unit, true)
+        addFortifyActions(actionList, unit, true)
+
+        addSwapAction(unit, actionList, worldScreen)
+        addExplorationActions(unit, actionList)
         addDisbandAction(actionList, unit, worldScreen)
         addGiftAction(unit, actionList, tile)
 
+
+
+        addToggleActionsAction(unit, actionList, unitTable)
+
         return actionList
     }
 
@@ -135,11 +149,11 @@ object UnitActions {
     /** Produce a [UnitAction] for founding a city.
      * @param unit The unit to do the founding.
      * @param tile The tile to found a city on.
-     * @return null if impossible (the unit lacks the ability to found), 
+     * @return null if impossible (the unit lacks the ability to found),
-     * or else a [UnitAction] 'defining' the founding. 
+     * or else a [UnitAction] 'defining' the founding.
-     * The [action][UnitAction.action] field will be null if the action cannot be done here and now   
+     * The [action][UnitAction.action] field will be null if the action cannot be done here and now
      * (no movement left, too close to another city).
-      */ 
+      */
     fun getFoundCityAction(unit: MapUnit, tile: TileInfo): UnitAction? {
         if (!unit.hasUnique("Founds a new city") || tile.isWater) return null
 
@@ -154,7 +168,7 @@ object UnitActions {
             unit.destroy()
             UncivGame.Current.worldScreen.shouldUpdate = true
         }
-        
+
         if (unit.civInfo.playerType == PlayerType.AI)
             return UnitAction(UnitActionType.FoundCity,  uncivSound = UncivSound.Silent, action = foundAction)
 
@@ -177,8 +191,8 @@ object UnitActions {
 
     /**
      * Checks whether a civ founding a city on a certain tile would break a promise.
-     * @param civInfo The civilization trying to found a city 
+     * @param civInfo The civilization trying to found a city
-     * @param tile The tile where the new city would go 
+     * @param tile The tile where the new city would go
      * @return null if no promises broken, else a String listing the leader(s) we would p* off.
      */
     private fun TestPromiseNotToSettle(civInfo: CivilizationInfo, tile: TileInfo): String? {
@@ -320,34 +334,36 @@ object UnitActions {
                 })
     }
 
-    private fun addWorkerActions(unit: MapUnit, actionList: ArrayList<UnitAction>, tile: TileInfo, worldScreen: WorldScreen, unitTable: UnitTable) {
+    private fun addBuildingImprovementsAction(unit: MapUnit, actionList: ArrayList<UnitAction>, tile: TileInfo, worldScreen: WorldScreen, unitTable: UnitTable) {
         // Constants.workerUnique deprecated since 3.15.5
         if (!unit.hasUnique(Constants.canBuildImprovements) && !unit.hasUnique(Constants.workerUnique)) return
-
-        // Allow automate/unautomate when embarked, but not building improvements - see #1963
-        if (Constants.unitActionAutomation == unit.action) {
-            actionList += UnitAction(UnitActionType.StopAutomation) { unit.action = null }
-        } else {
-            actionList += UnitAction(UnitActionType.Automate,
-                    action = {
-                        unit.action = Constants.unitActionAutomation
-                        WorkerAutomation(unit).automateWorkerAction()
-                    }.takeIf { unit.currentMovement > 0 })
-        }
-
         if (unit.isEmbarked()) return
 
         val canConstruct = unit.currentMovement > 0
                 && !tile.isCityCenter()
                 && unit.civInfo.gameInfo.ruleSet.tileImprovements.values.any { tile.canBuildImprovement(it, unit.civInfo) && unit.canBuildImprovement(it) }
-        
+
         actionList += UnitAction(UnitActionType.ConstructImprovement,
-                isCurrentAction = unit.currentTile.hasImprovementInProgress(),
+            isCurrentAction = unit.currentTile.hasImprovementInProgress(),
-                action = {
+            action = {
-                    worldScreen.game.setScreen(ImprovementPickerScreen(tile, unit) { unitTable.selectUnit() })
+                worldScreen.game.setScreen(ImprovementPickerScreen(tile, unit) { unitTable.selectUnit() })
-                }.takeIf { canConstruct })
+            }.takeIf { canConstruct }
+        )
     }
 
+    private fun addAutomateBuildingImprovementsAction(unit: MapUnit, actionList: ArrayList<UnitAction>) {
+        // Constants.workerUnique deprecated since 3.15.5
+        if (!unit.hasUnique(Constants.canBuildImprovements) && !unit.hasUnique(Constants.workerUnique)) return
+
+        actionList += UnitAction(UnitActionType.Automate,
+            action = {
+                unit.action = Constants.unitActionAutomation
+                WorkerAutomation(unit).automateWorkerAction()
+            }.takeIf { unit.currentMovement > 0 }
+        )
+    }
+
+
     private fun addGreatPersonActions(unit: MapUnit, actionList: ArrayList<UnitAction>, tile: TileInfo) {
 
         if (unit.currentMovement > 0) for (unique in unit.getUniques()) when (unique.placeholderText) {
@@ -413,7 +429,7 @@ object UnitActions {
             }
         }
     }
-    
+
     private fun addFoundReligionAction(unit: MapUnit, actionList: ArrayList<UnitAction>, tile: TileInfo) {
         if (!unit.hasUnique("May found a religion")) return // should later also include enhance religion
         if (!unit.civInfo.religionManager.mayUseGreatProphetAtAll(unit)) return
@@ -426,7 +442,7 @@ object UnitActions {
             }.takeIf { unit.civInfo.religionManager.mayUseGreatProphetNow(unit) }
         )
     }
-    
+
     private fun addSpreadReligionActions(unit: MapUnit, actionList: ArrayList<UnitAction>, tile: TileInfo) {
         if (!unit.hasUnique("Can spread religion [] times")) return
         if (unit.religion == null) return
@@ -448,7 +464,7 @@ object UnitActions {
             }.takeIf { unit.currentMovement > 0 && city != null && city.civInfo == unit.civInfo } // For now you can only convert your own cities
         )
     }
-    
+
     fun getImprovementConstructionActions(unit: MapUnit, tile: TileInfo): ArrayList<UnitAction> {
         val finalActions = ArrayList<UnitAction>()
         var uniquesToCheck = unit.getMatchingUniques("Can construct []")
@@ -504,7 +520,7 @@ object UnitActions {
 
         // In the rare case more than one city owns tiles neighboring the citadel
         // this will prioritize the nearest one not being razed
-        var nearestCity = unit.currentTile.neighbors
+        val nearestCity = unit.currentTile.neighbors
             .filter { it.getOwner() == unit.civInfo }
             .minByOrNull { priority(it) }?.getCity()
             ?: fallbackNearestCity(unit)
@@ -541,49 +557,60 @@ object UnitActions {
         civInfo.addNotification("[${mausoleum.name}] has provided [$goldEarned] Gold!", cityWithMausoleum.location, NotificationIcon.Gold)
     }
 
-    private fun addFortifyActions(actionList: ArrayList<UnitAction>, unit: MapUnit, unitTable: UnitTable) {
+    private fun addFortifyActions(actionList: ArrayList<UnitAction>, unit: MapUnit, showingAdditionalActions: Boolean) {
-
+        if (unit.isFortified() && !showingAdditionalActions) {
-        val action = UnitAction(UnitActionType.Fortify,
+            actionList += UnitAction(
-                uncivSound = UncivSound.Fortify,
+                type = if (unit.action!!.endsWith(" until healed"))
-                action = {
+                    UnitActionType.FortifyUntilHealed else
-                    unit.fortify()
+                    UnitActionType.Fortify,
-                    unitTable.selectUnit()
+                isCurrentAction = true,
-                }.takeIf { unit.currentMovement > 0 })
+                title = "${"Fortification".tr()} ${unit.getFortificationTurns() * 20}%"
-
+            )
-        if (unit.health < 100) {
+            return
-            val actionForWounded = action.copy(UnitActionType.FortifyUntilHealed,
-                    title = UnitActionType.FortifyUntilHealed.value,
-                    action = {
-                        unit.fortifyUntilHealed()
-                        unitTable.selectUnit()
-                    }.takeIf { unit.currentMovement > 0 })
-            actionList += actionForWounded
         }
 
-        actionList += action
+        if (!unit.canFortify()) return
+        if (unit.currentMovement == 0f) return
+
+        val isFortified = unit.isFortified()
+        val isDamaged = unit.health < 100
+
+        if (isDamaged && !showingAdditionalActions)
+            actionList += UnitAction(UnitActionType.FortifyUntilHealed,
+            title = UnitActionType.FortifyUntilHealed.value,
+            action = {
+                unit.fortifyUntilHealed()
+            }.takeIf { !unit.isFortifyingUntilHealed() }
+        )
+        else if (isDamaged || !showingAdditionalActions)
+            actionList += UnitAction(UnitActionType.Fortify,
+            uncivSound = UncivSound.Fortify,
+            action = {
+                unit.fortify()
+            }.takeIf { !isFortified }
+        )
     }
 
-    private fun addSleepActions(actionList: ArrayList<UnitAction>, unit: MapUnit, unitTable: UnitTable) {
+    private fun addSleepActions(actionList: ArrayList<UnitAction>, unit: MapUnit, showingAdditionalActions: Boolean) {
+        if (unit.isFortified() || unit.canFortify() || unit.currentMovement == 0f) return
+        // If this unit is working on an improvement, it cannot sleep
+        if ((unit.currentTile.hasImprovementInProgress() && unit.canBuildImprovement(unit.currentTile.getTileImprovementInProgress()!!))) return
         val isSleeping = unit.isSleeping()
+        val isDamaged = unit.health < 100
 
-        val action = UnitAction(UnitActionType.Sleep,
+        if (isDamaged && !showingAdditionalActions) {
-                isCurrentAction = isSleeping,
+            actionList += UnitAction(UnitActionType.SleepUntilHealed,
+                action = {
+                    unit.action = Constants.unitActionSleepUntilHealed
+                }.takeIf { !unit.isSleepingUntilHealed() }
+            )
+        } else if (isDamaged || !showingAdditionalActions) {
+            actionList += UnitAction(UnitActionType.Sleep,
                 action = {
                     unit.action = Constants.unitActionSleep
-                    unitTable.selectUnit()
+                }.takeIf { !isSleeping }
-                }.takeIf { !isSleeping })
+            )
-
-        if (unit.health < 100 && !isSleeping) {
-            val actionForWounded = action.copy(UnitActionType.SleepUntilHealed,
-                    title = UnitActionType.SleepUntilHealed.value,
-                    action = {
-                        unit.action = Constants.unitActionSleepUntilHealed
-                        unitTable.selectUnit()
-                    })
-            actionList += actionForWounded
         }
-
-        actionList += action
     }
 
     fun canPillage(unit: MapUnit, tile: TileInfo): Boolean {
@@ -644,4 +671,15 @@ object UnitActions {
 
         return UnitAction(UnitActionType.GiftUnit, uncivSound = UncivSound.Silent, action = giftAction)
     }
+
+    private fun addToggleActionsAction(unit: MapUnit, actionList: ArrayList<UnitAction>, unitTable: UnitTable) {
+        actionList += UnitAction(UnitActionType.ShowAdditionalActions,
+            title = if (unit.showAdditionalActions) "Back" else "Show more",
+            action = {
+                unit.showAdditionalActions = !unit.showAdditionalActions
+                unitTable.update()
+            }
+        )
+    }
+
 }

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

@@ -4,6 +4,7 @@ import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.ui.Button
 import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.utils.Align
 import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.logic.map.MapUnit
@@ -25,13 +26,13 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
         when {
             unitAction.equalsPlaceholderText("Upgrade to [] ([] gold)") -> {
                 // Regexplaination: start with a [, take as many non-] chars as you can, until you reach a ].
-                // What you find between the first [ and the first ] that comes after it, will be group no. 1
+                // What you find between the first [ and the first ] that comes after it, will be group no. 0
                 val unitToUpgradeTo = unitAction.getPlaceholderParameters()[0]
                 return UnitIconAndKey(ImageGetter.getUnitIcon(unitToUpgradeTo), 'u')
             }
             unitAction.equalsPlaceholderText("Create []") -> {
                 // Regexplaination: start with a [, take as many non-] chars as you can, until you reach a ].
-                // What you find between the first [ and the first ] that comes after it, will be group no. 1
+                // What you find between the first [ and the first ] that comes after it, will be group no. 0
                 val improvementName = unitAction.getPlaceholderParameters()[0]
                 return UnitIconAndKey(ImageGetter.getImprovementIcon(improvementName), 'i')
             }
@@ -39,9 +40,12 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
                 // This should later include icons for the different religions. For now, just use the great prophet icon
                 return UnitIconAndKey(ImageGetter.getUnitIcon("Great Prophet"), 'g')
             }
-            unitAction.startsWith("Sleep") -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Sleep"), 'f')
-            unitAction.startsWith("Fortify") -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Shield").apply { color = Color.BLACK }, 'f')
             else -> when (unitAction) {
+                "Sleep" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Sleep"), 'f')
+                "Sleep until healed" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Sleep"), 'h')
+                "Fortify" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Shield").apply { color = Color.BLACK }, 'f')
+                "Fortify until healed" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Shield").apply { color = Color.BLACK }, 'h')
+                // Move unit is not actually used anywhere
                 "Move unit" -> return UnitIconAndKey(ImageGetter.getStatIcon("Movement"))
                 "Stop movement" -> return UnitIconAndKey(ImageGetter.getStatIcon("Movement").apply { color = Color.RED }, '.')
                 "Swap units" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Swap"), 'y')
@@ -62,7 +66,13 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
                 "Pillage" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Pillage"), 'p')
                 "Disband unit" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/DisbandUnit"))
                 "Gift unit" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Present"))
-                else -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Star"))
+                "Show more" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/ArrowRight"), 'm')
+                "Back" -> return UnitIconAndKey(ImageGetter.getImage("OtherIcons/ArrowLeft"))
+                else -> {
+                    // If the unit has been fortifying for some turns
+                    if (unitAction.startsWith("Fortification")) return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Shield"))
+                    return UnitIconAndKey(ImageGetter.getImage("OtherIcons/Star"))
+                }
             }
         }
     }

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

@@ -40,7 +40,10 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
     fun selectUnit(unit:MapUnit?=null, append:Boolean=false) {
         if (!append) selectedUnits.clear()
         selectedCity = null
-        if (unit != null) selectedUnits.add(unit)
+        if (unit != null) {
+            selectedUnits.add(unit)
+            unit.actionsOnDeselect()
+        }
         selectedUnitIsSwapping = false
     }