Tweak Language Pickers to scroll the selected one into view when appropriate, and allow selection with letter keys (#10569)

core/src/com/unciv/ui/components/extensions/Scene2dExtensions.kt

@@ -19,6 +19,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.CheckBox
 import com.badlogic.gdx.scenes.scene2d.ui.Image
 import com.badlogic.gdx.scenes.scene2d.ui.ImageButton
 import com.badlogic.gdx.scenes.scene2d.ui.Label
+import com.badlogic.gdx.scenes.scene2d.ui.ScrollPane
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.scenes.scene2d.ui.TextButton
 import com.badlogic.gdx.scenes.scene2d.ui.TextButton.TextButtonStyle
@@ -130,6 +131,11 @@ fun Actor.getAscendant(predicate: (Actor) -> Boolean): Actor? {
     return null
 }
 
+/** Gets the nearest parent of this actor that is a [T], or null if none of its parents is of that type. */
+inline fun <reified T> Actor.getAscendant(): T? {
+    return  getAscendant { it is T } as? T
+}
+
 /** The actors bounding box in stage coordinates */
 val Actor.stageBoundingBox: Rectangle get() {
     val bottomLeft = localToStageCoordinates(Vector2.Zero.cpy())
@@ -225,6 +231,10 @@ fun Image.setSize(size: Float) {
     setSize(size, size)
 }
 
+/** Proxy for [ScrollPane.scrollTo] using the [bounds][Actor.setBounds] of a given [actor] for its parameters */
+fun ScrollPane.scrollTo(actor: Actor, center: Boolean = false) =
+    scrollTo(actor.x, actor.y, actor.width, actor.height, center, center)
+
 /** Translate a [String] and make a [TextButton] widget from it */
 fun String.toTextButton(style: TextButtonStyle? = null, hideIcons: Boolean = false): TextButton {
     val text = this.tr(hideIcons)

core/src/com/unciv/ui/components/widgets/LanguageTable.kt

@@ -1,18 +1,29 @@
 package com.unciv.ui.components.widgets
 
+import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.ui.components.extensions.darken
 import com.unciv.ui.components.extensions.toLabel
+import com.unciv.ui.components.input.KeyCharAndCode
+import com.unciv.ui.components.input.KeyShortcutDispatcher
+import com.unciv.ui.components.input.KeyboardBinding
+import com.unciv.ui.components.input.keyShortcuts
+import com.unciv.ui.components.widgets.LanguageTable.Companion.addLanguageTables
 import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.popups.options.OptionsPopup
+import com.unciv.ui.screens.LanguagePickerScreen
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.civilopediascreen.FormattedLine
 import com.unciv.ui.screens.civilopediascreen.MarkupRenderer
 import java.util.Locale
 
-/** Represents a row in the Language picker, used both in OptionsPopup and in LanguagePickerScreen */
+
+/** Represents a row in the Language picker, used both in [OptionsPopup] and in [LanguagePickerScreen]
+ *  @see addLanguageTables
+ */
 internal class LanguageTable(val language:String, val percentComplete: Int) : Table() {
     private val baseColor = BaseScreen.skinStrings.skinConfig.baseColor
     private val darkBaseColor = baseColor.darken(0.5f)
@@ -41,7 +52,7 @@ internal class LanguageTable(val language:String, val percentComplete: Int) : Ta
 
     companion object {
         /** Extension to add the Language boxes to a Table, used both in OptionsPopup and in LanguagePickerScreen */
-        internal fun Table.addLanguageTables(expectedWidth: Float): ArrayList<LanguageTable> {
+        fun Table.addLanguageTables(expectedWidth: Float): ArrayList<LanguageTable> {
             val languageTables = ArrayList<LanguageTable>()
 
             val translationDisclaimer = FormattedLine(
@@ -54,27 +65,21 @@ internal class LanguageTable(val language:String, val percentComplete: Int) : Ta
             add(MarkupRenderer.render(listOf(translationDisclaimer),expectedWidth)).pad(5f).row()
 
             val tableLanguages = Table()
-            tableLanguages.defaults().uniformX()
-            tableLanguages.defaults().pad(10.0f)
-            tableLanguages.defaults().fillX()
+            tableLanguages.defaults().uniformX().fillX().pad(10.0f)
 
             val systemLanguage = Locale.getDefault().getDisplayLanguage(Locale.ENGLISH)
 
             val languageCompletionPercentage = UncivGame.Current.translations
                 .percentCompleteOfLanguages
-            languageTables.addAll(languageCompletionPercentage
+            languageTables.addAll(
+                languageCompletionPercentage
                 .map { LanguageTable(it.key, if (it.key == Constants.english) 100 else it.value) }
-                .sortedWith { p0, p1 ->
-                    when {
-                        p0.language == Constants.english -> -1
-                        p1.language == Constants.english -> 1
-                        p0.language == systemLanguage -> -1
-                        p1.language == systemLanguage -> 1
-                        p0.percentComplete > p1.percentComplete -> -1
-                        p0.percentComplete == p1.percentComplete -> 0
-                        else -> 1
-                    }
-                })
+                .sortedWith(
+                    compareBy<LanguageTable> { it.language != Constants.english }
+                    .thenBy { it.language != systemLanguage }
+                    .thenByDescending { it.percentComplete }
+                )
+            )
 
             languageTables.forEach {
                 tableLanguages.add(it).row()
@@ -83,5 +88,28 @@ internal class LanguageTable(val language:String, val percentComplete: Int) : Ta
 
             return languageTables
         }
+
+        /** Create round-robin letter key handling, such that repeatedly pressing 'R' will cycle through all languages starting with 'R' */
+        fun Actor.addLanguageKeyShortcuts(languageTables: ArrayList<LanguageTable>, getSelection: ()->String, action: (String)->Unit) {
+            // Yes this is too complicated. Trying to preserve existing architecture choices.
+            // One - extending KeyShortcut to allow another type filtering by a lambda,
+            //       then teach KeyShortcutDispatcher.Resolver to recognize that - and pass on the actual key to its activation - could help.
+            // Two - Changing addLanguageTables above to an actual container class holding the LanguageTables - could help.
+            fun activation(letter: Char) {
+                val candidates = languageTables.filter { it.language.first() == letter }
+                if (candidates.isEmpty()) return
+                if (candidates.size == 1) return action(candidates.first().language)
+                val currentIndex = candidates.indexOfFirst { it.language == getSelection() }
+                val newSelection = candidates[(currentIndex + 1) % candidates.size]
+                action(newSelection.language)
+            }
+
+            val letters = languageTables.map { it.language.first() }.toSet()
+            for (letter in letters) {
+                keyShortcuts.add(KeyShortcutDispatcher.KeyShortcut(KeyboardBinding.None, KeyCharAndCode(letter), 0)) {
+                    activation(letter)
+                }
+            }
+        }
     }
 }

core/src/com/unciv/ui/components/widgets/TabbedPager.kt

@@ -13,6 +13,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.ScrollPane
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.scenes.scene2d.ui.WidgetGroup
 import com.badlogic.gdx.scenes.scene2d.utils.ActorGestureListener
+import com.badlogic.gdx.scenes.scene2d.utils.Layout
 import com.badlogic.gdx.utils.Align
 import com.unciv.Constants
 import com.unciv.UncivGame
@@ -23,6 +24,7 @@ import com.unciv.ui.components.extensions.darken
 import com.unciv.ui.components.extensions.isEnabled
 import com.unciv.ui.components.extensions.packIfNeeded
 import com.unciv.ui.components.extensions.pad
+import com.unciv.ui.components.extensions.scrollTo
 import com.unciv.ui.components.input.KeyCharAndCode
 import com.unciv.ui.components.input.keyShortcuts
 import com.unciv.ui.components.input.onActivation
@@ -459,6 +461,18 @@ open class TabbedPager(
         contentScroll.scrollY = scrollY
         if (!animation) contentScroll.updateVisualScroll()
     }
+    /** Change the vertical scroll position af the [active][activePage] page's contents to scroll a given Actor into view
+     *
+     *  Assumes [actor] is a direct child of the content page, and **if** the page does **not** implement [Layout],
+     *  that [actor] has somehow been given valid [bounds][Actor.setBounds] within the page.
+     */
+    fun pageScrollTo(actor: Actor, animation: Boolean = false) {
+        if (activePage < 0) return
+        (contentScroll.actor as? Layout)?.validate()
+        contentScroll.scrollTo(actor)
+        pages[activePage].scrollY = contentScroll.scrollY
+        if (!animation) contentScroll.updateVisualScroll()
+    }
 
     /** Disable/Enable built-in ScrollPane for content pages, including focus stealing prevention */
     fun setScrollDisabled(disabled: Boolean) {

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

@@ -2,37 +2,52 @@ package com.unciv.ui.popups.options
 
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.UncivGame
-import com.unciv.ui.screens.basescreen.BaseScreen
-import com.unciv.ui.components.widgets.LanguageTable.Companion.addLanguageTables
+import com.unciv.ui.components.extensions.getAscendant
 import com.unciv.ui.components.input.onClick
+import com.unciv.ui.components.widgets.LanguageTable.Companion.addLanguageKeyShortcuts
+import com.unciv.ui.components.widgets.LanguageTable.Companion.addLanguageTables
+import com.unciv.ui.components.widgets.TabbedPager
 
-fun languageTab(
+class LanguageTab(
     optionsPopup: OptionsPopup,
-    onLanguageSelected: () -> Unit
-): Table = Table(BaseScreen.skin).apply {
-    val settings = optionsPopup.settings
+    private val onLanguageSelected: () -> Unit
+): Table(), TabbedPager.IPageExtensions {
+    private val languageTables = this.addLanguageTables(optionsPopup.tabs.prefWidth * 0.9f - 10f)
+    private val settings = optionsPopup.settings
+    private var chosenLanguage = settings.language
 
-    val languageTables = this.addLanguageTables(optionsPopup.tabs.prefWidth * 0.9f - 10f)
-
-    var chosenLanguage = settings.language
-    fun selectLanguage() {
+    private fun selectLanguage() {
         settings.language = chosenLanguage
         settings.updateLocaleFromLanguage()
         UncivGame.Current.translations.tryReadTranslationForCurrentLanguage()
         onLanguageSelected()
     }
 
-    fun updateSelection() {
+    private fun updateSelection() {
         languageTables.forEach { it.update(chosenLanguage) }
         if (chosenLanguage != settings.language)
             selectLanguage()
     }
-    updateSelection()
 
-    languageTables.forEach {
-        it.onClick {
-            chosenLanguage = it.language
-            updateSelection()
+    init {
+        for (langTable in languageTables) {
+            langTable.onClick {
+                chosenLanguage = langTable.language
+                updateSelection()
+            }
+        }
+        addLanguageKeyShortcuts(languageTables, getSelection = { chosenLanguage }) {
+            chosenLanguage = it
+            val pager = this.getAscendant<TabbedPager>()
+                ?: return@addLanguageKeyShortcuts
+            activated(pager.activePage, "", pager)
         }
     }
+
+    override fun activated(index: Int, caption: String, pager: TabbedPager) {
+        updateSelection()
+        val selectedTable = languageTables.firstOrNull { it.language == chosenLanguage }
+            ?: return
+        pager.pageScrollTo(selectedTable, true)
+    }
 }

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

@@ -90,7 +90,7 @@ class OptionsPopup(
         )
         tabs.addPage(
             "Language",
-            languageTab(this, ::reloadWorldAndOptions),
+            LanguageTab(this, ::reloadWorldAndOptions),
             ImageGetter.getImage("FlagIcons/${settings.language}"), 24f
         )
         tabs.addPage(

core/src/com/unciv/ui/screens/LanguagePickerScreen.kt

@@ -2,20 +2,22 @@ package com.unciv.ui.screens
 
 import com.unciv.Constants
 import com.unciv.models.translations.tr
-import com.unciv.ui.popups.options.OptionsPopup
-import com.unciv.ui.screens.pickerscreens.PickerScreen
-import com.unciv.ui.components.widgets.LanguageTable
-import com.unciv.ui.components.widgets.LanguageTable.Companion.addLanguageTables
 import com.unciv.ui.components.extensions.enable
+import com.unciv.ui.components.extensions.scrollTo
 import com.unciv.ui.components.input.onClick
+import com.unciv.ui.components.widgets.LanguageTable
+import com.unciv.ui.components.widgets.LanguageTable.Companion.addLanguageKeyShortcuts
+import com.unciv.ui.components.widgets.LanguageTable.Companion.addLanguageTables
+import com.unciv.ui.popups.options.OptionsPopup
 import com.unciv.ui.screens.mainmenuscreen.MainMenuScreen
+import com.unciv.ui.screens.pickerscreens.PickerScreen
 
 /** A [PickerScreen] to select a language, used once on the initial run after a fresh install.
  *  After that, [OptionsPopup] provides the functionality.
  *  Reusable code is in [LanguageTable] and [addLanguageTables].
  */
 class LanguagePickerScreen : PickerScreen() {
-    var chosenLanguage = Constants.english
+    private var chosenLanguage = Constants.english
 
     private val languageTables: ArrayList<LanguageTable>
 
@@ -28,21 +30,32 @@ class LanguagePickerScreen : PickerScreen() {
 
         languageTables = topTable.addLanguageTables(stage.width - 60f)
 
-        languageTables.forEach {
-            it.onClick {
-                chosenLanguage = it.language
-                rightSideButton.enable()
-                update()
+        for (languageTable in languageTables) {
+            languageTable.onClick {
+                onChoice(languageTable.language)
             }
         }
 
+        topTable.addLanguageKeyShortcuts(languageTables, { chosenLanguage }) { language ->
+            onChoice(language)
+            val selectedTable = languageTables.firstOrNull { it.language == language }
+                ?: return@addLanguageKeyShortcuts
+            scrollPane.scrollTo(selectedTable, true)
+        }
+
         rightSideButton.setText("Pick language".tr())
         rightSideButton.onClick {
             pickLanguage()
         }
     }
 
-    fun pickLanguage() {
+    private fun onChoice(choice: String) {
+        chosenLanguage = choice
+        rightSideButton.enable()
+        update()
+    }
+
+    private fun pickLanguage() {
         game.settings.language = chosenLanguage
         game.settings.updateLocaleFromLanguage()
         game.settings.isFreshlyCreated = false     // mark so the picker isn't called next launch