Keyboard bindings - collision check (#9580)

android/assets/jsons/Tutorials.json

@@ -398,7 +398,6 @@
             {"text":"This is a work in progress.","color":"#b22222","starred":true},
             {"text":"For technical reasons, only direct keys or Ctrl-Letter combinations can be used.","starred":true},
             {"text":"The Escape key is intentionally excluded from being reassigned.","starred":true},
-            {"text":"Currently, there are no checks to prevent conflicting assignments.","starred":true},
             {},
             {"text":"Using the Keys page","header":3},
             {"text":"Each binding has a button with an image looking like this:"},
@@ -407,6 +406,7 @@
             {"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."},
+            {"text":"Conflicting assignments are marked red. Conflicts can exist across categories, like World Screen / Unit Actions. Note that at the moment, the game does not prevent saving conflicting assignments, though the result may be unexpected."},
             {},
             {"text":"For discussion about missing entries, see the linked github issue.","link":"https://github.com/yairm210/Unciv/issues/8862"}
         ]

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

@@ -68,9 +68,16 @@ class KeyCapturingButton(
             updateLabel()
         }
 
+    var markConflict = false
+        set(value) {
+            field = value
+            updateStyle()
+        }
+
     private var savedFocus: Actor? = null
     private val normalStyle: ImageTextButtonStyle
     private val defaultStyle: ImageTextButtonStyle
+    private val conflictStyle: ImageTextButtonStyle
 
     init {
         imageCell.size((style as KeyCapturingButtonStyle).imageSize)
@@ -80,13 +87,23 @@ class KeyCapturingButton(
         normalStyle = style
         defaultStyle = ImageTextButtonStyle(normalStyle)
         defaultStyle.fontColor = Color.GRAY.cpy()
+        conflictStyle = ImageTextButtonStyle(normalStyle)
+        conflictStyle.fontColor = Color.RED.cpy()
         addListener(ButtonListener(this))
     }
 
     private fun updateLabel() {
         label.setText(if (current == KeyCharAndCode.UNKNOWN) "" else current.toString())
-        style = if (current == default) defaultStyle else normalStyle
+        updateStyle()
     }
+    private fun updateStyle() {
+        style = when {
+            markConflict -> conflictStyle
+            current == default -> defaultStyle
+            else -> normalStyle
+        }
+    }
+
     private fun handleKey(code: Int, control: Boolean) {
         current = if (control) KeyCharAndCode.ctrlFromCode(code) else KeyCharAndCode(code)
         onKeyHit?.invoke(current)

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

@@ -12,6 +12,8 @@ enum class KeyboardBinding(
     label: String? = null,
     key: KeyCharAndCode? = null
 ) {
+    //region Enum Instances
+
     /** Used by [KeyShortcutDispatcher.KeyShortcut] to mark an old-style shortcut with a hardcoded key */
     None(Category.None, KeyCharAndCode.UNKNOWN),
 
@@ -89,10 +91,22 @@ enum class KeyboardBinding(
     Confirm(Category.Popups, "Confirm Dialog", 'y'),
     Cancel(Category.Popups, "Cancel Dialog", 'n'),
     ;
+    //endregion
 
     enum class Category {
-        None, WorldScreen, UnitActions, Popups;
+        None,
+        WorldScreen {
+            // Conflict checking within group plus keys assigned to UnitActions are a problem
+            override fun checkConflictsIn() = sequenceOf(this, UnitActions)
+        },
+        UnitActions {
+            // Conflict checking within group disabled, but any key assigned on WorldScreen is a problem
+            override fun checkConflictsIn() = sequenceOf(WorldScreen)
+        },
+        Popups
+        ;
         val label = unCamelCase(name)
+        open fun checkConflictsIn() = sequenceOf(this)
     }
 
     val label: String
@@ -104,12 +118,13 @@ enum class KeyboardBinding(
         this.defaultKey = key ?: KeyCharAndCode(name[0])
     }
 
-    // Helpers to make enum instance initializations shorter
+    //region Helper constructors
     constructor(category: Category, label: String, key: Char) : this(category, label, KeyCharAndCode(key))
     constructor(category: Category, label: String, key: Int) : this(category, label, KeyCharAndCode(key))
     constructor(category: Category, key: KeyCharAndCode) : this(category, null, key)
     constructor(category: Category, key: Char) : this(category, KeyCharAndCode(key))
     constructor(category: Category, key: Int) : this(category, KeyCharAndCode(key))
+    //endregion
 
     /** Debug helper */
     override fun toString() = "$category.$name($defaultKey)"

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

@@ -7,6 +7,7 @@ import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.translations.tr
 import com.unciv.ui.components.ExpanderTab
 import com.unciv.ui.components.KeyCapturingButton
+import com.unciv.ui.components.input.KeyCharAndCode
 import com.unciv.ui.components.input.KeyboardBinding
 import com.unciv.ui.components.TabbedPager
 import com.unciv.ui.components.extensions.toLabel
@@ -47,7 +48,7 @@ class KeyBindingsTab(
                 .map { (category, bindings) ->
                     category to bindings.asSequence()
                         .sortedWith(compareBy(collator) { it.label.tr() })
-                        .map { it to KeyCapturingButton(it.defaultKey) }  // associate would materialize a map
+                        .map { it to KeyCapturingButton(it.defaultKey) { onKeyHit() } }  // associate would materialize a map
                         .toMap(LinkedHashMap())
                 }
                 .sortedBy { it.first.name.tr() }
@@ -81,6 +82,33 @@ class KeyBindingsTab(
         }
     }
 
+    private fun onKeyHit() {
+        for ((category, bindings) in groupedWidgets) {
+            val scope = category.checkConflictsIn()
+            if (scope.none()) continue
+
+            val usedKeys = mutableSetOf<KeyCharAndCode>()
+            val conflictingKeys = mutableSetOf<KeyCharAndCode>()
+            val widgetsInScope = scope
+                .mapNotNull { groupedWidgets[it] }
+                .flatMap { scopeMap -> scopeMap.map { it.key.category to it.value } }
+
+            for ((scopeCategory ,widget) in widgetsInScope) {
+                val key = widget.current
+                // We shall not use any key of a different category in scope,
+                // nor use a key within this category twice - if this category _is_ in scope.
+                if (key in usedKeys || scopeCategory != category)
+                    conflictingKeys += key
+                else
+                    usedKeys += key
+            }
+
+            for (widget in bindings.values) {
+                widget.markConflict = widget.current in conflictingKeys
+            }
+        }
+    }
+
     fun save () {
         for ((binding, widget) in groupedWidgets.asSequence().flatMap { it.value.entries }) {
             keyBindings[binding] = widget.current