Solved crash of #5728 - AIs ignore trade requests that have become invalid mid-turn

They also no longer spam each other with requests

core/src/com/unciv/logic/automation/NextTurnAutomation.kt

@@ -75,6 +75,9 @@ object NextTurnAutomation {
     private fun respondToTradeRequests(civInfo: CivilizationInfo) {
         for (tradeRequest in civInfo.tradeRequests.toList()) {
             val otherCiv = civInfo.gameInfo.getCivilization(tradeRequest.requestingCiv)
+            if (!TradeEvaluation().isTradeValid(tradeRequest.trade, civInfo, otherCiv))
+                continue
+
             val tradeLogic = TradeLogic(civInfo, otherCiv)
             tradeLogic.currentTrade.set(tradeRequest.trade)
             /** We need to remove this here, so that if the trade is accepted, the updateDetailedCivResources()
@@ -87,6 +90,7 @@ object NextTurnAutomation {
                 tradeLogic.acceptTrade()
                 otherCiv.addNotification("[${civInfo.civName}] has accepted your trade request", NotificationIcon.Trade, civInfo.civName)
             } else {
+                tradeRequest.decline(civInfo)
                 /* Currently disabled until we solve the problems in https://github.com/yairm210/Unciv/issues/5728
 
                 val counteroffer = getCounteroffer(civInfo, tradeRequest)

core/src/com/unciv/logic/trade/Trade.kt

@@ -1,5 +1,10 @@
 package com.unciv.logic.trade
 
+import com.unciv.Constants
+import com.unciv.logic.civilization.CivilizationInfo
+import com.unciv.logic.civilization.NotificationIcon
+import com.unciv.logic.civilization.diplomacy.DiplomacyFlags
+
 class Trade{
 
     val theirOffers = TradeOffersList()
@@ -38,10 +43,26 @@ class Trade{
         theirOffers.clear()
         theirOffers.addAll(trade.theirOffers)
     }
+
+    fun isPeaceTreaty() = ourOffers.any { it.type == TradeType.Treaty && it.name == Constants.peaceTreaty }
 }
 
 
 class TradeRequest {
+    fun decline(decliningCiv:CivilizationInfo) {
+        val requestingCivInfo = decliningCiv.gameInfo.getCivilization(requestingCiv)
+        val diplomacyManager = requestingCivInfo.getDiplomacyManager(decliningCiv)
+        // the numbers of the flags (20,5) are the amount of turns to wait until offering again
+        if (trade.ourOffers.all { it.type == TradeType.Luxury_Resource }
+            && trade.theirOffers.all { it.type==TradeType.Luxury_Resource })
+            diplomacyManager.setFlag(DiplomacyFlags.DeclinedLuxExchange,20)
+        if (trade.ourOffers.any { it.name == Constants.researchAgreement })
+            diplomacyManager.setFlag(DiplomacyFlags.DeclinedResearchAgreement,20)
+        if (trade.isPeaceTreaty()) diplomacyManager.setFlag(DiplomacyFlags.DeclinedPeace, 5)
+
+        requestingCivInfo.addNotification("[${decliningCiv.civName}] has denied your trade request", decliningCiv.civName, NotificationIcon.Trade)
+    }
+
 
     lateinit var requestingCiv: String
 

core/src/com/unciv/logic/trade/TradeOffersList.kt

@@ -2,22 +2,22 @@ package com.unciv.logic.trade
 
 import java.util.*
 
-class TradeOffersList: ArrayList<TradeOffer>(){
+class TradeOffersList: ArrayList<TradeOffer>() {
     override fun add(element: TradeOffer): Boolean {
-        val equivalentOffer = firstOrNull { it.name==element.name&&it.type==element.type }
+        val equivalentOffer = firstOrNull { it.name == element.name && it.type == element.type }
-        if(equivalentOffer==null){
+        if (equivalentOffer == null) {
             super.add(element)
             return true
         }
         equivalentOffer.amount += element.amount
-        if(equivalentOffer.amount==0) remove(equivalentOffer)
+        if (equivalentOffer.amount == 0) remove(equivalentOffer)
         return true
     }
 
     fun without(otherTradeOffersList: TradeOffersList): TradeOffersList {
         val tradeOffersListCopy = TradeOffersList()
-        for(offer in this) tradeOffersListCopy.add(offer.copy())
+        for (offer in this) tradeOffersListCopy.add(offer.copy())
-        for(offer in otherTradeOffersList) tradeOffersListCopy.add(offer.copy(amount=-offer.amount))
+        for (offer in otherTradeOffersList) tradeOffersListCopy.add(offer.copy(amount = -offer.amount))
         return tradeOffersListCopy
     }
 }

core/src/com/unciv/ui/worldscreen/TradePopup.kt

@@ -41,12 +41,9 @@ class TradePopup(worldScreen: WorldScreen): Popup(worldScreen){
         val requestingCiv = worldScreen.gameInfo.getCivilization(tradeRequest.requestingCiv)
         val nation = requestingCiv.nation
         val trade = tradeRequest.trade
-        val isPeaceTreaty = trade.ourOffers.any { it.type == TradeType.Treaty && it.name == Constants.peaceTreaty }
-        val ourResources = viewingCiv.getCivResourcesByName()
-        val music = UncivGame.Current.musicController
 
-        if (isPeaceTreaty)
+
-            music.chooseTrack(nation.name, MusicMood.Peace, MusicTrackChooserFlags.setSpecific)
+        val ourResources = viewingCiv.getCivResourcesByName()
 
         val leaderIntroTable = LeaderIntroTable(requestingCiv)
         add(leaderIntroTable)
@@ -83,7 +80,7 @@ class TradePopup(worldScreen: WorldScreen): Popup(worldScreen){
 
         addGoodSizedLabel(nation.tradeRequest).pad(15f).row()
 
-        val soundsGoodButton = addButton("Sounds good!", 'y') {
+        addButton("Sounds good!", 'y') {
             val tradeLogic = TradeLogic(viewingCiv, requestingCiv)
             tradeLogic.currentTrade.set(trade)
             tradeLogic.acceptTrade()
@@ -92,20 +89,9 @@ class TradePopup(worldScreen: WorldScreen): Popup(worldScreen){
             requestingCiv.addNotification("[${viewingCiv.civName}] has accepted your trade request", viewingCiv.civName, NotificationIcon.Trade)
         }
 
-        // In the meantime this became invalid, perhaps because we accepted previous trades
-        if(!TradeEvaluation().isTradeValid(trade,viewingCiv,requestingCiv))
-            soundsGoodButton.actor.disable()
-
         addButton("Not this time.", 'n') {
-            val diplomacyManager = requestingCiv.getDiplomacyManager(viewingCiv)
+
-            if (trade.ourOffers.all { it.type == TradeType.Luxury_Resource } && trade.theirOffers.all { it.type==TradeType.Luxury_Resource })
+            tradeRequest.decline(viewingCiv)
-                diplomacyManager.setFlag(DiplomacyFlags.DeclinedLuxExchange,20) // offer again in 20 turns
-            if (trade.ourOffers.any { it.name == Constants.researchAgreement })
-                diplomacyManager.setFlag(DiplomacyFlags.DeclinedResearchAgreement,20) // offer again in 20 turns
-            if (isPeaceTreaty) {
-                diplomacyManager.setFlag(DiplomacyFlags.DeclinedPeace, 5)
-                music.chooseTrack(nation.name, MusicMood.War, MusicTrackChooserFlags.setSpecific)
-            }
 
             close()
             requestingCiv.addNotification("[${viewingCiv.civName}] has denied your trade request", viewingCiv.civName, NotificationIcon.Trade)

core/src/com/unciv/ui/worldscreen/WorldScreen.kt

@@ -19,6 +19,7 @@ import com.unciv.logic.GameSaver
 import com.unciv.logic.civilization.CivilizationInfo
 import com.unciv.logic.civilization.ReligionState
 import com.unciv.logic.civilization.diplomacy.DiplomaticStatus
+import com.unciv.logic.trade.TradeEvaluation
 import com.unciv.models.Tutorial
 import com.unciv.models.UncivSound
 import com.unciv.models.ruleset.tile.ResourceType
@@ -419,7 +420,16 @@ class WorldScreen(val gameInfo: GameInfo, val viewingCiv:CivilizationInfo) : Bas
                     UncivGame.Current.setScreen(DiplomaticVoteResultScreen(gameInfo.diplomaticVictoryVotesCast, viewingCiv))
                 viewingCiv.greatPeople.freeGreatPeople > 0 -> game.setScreen(GreatPersonPickerScreen(viewingCiv))
                 viewingCiv.popupAlerts.any() -> AlertPopup(this, viewingCiv.popupAlerts.first()).open()
-                viewingCiv.tradeRequests.isNotEmpty() -> TradePopup(this).open()
+                viewingCiv.tradeRequests.isNotEmpty() -> {
+                    // In the meantime this became invalid, perhaps because we accepted previous trades
+                    for (tradeRequest in viewingCiv.tradeRequests.toList())
+                        if (!TradeEvaluation().isTradeValid(tradeRequest.trade, viewingCiv,
+                                gameInfo.getCivilization(tradeRequest.requestingCiv)))
+                            viewingCiv.tradeRequests.remove(tradeRequest)
+
+                    if (viewingCiv.tradeRequests.isNotEmpty()) // if a valid one still exists
+                        TradePopup(this).open()
+                }
             }
         }
         updateNextTurnButton(hasOpenPopups()) // This must be before the notifications update, since its position is based on it

docs/uniques.md

@@ -1,6 +1,5 @@
 ## Table of Contents
 
- - [Improvement uniques](#improvement-uniques)
  - [Global uniques](#global-uniques)
  - [Building uniques](#building-uniques)
  - [CityState uniques](#citystate-uniques)
@@ -8,84 +7,18 @@
  - [Unit uniques](#unit-uniques)
  - [Terrain uniques](#terrain-uniques)
  - [Resource uniques](#resource-uniques)
+ - [Improvement uniques](#improvement-uniques)
  - [Conditional uniques](#conditional-uniques)
  - [Ruins uniques](#ruins-uniques)
  - [Promotion uniques](#promotion-uniques)
  - [Deprecated uniques](#deprecated-uniques)
 
-## Improvement uniques
+## Global uniques
 #### [stats]
 Example: "[+1 Gold, +2 Production]"
 
-Applicable to: Improvement, Global, FollowerBelief
+Applicable to: Global, FollowerBelief, Improvement
 
-#### Provides [amount] [resource]
-Example: "Provides [20] [Iron]"
-
-Applicable to: Improvement, Building
-
-#### Can also be built on tiles adjacent to fresh water
-Applicable to: Improvement
-
-#### [stats] from [tileFilter] tiles
-Example: "[+1 Gold, +2 Production] from [Farm] tiles"
-
-Applicable to: Improvement
-
-#### [stats] for each adjacent [tileFilter]
-Example: "[+1 Gold, +2 Production] for each adjacent [Farm]"
-
-Applicable to: Improvement
-
-#### Can be built outside your borders
-Applicable to: Improvement
-
-#### Can be built just outside your borders
-Applicable to: Improvement
-
-#### Cannot be built on [tileFilter] tiles until [tech] is discovered
-Example: "Cannot be built on [Farm] tiles until [tech] is discovered"
-
-Applicable to: Improvement
-
-#### Cannot be built on [tileFilter] tiles
-Example: "Cannot be built on [Farm] tiles"
-
-Applicable to: Improvement
-
-#### Does not need removal of [tileFilter]
-Example: "Does not need removal of [Farm]"
-
-Applicable to: Improvement
-
-#### Gives a defensive bonus of [amount]%
-Example: "Gives a defensive bonus of [20]%"
-
-Applicable to: Improvement
-
-#### Costs [amount] gold per turn when in your territory
-Example: "Costs [20] gold per turn when in your territory"
-
-Applicable to: Improvement
-
-#### Deal [amount] damage to adjacent enemy units
-Example: "Deal [20] damage to adjacent enemy units"
-
-Applicable to: Improvement
-
-#### Great Improvement
-Applicable to: Improvement
-
-#### Provides a random bonus when entered
-Applicable to: Improvement
-
-#### Unpillagable
-Applicable to: Improvement
-
-#### Indestructible
-Applicable to: Improvement
-
-## Global uniques
 #### [stats] [cityFilter]
 Example: "[+1 Gold, +2 Production] [in all cities]"
 
@@ -286,6 +219,29 @@ Applicable to: Global
 #### Triggers a Cultural Victory upon completion
 Applicable to: Global
 
+#### [amount]% Strength
+Example: "[20]% Strength"
+
+Applicable to: Global, Unit
+
+#### [amount] Movement
+Example: "[20] Movement"
+
+Applicable to: Global, Unit
+
+#### [amount] Sight
+Example: "[20] Sight"
+
+Applicable to: Global, Unit
+
+#### [amount]% Spread Religion Strength
+Example: "[20]% Spread Religion Strength"
+
+Applicable to: Global, Unit
+
+#### Normal vision when embarked
+Applicable to: Global, Unit
+
 #### Free [baseUnitFilter] appears
 Example: "Free [Melee] appears"
 
@@ -366,6 +322,37 @@ Applicable to: Global
 #### Remove extra unhappiness from annexed cities
 Applicable to: Building
 
+#### Consumes [amount] [resource]
+Example: "Consumes [20] [Iron]"
+
+Applicable to: Building, Unit, Improvement
+
+#### Provides [amount] [resource]
+Example: "Provides [20] [Iron]"
+
+Applicable to: Building, Improvement
+
+#### Unbuildable
+Applicable to: Building, Unit
+
+#### Cannot be purchased
+Applicable to: Building, Unit
+
+#### Can be purchased with [stat] [cityFilter]
+Example: "Can be purchased with [Culture] [in all cities]"
+
+Applicable to: Building, Unit
+
+#### Can be purchased for [amount] [stat] [cityFilter]
+Example: "Can be purchased for [20] [Culture] [in all cities]"
+
+Applicable to: Building, Unit
+
+#### Limited to [amount] per Civilization
+Example: "Limited to [20] per Civilization"
+
+Applicable to: Building, Unit
+
 #### Cost increases by [amount] per owned city
 Example: "Cost increases by [20] per owned city"
 
@@ -381,6 +368,11 @@ Example: "Requires a [Library] in this city"
 
 Applicable to: Building
 
+#### Not displayed as an available construction without [buildingName/tech/resource/policy]
+Example: "Not displayed as an available construction without [buildingName/tech/resource/policy]"
+
+Applicable to: Building, Unit
+
 #### Must be on [terrainFilter]
 Example: "Must be on [Grassland]"
 
@@ -443,37 +435,6 @@ Example: "Retain [20]% of the happiness from a luxury after the last copy has be
 Applicable to: Nation
 
 ## Unit uniques
-#### Consumes [amount] [resource]
-Example: "Consumes [20] [Iron]"
-
-Applicable to: Unit, Improvement, Building
-
-#### Unbuildable
-Applicable to: Unit, Building
-
-#### Cannot be purchased
-Applicable to: Unit, Building
-
-#### Can be purchased with [stat] [cityFilter]
-Example: "Can be purchased with [Culture] [in all cities]"
-
-Applicable to: Unit, Building
-
-#### Can be purchased for [amount] [stat] [cityFilter]
-Example: "Can be purchased for [20] [Culture] [in all cities]"
-
-Applicable to: Unit, Building
-
-#### Limited to [amount] per Civilization
-Example: "Limited to [20] per Civilization"
-
-Applicable to: Unit, Building
-
-#### Not displayed as an available construction without [buildingName/tech/resource/policy]
-Example: "Not displayed as an available construction without [buildingName/tech/resource/policy]"
-
-Applicable to: Unit, Building
-
 #### Founds a new city
 Applicable to: Unit
 
@@ -490,40 +451,17 @@ Example: "Can see invisible [Wounded] units"
 
 Applicable to: Unit
 
-#### [amount]% Strength
-Example: "[20]% Strength"
-
-Applicable to: Unit, Global
-
 #### [amount]% Strength decreasing with distance from the capital
 Example: "[20]% Strength decreasing with distance from the capital"
 
 Applicable to: Unit
 
-#### [amount] Movement
-Example: "[20] Movement"
-
-Applicable to: Unit, Global
-
-#### [amount] Sight
-Example: "[20] Sight"
-
-Applicable to: Unit, Global
-
-#### [amount]% Spread Religion Strength
-Example: "[20]% Spread Religion Strength"
-
-Applicable to: Unit, Global
-
 #### May found a religion
 Applicable to: Unit
 
 #### May enhance a religion
 Applicable to: Unit
 
-#### Normal vision when embarked
-Applicable to: Unit, Global
-
 #### Cannot attack
 Applicable to: Unit
 
@@ -601,9 +539,6 @@ Applicable to: Unit
 #### Religious Unit
 Applicable to: Unit
 
-#### Hidden when religion is disabled
-Applicable to: Unit, Ruins, Building
-
 ## Terrain uniques
 #### Must be adjacent to [amount] [simpleTerrain] tiles
 Example: "Must be adjacent to [20] [simpleTerrain] tiles"
@@ -774,6 +709,68 @@ Applicable to: Resource
 #### Can only be created by Mercantile City-States
 Applicable to: Resource
 
+## Improvement uniques
+#### Can also be built on tiles adjacent to fresh water
+Applicable to: Improvement
+
+#### [stats] from [tileFilter] tiles
+Example: "[+1 Gold, +2 Production] from [Farm] tiles"
+
+Applicable to: Improvement
+
+#### [stats] for each adjacent [tileFilter]
+Example: "[+1 Gold, +2 Production] for each adjacent [Farm]"
+
+Applicable to: Improvement
+
+#### Can be built outside your borders
+Applicable to: Improvement
+
+#### Can be built just outside your borders
+Applicable to: Improvement
+
+#### Cannot be built on [tileFilter] tiles until [tech] is discovered
+Example: "Cannot be built on [Farm] tiles until [tech] is discovered"
+
+Applicable to: Improvement
+
+#### Cannot be built on [tileFilter] tiles
+Example: "Cannot be built on [Farm] tiles"
+
+Applicable to: Improvement
+
+#### Does not need removal of [tileFilter]
+Example: "Does not need removal of [Farm]"
+
+Applicable to: Improvement
+
+#### Gives a defensive bonus of [amount]%
+Example: "Gives a defensive bonus of [20]%"
+
+Applicable to: Improvement
+
+#### Costs [amount] gold per turn when in your territory
+Example: "Costs [20] gold per turn when in your territory"
+
+Applicable to: Improvement
+
+#### Deal [amount] damage to adjacent enemy units
+Example: "Deal [20] damage to adjacent enemy units"
+
+Applicable to: Improvement
+
+#### Great Improvement
+Applicable to: Improvement
+
+#### Provides a random bonus when entered
+Applicable to: Improvement
+
+#### Unpillagable
+Applicable to: Improvement
+
+#### Indestructible
+Applicable to: Improvement
+
 ## Conditional uniques
 #### <when at war>
 Applicable to: Conditional
@@ -942,6 +939,9 @@ Applicable to: Ruins
 #### This Unit upgrades for free including special upgrades
 Applicable to: Ruins
 
+#### Hidden when religion is disabled
+Applicable to: Ruins, Building, Unit
+
 #### Hidden before founding a Pantheon
 Applicable to: Ruins
 
@@ -963,9 +963,6 @@ Example: "Heal this unit by [20] HP"
 Applicable to: Promotion
 
 ## Deprecated uniques
- - "[stats] on [tileFilter] tiles once [tech] is discovered" - Deprecated As of 3.17.10, replace with "[stats] from [tileFilter] tiles <after discovering [tech]>"
- - "[stats] once [tech] is discovered" - Deprecated As of 3.17.10, replace with "[stats] <after discovering [tech]>"
- - "Deal 30 damage to adjacent enemy units" - Deprecated As of 3.17.10, replace with "Adjacent enemy units ending their turn take [30] damage"
  - "+[amount]% [stat] [cityFilter]" - Deprecated As of 3.17.10, replace with "[+amount]% [stat] [cityFilter]"
  - "+[amount]% [stat] in all cities" - Deprecated As of 3.17.10, replace with "[+amount]% [stat] [in all cities]"
  - "[amount]% [stat] while the empire is happy" - Deprecated As of 3.17.1, replace with "[amount]% [stat] [in all cities] <while the empire is happy>"
@@ -999,6 +996,7 @@ Applicable to: Promotion
  - "[stats] from every specialist" - Deprecated As of 3.16.16 - removed 3.17.11, replace with "[stats] from every specialist [in all cities]"
  - "[stats] if this city has at least [amount] specialists" - Deprecated As of 3.16.16 - removed 3.17.11, replace with "[stats] <if this city has at least [amount] specialists>"
  - "Not displayed as an available construction unless [buildingName] is built" - Deprecated As of 3.16.11, replace with "Not displayed as an available construction without [buildingName]"
+ - "[stats] once [tech] is discovered" - Deprecated As of 3.17.10, replace with "[stats] <after discovering [tech]>"
  - "Double movement in coast" - Deprecated As of 3.17.1 - removed 3.17.13, replace with "Double movement in [terrainFilter]"
  - "Double movement rate through Forest and Jungle" - Deprecated As of 3.17.1 - removed 3.17.13, replace with "Double movement in [terrainFilter]"
  - "Double movement in Snow, Tundra and Hills" - Deprecated As of 3.17.1 - removed 3.17.13, replace with "Double movement in [terrainFilter]"
@@ -1013,4 +1011,6 @@ Applicable to: Promotion
  - "+[amount]% defence in [tileFilter] tiles" - Deprecated As of 3.17.5 - removed 3.18.5, replace with "[amount]% Strength <when fighting in [tileFilter] tiles> <when defending>"
  - "+[amount]% Strength in [tileFilter]" - Deprecated As of 3.17.5 - removed 3.18.5, replace with "[amount]% Strength <when fighting in [tileFilter] tiles>"
  - "[amount] Visibility Range" - Deprecated As of 3.17.5 - removed 3.18.5, replace with "[amount] Sight"
- - "Limited Visibility" - Deprecated As of 3.17.5 - removed 3.18.5, replace with "[-1] Sight"
+ - "Limited Visibility" - Deprecated As of 3.17.5 - removed 3.18.5, replace with "[-1] Sight"
+ - "[stats] on [tileFilter] tiles once [tech] is discovered" - Deprecated As of 3.17.10, replace with "[stats] from [tileFilter] tiles <after discovering [tech]>"
+ - "Deal 30 damage to adjacent enemy units" - Deprecated As of 3.17.10, replace with "Adjacent enemy units ending their turn take [30] damage"