Refactor sounds and unitactions data class (#1596)

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

@@ -28,9 +28,9 @@ class SpecificUnitAutomation{
             unit.movement.headTowards(closestReachableResource)
             if (unit.currentMovement > 0 && unit.currentTile == closestReachableResource) {
                 val createImprovementAction = UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen)
-                        .firstOrNull { it.name.startsWith("Create") } // could be either fishing boats or oil well
+                        .firstOrNull { it.name.startsWith("Create") }?.action // could be either fishing boats or oil well
                 if (createImprovementAction != null)
-                    return createImprovementAction.action() // unit is already gone, can't "Explore"
+                    return createImprovementAction() // unit is already gone, can't "Explore"
             }
         }
         else UnitAutomation().tryExplore(unit, unit.movement.getDistanceToTiles())
@@ -133,11 +133,11 @@ class SpecificUnitAutomation{
             throw Exception("City within distance")
 
         if (unit.getTile() == bestCityLocation)
-            UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen).first { it.name == "Found city" }.action()
+            UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen).first { it.name == "Found city" }.action?.invoke()
         else {
             unit.movement.headTowards(bestCityLocation)
             if (unit.currentMovement > 0 && unit.getTile() == bestCityLocation)
-                UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen).first { it.name == "Found city" }.action()
+                UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen).first { it.name == "Found city" }.action?.invoke()
         }
     }
 
@@ -170,9 +170,9 @@ class SpecificUnitAutomation{
             if(chosenTile==null) continue // to another city
 
             unit.movement.headTowards(chosenTile)
-            if(unit.currentTile==chosenTile && unit.currentMovement>0)
+            if(unit.currentTile==chosenTile && unit.currentMovement > 0)
                 UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen)
-                        .first { it.name.startsWith("Create") }.action()
+                        .first { it.name.startsWith("Create") }.action?.invoke()
             return
         }
 

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

@@ -10,8 +10,8 @@ import com.unciv.logic.civilization.diplomacy.DiplomaticStatus
 import com.unciv.logic.map.MapUnit
 import com.unciv.logic.map.PathsToTilesWithinTurn
 import com.unciv.logic.map.TileInfo
+import com.unciv.models.UnitAction
 import com.unciv.models.ruleset.unit.UnitType
-import com.unciv.ui.worldscreen.unit.UnitAction
 import com.unciv.ui.worldscreen.unit.UnitActions
 
 
@@ -163,7 +163,7 @@ class UnitAutomation{
             unit.movement.moveToTile(tileToPillage)
 
         UnitActions().getUnitActions(unit, UncivGame.Current.worldScreen)
-                .first { it.name == "Pillage" }.action()
+                .first { it.name == "Pillage" }.action?.invoke()
         return true
     }
 
