Modding: Fixed chance conditionals attached to triggered uniques

core/src/com/unciv/logic/battle/Battle.kt

@@ -618,6 +618,8 @@ object Battle {
         if (attackerCiv.isCurrentPlayer())
             UncivGame.Current.settings.addCompletedTutorialTask("Conquer a city")
 
+        // Here, we DO need the stateForConditionals - since it contains MORE than the civ+city+unit+tile that is checked when triggering
+        // This means that the conditionalChance WILL be checked twice, and will not function as expected
         for (unique in attackerCiv.getTriggeredUniques(UniqueType.TriggerUponConqueringCity, stateForConditionals)
                 + attacker.unit.getTriggeredUniques(UniqueType.TriggerUponConqueringCity, stateForConditionals))
             UniqueTriggerActivation.triggerUnique(unique, attacker.unit)

core/src/com/unciv/logic/city/CityConstructions.kt

@@ -141,7 +141,7 @@ class CityConstructions : IsPartOfGameInfoSerialization {
     }
 
     /** @param constructionName needs to be a non-perpetual construction, else an empty string is returned */
-    internal fun getTurnsToConstructionString(constructionName: String, useStoredProduction: Boolean = true) =
+    private fun getTurnsToConstructionString(constructionName: String, useStoredProduction: Boolean = true) =
         getTurnsToConstructionString(getConstruction(constructionName), useStoredProduction)
 
     /** @param construction needs to be a non-perpetual construction, else an empty string is returned */
@@ -588,19 +588,19 @@ class CityConstructions : IsPartOfGameInfoSerialization {
             city.civ.civConstructions.tryAddFreeBuildings()
     }
 
-    fun triggerNewBuildingUniques(building: Building) {
+    private fun triggerNewBuildingUniques(building: Building) {
         val stateForConditionals = city.state
         val triggerNotificationText ="due to constructing [${building.name}]"
 
         for (unique in building.uniqueObjects)
-            if (!unique.hasTriggerConditional() && unique.conditionalsApply(stateForConditionals))
+            if (!unique.hasTriggerConditional())
                 UniqueTriggerActivation.triggerUnique(unique, city, triggerNotificationText = triggerNotificationText)
 
-        for (unique in city.civ.getTriggeredUniques(UniqueType.TriggerUponConstructingBuilding, stateForConditionals)
+        for (unique in city.civ.getTriggeredUniques(UniqueType.TriggerUponConstructingBuilding)
                 { building.matchesFilter(it.params[0], stateForConditionals) })
             UniqueTriggerActivation.triggerUnique(unique, city, triggerNotificationText = triggerNotificationText)
 
-        for (unique in city.civ.getTriggeredUniques(UniqueType.TriggerUponConstructingBuildingCityFilter, stateForConditionals)
+        for (unique in city.civ.getTriggeredUniques(UniqueType.TriggerUponConstructingBuildingCityFilter)
                 { building.matchesFilter(it.params[0], stateForConditionals) && city.matchesFilter(it.params[1]) })
             UniqueTriggerActivation.triggerUnique(unique, city, triggerNotificationText = triggerNotificationText)
     }
@@ -628,7 +628,7 @@ class CityConstructions : IsPartOfGameInfoSerialization {
         setTransients()
     }
 
-    fun updateUniques(onLoadGame: Boolean = false) {
+    private fun updateUniques(onLoadGame: Boolean = false) {
         builtBuildingUniqueMap.clear()
         for (building in getBuiltBuildings())
             builtBuildingUniqueMap.addUniques(building.uniqueObjects)
@@ -799,7 +799,7 @@ class CityConstructions : IsPartOfGameInfoSerialization {
         PerpetualConstruction.isNamePerpetual(constructionQueue.last())
         // `getConstruction(constructionQueue.last()) is PerpetualConstruction` is clear but more expensive
 
-    fun isQueueEmptyOrIdle() = currentConstructionFromQueue.isEmpty()
+    private fun isQueueEmptyOrIdle() = currentConstructionFromQueue.isEmpty()
         || currentConstructionFromQueue == PerpetualConstruction.idle.name
 
     /** Add [construction] to the end or top (controlled by [addToTop]) of the queue with all checks (does nothing if not possible)

core/src/com/unciv/logic/city/managers/CityFounder.kt

@@ -9,7 +9,6 @@ import com.unciv.logic.civilization.diplomacy.DiplomacyFlags
 import com.unciv.logic.civilization.managers.ReligionState
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.models.ruleset.nation.Nation
-import com.unciv.models.ruleset.unique.StateForConditionals
 import com.unciv.models.ruleset.unique.UniqueTriggerActivation
 import com.unciv.models.ruleset.unique.UniqueType
 
@@ -86,13 +85,10 @@ class CityFounder {
 
         addStartingBuildings(city, civInfo, startingEra)
 
-        for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponFoundingCity,
+        for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponFoundingCity))
-            StateForConditionals(civInfo, city, unit)
-        ))
             UniqueTriggerActivation.triggerUnique(unique, civInfo, city, unit, triggerNotificationText = "due to founding a city")
         if (unit != null)
-            for (unique in unit.getTriggeredUniques(UniqueType.TriggerUponFoundingCity,
+            for (unique in unit.getTriggeredUniques(UniqueType.TriggerUponFoundingCity))
-                StateForConditionals(civInfo, city, unit)))
                 UniqueTriggerActivation.triggerUnique(unique, civInfo, city, unit, triggerNotificationText = "due to founding a city")
 
         return city

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

@@ -552,10 +552,20 @@ class Civilization : IsPartOfGameInfoSerialization {
         yieldAll(civResourcesUniqueMap.getMatchingUniques(uniqueType, stateForConditionals))
         yieldAll(gameInfo.ruleset.globalUniques.getMatchingUniques(uniqueType, stateForConditionals))
     }
+    
+    /** Good for generic, non-filtered triggers */
+    fun triggerUniques(uniqueType: UniqueType){
+
+        for (unique in getTriggeredUniques(uniqueType))
+            UniqueTriggerActivation.triggerUnique(unique, this)
+    }
 
     fun getTriggeredUniques(
         trigger: UniqueType,
-        stateForConditionals: StateForConditionals = state,
+        // Ignore conditionals, as triggerUnique will check again.
+        // If we check twice, that breaks UniqueType.ConditionalChance - 25% declared chance would work as 6% actual chance
+        /** Only set this if the state contains something other than civ, city, unit, tile - since those are checked in triggerUnique() */
+        stateForConditionals: StateForConditionals = StateForConditionals.IgnoreConditionals,
         triggerFilter: (Unique) -> Boolean = { true }
     ) : Iterable<Unique> = sequence {
         yieldAll(nation.uniqueMap.getTriggeredUniques(trigger, stateForConditionals, triggerFilter))

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

@@ -1,14 +1,8 @@
 package com.unciv.logic.civilization.diplomacy
 
 import com.unciv.Constants
-import com.unciv.logic.civilization.AlertType
+import com.unciv.logic.civilization.*
-import com.unciv.logic.civilization.Civilization
-import com.unciv.logic.civilization.DiplomacyAction
-import com.unciv.logic.civilization.NotificationCategory
-import com.unciv.logic.civilization.NotificationIcon
-import com.unciv.logic.civilization.PopupAlert
 import com.unciv.models.ruleset.nation.PersonalityValue
-import com.unciv.models.ruleset.unique.UniqueTriggerActivation
 import com.unciv.models.ruleset.unique.UniqueType
 
 object DeclareWar {
@@ -40,9 +34,7 @@ object DeclareWar {
 
         breakTreaties(diplomacyManager)
 
-        if (otherCiv.isMajorCiv())
+        if (otherCiv.isMajorCiv()) civInfo.triggerUniques(UniqueType.TriggerUponDeclaringWar)
-            for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponDeclaringWar))
-                UniqueTriggerActivation.triggerUnique(unique, civInfo)
     }
 
     private fun handleCityStateDirectAttack(diplomacyManager: DiplomacyManager) {

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

@@ -6,14 +6,8 @@ import com.unciv.logic.IsPartOfGameInfoSerialization
 import com.unciv.logic.civilization.Civilization
 import com.unciv.logic.civilization.NotificationCategory
 import com.unciv.logic.civilization.NotificationIcon
-import com.unciv.logic.trade.Trade
+import com.unciv.logic.trade.*
-import com.unciv.logic.trade.TradeEvaluation
-import com.unciv.logic.trade.TradeLogic
-import com.unciv.logic.trade.TradeOffer
-import com.unciv.logic.trade.TradeOfferType
 import com.unciv.models.ruleset.tile.ResourceSupplyList
-import com.unciv.models.ruleset.unique.StateForConditionals
-import com.unciv.models.ruleset.unique.UniqueTriggerActivation
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.ui.components.extensions.toPercent
 import kotlin.math.ceil
@@ -564,10 +558,8 @@ class DiplomacyManager() : IsPartOfGameInfoSerialization {
 
         // Ignore contitionals as triggerUnique will check again, and that would break
         // UniqueType.ConditionalChance - 25% declared chance would work as 6% actual chance
-        for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponDeclaringFriendship, StateForConditionals.IgnoreConditionals))
+        civInfo.triggerUniques(UniqueType.TriggerUponDeclaringFriendship)
-            UniqueTriggerActivation.triggerUnique(unique, civInfo)
+        otherCiv().triggerUniques(UniqueType.TriggerUponDeclaringFriendship)
-        for (unique in otherCiv().getTriggeredUniques(UniqueType.TriggerUponDeclaringFriendship, StateForConditionals.IgnoreConditionals))
-            UniqueTriggerActivation.triggerUnique(unique, otherCiv())
     }
 
     internal fun setFriendshipBasedModifier() {
@@ -614,10 +606,8 @@ class DiplomacyManager() : IsPartOfGameInfoSerialization {
 
         // Ignore contitionals as triggerUnique will check again, and that would break
         // UniqueType.ConditionalChance - 25% declared chance would work as 6% actual chance
-        for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponSigningDefensivePact, StateForConditionals.IgnoreConditionals))
+        civInfo.triggerUniques(UniqueType.TriggerUponSigningDefensivePact)
-            UniqueTriggerActivation.triggerUnique(unique, civInfo)
+        otherCiv().triggerUniques(UniqueType.TriggerUponSigningDefensivePact)
-        for (unique in otherCiv().getTriggeredUniques(UniqueType.TriggerUponSigningDefensivePact, StateForConditionals.IgnoreConditionals))
-            UniqueTriggerActivation.triggerUnique(unique, otherCiv())
     }
 
     internal fun setDefensivePactBasedModifier() {

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

@@ -384,18 +384,15 @@ class ReligionManager : IsPartOfGameInfoSerialization {
         when (religionState) {
             ReligionState.None -> {
                 religionState = ReligionState.Pantheon
-                for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponFoundingPantheon))
+                civInfo.triggerUniques(UniqueType.TriggerUponFoundingPantheon)
-                    UniqueTriggerActivation.triggerUnique(unique, civInfo)
             }
             ReligionState.FoundingReligion -> {
                 religionState = ReligionState.Religion
-                for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponFoundingReligion))
+                civInfo.triggerUniques(UniqueType.TriggerUponFoundingReligion)
-                    UniqueTriggerActivation.triggerUnique(unique, civInfo)
             }
             ReligionState.EnhancingReligion -> {
                 religionState = ReligionState.EnhancedReligion
-                for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponEnhancingReligion))
+                civInfo.triggerUniques(UniqueType.TriggerUponEnhancingReligion)
-                    UniqueTriggerActivation.triggerUnique(unique, civInfo)
             }
             else -> {}
         }

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

@@ -4,18 +4,11 @@ import com.unciv.UncivGame
 import com.unciv.logic.VictoryData
 import com.unciv.logic.automation.civilization.NextTurnAutomation
 import com.unciv.logic.city.managers.CityTurnManager
-import com.unciv.logic.civilization.AlertType
+import com.unciv.logic.civilization.*
-import com.unciv.logic.civilization.CivFlags
-import com.unciv.logic.civilization.Civilization
-import com.unciv.logic.civilization.NotificationCategory
-import com.unciv.logic.civilization.NotificationIcon
-import com.unciv.logic.civilization.PlayerType
-import com.unciv.logic.civilization.PopupAlert
 import com.unciv.logic.civilization.diplomacy.DiplomacyTurnManager.nextTurn
 import com.unciv.logic.map.mapunit.UnitTurnManager
 import com.unciv.logic.map.tile.Tile
 import com.unciv.logic.trade.TradeEvaluation
-import com.unciv.models.ruleset.unique.UniqueTriggerActivation
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.ruleset.unique.endTurn
 import com.unciv.models.stats.Stats
@@ -70,8 +63,7 @@ class TurnManager(val civInfo: Civilization) {
         startTurnFlags()
         updateRevolts()
 
-        for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponTurnStart, civInfo.state))
+        civInfo.triggerUniques(UniqueType.TriggerUponTurnStart)
-            UniqueTriggerActivation.triggerUnique(unique, civInfo)
 
         for (city in civInfo.cities) {
             progressBar?.increment()
@@ -234,8 +226,7 @@ class TurnManager(val civInfo: Civilization) {
         if (UncivGame.Current.settings.citiesAutoBombardAtEndOfTurn)
             NextTurnAutomation.automateCityBombardment(civInfo) // Bombard with all cities that haven't, maybe you missed one
 
-        for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponTurnEnd, civInfo.state))
+        civInfo.triggerUniques(UniqueType.TriggerUponTurnEnd)
-            UniqueTriggerActivation.triggerUnique(unique, civInfo)
 
         val notificationsLog = civInfo.notificationsLog
         val notificationsThisTurn = Civilization.NotificationsLog(civInfo.gameInfo.turns)

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

