All notifications from Overview are now temporary (#9455)

* Refactor notification history re-show functionality

* Refactor explored resources notification generator

* Refactor "Unimproved" to use existing providesResource

* Make overview resource "links" into temporary notifications

* Added link to overview resource "Unimproved" column

* Minor linting

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

@@ -15,6 +15,7 @@ import com.unciv.logic.city.City
 import com.unciv.logic.civilization.Civilization
 import com.unciv.logic.civilization.CivilizationInfoPreview
 import com.unciv.logic.civilization.LocationAction
+import com.unciv.logic.civilization.Notification
 import com.unciv.logic.civilization.NotificationCategory
 import com.unciv.logic.civilization.NotificationIcon
 import com.unciv.logic.civilization.PlayerType
@@ -437,18 +438,37 @@ class GameInfo : IsPartOfGameInfoSerialization, HasGameInfoSerializationVersion
         }
     }
 
-    /** Generate a notification pointing out resources.
+    /** Generate and show a notification pointing out resources.
      *  Used by [addTechnology][TechManager.addTechnology] and [ResourcesOverviewTab][com.unciv.ui.screens.overviewscreen.ResourcesOverviewTab]
      * @param maxDistance from next City, 0 removes distance limitation.
-     * @param showForeign Disables filter to exclude foreign territory.
+     * @param filter optional tile filter predicate, e.g. to exclude foreign territory.
      * @return `false` if no resources were found and no notification was added.
+     * @see getExploredResourcesNotification
      */
     fun notifyExploredResources(
         civInfo: Civilization,
         resourceName: String,
-        maxDistance: Int,
+        maxDistance: Int = Int.MAX_VALUE,
-        showForeign: Boolean
+        filter: (Tile) -> Boolean = { true }
     ): Boolean {
+        val notification = getExploredResourcesNotification(civInfo, resourceName, maxDistance, filter)
+            ?: return false
+        civInfo.notifications.add(notification)
+        return true
+    }
+
+    /** Generate a notification pointing out resources.
+     * @param maxDistance from next City, default removes distance limitation.
+     * @param filter optional tile filter predicate, e.g. to exclude foreign territory.
+     * @return `null` if no resources were found, otherwise a Notification instance.
+     * @see notifyExploredResources
+     */
+    fun getExploredResourcesNotification(
+        civInfo: Civilization,
+        resourceName: String,
+        maxDistance: Int = Int.MAX_VALUE,
+        filter: (Tile) -> Boolean = { true }
+    ): Notification? {
         data class CityTileAndDistance(val city: City, val tile: Tile, val distance: Int)
 
         val exploredRevealTiles: Sequence<Tile> =
@@ -476,11 +496,12 @@ class GameInfo : IsPartOfGameInfoSerialization, HasGameInfoSerializationVersion
                         CityTileAndDistance(city, tile, tile.aerialDistanceTo(city.getCenterTile()))
                     }
             }
-            .filter { (maxDistance == 0 || it.distance <= maxDistance) && (showForeign || it.tile.getOwner() == null || it.tile.getOwner() == civInfo) }
+            .filter { it.distance <= maxDistance && filter(it.tile) }
             .sortedWith(compareBy { it.distance })
             .distinctBy { it.tile }
 
-        val chosenCity = exploredRevealInfo.firstOrNull()?.city ?: return false
+        val chosenCity = exploredRevealInfo.firstOrNull()?.city
+            ?: return null
         val positions = exploredRevealInfo
             // re-sort to a more pleasant display order
             .sortedWith(compareBy { it.tile.aerialDistanceTo(chosenCity.getCenterTile()) })
@@ -492,13 +513,8 @@ class GameInfo : IsPartOfGameInfoSerialization, HasGameInfoSerializationVersion
         else
             "[$positionsCount] sources of [$resourceName] revealed, e.g. near [${chosenCity.name}]"
 
-        civInfo.addNotification(
+        return Notification(text, arrayListOf("ResourceIcons/$resourceName"),
-            text,
+            LocationAction(positions), NotificationCategory.General)
-            LocationAction(positions),
-            NotificationCategory.General,
-            "ResourceIcons/$resourceName"
-        )
-        return true
     }
 
     // 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/managers/TechManager.kt

@@ -320,7 +320,7 @@ class TechManager : IsPartOfGameInfoSerialization {
                 // notifyExploredResources scans the player's owned tiles and returns false if none
                 // found with a revealed resource - keep this knowledge to avoid the update call.
                 mayNeedUpdateResources = mayNeedUpdateResources ||
-                    civInfo.gameInfo.notifyExploredResources(civInfo, revealedResource.name, 5, false)
+                    civInfo.gameInfo.notifyExploredResources(civInfo, revealedResource.name, 5)
             }
         }
         // At least in the case of a human player hurrying research, this civ's resource availability

