Architectural update - Make animated menu reusable (#9685)

* Extract AnimatedMenuPopup from UnitUpgradeMenu to make its basic idea reusable

* Rebase UnitUpgradeMenu onto AnimatedMenuPopup

* Add SoundPlayer.playRepeated for future reusability

* Move UnitUpgradeMenu to popups package

* Reuse playRepeated in PromotionPickerScreen

* Reuse playRepeated in PromotionPickerScreen - clean up imports

core/src/com/unciv/ui/audio/SoundPlayer.kt

@@ -160,9 +160,12 @@ object SoundPlayer {
      * and lastly Unciv's own assets/sounds. Will fail silently if the sound file cannot be found.
      *
      * This will wait for the Stream to become ready (Android issue) if necessary, and do so on a
-     * separate thread. No new thread is created if the sound can be played immediately.
+     * separate thread. **No new thread is created** if the sound can be played immediately.
+     *
+     * That also means that it's the caller's responsibility to ensure calling this only on the GL thread.
      *
      * @param sound The sound to play
+     * @see playRepeated
      */
     fun play(sound: UncivSound) {
         val volume = UncivGame.Current.settings.soundEffectsVolume
@@ -177,4 +180,20 @@ object SoundPlayer {
             }
         }
     }
+
+    /** Play a sound repeatedly - e.g. to express that an action was applied multiple times or to multiple targets.
+     *
+     *  Runs the actual sound player decoupled on the GL thread unlike [SoundPlayer.play], which leaves that responsibility to the caller.
+     */
+    fun playRepeated(sound: UncivSound, count: Int = 2, delay: Long = 200) {
+        Concurrency.runOnGLThread {
+            SoundPlayer.play(sound)
+            if (count > 1) Concurrency.run {
+                repeat(count - 1) {
+                    delay(delay)
+                    Concurrency.runOnGLThread { SoundPlayer.play(sound) }
+                }
+            }
+        }
+    }
 }

core/src/com/unciv/ui/components/input/KeyboardBinding.kt