@@ -106,7 +106,7 @@ class UnitManager(val civInfo: Civilization) {
                     && unique.conditionalsApply(unit.cache.state))
                     UniqueTriggerActivation.triggerUnique(unique, unit, triggerNotificationText = triggerNotificationText)
 
-            for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponGainingUnit, unit.cache.state) 
+            for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponGainingUnit) 
                     { unit.matchesFilter(it.params[0]) })
                 UniqueTriggerActivation.triggerUnique(unique, unit, triggerNotificationText = triggerNotificationText)
 

core/src/com/unciv/logic/civilization/transients/CivInfoTransientCache.kt

@@ -270,9 +270,7 @@ class CivInfoTransientCache(val civInfo: Civilization) {
                     )
             }
 
-            for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponDiscoveringNaturalWonder,
+            for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponDiscoveringNaturalWonder))
-                StateForConditionals(civInfo, tile = tile)
-            ))
                 UniqueTriggerActivation.triggerUnique(unique, civInfo, tile=tile, triggerNotificationText = "due to discovering a Natural Wonder")
         }
     }

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

@@ -265,16 +265,15 @@ class TileImprovementFunctions(val tile: Tile) {
             civ.gainStockpiledResource(resource, -amount)
         }
 
-        for (unique in improvement.uniqueObjects.filter { !it.hasTriggerConditional()
+        for (unique in improvement.uniqueObjects.filter { !it.hasTriggerConditional() })
-            && it.conditionalsApply(stateForConditionals) })
             UniqueTriggerActivation.triggerUnique(unique, civ, unit = unit, tile = tile)
 
