Add a field for global unit uniques (#12775)

* Add a field for global unit uniques

* Whoops

* docs

* Fix this check only ever being done once

* Revert

* Add ruleset uniques when adding rulesets together

* Add ruleset to unitTypes for tests in case it's relevant

* My suggested changes: Implement a separate rulesetMap for units, remove any additional checks to the type where unnecessary

* Remove unit type code, update ruleset info by setter rather than by lazy

* Type information is needed before we set the ruleset here

* So should unique information

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

@@ -39,7 +39,6 @@ import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.ruleset.Speed
 import com.unciv.models.ruleset.nation.Difficulty
 import com.unciv.models.ruleset.unique.LocalUniqueCache
-import com.unciv.models.ruleset.unique.StateForConditionals
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.translations.tr
 import com.unciv.ui.audio.MusicMood
@@ -640,7 +639,7 @@ class GameInfo : IsPartOfGameInfoSerialization, HasGameInfoSerializationVersion
         removeMissingModReferences()
 
         for (baseUnit in ruleset.units.values)
-            baseUnit.ruleset = ruleset
+            baseUnit.setRuleset(ruleset)
 
         for (building in ruleset.buildings.values)
             building.ruleset = ruleset

core/src/com/unciv/logic/automation/unit/UnitAutomation.kt

@@ -594,7 +594,8 @@ object UnitAutomation {
 
     private fun chooseBombardTarget(city: City): ICombatant? {
         var targets = TargetHelper.getBombardableTiles(city).map { Battle.getMapCombatantOfTile(it)!! }
-            .filterNot { it.isCivilian() && !it.getUnitType().hasUnique(UniqueType.Uncapturable) } // Don't bombard capturable civilians
+            .filterNot { it is MapUnitCombatant &&
+                it.isCivilian() && !it.unit.hasUnique(UniqueType.Uncapturable) } // Don't bombard capturable civilians
         if (targets.none()) return null
 
         val siegeUnits = targets

core/src/com/unciv/logic/map/mapunit/MapUnit.kt

@@ -673,12 +673,9 @@ class MapUnit : IsPartOfGameInfoSerialization {
     }
 
     fun updateUniques() {
-        val unitUniqueSources =
-            baseUnit.uniqueObjects.asSequence() +
-                type.uniqueObjects
         val otherUniqueSources = promotions.getPromotions().flatMap { it.uniqueObjects } +
             statuses.flatMap { it.uniques }
-        val uniqueSources = unitUniqueSources + otherUniqueSources
+        val uniqueSources = baseUnit.rulesetUniqueObjects.asSequence() + otherUniqueSources
         
         tempUniquesMap = UniqueMap(uniqueSources)
         nonUnitUniquesMap = UniqueMap(otherUniqueSources)

core/src/com/unciv/models/ruleset/GlobalUniques.kt

@@ -7,6 +7,7 @@ import com.unciv.models.ruleset.unique.UniqueType
 class GlobalUniques: RulesetObject() {
     override var name = "GlobalUniques"
 
+    var unitUniques: ArrayList<String> = ArrayList()
     override fun getUniqueTarget() = UniqueTarget.Global
     override fun makeLink() = "" // No own category on Civilopedia screen
 

core/src/com/unciv/models/ruleset/Ruleset.kt

@@ -184,6 +184,8 @@ class Ruleset {
         globalUniques = GlobalUniques().apply {
             uniques.addAll(globalUniques.uniques)
             uniques.addAll(ruleset.globalUniques.uniques)
+            unitUniques.addAll(globalUniques.unitUniques)
+            unitUniques.addAll(ruleset.globalUniques.unitUniques)
         }
         ruleset.modOptions.nationsToRemove
             .flatMap { nationToRemove ->
@@ -214,7 +216,7 @@ class Ruleset {
         cityStateTypes.putAll(ruleset.cityStateTypes)
         ruleset.modOptions.unitsToRemove
             .flatMap { unitToRemove ->
-                units.filter { it.apply { value.ruleset = this@Ruleset }.value.matchesFilter(unitToRemove) }.keys
+                units.filter { it.apply { value.setRuleset(this@Ruleset) }.value.matchesFilter(unitToRemove) }.keys
             }.toSet().forEach {
                 units.remove(it)
             }

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

@@ -21,12 +21,18 @@ interface IHasUniques : INamed {
     val uniqueMap: UniqueMap
 
     fun uniqueObjectsProvider(): List<Unique> {
+        return uniqueObjectsProvider(uniques)
+    }
+    fun uniqueMapProvider(): UniqueMap {
+        return uniqueMapProvider(uniqueObjects)
+    }
+    fun uniqueObjectsProvider(uniques: List<String>): List<Unique> {
         if (uniques.isEmpty()) return emptyList()
         return uniques.map { Unique(it, getUniqueTarget(), name) }
     }
-    fun uniqueMapProvider(): UniqueMap {
+    fun uniqueMapProvider(uniqueObjects: List<Unique>): UniqueMap {
         val newUniqueMap = UniqueMap()
-        if (uniques.isNotEmpty())
+        if (uniqueObjects.isNotEmpty())
             newUniqueMap.addUniques(uniqueObjects)
         return newUniqueMap
     }

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

@@ -266,6 +266,8 @@ open class UniqueMap() {
         addUniques(uniques.asIterable())
     }
 
+    fun isEmpty(): Boolean = innerUniqueMap.isEmpty()
+
     /** Adds one [unique] unless it has a ConditionalTimedUnique conditional */
     open fun addUnique(unique: Unique) {
         val existingArrayList = innerUniqueMap[unique.placeholderText]

core/src/com/unciv/models/ruleset/unit/BaseUnit.kt

@@ -16,6 +16,7 @@ import com.unciv.models.ruleset.RulesetObject
 import com.unciv.models.ruleset.unique.Conditionals
 import com.unciv.models.ruleset.unique.StateForConditionals
 import com.unciv.models.ruleset.unique.Unique
+import com.unciv.models.ruleset.unique.UniqueMap
 import com.unciv.models.ruleset.unique.UniqueTarget
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.stats.Stat
@@ -69,7 +70,24 @@ class BaseUnit : RulesetObject(), INonPerpetualConstruction {
     val costFunctions = BaseUnitCost(this)
 
     lateinit var ruleset: Ruleset
+        private set
 
+    fun setRuleset(ruleset: Ruleset) {
+        this.ruleset = ruleset
+        val list = ArrayList(uniques)
+        list.addAll(ruleset.globalUniques.unitUniques)
+        list.addAll(type.uniques)
+        rulesetUniqueObjects = uniqueObjectsProvider(list)
+        rulesetUniqueMap = uniqueMapProvider(rulesetUniqueObjects) // Has global uniques by the unique objects already
+    }
+
+    @Transient
+    var rulesetUniqueObjects: List<Unique> = ArrayList()
+        private set
+
+    @Transient
+    var rulesetUniqueMap: UniqueMap = UniqueMap()
+        private set
 
     /** Generate short description as comma-separated string for Technology description "Units enabled" and GreatPersonPickerScreen */
     fun getShortDescription(uniqueExclusionFilter: Unique.() -> Boolean = {false}) = BaseUnitDescriptions.getShortDescription(this, uniqueExclusionFilter)
@@ -116,45 +134,33 @@ class BaseUnit : RulesetObject(), INonPerpetualConstruction {
         return unit
     }
 
-    
     override fun hasUnique(uniqueType: UniqueType, state: StateForConditionals?): Boolean {
-        return super<RulesetObject>.hasUnique(uniqueType, state) || ::ruleset.isInitialized && type.hasUnique(uniqueType, state)
+        val stateForConditionals = state ?: StateForConditionals.EmptyState
+        return if (::ruleset.isInitialized) rulesetUniqueMap.hasUnique(uniqueType, stateForConditionals)
+        else super<RulesetObject>.hasUnique(uniqueType, stateForConditionals)
     }
 
     override fun hasUnique(uniqueTag: String, state: StateForConditionals?): Boolean {
-        return super<RulesetObject>.hasUnique(uniqueTag, state) || ::ruleset.isInitialized && type.hasUnique(uniqueTag, state)
+        val stateForConditionals = state ?: StateForConditionals.EmptyState
+        return if (::ruleset.isInitialized) rulesetUniqueMap.hasUnique(uniqueTag, stateForConditionals)
+        else super<RulesetObject>.hasUnique(uniqueTag, stateForConditionals)
     }
 
     override fun hasTagUnique(tagUnique: String): Boolean {
-        return super<RulesetObject>.hasTagUnique(tagUnique) || ::ruleset.isInitialized && type.hasTagUnique(tagUnique) 
+        return if (::ruleset.isInitialized) rulesetUniqueMap.hasTagUnique(tagUnique)
+        else super<RulesetObject>.hasTagUnique(tagUnique)
     }
 
     /** Allows unique functions (getMatchingUniques, hasUnique) to "see" uniques from the UnitType */
     override fun getMatchingUniques(uniqueType: UniqueType, state: StateForConditionals): Sequence<Unique> {
-        val ourUniques = super<RulesetObject>.getMatchingUniques(uniqueType, state)
+        return if (::ruleset.isInitialized) rulesetUniqueMap.getMatchingUniques(uniqueType, state)
-        if (! ::ruleset.isInitialized) { // Not sure if this will ever actually happen, but better safe than sorry
+        else super<RulesetObject>.getMatchingUniques(uniqueType, state)
-            return ourUniques
-        }
-        val typeUniques = type.getMatchingUniques(uniqueType, state)
-        // Memory optimization - very rarely do we actually get uniques from both sources,
-        //   and sequence addition is expensive relative to the rare case that we'll actually need it
-        if (ourUniques.none()) return typeUniques
-        if (typeUniques.none()) return ourUniques
-        return ourUniques + type.getMatchingUniques(uniqueType, state)
     }
 
     /** Allows unique functions (getMatchingUniques, hasUnique) to "see" uniques from the UnitType */
     override fun getMatchingUniques(uniqueTag: String, state: StateForConditionals): Sequence<Unique> {
-        val ourUniques = super<RulesetObject>.getMatchingUniques(uniqueTag, state)
+        return if (::ruleset.isInitialized) rulesetUniqueMap.getMatchingUniques(uniqueTag, state)
-        if (! ::ruleset.isInitialized) { // Not sure if this will ever actually happen, but better safe than sorry
+        else super<RulesetObject>.getMatchingUniques(uniqueTag, state)
-            return ourUniques
-        }
-        val typeUniques = type.getMatchingUniques(uniqueTag, state)
-        // Memory optimization - very rarely do we actually get uniques from both sources,
-        //   and sequence addition is expensive relative to the rare case that we'll actually need it
-        if (ourUniques.none()) return typeUniques
-        if (typeUniques.none()) return ourUniques
-        return ourUniques + type.getMatchingUniques(uniqueTag, state)
     }
 
     override fun getProductionCost(civInfo: Civilization, city: City?): Int  = costFunctions.getProductionCost(civInfo, city)
@@ -519,7 +525,7 @@ class BaseUnit : RulesetObject(), INonPerpetualConstruction {
             power += 4000
 
         // Uniques
-        val allUniques = uniqueObjects.asSequence() +
+        val allUniques = rulesetUniqueObjects.asSequence() +
             promotions.asSequence()
                 .mapNotNull { ruleset.unitPromotions[it] }
                 .flatMap { it.uniqueObjects }

core/src/com/unciv/models/ruleset/unit/UnitType.kt

@@ -15,7 +15,6 @@ enum class UnitMovementType { // The types of tiles the unit can by default ente
 class UnitType() : RulesetObject() {
     private var movementType: String? = null
     private val unitMovementType: UnitMovementType? by lazy { if (movementType == null) null else UnitMovementType.valueOf(movementType!!) }
-
     override fun getUniqueTarget() = UniqueTarget.UnitType
     override fun makeLink() = "UnitType/$name"
 

docs/Modders/Mod-file-structure/5-Miscellaneous-JSON-files.md

@@ -292,7 +292,15 @@ With `civModifier` being the multiplicative aggregate of ["\[relativeAmount\]% G
 [link to original](https://github.com/yairm210/Unciv/tree/master/android/assets/jsons/GlobalUniques.json)
 
 GlobalUniques defines uniques that apply globally. e.g. Vanilla rulesets define the effects of Unhappiness here.
-Only the `uniques` field is used, but a name must still be set (the Ruleset validator might display it).
+
+It has the following structure:
+
+| Attribute   | Type            | Default         | Notes                                                                                       |
+|-------------|-----------------|-----------------|---------------------------------------------------------------------------------------------|
+| name        | String          | "GlobalUniques" | The name field is not used, but still must be set (the Ruleset validator might display it). |
+| uniques     | List of Strings | empty           | List of [unique abilities](../../uniques) that apply globally                               |
+| unitUniques | List of Strings | empty           | List of [unique abilities](../../uniques) that applies to each unit                         |
+
 When extension rulesets define GlobalUniques, all uniques are merged. At the moment there is no way to change/remove uniques set by a base mod.
 
 ## Tutorials.json

tests/src/com/unciv/logic/map/UnitMovementTests.kt

@@ -11,7 +11,6 @@ import com.unciv.logic.civilization.diplomacy.DiplomaticStatus
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.logic.map.tile.Tile
 import com.unciv.models.ruleset.nation.Nation
-import com.unciv.models.ruleset.unique.StateForConditionals
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.models.ruleset.unit.UnitType
@@ -54,8 +53,8 @@ class UnitMovementTests {
     fun addFakeUnit(unitType: UnitType, uniques: List<String> = listOf()): MapUnit {
         val baseUnit = BaseUnit()
         baseUnit.unitType = unitType.name
-        baseUnit.ruleset = testGame.ruleset
         baseUnit.uniques.addAll(uniques)
+        baseUnit.setRuleset(testGame.ruleset)
 
         val unit = MapUnit()
         unit.name = baseUnit.name

tests/src/com/unciv/testing/TestGame.kt

@@ -64,7 +64,7 @@ class TestGame {
         tileMap.gameInfo = gameInfo
 
         for (baseUnit in ruleset.units.values)
-            baseUnit.ruleset = ruleset
+            baseUnit.setRuleset(ruleset)
     }
 
     /** Makes a new rectangular tileMap and sets it in gameInfo. Removes all existing tiles. All new tiles have terrain [baseTerrain] */
@@ -183,7 +183,7 @@ class TestGame {
 
     fun addUnit(name: String, civInfo: Civilization, tile: Tile?): MapUnit {
         val baseUnit = ruleset.units[name]!!
-        baseUnit.ruleset = ruleset
+        baseUnit.setRuleset(ruleset)
         val mapUnit = baseUnit.getMapUnit(civInfo)
         civInfo.units.addUnit(mapUnit)
         if (tile!=null) {
@@ -238,8 +238,8 @@ class TestGame {
     fun createBaseUnit(unitType: String = createUnitType().name, vararg uniques: String) =
         createRulesetObject(ruleset.units, *uniques) {
             val baseUnit = BaseUnit()
-            baseUnit.ruleset = gameInfo.ruleset
             baseUnit.unitType = unitType
+            baseUnit.setRuleset(gameInfo.ruleset)
             baseUnit
         }
     fun createBelief(type: BeliefType = BeliefType.Any, vararg uniques: String) =