From 1345093d338aae8996952431fdd47bc467ccd656 Mon Sep 17 00:00:00 2001 From: yairm210 Date: Tue, 6 May 2025 23:36:58 +0300 Subject: [PATCH] Modding: Fixed chance conditionals attached to triggered uniques --- core/src/com/unciv/logic/battle/Battle.kt | 2 ++ .../com/unciv/logic/city/CityConstructions.kt | 14 ++++++------- .../unciv/logic/city/managers/CityFounder.kt | 8 ++------ .../unciv/logic/civilization/Civilization.kt | 12 ++++++++++- .../civilization/diplomacy/DeclareWar.kt | 12 ++--------- .../diplomacy/DiplomacyManager.kt | 20 +++++-------------- .../civilization/managers/ReligionManager.kt | 9 +++------ .../civilization/managers/TurnManager.kt | 15 +++----------- .../civilization/managers/UnitManager.kt | 2 +- .../transients/CivInfoTransientCache.kt | 4 +--- .../map/tile/TileImprovementFunctions.kt | 7 +++---- .../unciv/models/ruleset/unique/UniqueMap.kt | 8 ++++++++ 12 files changed, 48 insertions(+), 65 deletions(-) diff --git a/core/src/com/unciv/logic/battle/Battle.kt b/core/src/com/unciv/logic/battle/Battle.kt index fe552c310c..5ce06dc8c9 100644 --- a/core/src/com/unciv/logic/battle/Battle.kt +++ b/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) diff --git a/core/src/com/unciv/logic/city/CityConstructions.kt b/core/src/com/unciv/logic/city/CityConstructions.kt index e4a43fe4db..198f5e9e5b 100644 --- a/core/src/com/unciv/logic/city/CityConstructions.kt +++ b/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) diff --git a/core/src/com/unciv/logic/city/managers/CityFounder.kt b/core/src/com/unciv/logic/city/managers/CityFounder.kt index 16488f0006..8665d3a6df 100644 --- a/core/src/com/unciv/logic/city/managers/CityFounder.kt +++ b/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, - StateForConditionals(civInfo, city, unit) - )) + for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponFoundingCity)) UniqueTriggerActivation.triggerUnique(unique, civInfo, city, unit, triggerNotificationText = "due to founding a city") if (unit != null) - for (unique in unit.getTriggeredUniques(UniqueType.TriggerUponFoundingCity, - StateForConditionals(civInfo, city, unit))) + for (unique in unit.getTriggeredUniques(UniqueType.TriggerUponFoundingCity)) UniqueTriggerActivation.triggerUnique(unique, civInfo, city, unit, triggerNotificationText = "due to founding a city") return city diff --git a/core/src/com/unciv/logic/civilization/Civilization.kt b/core/src/com/unciv/logic/civilization/Civilization.kt index f95768eb74..5b1f1c9fba 100644 --- a/core/src/com/unciv/logic/civilization/Civilization.kt +++ b/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 = sequence { yieldAll(nation.uniqueMap.getTriggeredUniques(trigger, stateForConditionals, triggerFilter)) diff --git a/core/src/com/unciv/logic/civilization/diplomacy/DeclareWar.kt b/core/src/com/unciv/logic/civilization/diplomacy/DeclareWar.kt index bfb062d677..d869350779 100644 --- a/core/src/com/unciv/logic/civilization/diplomacy/DeclareWar.kt +++ b/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.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.logic.civilization.* 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()) - for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponDeclaringWar)) - UniqueTriggerActivation.triggerUnique(unique, civInfo) + if (otherCiv.isMajorCiv()) civInfo.triggerUniques(UniqueType.TriggerUponDeclaringWar) } private fun handleCityStateDirectAttack(diplomacyManager: DiplomacyManager) { diff --git a/core/src/com/unciv/logic/civilization/diplomacy/DiplomacyManager.kt b/core/src/com/unciv/logic/civilization/diplomacy/DiplomacyManager.kt index 17ac1ebd91..74f29f1f6a 100644 --- a/core/src/com/unciv/logic/civilization/diplomacy/DiplomacyManager.kt +++ b/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.TradeEvaluation -import com.unciv.logic.trade.TradeLogic -import com.unciv.logic.trade.TradeOffer -import com.unciv.logic.trade.TradeOfferType +import com.unciv.logic.trade.* 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)) - UniqueTriggerActivation.triggerUnique(unique, civInfo) - for (unique in otherCiv().getTriggeredUniques(UniqueType.TriggerUponDeclaringFriendship, StateForConditionals.IgnoreConditionals)) - UniqueTriggerActivation.triggerUnique(unique, otherCiv()) + civInfo.triggerUniques(UniqueType.TriggerUponDeclaringFriendship) + otherCiv().triggerUniques(UniqueType.TriggerUponDeclaringFriendship) } 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)) - UniqueTriggerActivation.triggerUnique(unique, civInfo) - for (unique in otherCiv().getTriggeredUniques(UniqueType.TriggerUponSigningDefensivePact, StateForConditionals.IgnoreConditionals)) - UniqueTriggerActivation.triggerUnique(unique, otherCiv()) + civInfo.triggerUniques(UniqueType.TriggerUponSigningDefensivePact) + otherCiv().triggerUniques(UniqueType.TriggerUponSigningDefensivePact) } internal fun setDefensivePactBasedModifier() { diff --git a/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt b/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt index 38a5366b09..fb80f431a4 100644 --- a/core/src/com/unciv/logic/civilization/managers/ReligionManager.kt +++ b/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)) - UniqueTriggerActivation.triggerUnique(unique, civInfo) + civInfo.triggerUniques(UniqueType.TriggerUponFoundingPantheon) } ReligionState.FoundingReligion -> { religionState = ReligionState.Religion - for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponFoundingReligion)) - UniqueTriggerActivation.triggerUnique(unique, civInfo) + civInfo.triggerUniques(UniqueType.TriggerUponFoundingReligion) } ReligionState.EnhancingReligion -> { religionState = ReligionState.EnhancedReligion - for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponEnhancingReligion)) - UniqueTriggerActivation.triggerUnique(unique, civInfo) + civInfo.triggerUniques(UniqueType.TriggerUponEnhancingReligion) } else -> {} } diff --git a/core/src/com/unciv/logic/civilization/managers/TurnManager.kt b/core/src/com/unciv/logic/civilization/managers/TurnManager.kt index dd2ec59a8e..1791444536 100644 --- a/core/src/com/unciv/logic/civilization/managers/TurnManager.kt +++ b/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.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.* 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)) - UniqueTriggerActivation.triggerUnique(unique, civInfo) + civInfo.triggerUniques(UniqueType.TriggerUponTurnStart) 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)) - UniqueTriggerActivation.triggerUnique(unique, civInfo) + civInfo.triggerUniques(UniqueType.TriggerUponTurnEnd) val notificationsLog = civInfo.notificationsLog val notificationsThisTurn = Civilization.NotificationsLog(civInfo.gameInfo.turns) diff --git a/core/src/com/unciv/logic/civilization/managers/UnitManager.kt b/core/src/com/unciv/logic/civilization/managers/UnitManager.kt index e8896a253f..af652d6f84 100644 --- a/core/src/com/unciv/logic/civilization/managers/UnitManager.kt +++ b/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) diff --git a/core/src/com/unciv/logic/civilization/transients/CivInfoTransientCache.kt b/core/src/com/unciv/logic/civilization/transients/CivInfoTransientCache.kt index 412d64f515..cb96c6daa9 100644 --- a/core/src/com/unciv/logic/civilization/transients/CivInfoTransientCache.kt +++ b/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, - StateForConditionals(civInfo, tile = tile) - )) + for (unique in civInfo.getTriggeredUniques(UniqueType.TriggerUponDiscoveringNaturalWonder)) UniqueTriggerActivation.triggerUnique(unique, civInfo, tile=tile, triggerNotificationText = "due to discovering a Natural Wonder") } } diff --git a/core/src/com/unciv/logic/map/tile/TileImprovementFunctions.kt b/core/src/com/unciv/logic/map/tile/TileImprovementFunctions.kt index 0b58780f2f..d19cb73deb 100644 --- a/core/src/com/unciv/logic/map/tile/TileImprovementFunctions.kt +++ b/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() - && it.conditionalsApply(stateForConditionals) }) + for (unique in improvement.uniqueObjects.filter { !it.hasTriggerConditional() }) 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) } diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueMap.kt b/core/src/com/unciv/models/ruleset/unique/UniqueMap.kt index 2ab611e9d3..10eb9b306d 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueMap.kt +++ b/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::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) : 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 { + 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) }