-        for (unique in civ.getTriggeredUniques(UniqueType.TriggerUponBuildingImprovement, stateForConditionals)
+        for (unique in civ.getTriggeredUniques(UniqueType.TriggerUponBuildingImprovement)
             { improvement.matchesFilter(it.params[0], stateForConditionals) })
             UniqueTriggerActivation.triggerUnique(unique, civ, unit = unit, tile = tile)
 
         if (unit == null) return
-        for (unique in unit.getTriggeredUniques(UniqueType.TriggerUponBuildingImprovement, stateForConditionals)
+        for (unique in unit.getTriggeredUniques(UniqueType.TriggerUponBuildingImprovement)
             { improvement.matchesFilter(it.params[0], stateForConditionals) })
             UniqueTriggerActivation.triggerUnique(unique, civ, unit = unit, tile = tile)
     }

core/src/com/unciv/models/ruleset/unique/UniqueMap.kt

@@ -9,6 +9,12 @@ open class UniqueMap() {
     // This is a memory/speed tradeoff, since there are *600 unique types*,
     // 750 including deprecated, and EnumMap creates a N-sized array where N is the number of objects in the enum
     private val typedUniqueMap = EnumMap<UniqueType, ArrayList<Unique>>(UniqueType::class.java)
+    
+    // Another memory-speed tradeoff - enumset is super fast and also super cheap, but it's not nothing
+    // This is used to speed up triggered uniques - in other words, when we want to find all uniques with a certain modifier.
+    // Rather than mapping all uniques thus triggered, this just stores whether any unique has that trigger - 
+    //   because most of the time is spent iterating on uniques, in uniquemaps that have no such trigger in the first place!
+    private val triggerEnumSet = EnumSet.noneOf(UniqueType::class.java)
 
     constructor(uniques: Sequence<Unique>) : this() {
         addUniques(uniques.asIterable())
@@ -25,6 +31,7 @@ open class UniqueMap() {
         if (unique.type == null) return
         if (typedUniqueMap[unique.type] != null) return
         typedUniqueMap[unique.type] = innerUniqueMap[unique.placeholderText]
+        triggerEnumSet.add(unique.type)
     }
 
     /** Calls [addUnique] on each item from [uniques] */
@@ -95,6 +102,7 @@ open class UniqueMap() {
 
     fun getTriggeredUniques(trigger: UniqueType, stateForConditionals: StateForConditionals,
                             triggerFilter: (Unique) -> Boolean = { true }): Sequence<Unique> {
+        if (!triggerEnumSet.contains(trigger)) return emptySequence() // Common case - no such unique exists
         return getAllUniques().filter { unique ->
             unique.getModifiers(trigger).any(triggerFilter) && unique.conditionalsApply(stateForConditionals)
         }.flatMap { it.getMultiplied(stateForConditionals) }