Mix of issue fixes around diplomacy and trade (#10462)

* Fix "AskForAssistance" Quest notification untranslatable

* Extend DiplomacyAction to allow showing the TradeTable of the other civ

* Make "trade ends" notifications clickable

* Make entries in Trade page of Empire Overview clickable

* Highlight selected civ in DiplomacyScreen, moddable

* A little proactive comment

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

@@ -76,10 +76,13 @@ class CityAction(private val city: Vector2 = Vector2.Zero): NotificationAction {
 }
 
 /** enter diplomacy screen */
-class DiplomacyAction(private val otherCivName: String = ""): NotificationAction {
+class DiplomacyAction(
+    private val otherCivName: String = "",
+    private val showTrade: Boolean = false
+): NotificationAction {
     override fun execute(worldScreen: WorldScreen) {
         val otherCiv = worldScreen.gameInfo.getCivilization(otherCivName)
-        worldScreen.game.pushScreen(DiplomacyScreen(worldScreen.viewingCiv, otherCiv))
+        worldScreen.game.pushScreen(DiplomacyScreen(worldScreen.viewingCiv, otherCiv, showTrade = showTrade))
     }
 }
 

core/src/com/unciv/logic/civilization/diplomacy/DeclareWar.kt

@@ -2,6 +2,7 @@ package com.unciv.logic.civilization.diplomacy
 
 import com.unciv.Constants
 import com.unciv.logic.civilization.AlertType
+import com.unciv.logic.civilization.DiplomacyAction
 import com.unciv.logic.civilization.NotificationCategory
 import com.unciv.logic.civilization.NotificationIcon
 import com.unciv.logic.civilization.PopupAlert
@@ -120,6 +121,7 @@ object DeclareWar {
         for (trade in diplomacyManager.trades)
             for (offer in trade.theirOffers.filter { it.duration > 0 && it.name != Constants.defensivePact})
                 diplomacyManager.civInfo.addNotification("[${offer.name}] from [${diplomacyManager.otherCivName}] has ended",
+                    DiplomacyAction(diplomacyManager.otherCivName, true),
                     NotificationCategory.Trade, diplomacyManager.otherCivName, NotificationIcon.Trade)
         diplomacyManager.trades.clear()
 

core/src/com/unciv/logic/civilization/diplomacy/DiplomacyTurnManager.kt

@@ -1,6 +1,7 @@
 package com.unciv.logic.civilization.diplomacy
 
 import com.unciv.Constants
+import com.unciv.logic.civilization.DiplomacyAction
 import com.unciv.logic.civilization.NotificationCategory
 import com.unciv.logic.civilization.NotificationIcon
 import com.unciv.logic.trade.Trade
@@ -45,8 +46,12 @@ object DiplomacyTurnManager {
                         remakePeaceTreaty(trade.theirOffers.first { it.name == Constants.peaceTreaty }.duration)
                     }
 
-                    civInfo.addNotification("One of our trades with [$otherCivName] has been cut short", NotificationCategory.Trade, NotificationIcon.Trade, otherCivName)
-                    otherCiv().addNotification("One of our trades with [${civInfo.civName}] has been cut short", NotificationCategory.Trade, NotificationIcon.Trade, civInfo.civName)
+                    civInfo.addNotification("One of our trades with [$otherCivName] has been cut short",
+                        DiplomacyAction(otherCivName, true),
+                        NotificationCategory.Trade, NotificationIcon.Trade, otherCivName)
+                    otherCiv().addNotification("One of our trades with [${civInfo.civName}] has been cut short",
+                        DiplomacyAction(civInfo.civName, true),
+                        NotificationCategory.Trade, NotificationIcon.Trade, civInfo.civName)
                     civInfo.cache.updateCivResources()
                 }
             }
@@ -224,9 +229,10 @@ object DiplomacyTurnManager {
             if (trade.ourOffers.all { it.duration <= 0 } && trade.theirOffers.all { it.duration <= 0 }) {
                 trades.remove(trade)
                 for (offer in trade.ourOffers.union(trade.theirOffers).filter { it.duration == 0 }) { // this was a timed trade
-                    if (offer in trade.theirOffers)
-                        civInfo.addNotification("[${offer.name}] from [$otherCivName] has ended", NotificationCategory.Trade, otherCivName, NotificationIcon.Trade)
-                    else civInfo.addNotification("[${offer.name}] to [$otherCivName] has ended", NotificationCategory.Trade, otherCivName, NotificationIcon.Trade)
+                    val direction = if (offer in trade.theirOffers) "from" else "to"
+                    civInfo.addNotification("[${offer.name}] $direction [$otherCivName] has ended",
+                        DiplomacyAction(otherCivName, true),
+                        NotificationCategory.Trade, otherCivName, NotificationIcon.Trade)
 
                     civInfo.updateStatsForNextTurn() // if they were bringing us gold per turn
                     if (trade.theirOffers.union(trade.ourOffers) // if resources were involved
@@ -238,9 +244,13 @@ object DiplomacyTurnManager {
             for (offer in trade.theirOffers.filter { it.duration <= 3 })
             {
                 if (offer.duration == 3)
-                    civInfo.addNotification("[${offer.name}] from [$otherCivName] will end in [3] turns", NotificationCategory.Trade, otherCivName, NotificationIcon.Trade)
+                    civInfo.addNotification("[${offer.name}] from [$otherCivName] will end in [3] turns",
+                        DiplomacyAction(otherCivName, true),
+                        NotificationCategory.Trade, otherCivName, NotificationIcon.Trade)
                 else if (offer.duration == 1)
-                    civInfo.addNotification("[${offer.name}] from [$otherCivName] will end next turn", NotificationCategory.Trade, otherCivName, NotificationIcon.Trade)
+                    civInfo.addNotification("[${offer.name}] from [$otherCivName] will end next turn",
+                        DiplomacyAction(otherCivName, true),
+                        NotificationCategory.Trade, otherCivName, NotificationIcon.Trade)
             }
         }
     }

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

@@ -571,7 +571,8 @@ class QuestManager : IsPartOfGameInfoSerialization {
     private fun notifyAskForAssistance(assignee: Civilization, attackerName: String, unitsToKill: Int, location: Vector2?) {
         if (attackerName == assignee.civName) return  // No "Hey Bob help us against Bob"
         val message = "[${civInfo.civName}] is being attacked by [$attackerName]!" +
-            "Kill [$unitsToKill] of the attacker's military units and they will be immensely grateful."
+            // Space relevant in template!
+            " Kill [$unitsToKill] of the attacker's military units and they will be immensely grateful."
         // Note: that LocationAction pseudo-constructor is able to filter out null location(s), no need for `if`
         assignee.addNotification(message, LocationAction(location), NotificationCategory.Diplomacy, civInfo.civName, "OtherIcons/Quest")
     }

core/src/com/unciv/ui/screens/diplomacyscreen/DiplomacyScreen.kt

@@ -39,22 +39,35 @@ import com.unciv.ui.components.widgets.AutoScrollPane as ScrollPane
  * Creates the diplomacy screen for [viewingCiv].
  *
  * When [selectCiv] is given and [selectTrade] is not, that Civilization is selected as if clicked on the left side.
+ * When [selectCiv] is given and [selectTrade] is not but [showTrade] is set, the [TradeTable] for that Civilization is shown.
  * When [selectCiv] and [selectTrade] are supplied, that Trade for that Civilization is selected, used for the counter-offer option from `TradePopup`.
  * Note calling this with [selectCiv] a City State and [selectTrade] supplied is **not allowed**.
  */
 class DiplomacyScreen(
     internal val viewingCiv: Civilization,
     private val selectCiv: Civilization? = null,
-    private val selectTrade: Trade? = null
+    private val selectTrade: Trade? = null,
+    private val showTrade: Boolean = selectTrade != null
 ): BaseScreen(), RecreateOnResize {
     companion object {
         private const val nationIconSize = 100f
         private const val nationIconPad = 10f
     }
 
-    private val leftSideTable = Table().apply { defaults().pad(nationIconPad) }
+    private val highlightColor: Color = clearColor.cpy().lerp(skin.getColor("color"), 0.333f)
+
+    private val leftSideTable = Table().apply {
+        background = skinStrings.getUiBackground("DiplomacyScreen/LeftSide", tintColor = clearColor)
+    }
     private val leftSideScroll = ScrollPane(leftSideTable)
-    internal val rightSideTable = Table()
+
+    private var highlightedCivButton: Table? = null
+    private val highlightBackground = skinStrings.getUiBackground("DiplomacyScreen/SelectedCiv", tintColor = highlightColor)
+
+    internal val rightSideTable = Table().apply {
+        background = skinStrings.getUiBackground("DiplomacyScreen/RightSide", tintColor = highlightColor)
+    }
+
     private val closeButton = Constants.close.toTextButton()
 
     internal fun isNotPlayersTurn() = !GUI.isAllowedChangeState()
@@ -77,9 +90,10 @@ class DiplomacyScreen(
         stage.addActor(closeButton) // This must come after the split pane so it will be above, that the button will be clickable
 
         if (selectCiv != null) {
-            if (selectTrade != null) {
+            if (showTrade) {
                 val tradeTable = setTrade(selectCiv)
-                tradeTable.tradeLogic.currentTrade.set(selectTrade)
+                if (selectTrade != null)
+                    tradeTable.tradeLogic.currentTrade.set(selectTrade)
                 tradeTable.offerColumnsTable.update()
             } else
                 updateRightSide(selectCiv)
@@ -92,7 +106,7 @@ class DiplomacyScreen(
 
     internal fun updateLeftSideTable(selectCiv: Civilization?) {
         leftSideTable.clear()
-        leftSideTable.add().padBottom(60f).row()  // room so the close button does not cover the first
+        leftSideTable.add().padBottom(70f).row()  // room so the close button does not cover the first
 
         var selectCivY = 0f
 
@@ -135,11 +149,20 @@ class DiplomacyScreen(
             }
 
             val civNameLabel = civ.civName.toLabel(hideIcons = true)
-            leftSideTable.add(civIndicator).row()
-            leftSideTable.add(civNameLabel).padBottom(20f).row()
 
-            civIndicator.onClick { updateRightSide(civ) }
-            civNameLabel.onClick { updateRightSide(civ) }
+            // The wrapper serves only to highlight the selected civ better
+            val civButton = Table().apply {
+                defaults().pad(nationIconPad)
+                add(civIndicator).row()
+                add(civNameLabel).row()
+                onClick {
+                    updateRightSide(civ)
+                    highlightCiv(this)
+                }
+                if (civ == selectCiv) highlightCiv(this)
+            }
+
+            leftSideTable.add(civButton).padBottom(20f - nationIconPad).growX().row()
         }
 
         if (selectCivY != 0f) {
@@ -149,6 +172,12 @@ class DiplomacyScreen(
         }
     }
 
+    private fun highlightCiv(civButton: Table) {
+        highlightedCivButton?.background = null
+        civButton.background = highlightBackground
+        highlightedCivButton = civButton
+    }
+
     internal fun updateRightSide(otherCiv: Civilization) {
         rightSideTable.clear()
         UncivGame.Current.musicController.chooseTrack(otherCiv.civName,
@@ -159,10 +188,8 @@ class DiplomacyScreen(
         )).height(stage.height)
     }
 
-    //region City State Diplomacy
-
-    //endregion
     //region Major Civ Diplomacy
+
     internal fun setTrade(civ: Civilization): TradeTable {
         rightSideTable.clear()
         val tradeTable = TradeTable(civ, this)
@@ -304,5 +331,5 @@ class DiplomacyScreen(
         positionCloseButton()
     }
 
-    override fun recreate(): BaseScreen = DiplomacyScreen(viewingCiv, selectCiv, selectTrade)
+    override fun recreate(): BaseScreen = DiplomacyScreen(viewingCiv, selectCiv, selectTrade, showTrade)
 }

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

@@ -3,14 +3,16 @@ package com.unciv.ui.screens.overviewscreen
 import com.badlogic.gdx.utils.Align
 import com.unciv.logic.civilization.Civilization
 import com.unciv.models.ruleset.tile.ResourceType
-import com.unciv.ui.screens.overviewscreen.EmpireOverviewTab.EmpireOverviewTabPersistableData
 import com.unciv.ui.components.input.KeyCharAndCode
+import com.unciv.ui.screens.overviewscreen.EmpireOverviewTab.EmpireOverviewTabPersistableData
 
 
 /** This controls which Tabs for the [EmpireOverviewScreen] exist and their order.
  *
  *  To add a Tab, build a new [EmpireOverviewTab] subclass and fill out a new entry here, that's all.
- *  Note the enum value's name is used as Tab caption, so if you ever need a non-alphanumeric caption please redesign to include a property for the caption.
+ *  Note the enum value's name is used as Tab caption, so if you ever need a non-alphanumeric caption
+ *  please redesign to include a property for the caption - and don't forget GameSettings.lastOverviewPage
+ *  currently looks for name when applied but uses tab caption when saving.
  */
 enum class EmpireOverviewCategories(
     val iconName: String,

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

@@ -1,18 +1,22 @@
 package com.unciv.ui.screens.overviewscreen
 
+import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.Constants
 import com.unciv.logic.civilization.Civilization
 import com.unciv.logic.trade.Trade
 import com.unciv.logic.trade.TradeOffersList
-import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.components.extensions.addSeparator
 import com.unciv.ui.components.extensions.toLabel
+import com.unciv.ui.components.input.onActivation
+import com.unciv.ui.screens.basescreen.BaseScreen
+import com.unciv.ui.screens.diplomacyscreen.DiplomacyScreen
 
 class TradesOverviewTab(
     viewingPlayer: Civilization,
     overviewScreen: EmpireOverviewScreen
 ) : EmpireOverviewTab(viewingPlayer, overviewScreen) {
+    val game = overviewScreen.game
 
     init {
         defaults().pad(10f)
@@ -44,11 +48,11 @@ class TradesOverviewTab(
         }
     }
 
-    private fun createTradeTable(trade: Trade, otherCiv: Civilization): Table {
-        val generalTable = Table()
-        generalTable.add(createOffersTable(viewingPlayer, trade.ourOffers, trade.theirOffers.size)).minWidth(overviewScreen.stage.width/4).fillY()
-        generalTable.add(createOffersTable(otherCiv, trade.theirOffers, trade.ourOffers.size)).minWidth(overviewScreen.stage.width/4).fillY()
-        return generalTable
+    private fun createTradeTable(trade: Trade, otherCiv: Civilization) = Table().apply {
+        add(createOffersTable(viewingPlayer, trade.ourOffers, trade.theirOffers.size)).minWidth(overviewScreen.stage.width/4).fillY()
+        add(createOffersTable(otherCiv, trade.theirOffers, trade.ourOffers.size)).minWidth(overviewScreen.stage.width/4).fillY()
+        touchable = Touchable.enabled
+        onActivation { game.pushScreen(DiplomacyScreen(viewingPlayer, otherCiv, trade)) }
     }
 
     private fun createOffersTable(civ: Civilization, offersList: TradeOffersList, numberOfOtherSidesOffers: Int): Table {

docs/Modders/Creating-a-UI-skin.md

@@ -55,6 +55,9 @@ These shapes are used all over Unciv and can be replaced to make a lot of UI ele
 | CityScreen/ConstructionInfoTable/ | Background | null | |
 | CityScreen/ConstructionInfoTable/ | SelectedConstructionTable | null | |
 | CivilopediaScreen/ | EntryButton | null | |
+| DiplomacyScreen/ | LeftSide | null | |
+| DiplomacyScreen/ | RightSide | null | |
+| DiplomacyScreen/ | SelectedCiv | null | |
 | General/ | AnimatedMenu | roundedEdgeRectangle | |
 | General/ | Border | null | |
 | General/ | ExpanderTab | null | |