core/src/com/unciv/logic/map/tile/Tile.kt

@@ -497,13 +497,12 @@ open class Tile : IsPartOfGameInfoSerialization {
         if (isCityCenter()) return true
         val improvement = getUnpillagedTileImprovement()
         if (improvement != null && improvement.name in tileResource.getImprovements()
-                && (improvement.techRequired==null || civInfo.tech.isResearched(improvement.techRequired!!))) return true
+                && (improvement.techRequired == null || civInfo.tech.isResearched(improvement.techRequired!!))
+            ) return true
         // TODO: Generic-ify to unique
-        if (tileResource.resourceType==ResourceType.Strategic
+        return (tileResource.resourceType == ResourceType.Strategic
-                && improvement!=null
+            && improvement != null
-                && improvement.isGreatImprovement())
+            && improvement.isGreatImprovement())
-            return true
-        return false
     }
 
     // This should be the only adjacency function

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

@@ -231,11 +231,9 @@ class CityOverviewTab(
                 city.demandedResource.isNotEmpty() -> {
                     val image = ImageGetter.getResourcePortrait(city.demandedResource, iconSize *0.7f).apply {
                         addTooltip("Demanding [${city.demandedResource}]", 18f, tipAlign = Align.topLeft)
-                        onClick {
+                        onClick { showOneTimeNotification(
-                            if (gameInfo.notifyExploredResources(viewingPlayer, city.demandedResource, 0, true)) {
+                            gameInfo.getExploredResourcesNotification(viewingPlayer, city.demandedResource)
-                                overviewScreen.game.popScreen()
+                        ) }
-                            }
-                        }
                     }
                     cityInfoTableDetails.add(image)
                 }

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

@@ -1,7 +1,10 @@
 package com.unciv.ui.screens.overviewscreen
 
 import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.unciv.GUI
+import com.unciv.UncivGame
 import com.unciv.logic.civilization.Civilization
+import com.unciv.logic.civilization.Notification
 import com.unciv.ui.components.TabbedPager
 import com.unciv.ui.screens.basescreen.BaseScreen
 
@@ -26,4 +29,13 @@ abstract class EmpireOverviewTab (
 
     val gameInfo = viewingPlayer.gameInfo
 
+    /** Helper to show the world screen with a temporary "one-time" notification */
+    // Here because it's common to notification history and resource finder
+    internal fun showOneTimeNotification(notification: Notification?) {
+        if (notification == null) return  // Convenience - easier than a return@lambda for a caller
+        val worldScreen = GUI.getWorldScreen()
+        worldScreen.notificationsScroll.oneTimeNotification = notification
+        UncivGame.Current.resetToWorldScreen()
+        notification.action?.execute(worldScreen)
+    }
 }

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

@@ -3,10 +3,7 @@ package com.unciv.ui.screens.overviewscreen
 import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.ui.Table
-import com.unciv.GUI
-import com.unciv.UncivGame
 import com.unciv.logic.civilization.Civilization
-import com.unciv.logic.civilization.LocationAction
 import com.unciv.logic.civilization.Notification
 import com.unciv.logic.civilization.NotificationCategory
 import com.unciv.ui.components.ColorMarkupLabel
@@ -37,7 +34,7 @@ class NotificationsOverviewTable(
     }
 
 
-    private val worldScreen = GUI.getWorldScreen()
+    private val stageWidth = overviewScreen.stage.width
 
     private val notificationLog = viewingPlayer.notificationsLog
     private val notificationTable = Table(BaseScreen.skin)
@@ -70,9 +67,9 @@ class NotificationsOverviewTable(
                         else
                             "Current turn".toLabel()
         turnTable.add(Table().apply {
-            add(ImageGetter.getWhiteDot()).minHeight(2f).width(worldScreen.stage.width/4)
+            add(ImageGetter.getWhiteDot()).minHeight(2f).width(stageWidth / 4)
             add(turnLabel).pad(3f)
-            add(ImageGetter.getWhiteDot()).minHeight(2f).width(worldScreen.stage.width/4)
+            add(ImageGetter.getWhiteDot()).minHeight(2f).width(stageWidth / 4)
         }).row()
 
         for (category in NotificationCategory.values()){
@@ -88,17 +85,13 @@ class NotificationsOverviewTable(
                 val label = ColorMarkupLabel(notification.text, Color.BLACK, fontSize = 20)
                     .apply { wrap = true }
 
-                notificationTable.add(label).width(worldScreen.stage.width/2 - iconSize * notification.icons.size)
+                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)
-                    notificationTable.onClick {
+                    notificationTable.onClick { showOneTimeNotification(notification) }
-                        worldScreen.notificationsScroll.oneTimeNotification = notification
-                        UncivGame.Current.resetToWorldScreen()
-                        notification.action?.execute(worldScreen)
-                    }
 
-                notification.addNotificationIconsTo(notificationTable, worldScreen.gameInfo.ruleset, iconSize)
+                notification.addNotificationIconsTo(notificationTable, gameInfo.ruleset, iconSize)
 
                 turnTable.add(notificationTable).padTop(5f)
                 turnTable.padTop(20f).row()

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

@@ -6,6 +6,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.utils.Align
 import com.unciv.UncivGame
 import com.unciv.logic.civilization.Civilization
+import com.unciv.logic.map.tile.Tile
 import com.unciv.logic.trade.TradeType
 import com.unciv.models.ruleset.tile.ResourceSupplyList
 import com.unciv.models.ruleset.tile.ResourceType
@@ -75,20 +76,28 @@ class ResourcesOverviewTab(
 
     private fun ResourceSupplyList.getLabel(resource: TileResource, origin: String): Label? {
         val amount = get(resource, origin)?.amount ?: return null
-        return if (resource.isStockpiled() && amount > 0) "+$amount".toLabel()
+        val label = if (resource.isStockpiled() && amount > 0) "+$amount".toLabel()
-        else amount.toLabel()
+            else amount.toLabel()
+        if (origin == ExtraInfoOrigin.Unimproved.name)
+            label.onClick { showOneTimeNotification(
+                gameInfo.getExploredResourcesNotification(viewingPlayer, resource.name) {
+                    it.getOwner() == viewingPlayer && it.countAsUnimproved()
+                }
+            ) }
+        return label
     }
+
     private fun ResourceSupplyList.getTotalLabel(resource: TileResource): Label {
         val total = filter { it.resource == resource }.sumOf { it.amount }
         return if (resource.isStockpiled() && total > 0) "+$total".toLabel()
         else total.toLabel()
     }
+
     private fun getResourceImage(name: String) =
         ImageGetter.getResourcePortrait(name, iconSize).apply {
-            onClick {
+            onClick { showOneTimeNotification(
-                if (viewingPlayer.gameInfo.notifyExploredResources(viewingPlayer, name, 0, true))
+                gameInfo.getExploredResourcesNotification(viewingPlayer, name)
-                    overviewScreen.game.popScreen()
+            ) }
-            }
         }
     private fun TileResource.getLabel() = name.toLabel().apply {
         onClick {
@@ -228,10 +237,15 @@ class ResourcesOverviewTab(
         overviewScreen.resizePage(this)  // Without the height is miscalculated - shouldn't be
     }
 
+    private fun Tile.countAsUnimproved(): Boolean = resource != null &&
+            tileResource.resourceType != ResourceType.Bonus &&
+            hasViewableResource(viewingPlayer) &&
+            !providesResources(viewingPlayer)
+
     private fun getExtraDrilldown(): ResourceSupplyList {
         val newResourceSupplyList = ResourceSupplyList()
         for (city in viewingPlayer.cities) {
-            if (!city.demandedResource.isEmpty()) {
+            if (city.demandedResource.isNotEmpty()) {
                 val wltkResource = gameInfo.ruleset.tileResources[city.demandedResource]!!
                 if (city.isWeLoveTheKingDayActive()) {
                     newResourceSupplyList.add(wltkResource, ExtraInfoOrigin.CelebratingWLKT.name)
@@ -239,15 +253,9 @@ class ResourcesOverviewTab(
                     newResourceSupplyList.add(wltkResource, ExtraInfoOrigin.DemandingWLTK.name)
                 }
             }
-            for (tile in city.getTiles()) {
+            for (tile in city.getTiles())
-                if (tile.isCityCenter()) continue
+                if (tile.countAsUnimproved())
-                if (!tile.hasViewableResource(viewingPlayer)) continue
+                    newResourceSupplyList.add(tile.tileResource, ExtraInfoOrigin.Unimproved.name)
-                val tileResource = tile.tileResource
-                if (tileResource.resourceType == ResourceType.Bonus) continue
-                if (tile.getUnpillagedImprovement() != null && tileResource.isImprovedBy(tile.improvement!!)) continue
-                if (tileResource.resourceType == ResourceType.Strategic && tile.getTileImprovement()?.isGreatImprovement() == true) continue
-                newResourceSupplyList.add(tileResource, ExtraInfoOrigin.Unimproved.name)
-            }
         }
 
         for (otherCiv in viewingPlayer.getKnownCivs())