User option to control NotificationScroll behaviour (#9148)

core/src/com/unciv/models/metadata/GameSettings.kt

@@ -21,7 +21,11 @@ import kotlin.reflect.KMutableProperty0
 
 data class WindowState (val width: Int = 900, val height: Int = 600)
 
-enum class ScreenSize(val virtualWidth:Float, val virtualHeight:Float){
+enum class ScreenSize(
+    @Suppress("unused")  // Actual width determined by screen aspect ratio, this as comment only
+    val virtualWidth: Float,
+    val virtualHeight: Float
+) {
     Tiny(750f,500f),
     Small(900f,600f),
     Medium(1050f,700f),
@@ -111,6 +115,9 @@ class GameSettings {
 
     var keyBindings = KeyboardBindings()
 
+    /** NotificationScroll on Word Screen visibility control - mapped to NotificationsScroll.UserSetting enum */
+    var notificationScroll: String = ""
+
     /** used to migrate from older versions of the settings */
     var version: Int? = null
 
@@ -153,7 +160,7 @@ class GameSettings {
         return (Fonts.ORIGINAL_FONT_SIZE * fontSizeMultiplier).toInt()
     }
 
-    fun getCurrentLocale(): Locale {
+    private fun getCurrentLocale(): Locale {
         if (locale == null)
             updateLocaleFromLanguage()
         return locale!!
@@ -213,15 +220,16 @@ enum class LocaleCode(var language: String, var country: String) {
 class GameSettingsMultiplayer {
     var userId = ""
     var passwords = mutableMapOf<String, String>()
+    @Suppress("unused")  // @GGuenni knows what he intended with this field
     var userName: String = ""
     var server = Constants.uncivXyzServer
     var friendList: MutableList<FriendList.Friend> = mutableListOf()
     var turnCheckerEnabled = true
     var turnCheckerPersistentNotificationEnabled = true
-    var turnCheckerDelay = Duration.ofMinutes(5)
+    var turnCheckerDelay: Duration = Duration.ofMinutes(5)
     var statusButtonInSinglePlayer = false
-    var currentGameRefreshDelay = Duration.ofSeconds(10)
+    var currentGameRefreshDelay: Duration = Duration.ofSeconds(10)
-    var allGameRefreshDelay = Duration.ofMinutes(5)
+    var allGameRefreshDelay: Duration = Duration.ofMinutes(5)
     var currentGameTurnNotificationSound: UncivSound = UncivSound.Silent
     var otherGameTurnNotificationSound: UncivSound = UncivSound.Silent
     var hideDropboxWarning = false
@@ -233,6 +241,7 @@ class GameSettingsMultiplayer {
     }
 }
 
+@Suppress("SuspiciousCallableReferenceInLambda")  // By @Azzurite, safe as long as that warning below is followed
 enum class GameSetting(
     val kClass: KClass<*>,
     private val propertyGetter: (GameSettings) -> KMutableProperty0<*>

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

@@ -7,7 +7,6 @@ import com.badlogic.gdx.scenes.scene2d.ui.SelectBox
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.utils.Array
 import com.unciv.GUI
-import com.unciv.UncivGame
 import com.unciv.models.metadata.GameSettings
 import com.unciv.models.metadata.ScreenSize
 import com.unciv.models.skins.SkinCache
@@ -24,10 +23,13 @@ import com.unciv.ui.components.extensions.onChange
 import com.unciv.ui.components.extensions.onClick
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.screens.worldscreen.NotificationsScroll
 import com.unciv.utils.Display
 import com.unciv.utils.ScreenMode
 
-
+/**
+ *  @param onChange Callback for _major_ changes, OptionsPopup will rebuild itself and the WorldScreen
+ */
 fun displayTab(
     optionsPopup: OptionsPopup,
     onChange: () -> Unit,
@@ -52,12 +54,14 @@ fun displayTab(
     optionsPopup.addCheckbox(this, "Show tile yields", settings.showTileYields, true) { settings.showTileYields = it } // JN
     optionsPopup.addCheckbox(this, "Show worked tiles", settings.showWorkedTiles, true) { settings.showWorkedTiles = it }
     optionsPopup.addCheckbox(this, "Show resources and improvements", settings.showResourcesAndImprovements, true) { settings.showResourcesAndImprovements = it }
-    optionsPopup.addCheckbox(this, "Show tutorials", settings.showTutorials, true, false) { settings.showTutorials = it }
+    optionsPopup.addCheckbox(this, "Show tutorials", settings.showTutorials, updateWorld = true, newRow = false) { settings.showTutorials = it }
     addResetTutorials(this, settings)
     optionsPopup.addCheckbox(this, "Show pixel improvements", settings.showPixelImprovements, true) { settings.showPixelImprovements = it }
     optionsPopup.addCheckbox(this, "Experimental Demographics scoreboard", settings.useDemographics, true) { settings.useDemographics = it }
     optionsPopup.addCheckbox(this, "Show zoom buttons in world screen", settings.showZoomButtons, true) { settings.showZoomButtons = it }
 
+    addNotificationScrollSelect(this, settings, optionsPopup.selectBoxMinWidth)
+
     addMinimapSizeSlider(this, settings, optionsPopup.selectBoxMinWidth)
 
     addUnitIconAlphaSlider(this, settings, optionsPopup.selectBoxMinWidth)
@@ -115,8 +119,6 @@ private fun addMinimapSizeSlider(table: Table, settings: GameSettings, selectBox
             settings.showMinimap = true
             settings.minimapSize = size
         }
-        val worldScreen = GUI.getWorldScreenIfActive()
-        if (worldScreen != null)
         GUI.setUpdateWorldOnNextRender()
     }
     table.add(minimapSlider).minWidth(selectBoxMinWidth).pad(10f).row()
@@ -145,11 +147,7 @@ private fun addUnitIconAlphaSlider(table: Table, settings: GameSettings, selectB
         0f, 1f, 0.1f, initial = settings.unitIconOpacity, getTipText = getTipText
     ) {
         settings.unitIconOpacity = it
-
+        GUI.setUpdateWorldOnNextRender()
-        val worldScreen = UncivGame.Current.getWorldScreenIfActive()
-        if (worldScreen != null)
-            worldScreen.shouldUpdate = true
-
     }
     table.add(unitIconAlphaSlider).minWidth(selectBoxMinWidth).pad(10f).row()
 }
@@ -266,3 +264,19 @@ private fun addResetTutorials(table: Table, settings: GameSettings) {
     }
     table.add(resetTutorialsButton).center().row()
 }
+
+private fun addNotificationScrollSelect(table: Table, settings: GameSettings, selectBoxMinWidth: Float) {
+    table.add("Notifications on world screen".toLabel()).left().fillX()
+
+    val selectBox = TranslatedSelectBox(
+        NotificationsScroll.UserSetting.values().map { it.name },
+        settings.notificationScroll,
+        table.skin
+    )
+    table.add(selectBox).minWidth(selectBoxMinWidth).pad(10f).row()
+
+    selectBox.onChange {
+        settings.notificationScroll = selectBox.selected.value
+        GUI.setUpdateWorldOnNextRender()
+    }
+}

core/src/com/unciv/ui/screens/overviewscreen/GlobalPoliticsOverviewTable.kt

@@ -183,7 +183,6 @@ class GlobalPoliticsOverviewTable (
         // wars
         for (otherCiv in civ.getKnownCivs()) {
             if (civ.isAtWarWith(otherCiv)) {
-                println(getCivName(otherCiv))
                 val warText = "At war with [${getCivName(otherCiv)}]".toLabel()
                 warText.color = Color.RED
                 politicsTable.add(warText).row()

core/src/com/unciv/ui/screens/worldscreen/NotificationsScroll.kt

@@ -10,6 +10,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.Image
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.scenes.scene2d.utils.NinePatchDrawable
 import com.badlogic.gdx.utils.Align
+import com.unciv.GUI
 import com.unciv.logic.civilization.Notification
 import com.unciv.logic.civilization.NotificationCategory
 import com.unciv.ui.components.ColorMarkupLabel
@@ -23,15 +24,18 @@ import com.unciv.ui.images.IconCircleGroup
 import com.unciv.ui.components.AutoScrollPane as ScrollPane
 
 /*TODO
- *  Some persistence - over game or over settings?
+ *  Un-hiding the notifications when new ones arrive is a little pointless due to Categories:
- *  Check state after Undo
+ *      * try to scroll new into view? Very complicated as the "old" state is only "available" in the Widgets
- *  Idea: Blink the button when new notifications while "away"
+ *      * Don't unless a new user option disables categories, then scroll to top?
+ *  Idea: Blink or tint the button when new notifications while Hidden
  *  Idea: The little "1" on the bell - remove and draw actual count
  */
 
 class NotificationsScroll(
     private val worldScreen: WorldScreen
 ) : ScrollPane(null) {
+    enum class UserSetting(val static: Boolean = false) { Disabled(true), Hidden, Visible, Permanent(true) }
+
     private companion object {
         /** Scale the entire ScrollPane by this factor */
         const val scaleFactor = 0.5f
@@ -71,6 +75,9 @@ class NotificationsScroll(
 
     private val restoreButton = RestoreButton()
 
+    private var userSetting = UserSetting.Visible
+    private var userSettingChanged = false
+
     init {
         actor = notificationsTable.right()
         touchable = Touchable.childrenOnly
@@ -81,11 +88,12 @@ class NotificationsScroll(
 
     /** Access to hidden "state" - writing it will ensure this is fully visible or hidden and the
      *  restore button shown as needed - with animation. */
+    @Suppress("MemberVisibilityCanBePrivate")  // API for future use
     var isHidden: Boolean
         get () = scrollX <= scrolledAwayEpsilon
         set(value) {
-            restoreButton.blocked = false
+            restoreButton.unblock()
-            scrollX = if (value) 0f else width
+            scrollX = if (value) 0f else maxX
         }
 
     /**
@@ -102,25 +110,34 @@ class NotificationsScroll(
         coveredNotificationsTop: Float,
         coveredNotificationsBottom: Float
     ) {
-        // Initial scrollX should scroll all the way to the right - the setter automatically clamps
+        if (getUserSettingCheckDisabled()) return
-        val previousScrollX = if (notificationsTable.hasChildren()) scrollX else Float.MAX_VALUE
+
+        val previousScrollX = when {
+            isScrollingDisabledX -> width  // switching from Permanent - scrollX and maxX are 0
+            userSetting.static -> maxX  // Permanent: fully visible
+            notificationsTable.hasChildren() -> scrollX  // save current scroll
+            else -> 0f  // Swiching Hidden to Dynamic - animate "in" only
+        }
         val previousScrollY = scrollY
 
-        restoreButton.blocked = true  // For the update, since ScrollPane may layout and change scrollX
+        restoreButton.block()  // For the update, since ScrollPane may layout and change scrollX
-        if (updateContent(notifications, coveredNotificationsTop, coveredNotificationsBottom)) {
+        val contentChanged = updateContent(notifications, coveredNotificationsTop, coveredNotificationsBottom)
+        if (contentChanged) {
             updateLayout()
-            if (notifications.isEmpty())
-                scrollX = previousScrollX
-            else
-                isHidden = false
         } else {
             updateSpacers(coveredNotificationsTop, coveredNotificationsBottom)
-            scrollX = previousScrollX
         }
 
+        scrollX = previousScrollX
         scrollY = previousScrollY
         updateVisualScroll()
-        restoreButton.blocked = false
+
+        applyUserSettingChange()
+        if (!userSetting.static) {
+            restoreButton.unblock()
+            if (contentChanged && !userSettingChanged && isHidden)
+                isHidden = false
+        }
 
         // Do the positioning here since WorldScreen may also call update when just its geometry changed
         setPosition(stage.width - width * scaleFactor, 0f)
@@ -281,8 +298,9 @@ class NotificationsScroll(
     }
 
     inner class RestoreButton : Container<IconCircleGroup>() {
-        var blocked = true
+        private var blockCheck = true
-        var active = false
+        private var blockAct = true
+        private var active = false
 
         init {
             actor = ImageGetter.getImage("OtherIcons/Notifications")
@@ -292,37 +310,50 @@ class NotificationsScroll(
             color = color.cpy()  // So we don't mutate a skin element while fading
             color.a = 0f  // for first fade-in
             onClick {
-                scrollX = this@NotificationsScroll.width
+                scrollX = maxX
                 hide()
             }
             pack()  // `this` needs to adopt the size of `actor`, won't happen automatically (surprisingly)
         }
 
+        fun block() {
+            blockCheck = true
+            blockAct = true
+        }
+        fun unblock() {
+            blockCheck = false
+            blockAct = false
+        }
+
         fun show() {
             active = true
-            blocked = false
+            clearActions()
+            updateUserSetting(UserSetting.Hidden)
             if (parent == null)
-                worldScreen.stage.addActor(this)
+                // `addActorAfter` to stay behind any popups
+                worldScreen.stage.root.addActorAfter(this@NotificationsScroll, this)
+            blockAct = false
             addAction(Actions.fadeIn(1f))
         }
 
         fun hide() {
+            active = false
             clearActions()
-            blocked = false
+            updateUserSetting(UserSetting.Visible)
             if (parent == null) return
+            blockAct = false
             addAction(
                 Actions.sequence(
                     Actions.fadeOut(0.333f),
                     Actions.run {
                         remove()
-                        active = false
                     }
                 )
             )
         }
 
         fun checkScrollX(scrollX: Float) {
-            if (blocked) return
+            if (blockCheck) return
             if (active && scrollX >= scrolledAwayEpsilon * 2)
                 hide()
             if (!active && scrollX <= scrolledAwayEpsilon)
@@ -331,9 +362,63 @@ class NotificationsScroll(
 
         override fun act(delta: Float) {
             // Actions are blocked while update() is rebuilding the UI elements - to be safe from unexpected state changes
-            if (!blocked)
+            if (!blockAct) super.act(delta)
-                super.act(delta)
         }
     }
 
+    private fun getUserSettingCheckDisabled(): Boolean {
+        val settingString = GUI.getSettings().notificationScroll
+        val setting = UserSetting.values().firstOrNull { it.name == settingString }
+            ?: UserSetting.Visible
+        userSettingChanged = false
+        if (setting == userSetting)
+            return setting == UserSetting.Disabled
+
+        userSetting = setting
+        userSettingChanged = true
+        if (setting != UserSetting.Disabled) return false
+
+        notificationsTable.clear()
+        notificationsHash = 0
+        scrollX = 0f
+        updateVisualScroll()
+        setScrollingDisabled(false, false)
+        isVisible = false
+        restoreButton.hide()
+        return true
+    }
+
+    private fun applyUserSettingChange() {
+        if (!userSettingChanged) return
+        // Here the rebuild of content and restoring of scrollX/Y already happened
+        restoreButton.block()
+        val fromPermanent = isScrollingDisabledX
+        setScrollingDisabled(userSetting.static, false)
+        if (fromPermanent) {
+            validate()
+            scrollX = maxX
+            updateVisualScroll()
+        }
+        when (userSetting) {
+            UserSetting.Hidden -> {
+                if (!isHidden) isHidden = true
+                restoreButton.show()
+            }
+            UserSetting.Visible -> {
+                if (isHidden) isHidden = false
+                restoreButton.hide()
+            }
+            UserSetting.Permanent -> {
+                restoreButton.hide()
+            }
+            else -> return
+        }
+        isVisible = true
+    }
+
+    private fun updateUserSetting(newSetting: UserSetting) {
+        if (newSetting == userSetting || userSetting.static) return
+        userSetting = newSetting
+        GUI.getSettings().notificationScroll = newSetting.name
+    }
 }