Long press support (#9558)

* Refactor input-related components into own package

* Unified input event system with common routing for keys and gestures

* Minor Linting

* Apply input system to World map right-click

* Replace UnitActionsTable Upgrade info tooltip with UnitUpgradeMenu

* Optimize ShortcutListener's full-stage scan

* Post-merge fixes

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

@@ -0,0 +1,58 @@
+package com.unciv.ui.components.input
+
+import com.unciv.models.UncivSound
+import com.unciv.ui.audio.SoundPlayer
+import com.unciv.utils.Concurrency
+
+typealias ActivationAction = () -> Unit
+
+// The delegation inheritance is only done to reduce the signature and limit clients to *our* add functions
+internal class ActivationActionMap : MutableMap<ActivationTypes, ActivationActionMap.ActivationActionList> by LinkedHashMap() {
+    // todo Old listener said "happens if there's a double (or more) click function but no single click" -
+    //      means when we register a single-click but the listener *only* reports a double, the registered single-click action is invoked.
+
+    class ActivationActionList(val sound: UncivSound) : MutableList<ActivationAction> by ArrayList()
+
+    fun add(
+        type: ActivationTypes,
+        sound: UncivSound,
+        noEquivalence: Boolean = false,
+        action: ActivationAction
+    ) {
+        getOrPut(type) { ActivationActionList(sound) }.add(action)
+        if (noEquivalence) return
+        for (other in ActivationTypes.equivalentValues(type)) {
+            getOrPut(other) { ActivationActionList(sound) }.add(action)
+        }
+    }
+
+    fun clear(type: ActivationTypes) {
+        if (containsKey(type)) remove(type)
+    }
+
+    fun clear(type: ActivationTypes, noEquivalence: Boolean) {
+        clear(type)
+        if (noEquivalence) return
+        for (other in ActivationTypes.equivalentValues(type)) {
+            clear(other)
+        }
+    }
+
+    fun clearGestures() {
+        for (type in ActivationTypes.gestures()) {
+            clear(type)
+        }
+    }
+
+    fun isNotEmpty() = any { it.value.isNotEmpty() }
+
+    fun activate(type: ActivationTypes): Boolean {
+        val actions = get(type) ?: return false
+        if (actions.isEmpty()) return false
+        if (actions.sound != UncivSound.Silent)
+            Concurrency.runOnGLThread("Sound") { SoundPlayer.play(actions.sound) }
+        for (action in actions)
+            action.invoke()
+        return true
+    }
+}

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

@@ -1,116 +1,128 @@
 package com.unciv.ui.components.input
 
-import com.badlogic.gdx.Gdx
-import com.badlogic.gdx.Input
 import com.badlogic.gdx.scenes.scene2d.Actor
-import com.badlogic.gdx.scenes.scene2d.Group
-import com.badlogic.gdx.scenes.scene2d.InputEvent
-import com.badlogic.gdx.scenes.scene2d.InputListener
 import com.badlogic.gdx.scenes.scene2d.Stage
+import com.badlogic.gdx.scenes.scene2d.utils.ChangeListener
 import com.badlogic.gdx.scenes.scene2d.utils.Disableable
 import com.unciv.models.UncivSound
-import com.unciv.ui.audio.SoundPlayer
-import com.unciv.utils.Concurrency
 
+/** Used to stop activation events if this returns `true`. */
+internal fun Actor.isActive(): Boolean = isVisible && ((this as? Disableable)?.isDisabled != true)
 
-fun Actor.addActivationAction(action: (() -> Unit)?) {
+/** Routes events from the listener to [ActorAttachments] */
-    if (action != null)
+internal fun Actor.activate(type: ActivationTypes = ActivationTypes.Tap): Boolean {
-        ActorAttachments.get(this).addActivationAction(action)
+    if (!isActive()) return false
+    val attachment = ActorAttachments.getOrNull(this) ?: return false
+    return attachment.activate(type)
 }
 
-fun Actor.removeActivationAction(action: (() -> Unit)?) {
+/** Accesses the [shortcut dispatcher][ActorKeyShortcutDispatcher] for your actor
-    if (action != null)
+ *  (creates one if the actor has none).
-        ActorAttachments.getOrNull(this)?.removeActivationAction(action)
+ *
-}
+ *  Note that shortcuts you add with handlers are routed directly, those without are routed to [onActivation] with type [ActivationTypes.Keystroke]. */
-
-fun Actor.isActive(): Boolean = isVisible && ((this as? Disableable)?.isDisabled != true)
-
-fun Actor.activate() {
-    if (isActive())
-        ActorAttachments.getOrNull(this)?.activate()
-}
-
-val Actor.keyShortcutsOrNull
-    get() = ActorAttachments.getOrNull(this)?.keyShortcuts
 val Actor.keyShortcuts
     get() = ActorAttachments.get(this).keyShortcuts
 
-fun Actor.onActivation(sound: UncivSound = UncivSound.Click, action: () -> Unit): Actor {
+/** Routes input events of type [type] to your handler [action].
-    addActivationAction {
+ *  Will also be activated for events [equivalent][ActivationTypes.isEquivalent] to [type] unless [noEquivalence] is `true`.
-        Concurrency.run("Sound") { SoundPlayer.play(sound) }
+ *  A [sound] will be played (concurrently) on activation unless you specify [UncivSound.Silent].
-        action()
+ *  @return `this` to allow chaining
-    }
+ */
+fun Actor.onActivation(
+    type: ActivationTypes,
+    sound: UncivSound = UncivSound.Click,
+    noEquivalence: Boolean = false,
+    action: ActivationAction
+): Actor {
+    ActorAttachments.get(this).addActivationAction(type, sound, noEquivalence, action)
     return this
 }
 
-fun Actor.onActivation(action: () -> Unit): Actor = onActivation(UncivSound.Click, action)
+/** Routes clicks and [keyboard shortcuts][keyShortcuts] to your handler [action].
+ *  A [sound] will be played (concurrently) on activation unless you specify [UncivSound.Silent].
+ *  @return `this` to allow chaining
+ */
+fun Actor.onActivation(sound: UncivSound = UncivSound.Click, action: ActivationAction): Actor =
+    onActivation(ActivationTypes.Tap, sound, action = action)
 
+/** Routes clicks and [keyboard shortcuts][keyShortcuts] to your handler [action].
+ *  A [Click sound][UncivSound.Click] will be played (concurrently).
+ *  @return `this` to allow chaining
+ */
+fun Actor.onActivation(action: ActivationAction): Actor =
+    onActivation(ActivationTypes.Tap, action = action)
 
-enum class DispatcherVetoResult { Accept, Skip, SkipWithChildren }
+/** Routes clicks to your handler [action], ignoring [keyboard shortcuts][keyShortcuts].
-typealias DispatcherVetoer = (associatedActor: Actor?, keyDispatcher: KeyShortcutDispatcher?) -> DispatcherVetoResult
+ *  A [sound] will be played (concurrently) on activation unless you specify [UncivSound.Silent].
+ *  @return `this` to allow chaining
+ */
+fun Actor.onClick(sound: UncivSound = UncivSound.Click, action: ActivationAction): Actor =
+    onActivation(ActivationTypes.Tap, sound, noEquivalence = true, action)
+
+/** Routes clicks to your handler [action], ignoring [keyboard shortcuts][keyShortcuts].
+ *  A [Click sound][UncivSound.Click] will be played (concurrently).
+ *  @return `this` to allow chaining
+ */
+fun Actor.onClick(action: ActivationAction): Actor =
+    onActivation(ActivationTypes.Tap, noEquivalence = true, action = action)
+
+/** Routes double-clicks to your handler [action].
+ *  A [sound] will be played (concurrently) on activation unless you specify [UncivSound.Silent].
+ *  @return `this` to allow chaining
+ */
+fun Actor.onDoubleClick(sound: UncivSound = UncivSound.Click, action: ActivationAction): Actor =
+    onActivation(ActivationTypes.Doubletap, sound, action = action)
+
+/** Routes right-clicks and long-presses to your handler [action].
+ *  These are treated as equivalent so both desktop and mobile can access the same functionality with methods common to the platform.
+ *  A [sound] will be played (concurrently) on activation unless you specify [UncivSound.Silent].
+ *  @return `this` to allow chaining
+ */
+fun Actor.onRightClick(sound: UncivSound = UncivSound.Click, action: ActivationAction): Actor =
+    onActivation(ActivationTypes.RightClick, sound, action = action)
+
+/** Routes long-presses (but not right-clicks) to your handler [action].
+ *  A [sound] will be played (concurrently) on activation unless you specify [UncivSound.Silent].
+ *  @return `this` to allow chaining
+ */
+@Suppress("unused")  // Just in case - for now, only onRightClick is used
+fun Actor.onLongPress(sound: UncivSound = UncivSound.Click, action: ActivationAction): Actor =
+    onActivation(ActivationTypes.Longpress, sound, noEquivalence = true, action)
+
+/** Clears activation actions for a specific [type], and, if [noEquivalence] is `true`,
+ *  its [equivalent][ActivationTypes.isEquivalent] types.
+ */
+@Suppress("unused")  // Just in case - for now, it's automatic clear via clearListener
+fun Actor.clearActivationActions(type: ActivationTypes, noEquivalence: Boolean = true) {
+    ActorAttachments.get(this).clearActivationActions(type, noEquivalence)
+}
 
 /**
  * Install shortcut dispatcher for this stage. It activates all actions associated with the
- * pressed key in [additionalShortcuts] (if specified) and all actors in the stage. It is
+ * pressed key in [additionalShortcuts] (if specified) and **all** actors in the stage - recursively.
- * possible to temporarily disable or veto some shortcut dispatchers by passing an appropriate
+ *
+ * It is possible to temporarily disable or veto some shortcut dispatchers by passing an appropriate
  * [dispatcherVetoerCreator] function. This function may return a [DispatcherVetoer], which
  * will then be used to evaluate all shortcut sources in the stage. This two-step vetoing
- * mechanism allows the callback ([dispatcherVetoerCreator]) perform expensive preparations
+ * mechanism allows the callback ([dispatcherVetoerCreator]) to perform expensive preparations
- * only one per keypress (doing them in the returned [DispatcherVetoer] would instead be once
+ * only once per keypress (doing them in the returned [DispatcherVetoer] would instead be once
  * per keypress/actor combination).
+ *
+ * Note - screens containing a TileGroupMap **should** supply a vetoer skipping that actor, or else
+ * the scanning Deque will be several thousand entries deep.
  */
-fun Stage.installShortcutDispatcher(additionalShortcuts: KeyShortcutDispatcher? = null, dispatcherVetoerCreator: (() -> DispatcherVetoer?)? = null) {
+fun Stage.installShortcutDispatcher(additionalShortcuts: KeyShortcutDispatcher? = null, dispatcherVetoerCreator: () -> DispatcherVetoer?) {
-    addListener(object: InputListener() {
+    addListener(KeyShortcutListener(actors.asSequence(), additionalShortcuts, dispatcherVetoerCreator))
-        override fun keyDown(event: InputEvent?, keycode: Int): Boolean {
+}
-            val key = when {
+
-                event == null ->
+private class OnChangeListener(val function: (event: ChangeEvent?) -> Unit): ChangeListener() {
-                    KeyCharAndCode.UNKNOWN
+    override fun changed(event: ChangeEvent?, actor: Actor?) {
-                Gdx.input.isKeyPressed(Input.Keys.CONTROL_LEFT) || Gdx.input.isKeyPressed(Input.Keys.CONTROL_RIGHT) ->
+        function(event)
-                    KeyCharAndCode.ctrlFromCode(event.keyCode)
+    }
-                else ->
+}
-                    KeyCharAndCode(event.keyCode)
+
-            }
+/** Attach a ChangeListener to [this] and route its changed event to [action] */
-
+fun Actor.onChange(action: (event: ChangeListener.ChangeEvent?) -> Unit): Actor {
-            if (key != KeyCharAndCode.UNKNOWN) {
+    this.addListener(OnChangeListener(action))
-                var dispatcherVetoer = when { dispatcherVetoerCreator != null -> dispatcherVetoerCreator() else -> null }
+    return this
-                if (dispatcherVetoer == null) dispatcherVetoer = { _, _ -> DispatcherVetoResult.Accept }
-
-                if (activate(key, dispatcherVetoer))
-                    return true
-                // Make both Enter keys equivalent.
-                if ((key == KeyCharAndCode.NUMPAD_ENTER && activate(KeyCharAndCode.RETURN, dispatcherVetoer))
-                    || (key == KeyCharAndCode.RETURN && activate(KeyCharAndCode.NUMPAD_ENTER, dispatcherVetoer)))
-                    return true
-                // Likewise always match Back to ESC.
-                if ((key == KeyCharAndCode.ESC && activate(KeyCharAndCode.BACK, dispatcherVetoer))
-                    || (key == KeyCharAndCode.BACK && activate(KeyCharAndCode.ESC, dispatcherVetoer)))
-                    return true
-            }
-
-            return false
-        }
-
-        private fun activate(key: KeyCharAndCode, dispatcherVetoer: DispatcherVetoer): Boolean {
-            val shortcutResolver = KeyShortcutDispatcher.Resolver(key)
-            val pendingActors = ArrayDeque<Actor>(actors.toList())
-
-            if (additionalShortcuts != null && dispatcherVetoer(null, additionalShortcuts) == DispatcherVetoResult.Accept)
-                shortcutResolver.updateFor(additionalShortcuts)
-
-            while (pendingActors.any()) {
-                val actor = pendingActors.removeFirst()
-                val shortcuts = actor.keyShortcutsOrNull
-                val vetoResult = dispatcherVetoer(actor, shortcuts)
-
-                if (shortcuts != null && vetoResult == DispatcherVetoResult.Accept)
-                    shortcutResolver.updateFor(shortcuts)
-                if (actor is Group && vetoResult != DispatcherVetoResult.SkipWithChildren)
-                    pendingActors.addAll(actor.children)
-            }
-
-            for (action in shortcutResolver.triggeredActions)
-                action()
-            return shortcutResolver.triggeredActions.any()
-        }
-    })
 }

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

@@ -0,0 +1,22 @@
+package com.unciv.ui.components.input
+
+import com.badlogic.gdx.scenes.scene2d.Actor
+import com.badlogic.gdx.scenes.scene2d.InputEvent
+import com.badlogic.gdx.scenes.scene2d.utils.ActorGestureListener
+
+class ActivationListener : ActorGestureListener(20f, 0.25f, 1.1f, Int.MAX_VALUE.toFloat()) {
+    // defaults are: halfTapSquareSize = 20, tapCountInterval = 0.4f, longPressDuration = 1.1f, maxFlingDelay = Integer.MAX_VALUE
+
+    override fun tap(event: InputEvent?, x: Float, y: Float, count: Int, button: Int) {
+        val actor = event?.listenerActor ?: return
+        val type = ActivationTypes.values().firstOrNull {
+            it.isGesture && it.tapCount == count && it.button == button
+        } ?: return
+        actor.activate(type)
+    }
+
+    override fun longPress(actor: Actor?, x: Float, y: Float): Boolean {
+        if (actor == null) return false
+        return actor.activate(ActivationTypes.Longpress)
+    }
+}

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

@@ -0,0 +1,32 @@
+package com.unciv.ui.components.input
+
+/** Formal encapsulation of the input interaction types */
+
+enum class ActivationTypes(
+    internal val tapCount: Int = 0,
+    internal val button: Int = 0,
+    internal val isGesture: Boolean = true,
+    private val equivalentTo: ActivationTypes? = null
+) {
+    Keystroke(isGesture = false),
+
+    Tap(1, equivalentTo = Keystroke),
+    Doubletap(2),
+    Tripletap(3),  // Just to clarify it ends here
+
+    RightClick(1, 1),
+    DoubleRightClick(1, 1),
+    Longpress(equivalentTo = RightClick),
+    ;
+
+    /** Checks whether two [ActivationTypes] are declared equivalent, e.g. [RightClick] and [Longpress]. */
+    internal fun isEquivalent(other: ActivationTypes) =
+        this == other.equivalentTo || other == this.equivalentTo
+
+    internal companion object {
+        fun equivalentValues(type: ActivationTypes) = values().asSequence()
+            .filter { it.isEquivalent(type) }
+        fun gestures() = values().asSequence()
+            .filter { it.isGesture }
+    }
+}

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

@@ -1,8 +1,7 @@
 package com.unciv.ui.components.input
 
 import com.badlogic.gdx.scenes.scene2d.Actor
-import com.badlogic.gdx.scenes.scene2d.InputEvent
+import com.unciv.models.UncivSound
-import com.badlogic.gdx.scenes.scene2d.utils.ClickListener
 
 internal class ActorAttachments private constructor(actor: Actor) {
     companion object {
@@ -21,38 +20,55 @@ internal class ActorAttachments private constructor(actor: Actor) {
         // Since 'keyShortcuts' has it anyway.
         get() = keyShortcuts.actor
 
-    private lateinit var activationActions: MutableList<() -> Unit>
+    private lateinit var activationActions: ActivationActionMap
-    private var clickActivationListener: ClickListener? = null
+    private var activationListener: ActivationListener? = null
 
+    /**
+     *  Keyboard dispatcher for the [actor] this is attached to.
+     *
+     *  Note the routing goes [KeyShortcutListener] -> [ActorKeyShortcutDispatcher] -> [ActivationActionMap],
+     *  meaning that shortcuts registered here _with_ explicit action parameter are independent of
+     *  other [activationActions] and do not reach [ActivationActionMap], only those added _without_
+     *  explicit action are routed through and get [ActivationTypes] equivalence to tap/click.
+     *
+     *  This also means the former are silent while the latter do the Click sound by default.
+     */
     val keyShortcuts = ActorKeyShortcutDispatcher(actor)
 
-    fun activate() {
+    fun activate(type: ActivationTypes): Boolean {
-        if (this::activationActions.isInitialized) {
+        if (!this::activationActions.isInitialized) return false
-            for (action in activationActions)
+        return activationActions.activate(type)
-                action()
-        }
     }
 
-    fun addActivationAction(action: () -> Unit) {
+    fun addActivationAction(
-        if (!this::activationActions.isInitialized) activationActions = mutableListOf()
+        type: ActivationTypes,
-        activationActions.add(action)
+        sound: UncivSound = UncivSound.Click,
+        noEquivalence: Boolean = false,
+        action: ActivationAction
+    ) {
+        if (!this::activationActions.isInitialized)
+            activationActions = ActivationActionMap()
 
-        if (clickActivationListener == null) {
+        else if (activationListener != null && activationListener !in actor.listeners) {
-            clickActivationListener = object: ClickListener() {
+            // We think our listener should be active but it isn't - Actor.clearListeners() was called.
-                override fun clicked(event: InputEvent?, x: Float, y: Float) {
+            // Decision: To keep existing code (which could have to call clearActivationActions otherwise),
-                    actor.activate()
+            // we start over clearing any registered actions using that listener.
-                }
+            actor.addListener(activationListener)
-            }
+            activationActions.clearGestures()
-            actor.addListener(clickActivationListener)
         }
+
+        activationActions.add(type, sound, noEquivalence, action)
+
+        if (!type.isGesture || activationListener != null) return
+        activationListener = ActivationListener()
+        actor.addListener(activationListener)
     }
 
-    fun removeActivationAction(action: () -> Unit) {
+    fun clearActivationActions(type: ActivationTypes, noEquivalence: Boolean = true) {
         if (!this::activationActions.isInitialized) return
-        activationActions.remove(action)
+        activationActions.clear(type, noEquivalence)
-        if (activationActions.none() && clickActivationListener != null) {
+        if (activationListener == null || activationActions.isNotEmpty()) return
-            actor.removeListener(clickActivationListener)
+        actor.removeListener(activationListener)
-            clickActivationListener = null
+        activationListener = null
-        }
     }
 }

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

@@ -7,11 +7,12 @@ import com.badlogic.gdx.scenes.scene2d.Actor
  * [activating][Actor.activate] the actor. However, other actions are possible too.
  */
 class ActorKeyShortcutDispatcher internal constructor(val actor: Actor): KeyShortcutDispatcher() {
-    fun add(shortcut: KeyShortcut?) = add(shortcut) { actor.activate() }
+    val action: ActivationAction = { actor.activate(ActivationTypes.Keystroke) }
-    fun add(binding: KeyboardBinding, priority: Int = 1) = add(binding, priority) { actor.activate() }
+    fun add(shortcut: KeyShortcut?) = add(shortcut, action)
-    fun add(key: KeyCharAndCode?) = add(key) { actor.activate() }
+    fun add(binding: KeyboardBinding, priority: Int = 1) = add(binding, priority, action)
-    fun add(char: Char?) = add(char) { actor.activate() }
+    fun add(key: KeyCharAndCode?) = add(key, action)
-    fun add(keyCode: Int?) = add(keyCode) { actor.activate() }
+    fun add(char: Char?) = add(char, action)
+    fun add(keyCode: Int?) = add(keyCode, action)
 
     override fun isActive(): Boolean = actor.isActive()
 }

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

@@ -1,46 +0,0 @@
-package com.unciv.ui.components.input
-
-import com.badlogic.gdx.scenes.scene2d.Actor
-import com.badlogic.gdx.scenes.scene2d.InputEvent
-import com.badlogic.gdx.scenes.scene2d.utils.ChangeListener
-import com.unciv.models.UncivSound
-
-// 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: UncivSound = UncivSound.Click, tapCount: Int = 1, tapInterval: Float = 0.0f, function: () -> Unit) {
-    onClickEvent(sound, tapCount, tapInterval) { _, _, _ -> function() }
-}
-
-/** same as [onClick], but sends the [InputEvent] and coordinates along */
-fun Actor.onClickEvent(sound: UncivSound = UncivSound.Click,
-                       tapCount: Int = 1,
-                       tapInterval: Float = 0.0f,
-                       function: (event: InputEvent?, x: Float, y: Float) -> Unit) {
-    val previousListener = this.listeners.firstOrNull { it is OnClickListener }
-    if (previousListener != null && previousListener is OnClickListener) {
-        previousListener.addClickFunction(sound, tapCount, function)
-        previousListener.setTapCountInterval(tapInterval)
-    } else {
-        this.addListener(OnClickListener(sound, function, tapCount, tapInterval))
-    }
-}
-
-fun Actor.onClick(function: () -> Unit): Actor {
-    onClick(UncivSound.Click, 1, 0f, function)
-    return this
-}
-
-fun Actor.onDoubleClick(sound: UncivSound = UncivSound.Click, tapInterval: Float = 0.25f, function: () -> Unit): Actor {
-    onClick(sound, 2, tapInterval, function)
-    return this
-}
-
-class OnChangeListener(val function: (event: ChangeEvent?) -> Unit): ChangeListener(){
-    override fun changed(event: ChangeEvent?, actor: Actor?) {
-        function(event)
-    }
-}
-
-fun Actor.onChange(function: (event: ChangeListener.ChangeEvent?) -> Unit): Actor {
-    this.addListener(OnChangeListener(function))
-    return this
-}

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

@@ -11,32 +11,32 @@ open class KeyShortcutDispatcher {
         override fun toString() = if (binding.hidden) "$key@$priority" else "$binding@$priority"
     }
 
-    private data class ShortcutAction(val shortcut: KeyShortcut, val action: () -> Unit)
+    private data class ShortcutAction(val shortcut: KeyShortcut, val action: ActivationAction)
     private val shortcuts: MutableList<ShortcutAction> = mutableListOf()
 
     fun clear() = shortcuts.clear()
 
-    fun add(shortcut: KeyShortcut?, action: (() -> Unit)?) {
+    fun add(shortcut: KeyShortcut?, action: ActivationAction?) {
         if (action == null || shortcut == null) return
         shortcuts.removeIf { it.shortcut == shortcut }
         shortcuts.add(ShortcutAction(shortcut, action))
     }
 
-    fun add(binding: KeyboardBinding, priority: Int = 1, action: (() -> Unit)?) {
+    fun add(binding: KeyboardBinding, priority: Int = 1, action: ActivationAction?) {
         add(KeyShortcut(binding, KeyCharAndCode.UNKNOWN, priority), action)
     }
 
-    fun add(key: KeyCharAndCode?, action: (() -> Unit)?) {
+    fun add(key: KeyCharAndCode?, action: ActivationAction?) {
         if (key != null)
             add(KeyShortcut(KeyboardBinding.None, key), action)
     }
 
-    fun add(char: Char?, action: (() -> Unit)?) {
+    fun add(char: Char?, action: ActivationAction?) {
         if (char != null)
             add(KeyCharAndCode(char), action)
     }
 
-    fun add(keyCode: Int?, action: (() -> Unit)?) {
+    fun add(keyCode: Int?, action: ActivationAction?) {
         if (keyCode != null)
             add(KeyCharAndCode(keyCode), action)
     }
@@ -66,7 +66,7 @@ open class KeyShortcutDispatcher {
 
     class Resolver(val key: KeyCharAndCode) {
         private var priority = Int.MIN_VALUE
-        val triggeredActions: MutableList<() -> Unit> = mutableListOf()
+        val triggeredActions: MutableList<ActivationAction> = mutableListOf()
 
         fun updateFor(dispatcher: KeyShortcutDispatcher) {
             if (!dispatcher.isActive()) return

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

@@ -0,0 +1,40 @@
+package com.unciv.ui.components.input
+
+import com.badlogic.gdx.scenes.scene2d.Actor
+import com.unciv.ui.components.tilegroups.TileGroupMap
+import com.unciv.ui.screens.basescreen.BaseScreen
+
+/**
+ * A lambda testing for a given *associatedActor* whether the shortcuts in *keyDispatcher*
+ * should be processed and whether the deep scan for child actors should continue.
+ * *associatedActor* == `null` means *keyDispatcher* is *BaseScreen.globalShortcuts*
+ */
+typealias DispatcherVetoer = (associatedActor: Actor?, keyDispatcher: KeyShortcutDispatcher?) -> KeyShortcutDispatcherVeto.DispatcherVetoResult
+
+object KeyShortcutDispatcherVeto {
+    enum class DispatcherVetoResult { Accept, Skip, SkipWithChildren }
+
+    internal val defaultDispatcherVetoer: DispatcherVetoer = { _, _ -> DispatcherVetoResult.Accept }
+
+    /** When a Popup ([activePopup]) is active, this creates a [DispatcherVetoer] that disables all
+     *  shortcuts on actors outside the popup and also the global shortcuts on the screen itself.
+     */
+    fun createPopupBasedDispatcherVetoer(activePopup: Actor): DispatcherVetoer? {
+        return { associatedActor: Actor?, _: KeyShortcutDispatcher? ->
+            when {
+                associatedActor == null -> DispatcherVetoResult.Skip
+                associatedActor.isDescendantOf(activePopup) -> DispatcherVetoResult.Accept
+                else -> DispatcherVetoResult.SkipWithChildren
+            }
+        }
+    }
+
+    /** Return this from [BaseScreen.getShortcutDispatcherVetoer] for Screens containing a [TileGroupMap] */
+    fun createTileGroupMapDispatcherVetoer(): DispatcherVetoer {
+        return { associatedActor: Actor?, _: KeyShortcutDispatcher? ->
+            if (associatedActor is TileGroupMap<*>) DispatcherVetoResult.SkipWithChildren
+            else DispatcherVetoResult.Accept
+        }
+    }
+
+}

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

@@ -0,0 +1,70 @@
+package com.unciv.ui.components.input
+
+import com.badlogic.gdx.Gdx
+import com.badlogic.gdx.Input
+import com.badlogic.gdx.scenes.scene2d.Actor
+import com.badlogic.gdx.scenes.scene2d.Group
+import com.badlogic.gdx.scenes.scene2d.InputEvent
+import com.badlogic.gdx.scenes.scene2d.InputListener
+import com.unciv.ui.components.input.KeyShortcutDispatcherVeto.DispatcherVetoResult
+
+
+/** @see installShortcutDispatcher */
+ class KeyShortcutListener(
+    private val actors: Sequence<Actor>,
+    private val additionalShortcuts: KeyShortcutDispatcher? = null,
+    private val dispatcherVetoerCreator: () -> DispatcherVetoer?
+) : InputListener() {
+
+    override fun keyDown(event: InputEvent?, keycode: Int): Boolean {
+        val key = when {
+            event == null ->
+                KeyCharAndCode.UNKNOWN
+            Gdx.input.isKeyPressed(Input.Keys.CONTROL_LEFT) || Gdx.input.isKeyPressed(Input.Keys.CONTROL_RIGHT) ->
+                KeyCharAndCode.ctrlFromCode(event.keyCode)
+            else ->
+                KeyCharAndCode(event.keyCode)
+        }
+        if (key == KeyCharAndCode.UNKNOWN) return false
+
+        val dispatcherVetoer = dispatcherVetoerCreator()
+            ?: KeyShortcutDispatcherVeto.defaultDispatcherVetoer
+        if (activate(key, dispatcherVetoer))
+            return true
+
+        // Make both Enter keys equivalent.
+        if ((key == KeyCharAndCode.NUMPAD_ENTER && activate(KeyCharAndCode.RETURN, dispatcherVetoer))
+            || (key == KeyCharAndCode.RETURN && activate(KeyCharAndCode.NUMPAD_ENTER, dispatcherVetoer)))
+            return true
+        // Likewise always match Back to ESC.
+        if ((key == KeyCharAndCode.ESC && activate(KeyCharAndCode.BACK, dispatcherVetoer))
+            || (key == KeyCharAndCode.BACK && activate(KeyCharAndCode.ESC, dispatcherVetoer)))
+            return true
+
+        return false
+    }
+
+    private fun activate(key: KeyCharAndCode, dispatcherVetoer: DispatcherVetoer): Boolean {
+        val shortcutResolver = KeyShortcutDispatcher.Resolver(key)
+        val pendingActors = ArrayDeque(actors.toList())
+
+        if (additionalShortcuts != null && dispatcherVetoer(null, additionalShortcuts) == DispatcherVetoResult.Accept)
+            shortcutResolver.updateFor(additionalShortcuts)
+
+        while (true) {
+            val actor = pendingActors.removeFirstOrNull()
+                ?: break
+            val shortcuts = ActorAttachments.getOrNull(actor)?.keyShortcuts
+            val vetoResult = dispatcherVetoer(actor, shortcuts)
+
+            if (shortcuts != null && vetoResult == DispatcherVetoResult.Accept)
+                shortcutResolver.updateFor(shortcuts)
+            if (actor is Group && vetoResult != DispatcherVetoResult.SkipWithChildren)
+                pendingActors.addAll(actor.children)
+        }
+
+        for (action in shortcutResolver.triggeredActions)
+            action()
+        return shortcutResolver.triggeredActions.isNotEmpty()
+    }
+}

core/src/com/unciv/ui/screens/basescreen/BaseScreen.kt

@@ -20,11 +20,13 @@ import com.unciv.models.TutorialTrigger
 import com.unciv.models.skins.SkinStrings
 import com.unciv.ui.components.Fonts
 import com.unciv.ui.components.input.KeyShortcutDispatcher
-import com.unciv.ui.components.input.DispatcherVetoResult
 import com.unciv.ui.components.input.DispatcherVetoer
 import com.unciv.ui.components.input.installShortcutDispatcher
+import com.unciv.ui.components.input.keyShortcuts
 import com.unciv.ui.components.extensions.isNarrowerThan4to3
+import com.unciv.ui.components.input.KeyShortcutDispatcherVeto
 import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.popups.Popup
 import com.unciv.ui.popups.activePopup
 import com.unciv.ui.popups.options.OptionsPopup
 
@@ -37,7 +39,7 @@ abstract class BaseScreen : Screen {
 
     /**
      * Keyboard shortcuts global to the screen. While this is public and can be modified,
-     * you most likely should use [keyShortcuts][Actor.keyShortcuts] on appropriate [Actor] instead.
+     * you most likely should use [keyShortcuts] on the appropriate [Actor] instead.
      */
     val globalShortcuts = KeyShortcutDispatcher()
 
@@ -54,22 +56,20 @@ abstract class BaseScreen : Screen {
             stage.setDebugParentUnderMouse(true)
         }
 
-        stage.installShortcutDispatcher(globalShortcuts, this::createPopupBasedDispatcherVetoer)
+        @Suppress("LeakingThis")
+        stage.installShortcutDispatcher(globalShortcuts, this::createDispatcherVetoer)
     }
 
-    private fun createPopupBasedDispatcherVetoer(): DispatcherVetoer? {
+    /** Hook allowing derived Screens to supply a key shortcut vetoer that can exclude parts of the
+     *  Stage Actor hierarchy from the search. Only called if no [Popup] is active.
+     *  @see installShortcutDispatcher
+     */
+    open fun getShortcutDispatcherVetoer(): DispatcherVetoer? = null
+
+    private fun createDispatcherVetoer(): DispatcherVetoer? {
         val activePopup = this.activePopup
-        if (activePopup == null)
+            ?: return getShortcutDispatcherVetoer()
-            return null
+        return KeyShortcutDispatcherVeto.createPopupBasedDispatcherVetoer(activePopup)
-        else {
-            // When any popup is active, disable all shortcuts on actor outside the popup
-            // and also the global shortcuts on the screen itself.
-            return { associatedActor: Actor?, _: KeyShortcutDispatcher? ->
-                when { associatedActor == null -> DispatcherVetoResult.Skip
-                       associatedActor.isDescendantOf(activePopup) -> DispatcherVetoResult.Accept
-                       else -> DispatcherVetoResult.SkipWithChildren }
-            }
-        }
     }
 
     override fun show() {}

core/src/com/unciv/ui/screens/cityscreen/CityScreen.kt

@@ -30,6 +30,7 @@ import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.input.onDoubleClick
 import com.unciv.ui.components.extensions.packIfNeeded
 import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.components.input.KeyShortcutDispatcherVeto
 import com.unciv.ui.components.tilegroups.CityTileGroup
 import com.unciv.ui.components.tilegroups.CityTileState
 import com.unciv.ui.components.tilegroups.TileGroupMap
@@ -341,6 +342,9 @@ class CityScreen(
         mapScrollPane.updateVisualScroll()
     }
 
+    // We contain a map...
+    override fun getShortcutDispatcherVetoer() = KeyShortcutDispatcherVeto.createTileGroupMapDispatcherVetoer()
+
     private fun tileWorkedIconOnClick(tileGroup: CityTileGroup, city: City) {
 
         if (!canChangeState || city.isPuppet) return

core/src/com/unciv/ui/screens/mainmenuscreen/MainMenuScreen.kt

@@ -30,6 +30,7 @@ import com.unciv.ui.components.input.keyShortcuts
 import com.unciv.ui.components.input.onActivation
 import com.unciv.ui.components.extensions.surroundWithCircle
 import com.unciv.ui.components.extensions.toLabel
+import com.unciv.ui.components.input.KeyShortcutDispatcherVeto
 import com.unciv.ui.components.tilegroups.TileGroupMap
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.popups.Popup
@@ -347,4 +348,7 @@ class MainMenuScreen: BaseScreen(), RecreateOnResize {
         stopBackgroundMapGeneration()
         return MainMenuScreen()
     }
+
+    // We contain a map...
+    override fun getShortcutDispatcherVetoer() = KeyShortcutDispatcherVeto.createTileGroupMapDispatcherVetoer()
 }

core/src/com/unciv/ui/screens/mapeditorscreen/MapEditorScreen.kt

@@ -27,6 +27,7 @@ import com.unciv.ui.popups.ConfirmPopup
 import com.unciv.ui.components.tilegroups.TileGroup
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.components.input.KeyCharAndCode
+import com.unciv.ui.components.input.KeyShortcutDispatcherVeto
 import com.unciv.ui.components.input.KeyboardPanningListener
 import com.unciv.ui.images.ImageWithCustomSize
 import com.unciv.ui.popups.ToastPopup
@@ -206,6 +207,9 @@ class MapEditorScreen(map: TileMap? = null): BaseScreen(), RecreateOnResize {
         return newHolder
     }
 
+    // We contain a map...
+    override fun getShortcutDispatcherVetoer() = KeyShortcutDispatcherVeto.createTileGroupMapDispatcherVetoer()
+
     fun loadMap(map: TileMap, newRuleset: Ruleset? = null, selectPage: Int = 0) {
         clearOverlayImages()
         mapHolder.remove()

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

@@ -35,7 +35,7 @@ import com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsUpgrade
  *  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 (geve the impression to take away illumination and spotlight the menu) and fades in together
+ *  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.
  *
@@ -43,6 +43,7 @@ import com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsUpgrade
  *  @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(
@@ -50,12 +51,14 @@ class UnitUpgradeMenu(
     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()
@@ -136,8 +139,7 @@ class UnitUpgradeMenu(
     private fun doUpgrade() {
         SoundPlayer.play(unitAction.uncivSound)
         unitAction.action!!()
-        onButtonClicked()
+        launchCallbackAndClose()
-        close()
     }
 
     private fun doAllUpgrade() {
@@ -151,7 +153,12 @@ class UnitUpgradeMenu(
             val otherAction = UnitActionsUpgrade.getUpgradeAction(unit)
             otherAction?.action?.invoke()
         }
-        onButtonClicked()
+        launchCallbackAndClose()
+    }
+
+    private fun launchCallbackAndClose() {
+        if (callbackAfterAnimation) afterCloseCallback = onButtonClicked
+        else onButtonClicked()
         close()
     }
 
@@ -168,6 +175,7 @@ class UnitUpgradeMenu(
             Actions.run {
                 container.remove()
                 super.close()
+                afterCloseCallback?.invoke()
             }
         ))
     }

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

@@ -95,10 +95,10 @@ class PolicyButton(viewingCiv: Civilization, canChangeState: Boolean, val policy
     }
 
     fun onClick(function: () -> Unit): PolicyButton {
-        (this as Actor).onClick(function = {
+        (this as Actor).onClick {
             function()
             updateState()
-        })
+        }
         return this
     }
 

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

@@ -9,11 +9,9 @@ import com.badlogic.gdx.math.Vector2
 import com.badlogic.gdx.scenes.scene2d.Action
 import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.Group
-import com.badlogic.gdx.scenes.scene2d.InputEvent
 import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.actions.Actions
 import com.badlogic.gdx.scenes.scene2d.ui.Table
-import com.badlogic.gdx.scenes.scene2d.utils.ClickListener
 import com.badlogic.gdx.utils.Align
 import com.unciv.Constants
 import com.unciv.UncivGame
@@ -35,17 +33,18 @@ import com.unciv.models.helpers.MiscArrowTypes
 import com.unciv.models.ruleset.unique.LocalUniqueCache
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.ui.audio.SoundPlayer
-import com.unciv.ui.components.input.KeyCharAndCode
 import com.unciv.ui.components.UnitGroup
 import com.unciv.ui.components.ZoomableScrollPane
 import com.unciv.ui.components.extensions.center
 import com.unciv.ui.components.extensions.colorFromRGB
 import com.unciv.ui.components.extensions.darken
+import com.unciv.ui.components.extensions.surroundWithCircle
+import com.unciv.ui.components.extensions.toLabel
+import com.unciv.ui.components.input.KeyCharAndCode
 import com.unciv.ui.components.input.keyShortcuts
 import com.unciv.ui.components.input.onActivation
 import com.unciv.ui.components.input.onClick
-import com.unciv.ui.components.extensions.surroundWithCircle
+import com.unciv.ui.components.input.onRightClick
-import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.tilegroups.TileGroup
 import com.unciv.ui.components.tilegroups.TileGroupMap
 import com.unciv.ui.components.tilegroups.TileSetStrings
@@ -54,8 +53,8 @@ import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.basescreen.UncivStage
 import com.unciv.ui.screens.worldscreen.bottombar.BattleTableHelpers.battleAnimation
-import com.unciv.utils.Log
 import com.unciv.utils.Concurrency
+import com.unciv.utils.Log
 import com.unciv.utils.launchOnGLThread
 import java.lang.Float.max
 
@@ -131,19 +130,13 @@ class WorldMapHolder(
                 continue
 
             // Right mouse click listener
-            tileGroup.addListener(object : ClickListener() {
+            tileGroup.onRightClick {
-                init {
+                val unit = worldScreen.bottomUnitTable.selectedUnit
-                    button = Input.Buttons.RIGHT
+                    ?: return@onRightClick
+                Concurrency.run("WorldScreenClick") {
+                    onTileRightClicked(unit, tileGroup.tile)
                 }
-
+            }
-                override fun clicked(event: InputEvent?, x: Float, y: Float) {
-                    val unit = worldScreen.bottomUnitTable.selectedUnit
-                        ?: return
-                    Concurrency.run("WorldScreenClick") {
-                        onTileRightClicked(unit, tileGroup.tile)
-                    }
-                }
-            })
         }
         actor = tileGroupMap
         setSize(worldScreen.stage.width, worldScreen.stage.height)

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

@@ -33,6 +33,7 @@ import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.setFontSize
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.components.input.KeyShortcutDispatcherVeto
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.popups.AuthPopup
 import com.unciv.ui.popups.Popup
@@ -284,6 +285,9 @@ class WorldScreen(
         stage.addListener(KeyboardPanningListener(mapHolder, allowWASD = true))
     }
 
+    // We contain a map...
+    override fun getShortcutDispatcherVetoer() = KeyShortcutDispatcherVeto.createTileGroupMapDispatcherVetoer()
+
     private suspend fun loadLatestMultiplayerState(): Unit = coroutineScope {
         if (game.screen != this@WorldScreen) return@coroutineScope // User already went somewhere else
 

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

@@ -4,22 +4,20 @@ import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.math.Vector2
 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.GUI
 import com.unciv.UncivGame
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.models.UnitAction
 import com.unciv.models.UnitActionType
 import com.unciv.models.UpgradeUnitAction
-import com.unciv.ui.components.UncivTooltip
 import com.unciv.ui.components.UncivTooltip.Companion.addTooltip
 import com.unciv.ui.components.extensions.disable
 import com.unciv.ui.components.input.KeyboardBindings
 import com.unciv.ui.components.input.keyShortcuts
 import com.unciv.ui.components.input.onActivation
-import com.unciv.ui.components.extensions.packIfNeeded
+import com.unciv.ui.components.input.onRightClick
 import com.unciv.ui.images.IconTextButton
-import com.unciv.ui.objectdescriptions.BaseUnitDescriptions
+import com.unciv.ui.screens.overviewscreen.UnitUpgradeMenu
 import com.unciv.ui.screens.worldscreen.WorldScreen
 
 class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
@@ -30,12 +28,13 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
         if (!worldScreen.canChangeState) return // No actions when it's not your turn or spectator!
         for (unitAction in UnitActions.getUnitActions(unit)) {
             val button = getUnitActionButton(unit, unitAction)
-            if (unitAction is UpgradeUnitAction && GUI.keyboardAvailable) {
+            if (unitAction is UpgradeUnitAction) {
-                val tipTitle = "«RED»${KeyboardBindings[unitAction.type.binding]}«»: {Upgrade}"
+                button.onRightClick {
-                val tipActor = BaseUnitDescriptions.getUpgradeTooltipActor(tipTitle, unit.baseUnit, unitAction.unitToUpgradeTo)
+                    val pos = button.localToStageCoordinates(Vector2(button.width, button.height))
-                button.addListener(UncivTooltip(button, tipActor
+                    UnitUpgradeMenu(worldScreen.stage, pos, unit, unitAction, callbackAfterAnimation = true) {
-                    , offset = Vector2(0f, tipActor.packIfNeeded().height * 0.333f) // scaling fails to express size in parent coordinates
+                        worldScreen.shouldUpdate = true
-                    , tipAlign = Align.topLeft, targetAlign = Align.topRight))
+                    }
+                }
             }
             add(button).left().padBottom(2f).row()
         }
@@ -54,9 +53,9 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
         if (unitAction.type == UnitActionType.Promote && unitAction.action != null)
             actionButton.color = Color.GREEN.cpy().lerp(Color.WHITE, 0.5f)
 
-        if (unitAction !is UpgradeUnitAction)  // Does its own toolTip
+        actionButton.addTooltip(KeyboardBindings[binding])
-            actionButton.addTooltip(KeyboardBindings[binding])
         actionButton.pack()
+
         if (unitAction.action == null) {
             actionButton.disable()
         } else {