KeyPressManager now manages listener (and cityscreen usage) (#3966)

* KeyPressManager now manages listener (and cityscreen usage)

* KeyPressManager now manages listener - patch 1

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

@@ -2,8 +2,6 @@ package com.unciv.ui.cityscreen
 
 import com.badlogic.gdx.Input
 import com.badlogic.gdx.graphics.Color
-import com.badlogic.gdx.scenes.scene2d.InputEvent
-import com.badlogic.gdx.scenes.scene2d.InputListener
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.utils.Align
 import com.unciv.UncivGame
@@ -19,7 +17,6 @@ import com.unciv.ui.utils.AutoScrollPane as ScrollPane
 class CityScreen(internal val city: CityInfo): CameraStageBaseScreen() {
     var selectedTile: TileInfo? = null
     var selectedConstruction: IConstruction? = null
-    var keyListener: InputListener? = null
 
     /** Toggles or adds/removes all state changing buttons */
     val canChangeState = UncivGame.Current.worldScreen.canChangeState
@@ -75,8 +72,8 @@ class CityScreen(internal val city: CityInfo): CameraStageBaseScreen() {
         stage.addActor(cityInfoTable)
         update()
 
-        keyListener = getKeyboardListener()
+        keyPressDispatcher[Input.Keys.LEFT] = { page(-1) }
-        stage.addListener(keyListener)
+        keyPressDispatcher[Input.Keys.RIGHT] = { page(1) }
     }
 
     internal fun update() {
@@ -238,7 +235,6 @@ class CityScreen(internal val city: CityInfo): CameraStageBaseScreen() {
     }
 
     fun exit() {
-        stage.removeListener(keyListener)
         game.setWorldScreen()
         game.worldScreen.mapHolder.setCenterPosition(city.location)
         game.worldScreen.bottomUnitTable.selectUnit()
@@ -250,23 +246,10 @@ class CityScreen(internal val city: CityInfo): CameraStageBaseScreen() {
         if (numCities == 0) return
         val indexOfCity = civInfo.cities.indexOf(city)
         val indexOfNextCity = (indexOfCity + delta + numCities) % numCities
-        // not entirely sure this is necessary, since we're changing screens we're changing stages as well?
-        stage.removeListener(keyListener)
         val newCityScreen = CityScreen(civInfo.cities[indexOfNextCity])
         newCityScreen.showConstructionsTable = showConstructionsTable // stay on stats drilldown between cities
         newCityScreen.update()
         game.setScreen(newCityScreen)
     }
 
-    private fun getKeyboardListener(): InputListener = object : InputListener() {
-        override fun keyDown(event: InputEvent?, keyCode: Int): Boolean {
-            if (event == null) return super.keyDown(event, keyCode)
-            when (event.keyCode) {
-                Input.Keys.LEFT -> page(-1)
-                Input.Keys.RIGHT -> page(1)
-                else -> return super.keyDown(event, keyCode)
-            }
-            return true
-        }
-    }
 }

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

@@ -37,22 +37,7 @@ open class CameraStageBaseScreen : Screen {
         /** The ExtendViewport sets the _minimum_(!) world size - the actual world size will be larger, fitted to screen/window aspect ratio. */
         stage = Stage(ExtendViewport(height, height), SpriteBatch())
 
-        stage.addListener(
+        keyPressDispatcher.install(stage, this.javaClass.simpleName) { hasOpenPopups() }
-                object : InputListener() {
-                    override fun keyTyped(event: InputEvent?, character: Char): Boolean {
-                        val key = KeyCharAndCode(event, character)
-
-                        if (key !in keyPressDispatcher || hasOpenPopups())
-                            return super.keyTyped(event, character)
-
-                        //try-catch mainly for debugging. Breakpoints in the vicinity can make the event fire twice in rapid succession, second time the context can be invalid
-                        try {
-                            keyPressDispatcher[key]?.invoke()
-                        } catch (ex: Exception) {}
-                        return true
-                    }
-                }
-        )
     }
 
     override fun show() {}
@@ -75,7 +60,9 @@ open class CameraStageBaseScreen : Screen {
 
     override fun hide() {}
 
-    override fun dispose() {}
+    override fun dispose() {
+        keyPressDispatcher.uninstall()
+    }
 
     fun displayTutorial(tutorial: Tutorial, test: (() -> Boolean)? = null) {
         if (!game.settings.showTutorials) return
@@ -107,19 +94,9 @@ open class CameraStageBaseScreen : Screen {
         internal var batch: Batch = SpriteBatch()
     }
 
-    /** It returns the assigned [InputListener] */
+    fun onBackButtonClicked(action: () -> Unit) {
-    fun onBackButtonClicked(action: () -> Unit): InputListener {
+        keyPressDispatcher[Input.Keys.BACK] = action
-        val listener = object : InputListener() {
+        keyPressDispatcher['\u001B'] = action
-            override fun keyDown(event: InputEvent?, keycode: Int): Boolean {
-                if (keycode == Input.Keys.BACK || keycode == Input.Keys.ESCAPE) {
-                    action()
-                    return true
-                }
-                return false
-            }
-        }
-        stage.addListener(listener)
-        return listener
     }
 
     fun isPortrait() = stage.viewport.screenHeight > stage.viewport.screenWidth

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

@@ -1,7 +1,10 @@
 package com.unciv.ui.utils
 
 import com.badlogic.gdx.Input
+import com.badlogic.gdx.scenes.scene2d.EventListener
 import com.badlogic.gdx.scenes.scene2d.InputEvent
+import com.badlogic.gdx.scenes.scene2d.InputListener
+import com.badlogic.gdx.scenes.scene2d.Stage
 import java.util.HashMap
 
 /*
@@ -31,19 +34,37 @@ data class KeyCharAndCode(val char: Char, val code: Int) {
                 if (character == Char.MIN_VALUE && event!=null) event.keyCode else 0
             )
 
-    @ExperimentalStdlibApi
+    // From Kotlin 1.5 on the Ctrl- line will need Char(char.code+64)
+    // see https://github.com/Kotlin/KEEP/blob/master/proposals/stdlib/char-int-conversions.md
     override fun toString(): String {
         // debug helper
         return when {
             char == Char.MIN_VALUE -> Input.Keys.toString(code)
-            char < ' ' -> "Ctrl-" + Char(char.toInt()+64)
+            char < ' ' -> "Ctrl-" + (char.toInt()+64).toChar()
             else -> "\"$char\""
         }
     }
 }
 
+
+/** A manager for a [keyTyped][InputListener.keyTyped] [InputListener], based on [HashMap].
+ *  Uses [KeyCharAndCode] as keys to express bindings for both Ascii and function keys.
+ *
+ *  [install] and [uninstall] handle adding the listener to a [Stage].
+ *  Use indexed assignments to react to specific keys, e.g.:
+ *  ```
+ *      keyPressDispatcher[Input.Keys.F1] = { showHelp() }
+ *      keyPressDispatcher['+'] = { zoomIn() }
+ *  ```
+ *  Optionally use [setCheckpoint] and [revertToCheckPoint] to remember and restore one state.
+ */
 class KeyPressDispatcher: HashMap<KeyCharAndCode, (() -> Unit)>() {
-    private var checkpoint: Set<KeyCharAndCode> = setOf()
+    private var checkpoint: Set<KeyCharAndCode> = setOf()       // set of keys marking a checkpoint
+    private var listener: EventListener? = null     // holds listener code, captures its params in install() function
+    private var listenerInstalled = false           // flag for lazy Stage.addListener()
+    private var installStage: Stage? = null         // Keep stage passed by install() for lazy addListener and uninstall
+    var name: String? = null                        // optional debug label
+        private set
 
     // access by Char
     operator fun get(char: Char) = this[KeyCharAndCode(char)]
@@ -61,14 +82,98 @@ class KeyPressDispatcher: HashMap<KeyCharAndCode, (() -> Unit)>() {
     operator fun contains(code: Int) = this.contains(KeyCharAndCode(code))
     fun remove(code: Int) = this.remove(KeyCharAndCode(code))
 
+    // access by KeyCharAndCode
+    operator fun set(key: KeyCharAndCode, action: () -> Unit) {
+        super.put(key, action)
+        checkInstall()
+    }
+    override fun remove(key: KeyCharAndCode): (() -> Unit)? {
+        val result = super.remove(key)
+        checkInstall()
+        return result
+    }
+
+    override fun toString(): String {
+        return (if (name==null) "" else "$name.") +
+            "KeyPressDispatcher(" + keys.joinToString(limit = 6){ it.toString() } + ")"
+    }
+
+    /** Removes all of the mappings, including a checkpoint if set. */
     override fun clear() {
         checkpoint = setOf()
         super.clear()
+        checkInstall()
     }
+
+    /** Set a checkpoint: The current set of keys will not be removed on a subsequent [revertToCheckPoint] */
     fun setCheckpoint() {
         checkpoint = keys.toSet()
     }
+    /** Revert to a checkpoint: Remove all mappings except those that existed on a previous [setCheckpoint] call.
+     * If no checkpoint has been set, this is equivalent to [clear] */
     fun revertToCheckPoint() {
         keys.minus(checkpoint).forEach { remove(it) }
+        checkInstall()
     }
+
+    /** install our [EventListener] on a stage with optional inhibitor
+     * @param   stage The [Stage] to add the listener to
+     * @param   checkIgnoreKeys An optional lambda - when it returns true all keys are ignored
+     */
+    fun install(stage: Stage, name: String? = null, checkIgnoreKeys: (() -> Boolean)? = null) {
+        this.name = name
+        if (installStage != null) uninstall()
+        listener =
+            object : InputListener() {
+                override fun keyTyped(event: InputEvent?, character: Char): Boolean {
+                    val key = KeyCharAndCode(event, character)
+
+                    // see if we want to handle this key, and if not, let it propagate
+                    if (!contains(key) || (checkIgnoreKeys?.invoke() == true))
+                        return super.keyTyped(event, character)
+
+                    //try-catch mainly for debugging. Breakpoints in the vicinity can make the event fire twice in rapid succession, second time the context can be invalid
+                    try {
+                        this@KeyPressDispatcher[key]?.invoke()
+                    } catch (ex: Exception) {}
+                    return true
+                }
+            }
+        installStage = stage
+        checkInstall()
+    }
+
+    /** uninstall our [EventListener] from the stage it was installed on. */
+    fun uninstall() {
+        checkInstall(forceRemove = true)
+        listener = null
+        installStage = null
+    }
+
+    /** Implements lazy hooking of the listener into the stage.
+     *
+     *  The listener will be added to the stage's listeners only when - and as soon as -
+     *  [this][KeyPressDispatcher] contains mappings.
+     *  When all mappings are removed or cleared the listener is removed from the stage.
+     */
+    private fun checkInstall(forceRemove: Boolean = false) {
+        if (listener == null || installStage == null) return
+        if (listenerInstalled && (isEmpty() || isPaused || forceRemove)) {
+            println(toString() + ": Removing listener" + (if(forceRemove) " for uninstall" else ""))
+            listenerInstalled = false
+            installStage!!.removeListener(listener)
+        } else if (!listenerInstalled && !(isEmpty() || isPaused)) {
+            println(toString() + ": Adding listener")
+            installStage!!.addListener(listener)
+            listenerInstalled = true
+        }
+    }
+
+    /** Allows temporarily suspending this [KeyPressDispatcher] */
+    var isPaused: Boolean = false
+        set(value) {
+            field = value
+            checkInstall()
+        }
+
 }