Notifications architectural update (#9605)

* Improved Notifications architecture

* Revert NotificationIcon as class hierarchy

* Improved Notifications architecture - migration roadmap first step

core/src/com/unciv/json/UncivJson.kt

@@ -6,6 +6,7 @@ import com.badlogic.gdx.utils.Json
 import com.badlogic.gdx.utils.JsonWriter
 import com.badlogic.gdx.utils.SerializationException
 import com.unciv.logic.civilization.CivRankingHistory
+import com.unciv.logic.civilization.Notification
 import com.unciv.logic.map.tile.TileHistory
 import com.unciv.ui.components.input.KeyCharAndCode
 import com.unciv.ui.components.input.KeyboardBindings
@@ -29,6 +30,7 @@ fun json() = Json(JsonWriter.OutputType.json).apply {
     setSerializer(KeyboardBindings::class.java, KeyboardBindings.Serializer())
     setSerializer(TileHistory::class.java, TileHistory.Serializer())
     setSerializer(CivRankingHistory::class.java, CivRankingHistory.Serializer())
+    setSerializer(Notification::class.java, Notification.Serializer())
 }
 
 /**

core/src/com/unciv/logic/GameInfo.kt

@@ -529,8 +529,8 @@ class GameInfo : IsPartOfGameInfoSerialization, HasGameInfoSerializationVersion
         else
             "[$positionsCount] sources of [$resourceName] revealed, e.g. near [${chosenCity.name}]"
 
-        return Notification(text, arrayListOf("ResourceIcons/$resourceName"),
+        return Notification(text, arrayOf("ResourceIcons/$resourceName"),
-            LocationAction(positions), NotificationCategory.General)
+            LocationAction(positions).asIterable(), NotificationCategory.General)
     }
 
     // All cross-game data which needs to be altered (e.g. when removing or changing a name of a building/tech)

core/src/com/unciv/logic/civilization/Civilization.kt

@@ -756,18 +756,24 @@ class Civilization : IsPartOfGameInfoSerialization {
         }
     }
 
-    fun addNotification(text: String, location: Vector2, category: NotificationCategory, vararg notificationIcons: String) {
+    // region addNotification
+    fun addNotification(text: String, category: NotificationCategory, vararg notificationIcons: String) =
+        addNotification(text, null, category, *notificationIcons)
+
+    fun addNotification(text: String, location: Vector2, category: NotificationCategory, vararg notificationIcons: String) =
         addNotification(text, LocationAction(location), category, *notificationIcons)
-    }
 
-    fun addNotification(text: String, category: NotificationCategory, vararg notificationIcons: String) = addNotification(text, null, category, *notificationIcons)
+    fun addNotification(text: String, action: NotificationAction, category: NotificationCategory, vararg notificationIcons: String) =
+        addNotification(text, listOf(action), category, *notificationIcons)
 
-    fun addNotification(text: String, action: NotificationAction?, category: NotificationCategory, vararg notificationIcons: String) {
+    fun addNotification(text: String, actions: Sequence<NotificationAction>, category:NotificationCategory, vararg notificationIcons: String) =
+        addNotification(text, actions.asIterable(), category, *notificationIcons)
+
+    fun addNotification(text: String, actions: Iterable<NotificationAction>?, category: NotificationCategory, vararg notificationIcons: String) {
         if (playerType == PlayerType.AI) return // no point in lengthening the saved game info if no one will read it
-        val arrayList = notificationIcons.toCollection(ArrayList())
+        notifications.add(Notification(text, notificationIcons, actions, category))
-        notifications.add(Notification(text, arrayList,
-                if (action is LocationAction && action.locations.isEmpty()) null else action, category))
     }
+    // endregion
 
     fun addCity(location: Vector2) {
         val newCity = CityFounder().foundCity(this, location)

core/src/com/unciv/logic/civilization/Notification.kt

@@ -3,79 +3,71 @@ package com.unciv.logic.civilization
 import com.badlogic.gdx.math.Vector2
 import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.utils.Json
+import com.badlogic.gdx.utils.JsonValue
 import com.unciv.logic.IsPartOfGameInfoSerialization
 import com.unciv.models.ruleset.Ruleset
-import com.unciv.ui.screens.cityscreen.CityScreen
 import com.unciv.ui.images.ImageGetter
-import com.unciv.ui.screens.pickerscreens.TechPickerScreen
-import com.unciv.ui.screens.diplomacyscreen.DiplomacyScreen
-import com.unciv.ui.components.MayaCalendar
 import com.unciv.ui.screens.worldscreen.WorldScreen
 
-object NotificationIcon {
-    // Remember: The typical white-on-transparency icon will not be visible on Notifications
 
-    const val Barbarians = "ImprovementIcons/Barbarian encampment"
+typealias NotificationCategory = Notification.NotificationCategory
-    const val Citadel = "ImprovementIcons/Citadel"
-    const val City = "ImprovementIcons/City center"
-    const val CityState = "OtherIcons/CityState"
-    const val Crosshair = "OtherIcons/CrosshairB"
-    const val Culture = "StatIcons/Culture"
-    const val Construction = "StatIcons/Production"
-    const val Death = "OtherIcons/DisbandUnit"
-    const val Diplomacy = "OtherIcons/Diplomacy"
-    const val Faith = "StatIcons/Faith"
-    const val Food = "StatIcons/Food"
-    const val Gold = "StatIcons/Gold"
-    const val Growth = "StatIcons/Population"
-    const val Happiness = "StatIcons/Happiness"
-    const val Population = "StatIcons/Population"
-    const val Production = "StatIcons/Production"
-    const val Question = "OtherIcons/Question"
-    const val Ruins = "ImprovementIcons/Ancient ruins"
-    const val Science = "StatIcons/Science"
-    const val Scout = "UnitIcons/Scout"
-    const val Spy = "OtherIcons/Spy"
-    const val Trade = "StatIcons/Acquire"
-    const val War = "OtherIcons/Pillage"
-}
 
-enum class NotificationCategory {
-    General,
-    Trade,
-    Diplomacy,
-    Production,
-    Units,
-    War,
-    Religion,
-    Espionage,
-    Cities
-    ;
-    companion object {
-        fun safeValueOf(name: String): NotificationCategory? =
-                values().firstOrNull { it.name == name }
-    }
-}
-
-/**
- * [action] is not realized as lambda, as it would be too easy to introduce references to objects
- * there that should not be serialized to the saved game.
- */
 open class Notification() : IsPartOfGameInfoSerialization {
+    /** Category - UI grouping, within a Category the most recent Notification will be shown on top */
+    var category: NotificationCategory = NotificationCategory.General
+        private set
 
+    /** The notification text, untranslated - will be translated on the fly */
     var text: String = ""
+        private set
 
+    /** Icons to be shown */
     var icons: ArrayList<String> = ArrayList() // Must be ArrayList and not List so it can be deserialized
-    var action: NotificationAction? = null
+        private set
-    var category: String = NotificationCategory.General.name
 
-    constructor(text: String, notificationIcons: ArrayList<String>, action: NotificationAction?, category: NotificationCategory) : this() {
+    /** Actions on clicking a Notification - will be activated round-robin style */
+    var actions: ArrayList<NotificationAction> = ArrayList()
+        private set
+
+    constructor(
+        text: String,
+        notificationIcons: Array<out String>,
+        actions: Iterable<NotificationAction>?,
+        category: NotificationCategory = NotificationCategory.General
+    ) : this() {
+        this.category = category
         this.text = text
-        this.icons = notificationIcons
+        if (notificationIcons.isNotEmpty()) {
-        this.action = action
+            this.icons = notificationIcons.toCollection(ArrayList())
-        this.category = category.name
+        }
+        actions?.toCollection(this.actions)
     }
 
+    enum class NotificationCategory {
+        // These names are displayed, so remember to add a translation template
+        // - if there's no other source for one.
+        General,
+        Trade,
+        Diplomacy,
+        Production,
+        Units,
+        War,
+        Religion,
+        Espionage,
+        Cities
+        ;
+
+        companion object {
+            fun safeValueOf(name: String): NotificationCategory? =
+                values().firstOrNull { it.name == name }
+        }
+    }
+
+    @Transient
+    /** For round-robin activation in [execute] */
+    private var index = 0
+
     fun addNotificationIconsTo(table: Table, ruleset: Ruleset, iconSize: Float) {
         if (icons.isEmpty()) return
         for (icon in icons.reversed()) {
@@ -92,63 +84,120 @@ open class Notification() : IsPartOfGameInfoSerialization {
             table.add(image).size(iconSize).padRight(5f)
         }
     }
-}
 
-/** defines what to do if the user clicks on a notification */
+    fun execute(worldScreen: WorldScreen) {
-interface NotificationAction : IsPartOfGameInfoSerialization {
+        if (actions.isEmpty()) return
-    fun execute(worldScreen: WorldScreen)
+        actions[index].execute(worldScreen)
-}
+        index = ++index % actions.size // cycle through tiles
+    }
 
-/** A notification action that cycles through tiles.
+    /**
- *
+     *  Custom [Gdx.Json][Json] serializer/deserializer for one [Notification].
- * Constructors accept any kind of [Vector2] collection, including [Iterable], [Sequence], `vararg`.
+     *
- * `varargs` allows nulls which are ignored, a resulting empty list is allowed and equivalent to no [NotificationAction].
+     *  Migration roadmap:
- */
+     *
-data class LocationAction(var locations: ArrayList<Vector2> = ArrayList()) : NotificationAction, IsPartOfGameInfoSerialization {
+     *      1.) Change internal structures but write old json format
-    constructor(locations: Iterable<Vector2>) : this(locations.toCollection(ArrayList()))
+     *      2.) Wait for good distribution in multiplayer user base
-    constructor(locations: Sequence<Vector2>) : this(locations.toCollection(ArrayList()))
+     *      3.) Switch to writing new format
-    constructor(vararg locations: Vector2?) : this(locations.asSequence().filterNotNull())
+     *      4.) Wait for Versions prior to Step 3 to fade out, keep switch for quick revert
+     *      5.) Remove Switch, old format routines and this comment
+     *
+     *  Caveats:
+     *
+     *      * New format can express Notifications the old can't.
+     *        In that case, in Phase 1, reduce to first action and throw away the rest.
+     */
+    class Serializer : Json.Serializer<Notification> {
+        companion object {
+            /** The switch that starts Phase III and dies with Phase V
+            *   @see Serializer */
+            private const val compatibilityMode = true
+        }
 
-    @Transient
+        override fun write(json: Json, notification: Notification, knownType: Class<*>?) {
-    private var index = 0
+            json.writeObjectStart()
+            if (notification.category != NotificationCategory.General)
+                json.writeValue("category", notification.category)
+            if (notification.text.isNotEmpty())
+                json.writeValue("text", notification.text)
+            if (notification.icons.isNotEmpty())
+                json.writeValue("icons", notification.icons, null, String::class.java)
 
-    override fun execute(worldScreen: WorldScreen) {
+            if (compatibilityMode) writeOldFormatAction(json, notification)
-        if (locations.isNotEmpty()) {
+            else writeNewFormatActions(json, notification)
-            worldScreen.mapHolder.setCenterPosition(locations[index], selectUnit = false)
+
-            index = ++index % locations.size // cycle through tiles
+            json.writeObjectEnd()
+        }
+
+        private fun writeNewFormatActions(json: Json, notification: Notification) {
+            if (notification.actions.isEmpty()) return
+            json.writeArrayStart("actions")
+            for (action in notification.actions) {
+                json.writeObjectStart()
+                json.writeObjectStart(action::class.java.simpleName)
+                json.writeFields(action)
+                json.writeObjectEnd()
+                json.writeObjectEnd()
+            }
+            json.writeArrayEnd()
+        }
+
+        private fun writeOldFormatAction(json: Json, notification: Notification) {
+            if (notification.actions.isEmpty()) return
+            val firstAction = notification.actions.first()
+            if (firstAction !is LocationAction) {
+                json.writeValue("action", firstAction, null)
+                return
+            }
+            val locations = notification.actions.filterIsInstance<LocationAction>()
+                .map { it.location }.toTypedArray()
+            json.writeObjectStart("action")
+            json.writeValue("class", "com.unciv.logic.civilization.LocationAction")
+            json.writeValue("locations", locations, Array<Vector2>::class.java, Vector2::class.java)
+            json.writeObjectEnd()
+        }
+
+        override fun read(json: Json, jsonData: JsonValue, type: Class<*>?) = Notification().apply {
+            // Cannot be distinguished 100% certain by field names but if neither action / actions exist then both formats are compatible
+            json.readField(this, "category", jsonData)
+            json.readField(this, "text", jsonData)
+            readOldFormatAction(json, jsonData)
+            readNewFormatActions(json, jsonData)
+            json.readField(this, "icons", jsonData)
+        }
+
+        private fun Notification.readNewFormatActions(json: Json, jsonData: JsonValue) {
+            // New format looks like this: "notifications":[
+            //      {"category":"Cities","text":"[Stockholm] has expanded its borders!","icons":["StatIcons/Culture"],"actions":[{"LocationAction":{"location":{"x":7,"y":1}}},{"LocationAction":{"location":{"x":9,"y":3}}}]},
+            //      {"category":"Production","text":"[Nobel Foundation] has been built in [Stockholm]","icons":["BuildingIcons/Nobel Foundation"],"actions":[{"LocationAction":{"location":{"x":9,"y":3}}}]}
+            //  ]
+            if (!jsonData.hasChild("actions")) return
+            var entry = jsonData.get("actions").child
+            while (entry != null) {
+                actions.addAll(NotificationActionsDeserializer().read(json, entry))
+                entry = entry.next
+            }
+        }
+
+        private fun Notification.readOldFormatAction(json: Json, jsonData: JsonValue) {
+            // Old format looks like: "notifications":[
+            //      {"text":"[Stockholm] has expanded its borders!","icons":["StatIcons/Culture"],"action":{"class":"com.unciv.logic.civilization.LocationAction","locations":[{"x":7,"y":1},{"x":9,"y":3}]},"category":"Cities"},
+            //      {"text":"[Nobel Foundation] has been built in [Stockholm]","icons":["BuildingIcons/Nobel Foundation"],"action":{"class":"com.unciv.logic.civilization.LocationAction","locations":[{"x":9,"y":3}]},"category":"Production"}
+            //  ]
+            val actionData = jsonData.get("action") ?: return
+            val actionClass = actionData.getString("class")
+            when (actionClass.substring(actionClass.lastIndexOf('.') + 1)) {
+                "LocationAction" -> actions += getOldFormatLocations(json, actionData)
+                "TechAction" -> actions += json.readValue(TechAction::class.java, actionData)
+                "CityAction" -> actions += json.readValue(CityAction::class.java, actionData)
+                "DiplomacyAction" -> actions += json.readValue(DiplomacyAction::class.java, actionData)
+                "MayaLongCountAction" -> actions += MayaLongCountAction()
+            }
+        }
+
+        private fun getOldFormatLocations(json: Json, actionData: JsonValue): Sequence<LocationAction> {
+            val locations = json.readValue("locations", Array<Vector2>::class.java, actionData)
+            return locations.asSequence().map { LocationAction(it) }
         }
     }
 }
-
-/** show tech screen */
-class TechAction(val techName: String = "") : NotificationAction, IsPartOfGameInfoSerialization {
-    override fun execute(worldScreen: WorldScreen) {
-        val tech = worldScreen.gameInfo.ruleset.technologies[techName]
-        worldScreen.game.pushScreen(TechPickerScreen(worldScreen.viewingCiv, tech))
-    }
-}
-
-/** enter city */
-data class CityAction(val city: Vector2 = Vector2.Zero): NotificationAction, IsPartOfGameInfoSerialization {
-    override fun execute(worldScreen: WorldScreen) {
-        worldScreen.mapHolder.tileMap[city].getCity()?.let {
-            if (it.civ == worldScreen.viewingCiv)
-                worldScreen.game.pushScreen(CityScreen(it))
-        }
-    }
-}
-
-/** enter diplomacy screen */
-data class DiplomacyAction(val otherCivName: String = ""): NotificationAction, IsPartOfGameInfoSerialization {
-    override fun execute(worldScreen: WorldScreen) {
-        val otherCiv = worldScreen.gameInfo.getCivilization(otherCivName)
-        worldScreen.game.pushScreen(DiplomacyScreen(worldScreen.viewingCiv, otherCiv))
-    }
-}
-
-/** enter Maya Long Count popup */
-class MayaLongCountAction : NotificationAction, IsPartOfGameInfoSerialization {
-    override fun execute(worldScreen: WorldScreen) {
-        MayaCalendar.openPopup(worldScreen, worldScreen.selectedCiv, worldScreen.gameInfo.getYear())
-    }
-}

core/src/com/unciv/logic/civilization/NotificationActions.kt

@@ -0,0 +1,118 @@
+package com.unciv.logic.civilization
+
+import com.badlogic.gdx.math.Vector2
+import com.badlogic.gdx.utils.Json
+import com.badlogic.gdx.utils.JsonValue
+import com.unciv.logic.IsPartOfGameInfoSerialization
+import com.unciv.ui.components.MayaCalendar
+import com.unciv.ui.screens.cityscreen.CityScreen
+import com.unciv.ui.screens.civilopediascreen.CivilopediaCategories
+import com.unciv.ui.screens.civilopediascreen.CivilopediaScreen
+import com.unciv.ui.screens.diplomacyscreen.DiplomacyScreen
+import com.unciv.ui.screens.pickerscreens.PromotionPickerScreen
+import com.unciv.ui.screens.pickerscreens.TechPickerScreen
+import com.unciv.ui.screens.worldscreen.WorldScreen
+
+
+/** defines what to do if the user clicks on a notification */
+/*
+ * Not realized as lambda, as it would be too easy to introduce references to objects
+ * there that should not be serialized to the saved game.
+ */
+interface NotificationAction : IsPartOfGameInfoSerialization {
+    fun execute(worldScreen: WorldScreen)
+}
+
+/** A notification action that shows map places. */
+class LocationAction(var location: Vector2) : NotificationAction, IsPartOfGameInfoSerialization {
+    override fun execute(worldScreen: WorldScreen) {
+        worldScreen.mapHolder.setCenterPosition(location, selectUnit = false)
+    }
+    companion object {
+        operator fun invoke(locations: Sequence<Vector2>): Sequence<LocationAction> =
+            locations.map { LocationAction(it) }
+        operator fun invoke(locations: Iterable<Vector2>): Sequence<LocationAction> =
+            locations.asSequence().map { LocationAction(it) }
+        operator fun invoke(vararg locations: Vector2?): Sequence<LocationAction> =
+            locations.asSequence().filterNotNull().map { LocationAction(it) }
+    }
+}
+
+/** show tech screen */
+class TechAction(val techName: String = "") : NotificationAction, IsPartOfGameInfoSerialization {
+    override fun execute(worldScreen: WorldScreen) {
+        val tech = worldScreen.gameInfo.ruleset.technologies[techName]
+        worldScreen.game.pushScreen(TechPickerScreen(worldScreen.viewingCiv, tech))
+    }
+}
+
+/** enter city */
+class CityAction(val city: Vector2 = Vector2.Zero): NotificationAction,
+    IsPartOfGameInfoSerialization {
+    override fun execute(worldScreen: WorldScreen) {
+        val cityObject = worldScreen.mapHolder.tileMap[city].getCity()
+            ?: return
+        if (cityObject.civ == worldScreen.viewingCiv)
+            worldScreen.game.pushScreen(CityScreen(cityObject))
+    }
+}
+
+/** enter diplomacy screen */
+class DiplomacyAction(val otherCivName: String = ""): NotificationAction,
+    IsPartOfGameInfoSerialization {
+    override fun execute(worldScreen: WorldScreen) {
+        val otherCiv = worldScreen.gameInfo.getCivilization(otherCivName)
+        worldScreen.game.pushScreen(DiplomacyScreen(worldScreen.viewingCiv, otherCiv))
+    }
+}
+
+/** enter Maya Long Count popup */
+class MayaLongCountAction : NotificationAction, IsPartOfGameInfoSerialization {
+    override fun execute(worldScreen: WorldScreen) {
+        MayaCalendar.openPopup(worldScreen, worldScreen.selectedCiv, worldScreen.gameInfo.getYear())
+    }
+}
+
+/** A notification action that shows and selects units on the map. */
+class MapUnitAction(var location: Vector2) : NotificationAction, IsPartOfGameInfoSerialization {
+    override fun execute(worldScreen: WorldScreen) {
+        worldScreen.mapHolder.setCenterPosition(location, selectUnit = true)
+    }
+}
+
+/** A notification action that shows the Civilopedia entry for a Wonder. */
+class WonderAction(val wonderName: String) : NotificationAction, IsPartOfGameInfoSerialization {
+    override fun execute(worldScreen: WorldScreen) {
+        worldScreen.game.pushScreen(CivilopediaScreen(worldScreen.gameInfo.ruleset, CivilopediaCategories.Wonder, wonderName))
+    }
+}
+
+/** Show Promotion picker for a MapUnit - by name and location, as they lack a serialized unique ID */
+class PromoteUnitAction(val name: String, val location: Vector2) : NotificationAction, IsPartOfGameInfoSerialization {
+    override fun execute(worldScreen: WorldScreen) {
+        val tile = worldScreen.gameInfo.tileMap[location]
+        val unit = tile.militaryUnit?.takeIf { it.name == name && it.civ == worldScreen.selectedCiv }
+            ?: return
+        worldScreen.game.pushScreen(PromotionPickerScreen(unit))
+    }
+}
+
+@Suppress("PropertyName")
+internal class NotificationActionsDeserializer {
+    // This exists as trick to leverage readFields for Json deserialization
+    var LocationAction: LocationAction? = null
+    var TechAction: TechAction? = null
+    var CityAction: CityAction? = null
+    var DiplomacyAction: DiplomacyAction? = null
+    var MayaLongCountAction: MayaLongCountAction? = null
+    var MapUnitAction: MapUnitAction? = null
+    var WonderAction: WonderAction? = null
+    var PromoteUnitAction: PromoteUnitAction? = null
+    fun read(json: Json, jsonData: JsonValue): List<NotificationAction> {
+        json.readFields(this, jsonData)
+        return listOfNotNull(
+            LocationAction, TechAction, CityAction, DiplomacyAction,
+            MayaLongCountAction, MapUnitAction, WonderAction, PromoteUnitAction
+        )
+    }
+}

core/src/com/unciv/logic/civilization/NotificationIcons.kt

@@ -0,0 +1,29 @@
+package com.unciv.logic.civilization
+
+object NotificationIcon {
+    // Remember: The typical white-on-transparency icon will not be visible on Notifications
+
+    const val Barbarians = "ImprovementIcons/Barbarian encampment"
+    const val Citadel = "ImprovementIcons/Citadel"
+    const val City = "ImprovementIcons/City center"
+    const val CityState = "OtherIcons/CityState"
+    const val Crosshair = "OtherIcons/CrosshairB"
+    const val Culture = "StatIcons/Culture"
+    const val Construction = "StatIcons/Production"
+    const val Death = "OtherIcons/DisbandUnit"
+    const val Diplomacy = "OtherIcons/Diplomacy"
+    const val Faith = "StatIcons/Faith"
+    const val Food = "StatIcons/Food"
+    const val Gold = "StatIcons/Gold"
+    const val Growth = "StatIcons/Population"
+    const val Happiness = "StatIcons/Happiness"
+    const val Population = "StatIcons/Population"
+    const val Production = "StatIcons/Production"
+    const val Question = "OtherIcons/Question"
+    const val Ruins = "ImprovementIcons/Ancient ruins"
+    const val Science = "StatIcons/Science"
+    const val Scout = "UnitIcons/Scout"
+    const val Spy = "OtherIcons/Spy"
+    const val Trade = "StatIcons/Acquire"
+    const val War = "OtherIcons/Pillage"
+}

core/src/com/unciv/logic/civilization/managers/TechManager.kt

@@ -382,7 +382,7 @@ class TechManager : IsPartOfGameInfoSerialization {
         // Add notifications for obsolete units/constructions
         for ((unit, cities) in unitUpgrades) {
             if (cities.isEmpty()) continue
-            val locationAction = LocationAction(cities.mapTo(ArrayList(cities.size)) { it.location })
+            val locationAction = LocationAction(cities.asSequence().map { it.location })
             val cityText = if (cities.size == 1) "[${cities.first().name}]"
                 else "[${cities.size}] cities"
             val newUnit = obsoleteUnits[unit]?.name

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

@@ -36,6 +36,6 @@ abstract class EmpireOverviewTab (
         val worldScreen = GUI.getWorldScreen()
         worldScreen.notificationsScroll.oneTimeNotification = notification
         UncivGame.Current.resetToWorldScreen()
-        notification.action?.execute(worldScreen)
+        notification.execute(worldScreen)
     }
 }

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

@@ -73,7 +73,7 @@ class NotificationsOverviewTable(
         }).row()
 
         for (category in NotificationCategory.values()){
-            val categoryNotifications = notifications.filter { it.category == category.name }
+            val categoryNotifications = notifications.filter { it.category == category }
             if (categoryNotifications.isEmpty()) continue
 
             if (category != NotificationCategory.General)
@@ -88,7 +88,7 @@ class NotificationsOverviewTable(
                 notificationTable.add(label).width(stageWidth / 2 - iconSize * notification.icons.size)
                 notificationTable.background = BaseScreen.skinStrings.getUiBackground("OverviewScreen/NotificationOverviewTable/Notification", BaseScreen.skinStrings.roundedEdgeRectangleShape)
                 notificationTable.touchable = Touchable.enabled
-                if (notification.action != null)
+                if (notification.actions.isNotEmpty())
                     notificationTable.onClick { showOneTimeNotification(notification) }
 
                 notification.addNotificationIconsTo(notificationTable, gameInfo.ruleset, iconSize)

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

@@ -238,7 +238,7 @@ class NotificationsScroll(
         val backgroundDrawable = BaseScreen.skinStrings.getUiBackground("WorldScreen/Notification", BaseScreen.skinStrings.roundedEdgeRectangleShape)
 
         val orderedNotifications = (additionalNotification + notifications.asReversed())
-            .groupBy { NotificationCategory.safeValueOf(it.category) ?: NotificationCategory.General }
+            .groupBy { it.category }
             .toSortedMap()  // This sorts by Category ordinal, so far intentional - the order of the grouped lists are unaffected
         for ((category, categoryNotifications) in orderedNotifications) {
             if (category == NotificationCategory.General)
@@ -351,7 +351,7 @@ class NotificationsScroll(
             add(listItem).pad(topBottomPad, listItemPad, topBottomPad, rightPadToScreenEdge)
             touchable = Touchable.enabled
             onClick {
-                notification.action?.execute(worldScreen)
+                notification.execute(worldScreen)
                 clickedNotification = notification
                 GUI.setUpdateWorldOnNextRender()
             }