@@ -116,6 +116,7 @@ enum class KeyboardBinding(
     // Popups
     Confirm(Category.Popups, "Confirm Dialog", 'y'),
     Cancel(Category.Popups, "Cancel Dialog", 'n'),
+    UpgradeAll(Category.Popups, KeyCharAndCode.ctrl('a')),
     ;
     //endregion
 

core/src/com/unciv/ui/popups/AnimatedMenuPopup.kt

@@ -0,0 +1,182 @@
+package com.unciv.ui.popups
+
+import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.graphics.g2d.NinePatch
+import com.badlogic.gdx.math.Interpolation
+import com.badlogic.gdx.math.Vector2
+import com.badlogic.gdx.scenes.scene2d.Stage
+import com.badlogic.gdx.scenes.scene2d.Touchable
+import com.badlogic.gdx.scenes.scene2d.actions.Actions
+import com.badlogic.gdx.scenes.scene2d.ui.Container
+import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.scenes.scene2d.ui.TextButton
+import com.badlogic.gdx.scenes.scene2d.utils.NinePatchDrawable
+import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.components.input.KeyCharAndCode
+import com.unciv.ui.components.input.KeyboardBinding
+import com.unciv.ui.components.input.keyShortcuts
+import com.unciv.ui.components.input.onActivation
+import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.screens.basescreen.BaseScreen
+import com.unciv.utils.Concurrency
+
+/**
+ *  A popup menu that animates on open/close, centered on a given Position (unlike other [Popup]s which are always stage-centered).
+ *
+ *  You must provide content by overriding [createContentTable] - see its doc.
+ *
+ *  The Popup opens automatically once created. Meant to be used for small menus.
+ *  No default close button - recommended to simply use "click-behind".
+ *
+ *  The "click-behind" semi-transparent covering of the rest of the stage is much darker than a normal
+ *  Popup (give the impression to take away illumination and spotlight the menu) and fades in together
+ *  with the AnimatedMenuPopup itself. Closing the menu in any of the four ways will fade out everything
+ *  inverting the fade-and-scale-in. Callbacks registered with [Popup.closeListeners] will run before the animation starts.
+ *  Use [afterCloseCallback] instead if you need a notification after the animation finishes and the Popup is cleaned up.
+ *
+ *  @param stage The stage this will be shown on, passed to Popup and used for clamping **`position`**
+ *  @param position stage coordinates to show this centered over - clamped so that nothing is clipped outside the [stage]
+ */
+open class AnimatedMenuPopup(
+    stage: Stage,
+    position: Vector2
+) : Popup(stage, Scrollability.None) {
+    private val container: Container<Table> = Container()
+    private val animationDuration = 0.33f
+    private val backgroundColor = (background as NinePatchDrawable).patch.color
+    private val smallButtonStyle by lazy { SmallButtonStyle() }
+
+    /** Will be notified after this Popup is closed, the animation finished, and cleanup is done (removed from stage). */
+    var afterCloseCallback: (() -> Unit)? = null
+
+    /** Allows differentiating the close reason in [afterCloseCallback] or [closeListeners]
+     *  When still `false` in a callback, then ESC/BACK or the click-behind listener closed this. */
+    var anyButtonWasClicked = false
+        private set
+
+    /**
+     *  Provides the Popup content.
+     *
+     *  Call super to fetch an empty default with prepared padding and background.
+     *  You can use [getButton], which produces TextButtons slightly smaller than Unciv's default ones.
+     *  The content adding functions offered by [Popup] or [Table] won't work.
+     *  The content needs to be complete when the method finishes, it will be `pack()`ed and measured immediately.
+     */
+    open fun createContentTable() = Table().apply {
+        defaults().pad(5f, 15f, 5f, 15f).growX()
+        background = BaseScreen.skinStrings.getUiBackground("General/AnimatedMenu", BaseScreen.skinStrings.roundedEdgeRectangleShape, Color.DARK_GRAY)
+    }
+
+    init {
+        clickBehindToClose = true
+        keyShortcuts.add(KeyCharAndCode.BACK) { close() }
+        innerTable.remove()
+
+        // Decouple the content creation from object initialization so it can access its own fields
+        // (initialization order super->sub - see LeakingThis)
+        Concurrency.runOnGLThread { createAndShow(position) }
+    }
+
+    private fun createAndShow(position: Vector2) {
+        val newInnerTable = createContentTable()
+        newInnerTable.pack()
+        container.actor = newInnerTable
+        container.touchable = Touchable.childrenOnly
+        container.isTransform = true
+        container.setScale(0.05f)
+        container.color.a = 0f
+
+        open(true)  // this only does the screen-covering "click-behind" portion
+
+        container.setPosition(
+            position.x.coerceAtMost(stage.width - newInnerTable.width / 2),
+            position.y.coerceAtLeast(newInnerTable.height / 2)
+        )
+        super.addActor(container)
+
+        // This "zoomfades" the container "in"
+        container.addAction(
+            Actions.parallel(
+                Actions.scaleTo(1f, 1f, animationDuration, Interpolation.fade),
+                Actions.fadeIn(animationDuration, Interpolation.fade)
+            ))
+
+        // This gradually darkens the "outside" at the same time
+        backgroundColor.set(0)
+        super.addAction(Actions.alpha(0.35f, animationDuration, Interpolation.fade).apply {
+            color = backgroundColor
+        })
+    }
+
+    override fun close() {
+        val toNotify = closeListeners.toList()
+        closeListeners.clear()
+        for (listener in toNotify) listener()
+
+        addAction(Actions.alpha(0f, animationDuration, Interpolation.fade).apply {
+            color = backgroundColor
+        })
+        container.addAction(
+            Actions.sequence(
+                Actions.parallel(
+                    Actions.scaleTo(0.05f, 0.05f, animationDuration, Interpolation.fade),
+                    Actions.fadeOut(animationDuration, Interpolation.fade)
+                ),
+                Actions.run {
+                    container.remove()
+                    super.close()
+                    afterCloseCallback?.invoke()
+                }
+            )
+        )
+    }
+
+    /**
+     *  Creates a button - for use in [AnimatedMenuPopup]'s `contentBuilder` parameter.
+     *
+     *  On activation it will set [anyButtonWasClicked], call [action], then close the Popup.
+     */
+    fun getButton(text: String, binding: KeyboardBinding, action: () -> Unit) =
+        text.toTextButton(smallButtonStyle).apply {
+            onActivation(binding = binding) {
+                anyButtonWasClicked = true
+                action()
+                close()
+            }
+        }
+
+    class SmallButtonStyle : TextButton.TextButtonStyle(BaseScreen.skin[TextButton.TextButtonStyle::class.java]) {
+        /** Modify NinePatch geometry so the roundedEdgeRectangleMidShape button is 38f high instead of 48f,
+         *  Otherwise this excercise would be futile - normal roundedEdgeRectangleShape based buttons are 50f high.
+         */
+        private fun NinePatchDrawable.reduce(): NinePatchDrawable {
+            val patch = NinePatch(this.patch)
+            patch.padTop = 10f
+            patch.padBottom = 10f
+            patch.topHeight = 10f
+            patch.bottomHeight = 10f
+            return NinePatchDrawable(this).also { it.patch = patch }
+        }
+
+        init {
+            val upColor = BaseScreen.skin.getColor("color")
+            val downColor = BaseScreen.skin.getColor("pressed")
+            val overColor = BaseScreen.skin.getColor("highlight")
+            val disabledColor = BaseScreen.skin.getColor("disabled")
+            // UiElementDocsWriter inspects source, which is why this isn't prettified better
+            val shape = BaseScreen.run {
+                // Let's use _one_ skinnable background lookup but with different tints
+                val skinned = skinStrings.getUiBackground("AnimatedMenu/Button", skinStrings.roundedEdgeRectangleMidShape)
+                // Reduce height only if not skinned
+                val default = ImageGetter.getNinePatch(skinStrings.roundedEdgeRectangleMidShape)
+                if (skinned === default) default.reduce() else skinned
+            }
+            // Now get the tinted variants
+            up = shape.tint(upColor)
+            down = shape.tint(downColor)
+            over = shape.tint(overColor)
+            disabled = shape.tint(disabledColor)
+            disabledFontColor = Color.GRAY
+        }
+    }
+}

core/src/com/unciv/ui/popups/UnitUpgradeMenu.kt

@@ -0,0 +1,99 @@
+package com.unciv.ui.popups
+
+import com.badlogic.gdx.math.Vector2
+import com.badlogic.gdx.scenes.scene2d.Stage
+import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.unciv.logic.map.mapunit.MapUnit
+import com.unciv.models.UpgradeUnitAction
+import com.unciv.ui.audio.SoundPlayer
+import com.unciv.ui.components.extensions.pad
+import com.unciv.ui.components.input.KeyboardBinding
+import com.unciv.ui.objectdescriptions.BaseUnitDescriptions
+import com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsUpgrade
+
+/**
+ *  A popup menu showing info about an Unit upgrade, with buttons to upgrade "this" unit or _all_
+ *  similar units.
+ *
+ *  @param stage The stage this will be shown on, passed to Popup and used for clamping **`position`**
+ *  @param position stage coordinates to show this centered over - clamped so that nothing is clipped outside the [stage]
+ *  @param unit Who is ready to upgrade?
+ *  @param unitAction Holds pre-calculated info like unitToUpgradeTo, cost or resource requirements. Its action is mapped to the Upgrade button.
+ *  @param callbackAfterAnimation If true the following will be delayed until the Popup is actually closed (Stage.hasOpenPopups returns false).
+ *  @param onButtonClicked A callback after one or several upgrades have been performed (and the menu is about to close)
+ */
+/*
+    Note - callbackAfterAnimation has marginal value: When this is called from UnitOverview, where the
+    callback updates the upgrade symbol column, that can happen before/while the animation plays.
+    Called from the WorldScreen, to set shouldUpdate, that **needs** to fire late, or else the update is wasted.
+    Therefore, simplifying to always use afterCloseCallback would only be visible to the quick keen eye.
+ */
+class UnitUpgradeMenu(
+    stage: Stage,
+    position: Vector2,
+    private val unit: MapUnit,
+    private val unitAction: UpgradeUnitAction,
+    private val callbackAfterAnimation: Boolean = false,
+    private val onButtonClicked: () -> Unit
+) : AnimatedMenuPopup(stage, position) {
+
+    private val allUpgradableUnits: Sequence<MapUnit> by lazy {
+        unit.civ.units.getCivUnits()
+            .filter {
+                it.baseUnit.name == unit.baseUnit.name
+                    && it.currentMovement > 0f
+                    && it.currentTile.getOwner() == unit.civ
+                    && !it.isEmbarked()
+                    && it.upgrade.canUpgrade(unitAction.unitToUpgradeTo, ignoreResources = true)
+            }
+    }
+
+    init {
+        val action = {
+            if (anyButtonWasClicked) onButtonClicked()
+        }
+        if (callbackAfterAnimation) afterCloseCallback = action
+        else closeListeners.add(action)
+    }
+
+    override fun createContentTable(): Table {
+        val newInnerTable = BaseUnitDescriptions.getUpgradeInfoTable(
+            unitAction.title, unit.baseUnit, unitAction.unitToUpgradeTo
+        )
+        newInnerTable.row()
+        newInnerTable.add(getButton("Upgrade", KeyboardBinding.Upgrade, ::doUpgrade))
+            .pad(15f, 15f, 5f, 15f).growX().row()
+
+        val allCount = allUpgradableUnits.count()
+        if (allCount <= 1) return newInnerTable
+
+        // Note - all same-baseunit units cost the same to upgrade? What if a mod says e.g. 50% discount on Oasis?
+        // - As far as I can see the rest of the upgrading code doesn't support such conditions at the moment.
+        val allCost = unitAction.goldCostOfUpgrade * allCount
+        val allResources = unitAction.newResourceRequirements * allCount
+        val upgradeAllText = "Upgrade all [$allCount] [${unit.name}] ([$allCost] gold)"
+        val upgradeAllButton = getButton(upgradeAllText, KeyboardBinding.UpgradeAll, ::doAllUpgrade)
+        upgradeAllButton.isDisabled = unit.civ.gold < allCost ||
+            allResources.isNotEmpty() &&
+            unit.civ.getCivResourcesByName().run {
+                allResources.any {
+                    it.value > (this[it.key] ?: 0)
+                }
+            }
+        newInnerTable.add(upgradeAllButton).pad(2f, 15f).growX().row()
+        return newInnerTable
+    }
+
+    private fun doUpgrade() {
+        SoundPlayer.play(unitAction.uncivSound)
+        unitAction.action!!()
+    }
+
+    private fun doAllUpgrade() {
+        SoundPlayer.playRepeated(unitAction.uncivSound)
+        for (unit in allUpgradableUnits) {
+            val otherAction = UnitActionsUpgrade.getUpgradeAction(unit)
+            otherAction?.action?.invoke()
+        }
+    }
+}

core/src/com/unciv/ui/screens/overviewscreen/UnitOverviewTab.kt

@@ -32,6 +32,7 @@ import com.unciv.ui.components.extensions.toPrettyString
 import com.unciv.ui.components.input.onClick
 import com.unciv.ui.images.IconTextButton
 import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.popups.UnitUpgradeMenu
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.pickerscreens.PromotionPickerScreen
 import com.unciv.ui.screens.pickerscreens.UnitRenamePopup

core/src/com/unciv/ui/screens/overviewscreen/UnitUpgradeMenu.kt

@@ -1,217 +0,0 @@
-package com.unciv.ui.screens.overviewscreen
-
-import com.badlogic.gdx.graphics.Color
-import com.badlogic.gdx.graphics.g2d.NinePatch
-import com.badlogic.gdx.math.Interpolation
-import com.badlogic.gdx.math.Vector2
-import com.badlogic.gdx.scenes.scene2d.Stage
-import com.badlogic.gdx.scenes.scene2d.Touchable
-import com.badlogic.gdx.scenes.scene2d.actions.Actions
-import com.badlogic.gdx.scenes.scene2d.ui.Container
-import com.badlogic.gdx.scenes.scene2d.ui.Table
-import com.badlogic.gdx.scenes.scene2d.ui.TextButton
-import com.badlogic.gdx.scenes.scene2d.utils.NinePatchDrawable
-import com.unciv.logic.map.mapunit.MapUnit
-import com.unciv.models.UpgradeUnitAction
-import com.unciv.ui.audio.SoundPlayer
-import com.unciv.ui.components.input.KeyCharAndCode
-import com.unciv.ui.components.input.KeyboardBinding
-import com.unciv.ui.components.input.keyShortcuts
-import com.unciv.ui.components.input.onActivation
-import com.unciv.ui.components.extensions.pad
-import com.unciv.ui.components.extensions.toTextButton
-import com.unciv.ui.images.ImageGetter
-import com.unciv.ui.objectdescriptions.BaseUnitDescriptions
-import com.unciv.ui.popups.Popup
-import com.unciv.ui.screens.basescreen.BaseScreen
-import com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsUpgrade
-
-//TODO When this gets reused. e.g. from UnitActionsTable, move to another package.
-
-/**
- *  A popup menu showing info about an Unit upgrade, with buttons to upgrade "this" unit or _all_
- *  similar units.
- *
- *  Meant to animate "in" at a given position - unlike other [Popup]s which are always stage-centered.
- *  No close button - use "click-behind".
- *  The "click-behind" semi-transparent covering of the rest of the stage is much darker than a normal
- *  Popup (give the impression to take away illumination and spotlight the menu) and fades in together
- *  with the UnitUpgradeMenu itself. Closing the menu in any of the four ways will fade out everything
- *  inverting the fade-and-scale-in.
- *
- *  @param stage The stage this will be shown on, passed to Popup and used for clamping **`position`**
- *  @param position stage coortinates to show this centered over - clamped so that nothing is clipped outside the [stage]
- *  @param unit Who is ready to upgrade?
- *  @param unitAction Holds pre-calculated info like unitToUpgradeTo, cost or resource requirements. Its action is mapped to the Upgrade button.
- *  @param callbackAfterAnimation If true the following will be delayed until the Popup is actually closed (Stage.hasOpenPopups returns false).
- *  @param onButtonClicked A callback after one or several upgrades have been performed (and the menu is about to close)
- */
-class UnitUpgradeMenu(
-    stage: Stage,
-    position: Vector2,
-    private val unit: MapUnit,
-    private val unitAction: UpgradeUnitAction,
-    private val callbackAfterAnimation: Boolean = false,
-    private val onButtonClicked: () -> Unit
-) : Popup(stage, Scrollability.None) {
-    private val container: Container<Table>
-    private val allUpgradableUnits: Sequence<MapUnit>
-    private val animationDuration = 0.33f
-    private val backgroundColor = (background as NinePatchDrawable).patch.color
-    private var afterCloseCallback: (() -> Unit)? = null
-
-    init {
-        innerTable.remove()
-
-        // Note: getUpgradeInfoTable skins this as General/Tooltip, roundedEdgeRectangle, DARK_GRAY
-        // TODO - own skinnable path, possibly when tooltip use of getUpgradeInfoTable gets replaced
-        val newInnerTable = BaseUnitDescriptions.getUpgradeInfoTable(
-            unitAction.title, unit.baseUnit, unitAction.unitToUpgradeTo
-        )
-
-        newInnerTable.row()
-        val smallButtonStyle = SmallButtonStyle()
-        val upgradeButton = "Upgrade".toTextButton(smallButtonStyle)
-        upgradeButton.onActivation(::doUpgrade)
-        upgradeButton.keyShortcuts.add(KeyboardBinding.Confirm)
-        newInnerTable.add(upgradeButton).pad(15f, 15f, 5f, 15f).growX().row()
-
-        allUpgradableUnits = unit.civ.units.getCivUnits()
-            .filter {
-                it.baseUnit.name == unit.baseUnit.name
-                    && it.currentMovement > 0f
-                    && it.currentTile.getOwner() == unit.civ
-                    && !it.isEmbarked()
-                    && it.upgrade.canUpgrade(unitAction.unitToUpgradeTo, ignoreResources = true)
-            }
-        newInnerTable.tryAddUpgradeAllUnitsButton(smallButtonStyle)
-
-        clickBehindToClose = true
-        keyShortcuts.add(KeyCharAndCode.BACK) { close() }
-
-        newInnerTable.pack()
-        container = Container(newInnerTable)
-        container.touchable = Touchable.childrenOnly
-        container.isTransform = true
-        container.setScale(0.05f)
-        container.color.a = 0f
-
-        open(true)  // this only does the screen-covering "click-behind" portion
-
-        container.setPosition(
-            position.x.coerceAtMost(stage.width - newInnerTable.width / 2),
-            position.y.coerceAtLeast(newInnerTable.height / 2)
-        )
-        addActor(container)
-
-        container.addAction(
-            Actions.parallel(
-            Actions.scaleTo(1f, 1f, animationDuration, Interpolation.fade),
-            Actions.fadeIn(animationDuration, Interpolation.fade)
-        ))
-
-        backgroundColor.set(0)
-        addAction(Actions.alpha(0.35f, animationDuration, Interpolation.fade).apply {
-            color = backgroundColor
-        })
-    }
-
-    private fun Table.tryAddUpgradeAllUnitsButton(buttonStyle: TextButton.TextButtonStyle) {
-        val allCount = allUpgradableUnits.count()
-        if (allCount <= 1) return
-        // Note - all same-baseunit units cost the same to upgrade? What if a mod says e.g. 50% discount on Oasis?
-        // - As far as I can see the rest of the upgrading code doesn't support such conditions at the moment.
-        val allCost = unitAction.goldCostOfUpgrade * allCount
-        val allResources = unitAction.newResourceRequirements * allCount
-        val upgradeAllButton = "Upgrade all [$allCount] [${unit.name}] ([$allCost] gold)"
-            .toTextButton(buttonStyle)
-        upgradeAllButton.isDisabled = unit.civ.gold < allCost ||
-            allResources.isNotEmpty() &&
-            unit.civ.getCivResourcesByName().run {
-                allResources.any {
-                    it.value > (this[it.key] ?: 0)
-                }
-            }
-        upgradeAllButton.onActivation(::doAllUpgrade)
-        add(upgradeAllButton).pad(2f, 15f).growX().row()
-    }
-
-    private fun doUpgrade() {
-        SoundPlayer.play(unitAction.uncivSound)
-        unitAction.action!!()
-        launchCallbackAndClose()
-    }
-
-    private fun doAllUpgrade() {
-        stage.addAction(
-            Actions.sequence(
-            Actions.run { SoundPlayer.play(unitAction.uncivSound) },
-            Actions.delay(0.2f),
-            Actions.run { SoundPlayer.play(unitAction.uncivSound) }
-        ))
-        for (unit in allUpgradableUnits) {
-            val otherAction = UnitActionsUpgrade.getUpgradeAction(unit)
-            otherAction?.action?.invoke()
-        }
-        launchCallbackAndClose()
-    }
-
-    private fun launchCallbackAndClose() {
-        if (callbackAfterAnimation) afterCloseCallback = onButtonClicked
-        else onButtonClicked()
-        close()
-    }
-
-    override fun close() {
-        addAction(Actions.alpha(0f, animationDuration, Interpolation.fade).apply {
-            color = backgroundColor
-        })
-        container.addAction(
-            Actions.sequence(
-            Actions.parallel(
-                Actions.scaleTo(0.05f, 0.05f, animationDuration, Interpolation.fade),
-                Actions.fadeOut(animationDuration, Interpolation.fade)
-            ),
-            Actions.run {
-                container.remove()
-                super.close()
-                afterCloseCallback?.invoke()
-            }
-        ))
-    }
-
-    class SmallButtonStyle : TextButton.TextButtonStyle(BaseScreen.skin[TextButton.TextButtonStyle::class.java]) {
-        /** Modify NinePatch geometry so the roundedEdgeRectangleMidShape button is 38f high instead of 48f,
-         *  Otherwise this excercise would be futile - normal roundedEdgeRectangleShape based buttons are 50f high.
-         */
-        private fun NinePatchDrawable.reduce(): NinePatchDrawable {
-            val patch = NinePatch(this.patch)
-            patch.padTop = 10f
-            patch.padBottom = 10f
-            patch.topHeight = 10f
-            patch.bottomHeight = 10f
-            return NinePatchDrawable(this).also { it.patch = patch }
-        }
-
-        init {
-            val upColor = BaseScreen.skin.getColor("color")
-            val downColor = BaseScreen.skin.getColor("pressed")
-            val overColor = BaseScreen.skin.getColor("highlight")
-            val disabledColor = BaseScreen.skin.getColor("disabled")
-            // UiElementDocsWriter inspects source, which is why this isn't prettified better
-            val shape = BaseScreen.run {
-                // Let's use _one_ skinnable background lookup but with different tints
-                val skinned = skinStrings.getUiBackground("UnitUpgradeMenu/Button", skinStrings.roundedEdgeRectangleMidShape)
-                // Reduce height only if not skinned
-                val default = ImageGetter.getNinePatch(skinStrings.roundedEdgeRectangleMidShape)
-                if (skinned === default) default.reduce() else skinned
-            }
-            // Now get the tinted variants
-            up = shape.tint(upColor)
-            down = shape.tint(downColor)
-            over = shape.tint(overColor)
-            disabled = shape.tint(disabledColor)
-            disabledFontColor = Color.GRAY
-        }
-    }
-}

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

@@ -21,8 +21,6 @@ import com.unciv.ui.components.input.onDoubleClick
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.basescreen.RecreateOnResize
-import com.unciv.utils.Concurrency
-import kotlinx.coroutines.delay
 import kotlin.math.abs
 
 class PromotionPickerScreen(
@@ -95,19 +93,8 @@ class PromotionPickerScreen(
         // if user managed to click disabled button, still do nothing
         if (button == null || !button.isPickable) return
 
-        // Can't use stage.addAction as the screen is going to die immediately
         val path = tree.getPathTo(button.node.promotion)
-        if (path.size == 1) {
+        SoundPlayer.playRepeated(UncivSound.Promote, path.size.coerceAtMost(2))
-            Concurrency.runOnGLThread { SoundPlayer.play(UncivSound.Promote) }
-        } else {
-            Concurrency.runOnGLThread {
-                SoundPlayer.play(UncivSound.Promote)
-                Concurrency.run {
-                    delay(200)
-                    Concurrency.runOnGLThread { SoundPlayer.play(UncivSound.Promote) }
-                }
-            }
-        }
 
         for (promotion in path)
             unit.promotions.addPromotion(promotion.name)

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

@@ -16,7 +16,7 @@ import com.unciv.ui.components.input.keyShortcuts
 import com.unciv.ui.components.input.onActivation
 import com.unciv.ui.components.input.onRightClick
 import com.unciv.ui.images.IconTextButton
-import com.unciv.ui.screens.overviewscreen.UnitUpgradeMenu
+import com.unciv.ui.popups.UnitUpgradeMenu
 import com.unciv.ui.screens.worldscreen.WorldScreen
 
 class UnitActionsTable(val worldScreen: WorldScreen) : Table() {

docs/Modders/Creating-a-UI-skin.md

@@ -34,6 +34,7 @@ These shapes are used all over Unciv and can be replaced to make a lot of UI ele
 <!--- DO NOT REMOVE OR MODIFY THIS LINE UI_ELEMENT_TABLE_REGION -->
 | Directory | Name | Default shape | Image |
 |---|:---:|:---:|---|
+| AnimatedMenu/ | Button | roundedEdgeRectangleMid | |
 | CityScreen/ | CityPickerTable | roundedEdgeRectangle | |
 | CityScreen/CitizenManagementTable/ | AvoidCell | null | |
 | CityScreen/CitizenManagementTable/ | FocusCell | null | |
@@ -52,6 +53,7 @@ These shapes are used all over Unciv and can be replaced to make a lot of UI ele
 | CityScreen/ConstructionInfoTable/ | Background | null | |
 | CityScreen/ConstructionInfoTable/ | SelectedConstructionTable | null | |
 | CivilopediaScreen/ | EntryButton | null | |
+| General/ | AnimatedMenu | roundedEdgeRectangle | |
 | General/ | Border | null | |
 | General/ | ExpanderTab | null | |
 | General/ | HealthBar | null | |
@@ -103,7 +105,6 @@ These shapes are used all over Unciv and can be replaced to make a lot of UI ele
 | TechPickerScreen/ | ResearchedFutureTechColor | 127, 50, 0 | |
 | TechPickerScreen/ | ResearchedTechColor | 255, 215, 0 | |
 | TechPickerScreen/ | TechButtonIconsOutline | roundedEdgeRectangleSmall | |
-| UnitUpgradeMenu/ | Button | roundedEdgeRectangleMid | |
 | VictoryScreen/ | CivGroup | roundedEdgeRectangle | |
 | WorldScreen/ | AirUnitTable | null | |
 | WorldScreen/ | BattleTable | null | |