From e22c53ca96ce9257b30a51f085ca166d7e270bc4 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Thu, 3 Jun 2021 21:30:42 +0200 Subject: [PATCH] Key dispatcher precedence with several Popups open (#4040) * Key dispatcher precedence with several Popups open * Key dispatcher precedence with several Popups open - remove instrumentation --- .../unciv/ui/utils/CameraStageBaseScreen.kt | 4 ++-- .../com/unciv/ui/utils/ExtensionFunctions.kt | 1 - .../com/unciv/ui/utils/KeyPressDispatcher.kt | 15 ++++++-------- core/src/com/unciv/ui/utils/Popup.kt | 20 +++++++++++++------ 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/core/src/com/unciv/ui/utils/CameraStageBaseScreen.kt b/core/src/com/unciv/ui/utils/CameraStageBaseScreen.kt index 2229155746..b226f88c89 100644 --- a/core/src/com/unciv/ui/utils/CameraStageBaseScreen.kt +++ b/core/src/com/unciv/ui/utils/CameraStageBaseScreen.kt @@ -28,7 +28,7 @@ open class CameraStageBaseScreen : Screen { protected val tutorialController by lazy { TutorialController(this) } - val keyPressDispatcher = KeyPressDispatcher() + val keyPressDispatcher = KeyPressDispatcher(this.javaClass.simpleName) init { val resolutions: List = game.settings.resolution.split("x").map { it.toInt().toFloat() } @@ -37,7 +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()) - keyPressDispatcher.install(stage, this.javaClass.simpleName) { hasOpenPopups() } + keyPressDispatcher.install(stage) { hasOpenPopups() } } override fun show() {} diff --git a/core/src/com/unciv/ui/utils/ExtensionFunctions.kt b/core/src/com/unciv/ui/utils/ExtensionFunctions.kt index c5d46bdd18..b7404dd62e 100644 --- a/core/src/com/unciv/ui/utils/ExtensionFunctions.kt +++ b/core/src/com/unciv/ui/utils/ExtensionFunctions.kt @@ -36,7 +36,6 @@ fun Button.enable() { * which is more appropriate to toggle On/Off buttons, while this one is good for 'click-to-do-something' buttons. */ var Button.isEnabled: Boolean - //Todo: Use in PromotionPickerScreen, TradeTable, WorldScreen.updateNextTurnButton get() = touchable == Touchable.enabled set(value) = if (value) enable() else disable() diff --git a/core/src/com/unciv/ui/utils/KeyPressDispatcher.kt b/core/src/com/unciv/ui/utils/KeyPressDispatcher.kt index d29b4f3ca3..a5e6f471dd 100644 --- a/core/src/com/unciv/ui/utils/KeyPressDispatcher.kt +++ b/core/src/com/unciv/ui/utils/KeyPressDispatcher.kt @@ -5,7 +5,6 @@ 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 /* * For now, combination keys cannot easily be expressed. @@ -40,6 +39,7 @@ data class KeyCharAndCode(val char: Char, val code: Int) { // debug helper return when { char == Char.MIN_VALUE -> Input.Keys.toString(code) + char == '\u001B' -> "ESC" char < ' ' -> "Ctrl-" + (char.toInt()+64).toChar() else -> "\"$char\"" } @@ -57,14 +57,14 @@ data class KeyCharAndCode(val char: Char, val code: Int) { * keyPressDispatcher['+'] = { zoomIn() } * ``` * Optionally use [setCheckpoint] and [revertToCheckPoint] to remember and restore one state. + * + * @param name Optional name of the container screen or popup for debugging */ -class KeyPressDispatcher: HashMap Unit)>() { +class KeyPressDispatcher(val name: String? = null) : HashMap Unit)>() { private var checkpoint: Set = 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)] @@ -130,8 +130,7 @@ class KeyPressDispatcher: HashMap Unit)>() { * @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 + fun install(stage: Stage, checkIgnoreKeys: (() -> Boolean)? = null) { if (installStage != null) uninstall() listener = object : InputListener() { @@ -141,7 +140,7 @@ class KeyPressDispatcher: HashMap Unit)>() { // 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() @@ -169,11 +168,9 @@ class KeyPressDispatcher: HashMap Unit)>() { 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 } diff --git a/core/src/com/unciv/ui/utils/Popup.kt b/core/src/com/unciv/ui/utils/Popup.kt index 89151b7c42..cd1dab55eb 100644 --- a/core/src/com/unciv/ui/utils/Popup.kt +++ b/core/src/com/unciv/ui/utils/Popup.kt @@ -21,7 +21,7 @@ open class Popup(val screen: CameraStageBaseScreen): Table(CameraStageBaseScreen * while the popup is active through the [hasOpenPopups][CameraStageBaseScreen.hasOpenPopups] mechanism. * @see [KeyPressDispatcher.install] */ - val keyPressDispatcher = KeyPressDispatcher() + val keyPressDispatcher = KeyPressDispatcher(this.javaClass.simpleName) init { // Set actor name for debugging @@ -44,20 +44,21 @@ open class Popup(val screen: CameraStageBaseScreen): Table(CameraStageBaseScreen * closed. Use [force] = true if you want to open this popup above the other one anyway. */ fun open(force: Boolean = false) { - if (force || !screen.hasOpenPopups()) { - show() - } - screen.stage.addActor(this) innerTable.pack() pack() center(screen.stage) + if (force || !screen.hasOpenPopups()) { + show() + } } /** Subroutine for [open] handles only visibility and [keyPressDispatcher] */ private fun show() { this.isVisible = true - keyPressDispatcher.install(screen.stage, name) + val currentCount = screen.countOpenPopups() + // the lambda is for stacked key dispatcher precedence: + keyPressDispatcher.install(screen.stage) { screen.countOpenPopups() > currentCount } } /** @@ -160,6 +161,13 @@ open class Popup(val screen: CameraStageBaseScreen): Table(CameraStageBaseScreen */ fun CameraStageBaseScreen.hasOpenPopups(): Boolean = stage.actors.any { it is Popup && it.isVisible } +/** + * Counts number of visible[Popup]s. + * + * Used for key dispatcher precedence. + */ +fun CameraStageBaseScreen.countOpenPopups() = stage.actors.count { it is Popup && it.isVisible } + /** Closes all [Popup]s. */ fun CameraStageBaseScreen.closeAllPopups() = popups.forEach { it.close() }