@@ -296,7 +296,7 @@ class UnitAutomation{
             if (upgradedUnit.isBuildable(unit.civInfo)) {
                 val upgradeAction = unitActions.firstOrNull { it.name.startsWith("Upgrade to") }
                 if (upgradeAction != null && upgradeAction.canAct) {
-                    upgradeAction.action()
+                    upgradeAction.action?.invoke()
                     return true
                 }
             }

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

@@ -0,0 +1,16 @@
+package com.unciv.models
+
+enum class UncivSound(val value: String) {
+    Click("click"),
+    Fortify("fortify"),
+    Promote("promote"),
+    Upgrade("upgrade"),
+    Setup("setup"),
+    Chimes("chimes"),
+    Coin("coin"),
+    Choir("choir"),
+    Policy("policy"),
+    Paper("paper"),
+    Whoosh("whoosh"),
+    Silent("")
+}

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

@@ -0,0 +1,10 @@
+package com.unciv.models
+
+data class UnitAction(
+        var name: String, // TODO make it enum or sealed class
+        var canAct: Boolean,
+        var title: String = name,
+        var isCurrentAction: Boolean = false,
+        var uncivSound: UncivSound = UncivSound.Click,
+        var action: (() -> Unit)? = null
+)

core/src/com/unciv/ui/cityscreen/CityScreenTileTable.kt

@@ -7,6 +7,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.TextButton
 import com.unciv.UncivGame
 import com.unciv.logic.city.CityInfo
 import com.unciv.logic.map.TileInfo
+import com.unciv.models.UncivSound
 import com.unciv.models.translations.tr
 import com.unciv.ui.utils.CameraStageBaseScreen
 import com.unciv.ui.utils.ImageGetter
@@ -49,7 +50,7 @@ class CityScreenTileTable(val city: CityInfo): Table(){
             val goldCostOfTile = city.expansion.getGoldCostOfTile(selectedTile)
 
             val buyTileButton = TextButton("Buy for [$goldCostOfTile] gold".tr(), CameraStageBaseScreen.skin)
-            buyTileButton.onClick("coin") {
+            buyTileButton.onClick(UncivSound.Coin) {
                 city.expansion.buyTile(selectedTile)
                 UncivGame.Current.setScreen(CityScreen(city))
             }

core/src/com/unciv/ui/cityscreen/ConstructionsTable.kt

@@ -9,6 +9,7 @@ import com.badlogic.gdx.utils.Align
 import com.unciv.UncivGame
 import com.unciv.logic.city.CityInfo
 import com.unciv.logic.city.SpecialConstruction
+import com.unciv.models.UncivSound
 import com.unciv.models.ruleset.Building
 import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.models.translations.tr
@@ -155,7 +156,7 @@ class ConstructionsTable(val cityScreen: CityScreen) : Table(CameraStageBaseScre
         if (!city.isPuppet && !city.isInResistance() && construction.canBePurchased()) {
             val constructionGoldCost = construction.getGoldCost(city.civInfo)
             purchaseConstructionButton = TextButton("Buy for [$constructionGoldCost] gold".tr(), CameraStageBaseScreen.skin)
-            purchaseConstructionButton.onClick("coin") {
+            purchaseConstructionButton.onClick(UncivSound.Coin) {
                 YesNoPopupTable("Would you like to purchase [${construction.name}] for [$constructionGoldCost] gold?".tr(), {
                     cityConstructions.purchaseConstruction(construction.name)
                     if(lastConstruction!="" && cityConstructions.getConstruction(lastConstruction).isBuildable(cityConstructions))

core/src/com/unciv/ui/pickerscreens/GreatPersonPickerScreen.kt

@@ -4,6 +4,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.Button
 import com.unciv.UncivGame
 import com.unciv.logic.civilization.CivilizationInfo
 import com.unciv.logic.civilization.GreatPersonManager
+import com.unciv.models.UncivSound
 import com.unciv.models.translations.tr
 import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.ui.utils.ImageGetter
@@ -34,7 +35,7 @@ class GreatPersonPickerScreen(val civInfo:CivilizationInfo) : PickerScreen() {
             topTable.add(button).pad(10f).row()
         }
 
-        rightSideButton.onClick("choir") {
+        rightSideButton.onClick(UncivSound.Choir) {
             civInfo.placeUnitNearTile(civInfo.cities[0].location, theChosenOne!!.name)
             civInfo.greatPeople.freeGreatPeople--
             UncivGame.Current.setWorldScreen()

core/src/com/unciv/ui/pickerscreens/PolicyPickerScreen.kt

@@ -7,6 +7,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.TextButton
 import com.unciv.UncivGame
 import com.unciv.logic.civilization.CivilizationInfo
 import com.unciv.models.Tutorial
+import com.unciv.models.UncivSound
 import com.unciv.models.ruleset.Policy
 import com.unciv.models.translations.tr
 import com.unciv.ui.utils.*
@@ -31,7 +32,7 @@ class PolicyPickerScreen(val worldScreen: WorldScreen, civInfo: CivilizationInfo
         }
         else onBackButtonClicked { UncivGame.Current.setWorldScreen() }
 
-        rightSideButton.onClick("policy") {
+        rightSideButton.onClick(UncivSound.Policy) {
             viewingCiv.policies.adopt(pickedPolicy!!)
 
             // If we've moved to another screen in the meantime (great person pick, victory screen) ignore this

core/src/com/unciv/ui/pickerscreens/PromotionPickerScreen.kt

@@ -8,6 +8,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.VerticalGroup
 import com.badlogic.gdx.utils.Align
 import com.unciv.UncivGame
 import com.unciv.logic.map.MapUnit
+import com.unciv.models.UncivSound
 import com.unciv.models.ruleset.unit.Promotion
 import com.unciv.models.translations.tr
 import com.unciv.ui.utils.*
@@ -30,7 +31,7 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen() {
 
 
         rightSideButton.setText("Pick promotion".tr())
-        rightSideButton.onClick("promote") {
+        rightSideButton.onClick(UncivSound.Promote) {
           acceptPromotion(selectedPromotion)
         }
         val canBePromoted = unit.promotions.canBePromoted()

core/src/com/unciv/ui/pickerscreens/TechPickerScreen.kt

@@ -8,6 +8,7 @@ import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.logic.civilization.CivilizationInfo
 import com.unciv.logic.civilization.TechManager
+import com.unciv.models.UncivSound
 import com.unciv.models.ruleset.tech.Technology
 import com.unciv.models.translations.tr
 import com.unciv.ui.utils.*
@@ -50,7 +51,7 @@ class TechPickerScreen(internal val civInfo: CivilizationInfo, centerOnTech: Tec
 
         setButtonsInfo()
         rightSideButton.setText("Pick a tech".tr())
-        rightSideButton.onClick("paper") {
+        rightSideButton.onClick(UncivSound.Paper) {
             game.settings.addCompletedTutorialTask("Pick technology")
             if (isFreeTechPick) civTech.getFreeTechnology(selectedTech!!.name)
             else civTech.techsToResearch = tempTechsToResearch

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

@@ -27,6 +27,7 @@ import com.badlogic.gdx.utils.viewport.ExtendViewport
 import com.unciv.JsonParser
 import com.unciv.UncivGame
 import com.unciv.models.Tutorial
+import com.unciv.models.UncivSound
 import com.unciv.models.translations.tr
 import com.unciv.ui.tutorials.TutorialController
 import com.unciv.ui.tutorials.TutorialMiner
@@ -144,22 +145,22 @@ fun Actor.center(parent:Stage){ centerX(parent); centerY(parent)}
 
 
 /** same as [onClick], but sends the [InputEvent] and coordinates along */
-fun Actor.onClickEvent(sound: String = "click", function: (event: InputEvent?, x: Float, y: Float) -> Unit) {
+fun Actor.onClickEvent(sound: UncivSound = UncivSound.Click, function: (event: InputEvent?, x: Float, y: Float) -> Unit) {
     this.addListener(object : ClickListener() {
         override fun clicked(event: InputEvent?, x: Float, y: Float) {
-            if (sound != "") thread{Sounds.play(sound)}
+            thread { Sounds.play(sound) }
             function(event, x, y)
         }
     })
 }
 
 // If there are other buttons that require special clicks then we'll have an onclick that will accept a string parameter, no worries
-fun Actor.onClick(sound: String = "click", function: () -> Unit) {
+fun Actor.onClick(sound: UncivSound = UncivSound.Click, function: () -> Unit) {
     onClickEvent(sound) { _, _, _ -> function() }
 }
 
 fun Actor.onClick(function: () -> Unit): Actor {
-    onClick("click", function)
+    onClick(UncivSound.Click, function)
     return this
 }
 

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

@@ -3,18 +3,20 @@ package com.unciv.ui.utils
 import com.badlogic.gdx.Gdx
 import com.badlogic.gdx.audio.Sound
 import com.unciv.UncivGame
+import com.unciv.models.UncivSound
 
-object Sounds{
+object Sounds {
-    val soundMap = HashMap<String, Sound>()
+    private val soundMap = HashMap<UncivSound, Sound>()
 
-    fun get(name:String):Sound{
+    fun get(sound: UncivSound): Sound {
-        if(!soundMap.containsKey(name))
+        if (!soundMap.containsKey(sound)) {
-            soundMap[name] = Gdx.audio.newSound(Gdx.files.internal("sounds/$name.mp3"))
+            soundMap[sound] = Gdx.audio.newSound(Gdx.files.internal("sounds/${sound.value}.mp3"))
-        return soundMap[name]!!
+        }
+        return soundMap[sound]!!
     }
 
-
+    fun play(sound: UncivSound) {
-    fun play(name:String){
+        if (sound == UncivSound.Silent) return
-        get(name).play(UncivGame.Current.settings.soundEffectsVolume)
+        get(sound).play(UncivGame.Current.settings.soundEffectsVolume)
     }
 }

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

@@ -17,6 +17,7 @@ import com.unciv.logic.civilization.CivilizationInfo
 import com.unciv.logic.map.MapUnit
 import com.unciv.logic.map.TileInfo
 import com.unciv.logic.map.TileMap
+import com.unciv.models.UncivSound
 import com.unciv.models.ruleset.unit.UnitType
 import com.unciv.ui.tilegroups.TileSetStrings
 import com.unciv.ui.tilegroups.WorldTileGroup
@@ -52,7 +53,7 @@ class TileMapHolder(internal val worldScreen: WorldScreen, internal val tileMap:
         val allTiles = TileGroupMap(daTileGroups,worldScreen.stage.width)
 
         for(tileGroup in tileGroups.values){
-            tileGroup.cityButtonLayerGroup.onClick("") {
+            tileGroup.cityButtonLayerGroup.onClick(UncivSound.Silent) {
                 onTileClicked(tileGroup.tileInfo)
             }
             tileGroup.onClick { onTileClicked(tileGroup.tileInfo) }
@@ -204,7 +205,7 @@ class TileMapHolder(internal val worldScreen: WorldScreen, internal val tileMap:
         moveHereButton.addActor(unitIcon)
 
         if (dto.unit.currentMovement > 0)
-            moveHereButton.onClick(""){
+            moveHereButton.onClick(UncivSound.Silent) {
                 UncivGame.Current.settings.addCompletedTutorialTask("Move unit")
                 if(dto.unit.type.isAirUnit())
                     UncivGame.Current.settings.addCompletedTutorialTask("Move an air unit")

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

@@ -15,6 +15,7 @@ import com.unciv.logic.GameSaver
 import com.unciv.logic.civilization.CivilizationInfo
 import com.unciv.logic.civilization.diplomacy.DiplomaticStatus
 import com.unciv.models.Tutorial
+import com.unciv.models.UncivSound
 import com.unciv.models.ruleset.tile.ResourceType
 import com.unciv.models.ruleset.unit.UnitType
 import com.unciv.models.translations.tr
@@ -78,7 +79,7 @@ class WorldScreen(val viewingCiv:CivilizationInfo) : CameraStageBaseScreen() {
         tileMapHolder.addTiles()
 
         techButtonHolder.touchable=Touchable.enabled
-        techButtonHolder.onClick("paper") {
+        techButtonHolder.onClick(UncivSound.Paper) {
             game.setScreen(TechPickerScreen(viewingCiv))
         }
         techPolicyAndVictoryHolder.add(techButtonHolder)

core/src/com/unciv/ui/worldscreen/optionstable/WorldScreenOptionsTable.kt

@@ -7,6 +7,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.*
 import com.badlogic.gdx.scenes.scene2d.utils.ChangeListener
 import com.badlogic.gdx.utils.Array
 import com.unciv.UncivGame
+import com.unciv.models.UncivSound
 import com.unciv.models.translations.tr
 import com.unciv.ui.utils.*
 import com.unciv.ui.worldscreen.WorldScreen
@@ -151,7 +152,7 @@ class WorldScreenOptionsTable(val worldScreen:WorldScreen) : PopupTable(worldScr
             override fun changed(event: ChangeEvent?, actor: Actor?) {
                 UncivGame.Current.settings.soundEffectsVolume = soundEffectsVolumeSlider.value
                 UncivGame.Current.settings.save()
-                Sounds.play("click")
+                Sounds.play(UncivSound.Click)
             }
         })
         innerTable.add(soundEffectsVolumeSlider).row()

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

@@ -8,264 +8,301 @@ import com.unciv.logic.automation.WorkerAutomation
 import com.unciv.logic.map.MapUnit
 import com.unciv.logic.map.RoadStatus
 import com.unciv.logic.map.TileInfo
+import com.unciv.models.UncivSound
+import com.unciv.models.UnitAction
 import com.unciv.models.ruleset.Building
 import com.unciv.models.translations.tr
 import com.unciv.ui.pickerscreens.ImprovementPickerScreen
 import com.unciv.ui.pickerscreens.PromotionPickerScreen
 import com.unciv.ui.worldscreen.WorldScreen
 import com.unciv.ui.worldscreen.optionstable.YesNoPopupTable
-import java.util.*
-
-class UnitAction(var name: String, var canAct: Boolean, var currentAction: Boolean = false, var title: String = name, var action: () -> Unit = {}){
-    var sound="click"
-    fun sound(soundName:String): UnitAction {sound=soundName; return this}
-}
 
 class UnitActions {
 
-    fun getUnitActions(unit:MapUnit,worldScreen: WorldScreen): List<UnitAction> {
+    fun getUnitActions(unit: MapUnit, worldScreen: WorldScreen): List<UnitAction> {
         val tile = unit.getTile()
         val unitTable = worldScreen.bottomUnitTable
         val actionList = ArrayList<UnitAction>()
 
-        if(unit.action!=null && unit.action!!.startsWith("moveTo")) {
+        if (unit.action != null && unit.action!!.startsWith("moveTo")) {
-            actionList += UnitAction("Stop movement", true) {unit.action = null}
+            actionList += UnitAction(name = "Stop movement", canAct = true)
         }
 
         val workingOnImprovement = unit.hasUnique("Can build improvements on tiles") && unit.currentTile.hasImprovementInProgress()
-        if(!unit.isFortified() && (!unit.canFortify() || unit.health<100) && unit.currentMovement >0
+        if (!unit.isFortified() && (!unit.canFortify() || unit.health < 100) && unit.currentMovement > 0
-                && !workingOnImprovement) {
+            && !workingOnImprovement) {
             val isSleeping = unit.action == Constants.unitActionSleep
-            actionList += UnitAction("Sleep", !isSleeping, isSleeping) {
+            actionList += UnitAction(
-                unit.action = Constants.unitActionSleep
+                    name = "Sleep",
-                unitTable.selectedUnit = null
+                    canAct = !isSleeping,
-            }
+                    isCurrentAction = isSleeping,
+                    action = {
+                        unit.action = Constants.unitActionSleep
+                        unitTable.selectedUnit = null
+                    })
         }
 
-        if(unit.canFortify()) {
+        if (unit.canFortify()) {
-            actionList += UnitAction("Fortify", unit.currentMovement >0) {
+            actionList += UnitAction(
-                unit.fortify()
+                    name = "Fortify",
-                unitTable.selectedUnit = null
+                    canAct = unit.currentMovement > 0,
-            }.sound("fortify")
+                    uncivSound = UncivSound.Fortify,
+                    action = {
+                        unit.fortify()
+                        unitTable.selectedUnit = null
+                    })
         } else if (unit.isFortified()) {
             actionList += UnitAction(
-                    "Fortify",
+                    name = "Fortify",
-                    false,
+                    canAct = false,
-                    true,
+                    isCurrentAction = true,
-                    "Fortification".tr() + " " + unit.getFortificationTurns() * 20 + "%"
+                    title = "${"Fortification".tr()} ${unit.getFortificationTurns() * 20}%"
             )
         }
 
-        if(!unit.type.isAirUnit()){
+        if (!unit.type.isAirUnit()) {
-            if(unit.action != Constants.unitActionExplore)
+            if (unit.action != Constants.unitActionExplore) {
-                actionList += UnitAction("Explore",true) {
+                actionList += UnitAction(
-                    UnitAutomation().automatedExplore(unit)
+                        name = "Explore",
-                    unit.action = Constants.unitActionExplore
+                        canAct = true,
-                }
+                        action = {
-            else
+                            UnitAutomation().automatedExplore(unit)
-                actionList += UnitAction("Stop exploration", true) { unit.action = null }
+                            unit.action = Constants.unitActionExplore
+                        })
+            } else {
+                actionList += UnitAction(name = "Stop exploration", canAct = true)
+            }
         }
 
-        if(!unit.type.isCivilian() && unit.promotions.canBePromoted()) {
+        if (!unit.type.isCivilian() && unit.promotions.canBePromoted()) {
             // promotion does not consume movement points, so we can do it always
-            actionList += UnitAction("Promote", true) {
+            actionList += UnitAction(
-                UncivGame.Current.setScreen(PromotionPickerScreen(unit))
+                    name = "Promote",
-            }.sound("promote")
+                    canAct = true,
+                    uncivSound = UncivSound.Promote,
+                    action = {
+                        UncivGame.Current.setScreen(PromotionPickerScreen(unit))
+                    })
         }
 
-        if(unit.baseUnit().upgradesTo!=null && tile.getOwner()==unit.civInfo) {
+        if (unit.baseUnit().upgradesTo != null && tile.getOwner() == unit.civInfo) {
-
             if (unit.canUpgrade()) {
                 val goldCostOfUpgrade = unit.getCostOfUpgrade()
                 val upgradedUnit = unit.getUnitToUpgradeTo()
 
-                actionList += UnitAction("Upgrade to [${upgradedUnit.name}] ([$goldCostOfUpgrade] gold)",
+                actionList += UnitAction(
-                        unit.civInfo.gold >= goldCostOfUpgrade
+                        name = "Upgrade to [${upgradedUnit.name}] ([$goldCostOfUpgrade] gold)",
-                                && !unit.isEmbarked()
+                        canAct = unit.civInfo.gold >= goldCostOfUpgrade && !unit.isEmbarked() && unit.currentMovement == unit.getMaxMovement().toFloat(),
-                                && unit.currentMovement == unit.getMaxMovement().toFloat()
+                        uncivSound = UncivSound.Upgrade,
-                ) {
+                        action = {
-                    unit.civInfo.gold -= goldCostOfUpgrade
+                            unit.civInfo.gold -= goldCostOfUpgrade
-                    val unitTile = unit.getTile()
+                            val unitTile = unit.getTile()
-                    unit.destroy()
+                            unit.destroy()
-                    val newunit = unit.civInfo.placeUnitNearTile(unitTile.position, upgradedUnit.name)!!
+                            val newunit = unit.civInfo.placeUnitNearTile(unitTile.position, upgradedUnit.name)!!
-                    newunit.health = unit.health
+                            newunit.health = unit.health
-                    newunit.promotions = unit.promotions
+                            newunit.promotions = unit.promotions
 
-                    for(promotion in newunit.baseUnit.promotions)
+                            for (promotion in newunit.baseUnit.promotions)
-                        if(promotion !in newunit.promotions.promotions)
+                                if (promotion !in newunit.promotions.promotions)
-                            newunit.promotions.addPromotion(promotion, true)
+                                    newunit.promotions.addPromotion(promotion, true)
 
-                    newunit.updateUniques()
+                            newunit.updateUniques()
-                    newunit.updateVisibleTiles()
+                            newunit.updateVisibleTiles()
-                    newunit.currentMovement = 0f
+                            newunit.currentMovement = 0f
-                    worldScreen.shouldUpdate = true
+                            worldScreen.shouldUpdate = true
-                }.sound("upgrade")
+                        })
             }
         }
 
-        if(!unit.type.isCivilian() && tile.improvement !=null){
+        if (!unit.type.isCivilian() && tile.improvement != null) {
-            actionList += UnitAction("Pillage", unit.currentMovement>0 && canPillage(unit,tile))
+            actionList += UnitAction("Pillage", unit.currentMovement > 0 && canPillage(unit, tile))
             {
                 // http://well-of-souls.com/civ/civ5_improvements.html says that naval improvements are destroyed upon pilllage
                 //    and I can't find any other sources so I'll go with that
-                if(tile.isLand) {
+                if (tile.isLand) {
                     tile.improvementInProgress = tile.improvement
                     tile.turnsToImprovement = 2
                 }
                 tile.improvement = null
-                if(!unit.hasUnique("No movement cost to pillage")) unit.useMovementPoints(1f)
+                if (!unit.hasUnique("No movement cost to pillage")) unit.useMovementPoints(1f)
                 unit.healBy(25)
             }
         }
 
-        if(unit.hasUnique("Must set up to ranged attack") && !unit.isEmbarked()) {
+        if (unit.hasUnique("Must set up to ranged attack") && !unit.isEmbarked()) {
             val setUp = unit.action == "Set Up"
-            actionList+=UnitAction("Set up", unit.currentMovement >0 && !setUp, currentAction = setUp ) {
+            actionList += UnitAction(
-                unit.action=Constants.unitActionSetUp
+                    name = "Set up",
-                unit.useMovementPoints(1f)
+                    canAct = unit.currentMovement > 0 && !setUp,
-            }.sound("setup")
+                    isCurrentAction = setUp,
+                    uncivSound = UncivSound.Setup,
+                    action = {
+                        unit.action = Constants.unitActionSetUp
+                        unit.useMovementPoints(1f)
+                    })
         }
 
         if (unit.hasUnique("Founds a new city") && !unit.isEmbarked()) {
-            actionList += UnitAction("Found city",
+            actionList += UnitAction(
-                    unit.currentMovement >0 &&
+                    name = "Found city",
-                            !tile.getTilesInDistance(3).any { it.isCityCenter() })
+                    canAct = unit.currentMovement > 0 && !tile.getTilesInDistance(3).any { it.isCityCenter() },
-            {
+                    uncivSound = UncivSound.Chimes,
-                UncivGame.Current.settings.addCompletedTutorialTask("Found city")
+                    action = {
-                unit.civInfo.addCity(tile.position)
+                        UncivGame.Current.settings.addCompletedTutorialTask("Found city")
-                tile.improvement = null
+                        unit.civInfo.addCity(tile.position)
-                unit.destroy()
+                        tile.improvement = null
-            }.sound("chimes")
+                        unit.destroy()
+                    })
         }
 
         if (unit.hasUnique("Can build improvements on tiles") && !unit.isEmbarked()) {
-            actionList += UnitAction("Construct improvement",
+            actionList += UnitAction(
-                    unit.currentMovement > 0
+                    name = "Construct improvement",
-                            && !tile.isCityCenter()
+                    canAct = unit.currentMovement > 0
-                            && unit.civInfo.gameInfo.ruleSet.tileImprovements.values.any { tile.canBuildImprovement(it, unit.civInfo) },
+                             && !tile.isCityCenter()
-                    currentAction = unit.currentTile.hasImprovementInProgress()
+                             && unit.civInfo.gameInfo.ruleSet.tileImprovements.values.any { tile.canBuildImprovement(it, unit.civInfo) },
-            ) { worldScreen.game.setScreen(ImprovementPickerScreen(tile) { unitTable.selectedUnit = null }) }
+                    isCurrentAction = unit.currentTile.hasImprovementInProgress(),
+                    action = {
+                        worldScreen.game.setScreen(ImprovementPickerScreen(tile) { unitTable.selectedUnit = null })
+                    })
 
             if (Constants.unitActionAutomation == unit.action) {
-                actionList += UnitAction("Stop automation", true) { unit.action = null }
+                actionList += UnitAction(name = "Stop automation", canAct = true)
             } else {
-                actionList += UnitAction("Automate", unit.currentMovement > 0)
+                actionList += UnitAction(
-                {
+                        name = "Automate",
-                    unit.action = Constants.unitActionAutomation
+                        canAct = unit.currentMovement > 0,
-                    WorkerAutomation(unit).automateWorkerAction()
+                        action = {
-                }
+                            unit.action = Constants.unitActionAutomation
+                            WorkerAutomation(unit).automateWorkerAction()
+                        })
             }
         }
 
-        if(unit.hasUnique("Can construct roads")
+        if (unit.hasUnique("Can construct roads")
-                && tile.roadStatus==RoadStatus.None
+            && tile.roadStatus == RoadStatus.None
-                && tile.improvementInProgress != "Road"
+            && tile.improvementInProgress != "Road"
-                && tile.isLand
+            && tile.isLand
-                && unit.civInfo.tech.isResearched(RoadStatus.Road.improvement(unit.civInfo.gameInfo.ruleSet)!!.techRequired!!))
+            && unit.civInfo.tech.isResearched(RoadStatus.Road.improvement(unit.civInfo.gameInfo.ruleSet)!!.techRequired!!))
-            actionList+=UnitAction("Construct road", unit.currentMovement >0){
+            actionList += UnitAction("Construct road", unit.currentMovement > 0) {
-                tile.improvementInProgress="Road"
+                tile.improvementInProgress = "Road"
-                tile.turnsToImprovement=4
+                tile.turnsToImprovement = 4
             }
 
-        for(improvement in listOf("Fishing Boats","Oil well")) {
+        for (improvement in listOf("Fishing Boats", "Oil well")) {
             if (unit.hasUnique("May create improvements on water resources") && tile.resource != null
-                    && tile.isWater // because fishing boats can enter cities, and if there's oil in the city... ;)
+                && tile.isWater // because fishing boats can enter cities, and if there's oil in the city... ;)
-                    && tile.improvement==null
+                && tile.improvement == null
-                    && tile.getTileResource().improvement == improvement
+                && tile.getTileResource().improvement == improvement
-                    && unit.civInfo.tech.isResearched(unit.civInfo.gameInfo.ruleSet.tileImprovements[improvement]!!.techRequired!!)
+                && unit.civInfo.tech.isResearched(unit.civInfo.gameInfo.ruleSet.tileImprovements[improvement]!!.techRequired!!)
             )
-                actionList += UnitAction("Create [$improvement]", unit.currentMovement >0) {
+                actionList += UnitAction(
-                    tile.improvement = improvement
+                        name = "Create [$improvement]",
-                    unit.destroy()
+                        canAct = unit.currentMovement > 0,
-                }
+                        action = {
+                            tile.improvement = improvement
+                            unit.destroy()
+                        })
         }
 
-        for(unique in unit.getUniques().filter { it.startsWith("Can build improvement: ") }){
+        for (unique in unit.getUniques().filter { it.startsWith("Can build improvement: ") }) {
-            val improvementName = unique.replace("Can build improvement: ","")
+            val improvementName = unique.replace("Can build improvement: ", "")
-            actionList += UnitAction("Create [$improvementName]",
+            actionList += UnitAction(
-                    unit.currentMovement >0f && !tile.isWater && !tile.isCityCenter() && !tile.getLastTerrain().unbuildable
+                    name = "Create [$improvementName]",
-            ) {
+                    canAct = unit.currentMovement > 0f && !tile.isWater && !tile.isCityCenter() && !tile.getLastTerrain().unbuildable,
-                unit.getTile().terrainFeature=null // remove forest/jungle/marsh
+                    uncivSound = UncivSound.Chimes,
-                unit.getTile().improvement = improvementName
+                    action = {
-                unit.getTile().improvementInProgress = null
+                        unit.getTile().terrainFeature = null // remove forest/jungle/marsh
-                unit.getTile().turnsToImprovement = 0
+                        unit.getTile().improvement = improvementName
-                unit.destroy()
+                        unit.getTile().improvementInProgress = null
-            }.sound("chimes")
+                        unit.getTile().turnsToImprovement = 0
+                        unit.destroy()
+                    })
         }
 
 
         if (unit.name == "Great Scientist" && !unit.isEmbarked()) {
-            actionList += UnitAction("Hurry Research", unit.civInfo.tech.currentTechnologyName() != null
+            actionList += UnitAction(
-                    && unit.currentMovement >0
+                    name = "Hurry Research",
-            ) {
+                    canAct = unit.civInfo.tech.currentTechnologyName() != null && unit.currentMovement > 0,
-                unit.civInfo.tech.hurryResearch()
+                    uncivSound = UncivSound.Chimes,
-                unit.destroy()
+                    action = {
-            }.sound("chimes")
+                        unit.civInfo.tech.hurryResearch()
+                        unit.destroy()
+                    })
         }
 
         if (unit.hasUnique("Can start an 8-turn golden age") && !unit.isEmbarked()) {
-            actionList += UnitAction("Start Golden Age", unit.currentMovement >0
+            actionList += UnitAction(
-            ) {
+                    name = "Start Golden Age",
-                unit.civInfo.goldenAges.enterGoldenAge()
+                    canAct = unit.currentMovement > 0,
-                unit.destroy()
+                    uncivSound = UncivSound.Chimes,
-            }.sound("chimes")
+                    action = {
+                        unit.civInfo.goldenAges.enterGoldenAge()
+                        unit.destroy()
+                    })
         }
 
         if (unit.name == "Great Engineer" && !unit.isEmbarked()) {
-            val canHurryWonder = if(unit.currentMovement==0f || !tile.isCityCenter()) false
+            val canHurryWonder = if (unit.currentMovement == 0f || !tile.isCityCenter()) false
             else {
                 val currentConstruction = tile.getCity()!!.cityConstructions.getCurrentConstruction()
-                if(currentConstruction !is Building) false
+                if (currentConstruction !is Building) false
                 else currentConstruction.isWonder || currentConstruction.isNationalWonder
             }
-            actionList += UnitAction("Hurry Wonder",
+            actionList += UnitAction(
-                    canHurryWonder
+                    name = "Hurry Wonder",
-            ) {
+                    canAct = canHurryWonder,
-                tile.getCity()!!.cityConstructions.apply {
+                    uncivSound = UncivSound.Chimes,
-                    addProductionPoints(300 + 30 * tile.getCity()!!.population.population) //http://civilization.wikia.com/wiki/Great_engineer_(Civ5)
+                    action = {
-                    constructIfEnough()
+                        tile.getCity()!!.cityConstructions.apply {
-                }
+                            addProductionPoints(300 + 30 * tile.getCity()!!.population.population) //http://civilization.wikia.com/wiki/Great_engineer_(Civ5)
-                unit.destroy()
+                            constructIfEnough()
-            }.sound("chimes")
+                        }
+                        unit.destroy()
+                    })
         }
 
         if (unit.name == "Great Merchant" && !unit.isEmbarked()) {
-            val canConductTradeMission = tile.owningCity?.civInfo?.isCityState()== true
+            val canConductTradeMission = tile.owningCity?.civInfo?.isCityState() == true
-                    && tile.owningCity?.civInfo?.isAtWarWith(unit.civInfo)== false
+                                         && tile.owningCity?.civInfo?.isAtWarWith(unit.civInfo) == false
-                    && unit.currentMovement >0
+                                         && unit.currentMovement > 0
-            actionList += UnitAction("Conduct Trade Mission", canConductTradeMission
+            actionList += UnitAction(
-            ) {
+                    name = "Conduct Trade Mission",
-                // http://civilization.wikia.com/wiki/Great_Merchant_(Civ5)
+                    canAct = canConductTradeMission,
-                var goldEarned = (350 + 50 * unit.civInfo.getEra().ordinal) * unit.civInfo.gameInfo.gameParameters.gameSpeed.getModifier()
+                    uncivSound = UncivSound.Chimes,
-                if (unit.civInfo.policies.isAdopted("Commerce Complete"))
+                    action = {
-                    goldEarned *= 2
+                        // http://civilization.wikia.com/wiki/Great_Merchant_(Civ5)
-                unit.civInfo.gold += goldEarned.toInt()
+                        var goldEarned = (350 + 50 * unit.civInfo.getEra().ordinal) * unit.civInfo.gameInfo.gameParameters.gameSpeed.getModifier()
-                val relevantUnique = unit.getUniques().first { it.startsWith("Can undertake") }
+                        if (unit.civInfo.policies.isAdopted("Commerce Complete"))
-                val influenceEarned=Regex("\\d+").find(relevantUnique)!!.value.toInt()
+                            goldEarned *= 2
-                tile.owningCity!!.civInfo.getDiplomacyManager(unit.civInfo).influence += influenceEarned
+                        unit.civInfo.gold += goldEarned.toInt()
-                unit.civInfo.addNotification("Your trade mission to [${tile.owningCity!!.civInfo}] has earned you [${goldEarned.toInt()}] gold and [$influenceEarned] influence!",null, Color.GOLD)
+                        val relevantUnique = unit.getUniques().first { it.startsWith("Can undertake") }
-                unit.destroy()
+                        val influenceEarned = Regex("\\d+").find(relevantUnique)!!.value.toInt()
-            }.sound("chimes")
+                        tile.owningCity!!.civInfo.getDiplomacyManager(unit.civInfo).influence += influenceEarned
+                        unit.civInfo.addNotification("Your trade mission to [${tile.owningCity!!.civInfo}] has earned you [${goldEarned.toInt()}] gold and [$influenceEarned] influence!", null, Color.GOLD)
+                        unit.destroy()
+                    })
         }
 
-        actionList += UnitAction("Disband unit", unit.currentMovement >0
+        actionList += UnitAction(
-        ) {
+                name = "Disband unit",
-            val disbandText = if(unit.currentTile.getOwner()==unit.civInfo)
+                canAct = unit.currentMovement > 0,
-                "Disband this unit for [${unit.baseUnit.getDisbandGold()}] gold?".tr()
+                action = {
-            else "Do you really want to disband this unit?".tr()
+                    val disbandText = if (unit.currentTile.getOwner() == unit.civInfo)
-            YesNoPopupTable(disbandText,
+                        "Disband this unit for [${unit.baseUnit.getDisbandGold()}] gold?".tr()
-                    {unit.disband(); worldScreen.shouldUpdate=true} )
+                    else "Do you really want to disband this unit?".tr()
-        }
+                    YesNoPopupTable(disbandText, { unit.disband(); worldScreen.shouldUpdate = true })
+                })
 
         return actionList
     }
 
     fun canPillage(unit: MapUnit, tile: TileInfo): Boolean {
-        if(tile.improvement==null || tile.improvement==Constants.barbarianEncampment
+        if (tile.improvement == null || tile.improvement == Constants.barbarianEncampment
-                || tile.improvement=="City ruins") return false
+            || tile.improvement == "City ruins") return false
         val tileOwner = tile.getOwner()
         // Can't pillage friendly tiles, just like you can't attack them - it's an 'act of war' thing
-        return tileOwner==null || tileOwner==unit.civInfo || unit.civInfo.isAtWarWith(tileOwner)
+        return tileOwner == null || tileOwner == unit.civInfo || unit.civInfo.isAtWarWith(tileOwner)
     }
-
 }

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

@@ -8,6 +8,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.logic.map.MapUnit
+import com.unciv.models.UnitAction
 import com.unciv.ui.utils.*
 import com.unciv.ui.worldscreen.WorldScreen
 
@@ -67,10 +68,10 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table(){
     private fun getUnitActionButton(unitAction: UnitAction): Button {
         val actionButton = Button(CameraStageBaseScreen.skin)
         actionButton.add(getIconForUnitAction(unitAction.name)).size(20f).pad(5f)
-        val fontColor = if(unitAction.currentAction) Color.YELLOW else Color.WHITE
+        val fontColor = if(unitAction.isCurrentAction) Color.YELLOW else Color.WHITE
-        actionButton.add(unitAction.title.toLabel(fontColor)).pad(5f)
+        actionButton.add(unitAction.name.toLabel(fontColor)).pad(5f)
         actionButton.pack()
-        actionButton.onClick(unitAction.sound) { unitAction.action(); UncivGame.Current.worldScreen.shouldUpdate=true }
+        actionButton.onClick(unitAction.uncivSound) { unitAction.action?.invoke(); UncivGame.Current.worldScreen.shouldUpdate=true }
         if (!unitAction.canAct) actionButton.disable()
         return actionButton
     }

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

@@ -8,6 +8,7 @@ import com.unciv.logic.map.MapUnit
 import com.unciv.logic.map.TileInfo
 import com.unciv.logic.map.action.BuildLongRoadAction
 import com.unciv.logic.map.action.MapUnitAction
+import com.unciv.models.UncivSound
 import com.unciv.ui.utils.CameraStageBaseScreen
 import com.unciv.ui.utils.ImageGetter
 import com.unciv.ui.utils.Sounds
@@ -74,7 +75,7 @@ class UnitContextMenu(val tileMapHolder: TileMapHolder, val selectedUnit: MapUni
                     // 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)
-                    Sounds.play("whoosh")
+                    Sounds.play(UncivSound.Whoosh)
                     if (selectedUnit.currentTile != targetTile)
                         selectedUnit.action = "moveTo " + targetTile.position.x.toInt() + "," + targetTile.position.y.toInt()
                     if (selectedUnit.currentMovement > 0) {