Key binding simpler widget (#8946)

* Keyboard Bindings: Simpler Widget

* Keyboard Bindings: Reset binding

* Keyboard Bindings: Reset binding - patch

* Keyboard Bindings: Simpler Widget - revert opening feature

* Keyboard Bindings: Simpler Widget - improved

android/assets/jsons/Tutorials.json

@@ -401,10 +401,10 @@
             {"text":"Currently, there are no checks to prevent conflicting assignments.","starred":true},
             {},
             {"text":"Using the Keys page","header":3},
-            {"text":"Each binding has a label, a dropdown select, and a key button looking like this:"},
+            {"text":"Each binding has a button with an image looking like this:"},
             {"extraImage":"OtherIcons/Keyboard","imageSize":36},
             {"text":"While hovering the mouse over the key button, you can press a desired key directly to assign it."},
-            {"text":"Alternatively, you can select the key in the dropdown."},
+            {"text":"Double-click the image to reset the binding to default."},
             {},
             {"text":"Bindings mapped to their default keys are displayed in gray, those reassigned by you in white."},
             {},

android/assets/jsons/translations/template.properties

@@ -756,6 +756,7 @@ Enable Easter Eggs =
 
 ## Keys tab
 Keys = 
+Please see the Tutorial. = 
 
 ## Locate mod errors tab
 Locate mod errors = 

core/src/com/unciv/ui/components/KeyCapturingButton.kt

@@ -1,41 +1,105 @@
 package com.unciv.ui.components
 
+import com.badlogic.gdx.Gdx
 import com.badlogic.gdx.Input
 import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.InputEvent
-import com.badlogic.gdx.scenes.scene2d.ui.ImageButton
+import com.badlogic.gdx.scenes.scene2d.ui.Image
+import com.badlogic.gdx.scenes.scene2d.ui.ImageTextButton
 import com.badlogic.gdx.scenes.scene2d.utils.ClickListener
+import com.badlogic.gdx.scenes.scene2d.utils.NinePatchDrawable
 import com.badlogic.gdx.utils.Align
 import com.unciv.ui.components.UncivTooltip.Companion.addTooltip
 import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.screens.basescreen.BaseScreen
 
-/** A button that captures keyboard keys and reports them through [onKeyHit] */
+/** An ImageTextButton that captures keyboard keys
+ *
+ *  Its Label will reflect the pressed key and will be grayed if the [current] key equals [default].
+ *  Note this will start with an empty label and [current] == UNKNOWN. You must set an initial value yourself if needed.
+ *
+ *  @param default The key seen as default (label grayed) state
+ *  @param initialStyle Optionally configurable style details
+ *  @param onKeyHit Fires when a key was pressed with the cursor over it
+ */
 class KeyCapturingButton(
-    private val onKeyHit: (keyCode: Int, control: Boolean) -> Unit
+    private val default: KeyCharAndCode = KeyCharAndCode.UNKNOWN,
-) : ImageButton(getStyle()) {
+    initialStyle: KeyCapturingButtonStyle = KeyCapturingButtonStyle(),
-    companion object {
+    private val onKeyHit: ((key: KeyCharAndCode) -> Unit)? = null
-        private const val buttonSize = 36f
+) : ImageTextButton("", initialStyle) {
-        private const val buttonImage = "OtherIcons/Keyboard"
-        private val controlKeys = setOf(Input.Keys.CONTROL_LEFT, Input.Keys.CONTROL_RIGHT)
 
-        private fun getStyle() = ImageButtonStyle().apply {
+    /** A subclass of [ImageTextButtonStyle][ImageTextButton.ImageTextButtonStyle] that allows setting
-            val image = ImageGetter.getDrawable(buttonImage)
+     *  the image parts (imageUp and imageOver only as hovering is the only interaction) via ImageGetter.
-            imageUp = image
+     * @param imageSize     Size for the image part
-            imageOver = image.tint(Color.LIME)
+     * @param imageName     Name for the imagePart as understood by [ImageGetter.getDrawable]
+     * @param imageUpTint   If not Color.CLEAR, this tints the image for its **normal** state
+     * @param imageOverTint If not Color.CLEAR, this tints the image for its **hover** state
+     * @param minWidth      Overrides background [NinePatchDrawable.minWidth]
+     * @param minHeight     Overrides background [NinePatchDrawable.minHeight]
+     */
+    class KeyCapturingButtonStyle (
+        val imageSize: Float = 24f,
+        imageName: String = "OtherIcons/Keyboard",
+        imageUpTint: Color = Color.CLEAR,
+        imageOverTint: Color = Color.LIME,
+        minWidth: Float = 150f,
+        minHeight: Float = imageSize
+    ) : ImageTextButtonStyle() {
+        init {
+            font = Fonts.font
+            fontColor = Color.WHITE
+            val image = ImageGetter.getDrawable(imageName)
+            imageUp = if (imageUpTint == Color.CLEAR) image else image.tint(imageUpTint)
+            imageOver = if (imageOverTint == Color.CLEAR) imageUp else image.tint(imageOverTint)
+            up = BaseScreen.skinStrings.run {
+                getUiBackground("General/KeyCapturingButton", roundedEdgeRectangleSmallShape, skinConfig.baseColor)
+            }
+            up.minWidth = minWidth
+            up.minHeight = minHeight
         }
     }
 
+    /** Gets/sets the currently assigned [KeyCharAndCode] */
+    var current = KeyCharAndCode.UNKNOWN
+        set(value) {
+            field = value
+            updateLabel()
+        }
+
     private var savedFocus: Actor? = null
+    private val normalStyle: ImageTextButtonStyle
+    private val defaultStyle: ImageTextButtonStyle
 
     init {
-        setSize(buttonSize, buttonSize)
+        imageCell.size((style as KeyCapturingButtonStyle).imageSize)
-        addTooltip("Hit the desired key now", 18f, targetAlign = Align.bottomRight)
+        imageCell.align(Align.topLeft)
+        image.addTooltip("Hit the desired key now", 18f, targetAlign = Align.bottomRight)
+        labelCell.expandX()
+        normalStyle = style
+        defaultStyle = ImageTextButtonStyle(normalStyle)
+        defaultStyle.fontColor = Color.GRAY.cpy()
         addListener(ButtonListener(this))
     }
 
+    private fun updateLabel() {
+        label.setText(if (current == KeyCharAndCode.UNKNOWN) "" else current.toString())
+        style = if (current == default) defaultStyle else normalStyle
+    }
+    private fun handleKey(code: Int, control: Boolean) {
+        current = if (control) KeyCharAndCode.ctrlFromCode(code) else KeyCharAndCode(code)
+        onKeyHit?.invoke(current)
+    }
+    private fun resetKey() {
+        current = default
+        onKeyHit?.invoke(current)
+    }
+
+    // Instead of storing a button reference one could use `(event?.listenerActor as? KeyCapturingButton)?.`
     private class ButtonListener(private val myButton: KeyCapturingButton) : ClickListener() {
-        private var controlDown = false
+        private fun controlDown() =
+                Gdx.input.isKeyPressed(Input.Keys.CONTROL_LEFT) ||
+                Gdx.input.isKeyPressed(Input.Keys.CONTROL_RIGHT)
 
         override fun enter(event: InputEvent?, x: Float, y: Float, pointer: Int, fromActor: Actor?) {
             if (myButton.stage == null) return
@@ -50,20 +114,15 @@ class KeyCapturingButton(
         }
 
         override fun keyDown(event: InputEvent?, keycode: Int): Boolean {
-            if (keycode == Input.Keys.ESCAPE) return false
+            if (keycode == Input.Keys.ESCAPE || keycode == Input.Keys.UNKNOWN) return false
-            if (keycode in controlKeys) {
+            if (keycode == Input.Keys.CONTROL_LEFT || keycode == Input.Keys.CONTROL_RIGHT) return false
-                controlDown = true
+            myButton.handleKey(keycode, controlDown())
-            } else {
-                myButton.onKeyHit(keycode, controlDown)
-            }
             return true
         }
 
-        override fun keyUp(event: InputEvent?, keycode: Int): Boolean {
+        override fun clicked(event: InputEvent?, x: Float, y: Float) {
-            if (keycode == Input.Keys.ESCAPE) return false
+            if (tapCount < 2 || event?.target !is Image) return
-            if (keycode in controlKeys)
+            myButton.resetKey()
-                controlDown = false
-            return true
         }
     }
 }

core/src/com/unciv/ui/components/KeysSelectBox.kt

@@ -1,144 +0,0 @@
-package com.unciv.ui.components
-
-import com.badlogic.gdx.Input
-import com.badlogic.gdx.graphics.Color
-import com.badlogic.gdx.scenes.scene2d.Actor
-import com.badlogic.gdx.scenes.scene2d.ui.SelectBox
-import com.badlogic.gdx.scenes.scene2d.utils.ChangeListener
-import com.badlogic.gdx.utils.Align
-import com.unciv.models.translations.tr
-import com.unciv.ui.components.extensions.GdxKeyCodeFixes
-import com.unciv.ui.screens.basescreen.BaseScreen
-import com.badlogic.gdx.utils.Array as GdxArray
-
-
-/**
- *  A Widget to allow selecting keys from a [SelectBox].
- *
- *  Added value:
- *  *  Easier change callback as direct [changeCallback] parameter
- *  *  Pulls existing keys from Gdx.[Input.Keys] with an exclusion parameter
- *  *  Key names made translatable as e.g. `Key-"A"` or `Key-Space`
- *  *  Supports TranslationFileWriter by providing those translation keys
- */
-// Note this has similarities with TranslatedSelectBox, but having entries in the translation
-// files that consist only of a punctuation character wouldn't be pretty
-class KeysSelectBox(
-    private val default: String = "",
-    excludeKeys: Set<Int> = defaultExclusions,
-    private val changeCallback: (() -> Unit)? = null
-) : SelectBox<KeysSelectBox.KeysSelectBoxEntry>(BaseScreen.skin) {
-
-    class KeysSelectBoxEntry(val name: String) {
-        val translation = run {
-            val translationKey = getTranslationKey(name)
-            val translation = translationKey.tr()
-            if (translation == translationKey) name else translation
-        }
-
-        override fun toString() = translation
-        override fun equals(other: Any?) = other is KeysSelectBoxEntry && other.name == this.name
-        override fun hashCode() = name.hashCode()
-    }
-
-    companion object {
-        // Gdx.Input.Keys has a keyNames map internally that would serve,
-        // but it's private and only used for valueOf() - So we do the same here...
-        private val keyCodeMap = LinkedHashMap<String, Int>(200).apply {
-            for (code in 0..Input.Keys.MAX_KEYCODE) {
-                val name = GdxKeyCodeFixes.toString(code) ?: continue
-                put(name, code)
-            }
-        }
-
-        val defaultExclusions = setOf(
-            Input.Keys.UNKNOWN,         // Any key GLFW or Gdx fail to translate fire this - and we assign our own meaning
-            Input.Keys.ESCAPE,          // Married to Android Back, we want to keep this closing the Options
-            Input.Keys.CONTROL_LEFT,    // Captured to support control-letter combos
-            Input.Keys.CONTROL_RIGHT,
-            Input.Keys.PICTSYMBOLS,     // Ugly toString and unknown who could have such a key
-            Input.Keys.SWITCH_CHARSET,
-            Input.Keys.SYM,
-            Input.Keys.PRINT_SCREEN,    // Not captured by Gdx by default, these are handled by the OS
-            Input.Keys.VOLUME_UP,
-            Input.Keys.VOLUME_DOWN,
-            Input.Keys.MUTE,
-        )
-
-        fun getKeyNames(excludeKeys: Set<Int>): Sequence<String> {
-            val keyCodeNames = keyCodeMap.asSequence()
-                .filterNot { it.value in excludeKeys }
-                .map { it.key }
-            val controlNames = ('A'..'Z').asSequence()
-                .map { "Ctrl-$it" }
-            return (keyCodeNames + controlNames)
-                .sortedWith(
-                    compareBy<String> {
-                        when {
-                            it.length == 1 -> 0  // Characters first
-                            it.length in 2..3 && it[0] == 'F' && it[1].isDigit() ->
-                                it.drop(1).toInt()  // then F-Keys numerically
-                            it.startsWith("Ctrl-") -> 100  // Then Ctrl-*
-                            else -> 999  // Rest last
-                        }
-                    }.thenBy { it }  // should be by translated, but - later
-                )
-        }
-
-        private fun getTranslationKey(keyName: String): String {
-            val noSpaces = keyName.replace(' ','-')
-            if (noSpaces.last().isLetterOrDigit()) return "Key-$noSpaces"
-            return "Key-${noSpaces.dropLast(1)}\"${noSpaces.last()}\""
-        }
-
-        @Suppress("unused")  // TranslationFileWriter integration will be done later
-        fun getAllTranslationKeys() = getKeyNames(defaultExclusions).map { getTranslationKey(it) }
-    }
-
-    private val normalStyle: SelectBoxStyle
-    private val defaultStyle: SelectBoxStyle
-
-    init {
-        //TODO:
-        // * Dropdown doesn't listen to mouse wheel until after some focus setting click
-        //   (like any other SelectBox, but here it's more noticeable)
-
-        items = GdxArray<KeysSelectBoxEntry>(200).apply {
-            for (name in getKeyNames(excludeKeys))
-                add(KeysSelectBoxEntry(name))
-        }
-        setSelected(default)
-
-        maxListCount = 12  // or else the dropdown will fill as much vertical space as it can, including upwards
-        list.alignment = Align.center  // default left is ugly, especially when a Mod Skin removes padding
-        setAlignment(Align.center)
-
-        addListener(object : ChangeListener() {
-            override fun changed(event: ChangeEvent?, actor: Actor?) {
-                setColorForDefault()
-                changeCallback?.invoke()
-            }
-        })
-
-        // clone style to prevent bleeding our changes to other Widgets
-        normalStyle = SelectBoxStyle(style)
-        // Another instance of SelectBoxStyle for displaying whether the selection is the default one
-        defaultStyle = SelectBoxStyle(normalStyle)
-        defaultStyle.fontColor = Color.GRAY.cpy()
-
-        setColorForDefault()
-    }
-
-    fun setSelected(name: String) {
-        val newSelection = items.firstOrNull { it.name == name }
-            ?: return
-        selected = newSelection
-    }
-
-    /** Update fontColor to show whether the selection is the default one (as grayed) */
-    private fun setColorForDefault() {
-        // See Javadoc of underlying style - setStyle is needed to update visually
-        @Suppress("UsePropertyAccessSyntax")
-        setStyle(if (selected.name == default) defaultStyle else normalStyle)
-    }
-}

core/src/com/unciv/ui/popups/options/KeyBindingsTab.kt

@@ -4,10 +4,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.GUI
 import com.unciv.models.ruleset.RulesetCache
 import com.unciv.ui.components.KeyCapturingButton
-import com.unciv.ui.components.KeyCharAndCode
 import com.unciv.ui.components.KeyboardBinding
-import com.unciv.ui.components.KeyboardBindings
-import com.unciv.ui.components.KeysSelectBox
 import com.unciv.ui.components.TabbedPager
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.screens.basescreen.BaseScreen
@@ -21,7 +18,7 @@ class KeyBindingsTab(
     labelWidth: Float
 ) : Table(BaseScreen.skin), TabbedPager.IPageExtensions {
     private val keyBindings = optionsPopup.settings.keyBindings
-    private val keyFields = HashMap<KeyboardBinding, KeyboardBindingWidget>(KeyboardBinding.values().size)
+    private val keyFields = HashMap<KeyboardBinding, KeyCapturingButton>(KeyboardBinding.values().size)
     private val disclaimer = MarkupRenderer.render(listOf(
         FormattedLine("This is a work in progress.", color = "#b22222", centered = true),  // FIREBRICK
         FormattedLine(),
@@ -40,7 +37,7 @@ class KeyBindingsTab(
 
         for (binding in KeyboardBinding.values()) {
             if (binding.hidden) continue
-            keyFields[binding] = KeyboardBindingWidget(binding)
+            keyFields[binding] = KeyCapturingButton(binding.defaultKey)
         }
     }
 
@@ -52,14 +49,14 @@ class KeyBindingsTab(
             if (binding.hidden) continue
             add(binding.label.toLabel())
             add(keyFields[binding]).row()
-            keyFields[binding]!!.update(keyBindings)
+            keyFields[binding]!!.current = keyBindings[binding]
         }
     }
 
     fun save () {
         for (binding in KeyboardBinding.values()) {
             if (binding.hidden) continue
-            keyBindings.put(binding, keyFields[binding]!!.text)
+            keyBindings[binding] = keyFields[binding]!!.current
         }
     }
 
@@ -69,57 +66,4 @@ class KeyBindingsTab(
     override fun deactivated(index: Int, caption: String, pager: TabbedPager) {
         save()
     }
-
-    class KeyboardBindingWidget(
-        /** The specific binding to edit */
-        private val binding: KeyboardBinding
-    ) : Table(BaseScreen.skin) {
-        private val selectBox = KeysSelectBox(binding.defaultKey.toString()) {
-            validateSelection()
-        }
-
-        private val button = KeyCapturingButton { code, control ->
-            selectBox.hideScrollPane()
-            boundKey = if (control)
-                KeyCharAndCode.ctrlFromCode(code)
-                else KeyCharAndCode(code)
-            resetSelection()
-        }
-
-        private var boundKey: KeyCharAndCode = binding.defaultKey
-
-        init {
-            pad(0f)
-            defaults().pad(0f)
-            add(selectBox)
-            add(button).size(36f).padLeft(2f)
-        }
-
-        /** Get the (untranslated) key name selected by the Widget */
-        // we let the KeysSelectBox handle undesired mappings
-        val text: String
-            get() = selectBox.selected.name
-
-        /** Update control to show current binding */
-        fun update(keyBindings: KeyboardBindings) {
-            boundKey = keyBindings[binding]
-            resetSelection()
-        }
-
-        /** Set boundKey from selectBox */
-        private fun validateSelection() {
-            val value = text
-            val parsedKey = KeyCharAndCode.parse(value)
-            if (parsedKey == KeyCharAndCode.UNKNOWN) {
-                resetSelection()
-            } else {
-                boundKey = parsedKey
-            }
-        }
-
-        /** Set selectBox from boundKey */
-        private fun resetSelection() {
-            selectBox.setSelected(boundKey.toString())
-        }
-    }
 }