From d666c44697adaa524a80a43b37d35d471bf52a64 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Tue, 13 Jun 2023 06:31:02 +0200 Subject: [PATCH] Keyboard bindings - collision check (#9580) --- android/assets/jsons/Tutorials.json | 2 +- .../unciv/ui/components/KeyCapturingButton.kt | 19 +++++++++++- .../ui/components/input/KeyboardBinding.kt | 19 ++++++++++-- .../unciv/ui/popups/options/KeyBindingsTab.kt | 30 ++++++++++++++++++- 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/android/assets/jsons/Tutorials.json b/android/assets/jsons/Tutorials.json index ba173ff2d1..38a2808b2e 100644 --- a/android/assets/jsons/Tutorials.json +++ b/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"} ] diff --git a/core/src/com/unciv/ui/components/KeyCapturingButton.kt b/core/src/com/unciv/ui/components/KeyCapturingButton.kt index 1fc29207b9..67acc28679 100644 --- a/core/src/com/unciv/ui/components/KeyCapturingButton.kt +++ b/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) diff --git a/core/src/com/unciv/ui/components/input/KeyboardBinding.kt b/core/src/com/unciv/ui/components/input/KeyboardBinding.kt index d105913cb7..801785ffdc 100644 --- a/core/src/com/unciv/ui/components/input/KeyboardBinding.kt +++ b/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)" diff --git a/core/src/com/unciv/ui/popups/options/KeyBindingsTab.kt b/core/src/com/unciv/ui/popups/options/KeyBindingsTab.kt index e9a6f42eb1..034b48a3df 100644 --- a/core/src/com/unciv/ui/popups/options/KeyBindingsTab.kt +++ b/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() + val conflictingKeys = mutableSetOf() + 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