Great General typed uniques and improved moddability (#6818)

* Great General UniqueTyped and improved moddability - draft

* Great General UniqueTyped and improved moddability - reviews

* Great General UniqueTyped and improved moddability - no reason not to cache another

* Integration with JackRainy's solution

* Integration with JackRainy's solution - part 2

* Revert of maxGreatGeneralBonusRadius logic

* Minor refactoring

* Code review: minor refactoring

* Keep the warning for the modders about the obsolete unique

* Code review: Better wording for the unique

Co-authored-by: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com>

android/assets/jsons/Civ V - Gods & Kings/Units.json

@@ -1634,7 +1634,7 @@
 	{
 		"name": "Great General",
 		"unitType": "Civilian",
-		"uniques": ["Can start an [8]-turn golden age", "Bonus for units in 2 tile radius 15%", "Can construct [Citadel]",
+		"uniques": ["Can start an [8]-turn golden age", "[+15]% Strength bonus for [Military] units within [2] tiles", "Can construct [Citadel]",
 			"Great Person - [War]", "Unbuildable", "Uncapturable"],
 		"movement": 2
 	},
@@ -1643,7 +1643,7 @@
 		"unitType": "Civilian",
 		"uniqueTo": "Mongolia",
 		"replaces": "Great General",
-		"uniques": ["Can start an [8]-turn golden age","Bonus for units in 2 tile radius 15%",
+		"uniques": ["Can start an [8]-turn golden age","[+15]% Strength bonus for [Military] units within [2] tiles",
 			"All adjacent units heal [+15] HP when healing", "[+15] HP when healing", "Can construct [Citadel]", "Great Person - [War]", "Unbuildable", "Uncapturable"],
 		"movement": 5
 	},

android/assets/jsons/Civ V - Vanilla/Units.json

@@ -1310,7 +1310,7 @@
 	{
 		"name": "Great General",
 		"unitType": "Civilian",
-		"uniques": ["Can start an [8]-turn golden age", "Bonus for units in 2 tile radius 15%", "Can construct [Citadel]",
+		"uniques": ["Can start an [8]-turn golden age", "[+15]% Strength bonus for [Military] units within [2] tiles", "Can construct [Citadel]",
 			"Great Person - [War]", "Unbuildable", "Uncapturable"],
 		"movement": 2
 	},
@@ -1319,7 +1319,7 @@
 		"unitType": "Civilian",
 		"uniqueTo": "Mongolia",
 		"replaces": "Great General",
-		"uniques": ["Can start an [8]-turn golden age","Bonus for units in 2 tile radius 15%",
+		"uniques": ["Can start an [8]-turn golden age", "[+15]% Strength bonus for [Military] units within [2] tiles",
 			"All adjacent units heal [+15] HP when healing", "[+15] HP when healing", "Can construct [Citadel]", "Great Person - [War]", "Unbuildable", "Uncapturable"],
 		"movement": 5
 	},

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

@@ -144,6 +144,14 @@ object BackwardCompatibility {
                     unit.promotions.addPromotion(startingPromo, true)
     }
 
+    /** Upgrade the uniques from deprecated format to the new more general one **/
+    fun GameInfo.updateGreatGeneralUniques() {
+        ruleSet.units.values.filter { it.uniques.contains("Bonus for units in 2 tile radius 15%") }.forEach {
+            it.uniques.remove("Bonus for units in 2 tile radius 15%")
+            it.uniques.add("[+15]% Strength bonus for [Military] units within [2] tiles")
+        }
+    }
+
     /** Move max XP from barbarians to new home */
     @Suppress("DEPRECATION")
     fun ModOptions.updateDeprecations() {

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

@@ -6,6 +6,7 @@ import com.unciv.logic.BackwardCompatibility.guaranteeUnitPromotions
 import com.unciv.logic.BackwardCompatibility.migrateBarbarianCamps
 import com.unciv.logic.BackwardCompatibility.migrateSeenImprovements
 import com.unciv.logic.BackwardCompatibility.removeMissingModReferences
+import com.unciv.logic.BackwardCompatibility.updateGreatGeneralUniques
 import com.unciv.logic.automation.NextTurnAutomation
 import com.unciv.logic.civilization.*
 import com.unciv.logic.city.CityInfo
@@ -417,6 +418,7 @@ class GameInfo {
 
         removeMissingModReferences()
 
+        updateGreatGeneralUniques()
 
         for (baseUnit in ruleSet.units.values)
             baseUnit.ruleset = ruleSet

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

@@ -163,8 +163,7 @@ object NextTurnAutomation {
         while (delta > 0) {
             // Now remove the best offer valued below delta until the deal is barely acceptable
             val offerToRemove = counterofferAsks.filter { it.value <= delta }.maxByOrNull { it.value }
-            if (offerToRemove == null)
+                ?: break  // Nothing more can be removed, at least en bloc
-                break // Nothing more can be removed, at least en bloc
             delta -= offerToRemove.value
             counterofferAsks.remove(offerToRemove.key)
         }
@@ -831,7 +830,7 @@ object NextTurnAutomation {
             when {
                 unit.baseUnit.isRanged() -> rangedUnits.add(unit)
                 unit.baseUnit.isMelee() -> meleeUnits.add(unit)
-                unit.hasUnique("Bonus for units in 2 tile radius 15%")
+                unit.isGreatPersonOfType("War")
                     -> generals.add(unit) // Generals move after military units
                 else -> civilianUnits.add(unit)
             }

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

@@ -1,6 +1,7 @@
 package com.unciv.logic.automation
 
 import com.unciv.logic.battle.Battle
+import com.unciv.logic.battle.GreatGeneralImplementation
 import com.unciv.logic.battle.MapUnitCombatant
 import com.unciv.logic.city.CityInfo
 import com.unciv.logic.civilization.CivilizationInfo
@@ -45,28 +46,16 @@ object SpecificUnitAutomation {
         }
     }
 
-    fun automateGreatGeneral(unit: MapUnit) {
+    fun automateGreatGeneral(unit: MapUnit): Boolean {
         //try to follow nearby units. Do not garrison in city if possible
-        val militaryUnitTilesInDistance = unit.movement.getDistanceToTiles().asSequence()
+        val maxAffectedTroopsTile = GreatGeneralImplementation.getBestAffectedTroopsTile(unit)
-                .filter {
+            ?: return false
-                    val militant = it.key.militaryUnit
-                    militant != null && militant.civInfo == unit.civInfo
-                            && (it.key.civilianUnit == null || it.key.civilianUnit == unit)
-                            && militant.getMaxMovement() <= 2 && !it.key.isCityCenter()
-                }
 
-        val maxAffectedTroopsTile = militaryUnitTilesInDistance
+        unit.movement.headTowards(maxAffectedTroopsTile)
-                .maxByOrNull {
+        return true
-                    it.key.getTilesInDistance(2).count { tile ->
+    }
-                        val militaryUnit = tile.militaryUnit
-                        militaryUnit != null && militaryUnit.civInfo == unit.civInfo
-                    }
-                }?.key
-        if (maxAffectedTroopsTile != null) {
-            unit.movement.headTowards(maxAffectedTroopsTile)
-            return
-        }
 
+    fun automateCitadelPlacer(unit: MapUnit): Boolean {
         // try to revenge and capture their tiles
         val enemyCities = unit.civInfo.getKnownCivs()
                 .filter { unit.civInfo.getDiplomacyManager(it).hasModifier(DiplomaticModifiers.StealingTerritory) }
@@ -94,16 +83,19 @@ object SpecificUnitAutomation {
             unit.movement.headTowards(tileToSteal)
             if (unit.currentMovement > 0 && unit.currentTile == tileToSteal)
                 UnitActions.getImprovementConstructionActions(unit, unit.currentTile).firstOrNull()?.action?.invoke()
-            return
+            return true
         }
 
         // try to build a citadel for defensive purposes
         if (WorkerAutomation.evaluateFortPlacement(unit.currentTile, unit.civInfo, true)) {
             UnitActions.getImprovementConstructionActions(unit, unit.currentTile).firstOrNull()?.action?.invoke()
-            return
+            return true
         }
+        return false
+    }
 
-        //if no unit to follow, take refuge in city or build citadel there.
+    fun automateGreatGeneralFallback(unit: MapUnit) {
+        // if no unit to follow, take refuge in city or build citadel there.
         val reachableTest: (TileInfo) -> Boolean = {
             it.civilianUnit == null &&
                     unit.movement.canMoveTo(it)
@@ -112,22 +104,26 @@ object SpecificUnitAutomation {
         val cityToGarrison = unit.civInfo.cities.asSequence().map { it.getCenterTile() }
                 .sortedBy { it.aerialDistanceTo(unit.currentTile) }
                 .firstOrNull { reachableTest(it) }
-
+            ?: return
-        if (cityToGarrison != null) {
+        if (!unit.hasCitadelPlacementUnique) {
-            // try to find a good place for citadel nearby
+            unit.movement.headTowards(cityToGarrison)
-            val potentialTilesNearCity = cityToGarrison.getTilesInDistanceRange(3..4)
-            val tileForCitadel = potentialTilesNearCity.firstOrNull {
-                reachableTest(it) &&
-                        WorkerAutomation.evaluateFortPlacement(it, unit.civInfo, true)
-            }
-            if (tileForCitadel != null) {
-                unit.movement.headTowards(tileForCitadel)
-                if (unit.currentMovement > 0 && unit.currentTile == tileForCitadel)
-                    UnitActions.getImprovementConstructionActions(unit, unit.currentTile).firstOrNull()?.action?.invoke()
-            } else
-                unit.movement.headTowards(cityToGarrison)
             return
         }
+
+        // try to find a good place for citadel nearby
+        val tileForCitadel = cityToGarrison.getTilesInDistanceRange(3..4)
+            .firstOrNull {
+                reachableTest(it) &&
+                WorkerAutomation.evaluateFortPlacement(it, unit.civInfo, true)
+            }
+        if (tileForCitadel == null) {
+            unit.movement.headTowards(cityToGarrison)
+            return
+        }
+        unit.movement.headTowards(tileForCitadel)
+        if (unit.currentMovement > 0 && unit.currentTile == tileForCitadel)
+            UnitActions.getImprovementConstructionActions(unit, unit.currentTile)
+                .firstOrNull()?.action?.invoke()
     }
 
     private fun rankTileAsCityCenter(tileInfo: TileInfo, nearbyTileRankings: Map<TileInfo, Float>,

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

@@ -157,9 +157,15 @@ object UnitAutomation {
             // For now its a simple option to allow AI to win a science victory again
             if (unit.hasUnique(UniqueType.AddInCapital))
                 return SpecificUnitAutomation.automateAddInCapital(unit)
-            
+
-            if (unit.hasUnique("Bonus for units in 2 tile radius 15%"))
+            //todo this now supports "Great General"-like mod units not combining 'aura' and citadel
-                return SpecificUnitAutomation.automateGreatGeneral(unit)
+            // abilities, but not additional capabilities if automation finds no use for those two
+            if (unit.hasStrengthBonusInRadiusUnique && SpecificUnitAutomation.automateGreatGeneral(unit))
+                return
+            if (unit.hasCitadelPlacementUnique && SpecificUnitAutomation.automateCitadelPlacer(unit))
+                return
+            if (unit.hasCitadelPlacementUnique || unit.hasStrengthBonusInRadiusUnique)
+                return SpecificUnitAutomation.automateGreatGeneralFallback(unit)
 
             if (unit.hasUnique(UniqueType.ConstructImprovementConsumingUnit))
                 return SpecificUnitAutomation.automateImprovementPlacer(unit) // includes great people plus moddable units

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

@@ -40,10 +40,9 @@ object BattleDamage {
 
         val conditionalState = StateForConditionals(civInfo, cityInfo = (combatant as? CityCombatant)?.city, ourCombatant = combatant, theirCombatant = enemy,
             attackedTile = attackedTile, combatAction = combatAction)
-        
+
-        
         if (combatant is MapUnitCombatant) {
-            
+
             for (unique in combatant.getMatchingUniques(UniqueType.Strength, conditionalState, true)) {
                 modifiers.add(getModifierStringFromUnique(unique), unique.params[0].toInt())
             }
@@ -61,9 +60,7 @@ object BattleDamage {
 
             //https://www.carlsguides.com/strategy/civilization5/war/combatbonuses.php
             val adjacentUnits = combatant.getTile().neighbors.flatMap { it.getUnits() }
-
+            for (unique in adjacentUnits.filter { it.civInfo.isAtWarWith(civInfo) }
-
-            for (unique in adjacentUnits.filter { it.civInfo.isAtWarWith(combatant.getCivInfo()) }
                     .flatMap { it.getMatchingUniques(UniqueType.StrengthForAdjacentEnemies) })
                 if (combatant.matchesCategory(unique.params[1]) && combatant.getTile()
                         .matchesFilter(unique.params[2])
@@ -73,17 +70,11 @@ object BattleDamage {
             val civResources = civInfo.getCivResourcesByName()
             for (resource in combatant.unit.baseUnit.getResourceRequirements().keys)
                 if (civResources[resource]!! < 0 && !civInfo.isBarbarian())
-                    modifiers["Missing resource"] = -25
+                    modifiers["Missing resource"] = -25  //todo ModConstants
 
-
+            val (greatGeneralName, greatGeneralBonus) = GreatGeneralImplementation.getGreatGeneralBonus(combatant.unit)
-            val nearbyCivUnits = combatant.unit.getTile().getTilesInDistance(2)
+            if (greatGeneralBonus != 0)
-                .flatMap { it.getUnits() }.filter { it.civInfo == combatant.unit.civInfo }
+                modifiers[greatGeneralName] = greatGeneralBonus
-            if (nearbyCivUnits.any { it.hasUnique("Bonus for units in 2 tile radius 15%") }) {
-                val greatGeneralModifier =
-                    if (combatant.unit.civInfo.hasUnique(UniqueType.GreatGeneralProvidesDoubleCombatBonus)) 30 else 15
-
-                modifiers["Great General"] = greatGeneralModifier
-            }
 
             for (unique in combatant.unit.getMatchingUniques(UniqueType.StrengthWhenStacked)) {
                 var stackedUnitsBonus = 0

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

@@ -0,0 +1,98 @@
+package com.unciv.logic.battle
+
+import com.unciv.logic.map.MapUnit
+import com.unciv.logic.map.TileInfo
+import com.unciv.models.ruleset.unique.Unique
+import com.unciv.models.ruleset.unique.UniqueType
+import com.unciv.logic.automation.SpecificUnitAutomation  // for Kdoc
+
+
+object GreatGeneralImplementation {
+
+    private data class GeneralBonusData(val general: MapUnit, val radius: Int, val filter: String, val bonus: Int) {
+        constructor(general: MapUnit, unique: Unique) : this(
+            general,
+            radius = unique.params[2].toIntOrNull() ?: 0,
+            filter = unique.params[1],
+            bonus = unique.params[0].toIntOrNull() ?: 0
+        )
+    }
+
+    /**
+     * Determine the "Great General" bonus for [unit] by searching for units carrying the [UniqueType.StrengthBonusInRadius] in the vicinity.
+     *
+     * Used by [BattleDamage.getGeneralModifiers].
+     *
+     * @return A pair of unit's name and bonus (percentage) as Int (typically 15), or 0 if no applicable Great General equivalents found
+     */
+    fun getGreatGeneralBonus(unit: MapUnit): Pair<String, Int> {
+        val civInfo = unit.civInfo
+        val allGenerals = civInfo.getCivUnits()
+            .filter { it.hasStrengthBonusInRadiusUnique }
+        if (allGenerals.none()) return Pair("", 0)
+
+        val greatGeneral = allGenerals
+            .flatMap { general ->
+                general.getMatchingUniques(UniqueType.StrengthBonusInRadius)
+                    .map { GeneralBonusData(general, it) }
+            }.filter {
+                // Support the border case when a mod unit has several
+                // GreatGeneralAura uniques (e.g. +50% as radius 1, +25% at radius 2, +5% at radius 3)
+                // The "Military" test is also supported deep down in unit.matchesFilter, a small
+                // optimization for the most common case, as this function is only called for `MapUnitCombatant`s
+                it.general.currentTile.aerialDistanceTo(unit.getTile()) <= it.radius
+                        && (it.filter == "Military" || unit.matchesFilter(it.filter))
+            }
+        val greatGeneralModifier = greatGeneral.maxByOrNull { it.bonus } ?: return Pair("",0)
+
+        if (unit.hasUnique(UniqueType.GreatGeneralProvidesDoubleCombatBonus, checkCivInfoUniques = true)
+            && greatGeneralModifier.general.isGreatPersonOfType("War")) // apply only on "true" generals
+            return Pair(greatGeneralModifier.general.name, greatGeneralModifier.bonus * 2)
+        return Pair(greatGeneralModifier.general.name, greatGeneralModifier.bonus)
+    }
+
+    /**
+     * Find a tile for accompanying a military unit where the total bonus for all affected units is maximized.
+     *
+     * Used by [SpecificUnitAutomation.automateGreatGeneral].
+     */
+    fun getBestAffectedTroopsTile(general: MapUnit): TileInfo? {
+        // Normally we have only one Unique here. But a mix is not forbidden, so let's try to support mad modders.
+        // (imagine several GreatGeneralAura uniques - +50% at radius 1, +25% at radius 2, +5% at radius 3 - possibly learnable from promotions via buildings or natural wonders?)
+
+        // Map out the uniques sorted by bonus, as later only the best bonus will apply.
+        val generalBonusData = (
+                general.getMatchingUniques(UniqueType.StrengthBonusInRadius).map { GeneralBonusData(general, it) }
+            ).sortedWith(compareByDescending<GeneralBonusData> { it.bonus }.thenBy { it.radius })
+            .toList()
+
+        // Get candidate units to 'follow', coarsely.
+        // The mapUnitFilter of the unique won't apply here but in the ranking of the "Aura" effectiveness.
+        val unitMaxMovement = general.getMaxMovement()
+        val militaryUnitTilesInDistance = general.movement.getDistanceToTiles().asSequence()
+            .map { it.key }
+            .filter { tile ->
+                val militaryUnit = tile.militaryUnit
+                militaryUnit != null && militaryUnit.civInfo == general.civInfo
+                        && (tile.civilianUnit == null || tile.civilianUnit == general)
+                        && militaryUnit.getMaxMovement() <= unitMaxMovement
+                        && !tile.isCityCenter()
+            }
+
+        // rank tiles and find best 
+        val unitBonusRadius = generalBonusData.maxOfOrNull { it.radius }
+            ?: return null
+        return militaryUnitTilesInDistance
+            .maxByOrNull { unitTile ->
+                unitTile.getTilesInDistance(unitBonusRadius).sumOf { auraTile ->
+                    val militaryUnit = auraTile.militaryUnit
+                    if (militaryUnit == null || militaryUnit.civInfo != general.civInfo) 0
+                    else generalBonusData.firstOrNull {
+                        // "Military" as commented above only a small optimization
+                        auraTile.aerialDistanceTo(unitTile) <= it.radius
+                                && (it.filter == "Military" || militaryUnit.matchesFilter(it.filter))
+                    }?.bonus ?: 0
+                }
+            }
+    }
+}

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

@@ -1,15 +1,9 @@
 package com.unciv.logic.civilization
 
 import com.badlogic.gdx.math.Vector2
-import com.badlogic.gdx.utils.Json
-import com.badlogic.gdx.utils.Json.Serializer
-import com.badlogic.gdx.utils.JsonValue
 import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.json.HashMapVector2
-import com.unciv.json.json
-import com.unciv.logic.BarbarianManager
-import com.unciv.logic.Encampment
 import com.unciv.logic.GameInfo
 import com.unciv.logic.UncivShowableException
 import com.unciv.logic.automation.NextTurnAutomation
@@ -123,7 +117,7 @@ class CivilizationInfo {
 
     @Transient
     val lastEraResourceUsedForUnit = HashMap<String, Int>()
-    
+
     @Transient
     var thingsToFocusOnForVictory = setOf<Victory.Focus>()
 
@@ -757,7 +751,7 @@ class CivilizationInfo {
         goldenAges.civInfo = this
 
         civConstructions.setTransients(civInfo = this)
-        
+
         policies.civInfo = this
         if (policies.adoptedPolicies.size > 0 && policies.numberOfAdoptedPolicies == 0)
             policies.numberOfAdoptedPolicies = policies.adoptedPolicies.count { !Policy.isBranchCompleteByName(it) }
@@ -776,14 +770,14 @@ class CivilizationInfo {
         tech.setTransients()
 
         ruinsManager.setTransients(this)
-        
+
         for (diplomacyManager in diplomacy.values) {
             diplomacyManager.civInfo = this
             diplomacyManager.updateHasOpenBorders()
         }
 
         victoryManager.civInfo = this
-        
+
         thingsToFocusOnForVictory = getPreferredVictoryTypeObject()?.getThingsToFocus(this) ?: setOf()
 
         for (cityInfo in cities) {
@@ -811,6 +805,7 @@ class CivilizationInfo {
         }
 
         hasLongCountDisplayUnique = hasUnique(UniqueType.MayanCalendarDisplay)
+
     }
 
     fun updateSightAndResources() {

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

@@ -23,7 +23,6 @@ import com.unciv.models.ruleset.unit.UnitType
 import com.unciv.models.stats.Stats
 import com.unciv.ui.utils.filterAndLogic
 import com.unciv.ui.utils.toPercent
-import com.unciv.ui.worldscreen.unit.UnitActions
 import java.text.DecimalFormat
 import kotlin.math.pow
 
@@ -120,6 +119,11 @@ class MapUnit {
     @Transient
     var hasUniqueToBuildImprovements = false    // not canBuildImprovements to avoid confusion
 
+    @Transient
+    var hasStrengthBonusInRadiusUnique = false
+    @Transient
+    var hasCitadelPlacementUnique = false
+
     /** civName owning the unit */
     lateinit var owner: String
 
@@ -340,6 +344,11 @@ class MapUnit {
         hasUniqueToBuildImprovements = hasUnique(UniqueType.BuildImprovements)
         canEnterForeignTerrain = hasUnique(UniqueType.CanEnterForeignTiles)
             || hasUnique(UniqueType.CanEnterForeignTilesButLosesReligiousStrength)
+
+        hasStrengthBonusInRadiusUnique = hasUnique(UniqueType.StrengthBonusInRadius)
+        hasCitadelPlacementUnique = getMatchingUniques(UniqueType.ConstructImprovementConsumingUnit)
+            .mapNotNull { civInfo.gameInfo.ruleSet.tileImprovements[it.params[0]] }
+            .any { it.hasUnique(UniqueType.TakeOverTilesAroundWhenBuilt) }
     }
 
     fun copyStatisticsTo(newUnit: MapUnit) {
@@ -568,6 +577,7 @@ class MapUnit {
     fun canGarrison() = isMilitary() && baseUnit.isLandUnit()
 
     fun isGreatPerson() = baseUnit.isGreatPerson()
+    fun isGreatPersonOfType(type: String) = baseUnit.isGreatPersonOfType(type)
 
     //endregion
 

core/src/com/unciv/logic/map/UnitPromotions.kt

@@ -73,7 +73,7 @@ class UnitPromotions {
         // If we upgrade this unit to its new version, we already need to have this promotion added,
         // so this has to go after the `promotions.add(promotionname)` line.
         doDirectPromotionEffects(promotion)
-        
+
         unit.updateUniques(ruleset)
 
         // Since some units get promotions upon construction, they will get the addPromotion from the unit.postBuildEvent

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

@@ -632,7 +632,7 @@ class Ruleset {
                 if (tileImprovements[improvementName]==null) continue // this will be caught in the checkUniques
                 if ((tileImprovements[improvementName] as Stats).none() &&
                         unit.isCivilian() &&
-                        !unit.hasUnique("Bonus for units in 2 tile radius 15%")) {
+                        !unit.isGreatPersonOfType("War")) {
                     lines.add("${unit.name} can place improvement $improvementName which has no stats, preventing unit automation!",
                         RulesetErrorSeverity.Warning)
                 }

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

@@ -232,7 +232,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
     UnitsFightFullStrengthWhenDamaged("Units fight as though they were at full strength even when damaged", UniqueTarget.Global),
     GoldWhenDiscoveringNaturalWonder("100 Gold for discovering a Natural Wonder (bonus enhanced to 500 Gold if first to discover it)", UniqueTarget.Global),
     UnhappinessFromCitiesDoubled("Unhappiness from number of Cities doubled", UniqueTarget.Global),
-    GreatGeneralProvidesDoubleCombatBonus("Great General provides double combat bonus", UniqueTarget.Global),
+    GreatGeneralProvidesDoubleCombatBonus("Great General provides double combat bonus", UniqueTarget.Unit, UniqueTarget.Global),
     TechBoostWhenScientificBuildingsBuiltInCapital("Receive a tech boost when scientific buildings/wonders are built in capital", UniqueTarget.Global),
     MayNotGenerateGreatProphet("May not generate great prophet equivalents naturally", UniqueTarget.Global),
     @Deprecated("as of 4.0.3", ReplaceWith("When conquering an encampment, earn [25] Gold and recruit a Barbarian unit <with [67]% chance>"))
@@ -413,6 +413,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
     StrengthBonusVsCityStates("+30% Strength when fighting City-State units and cities", UniqueTarget.Global),
     StrengthForAdjacentEnemies("[relativeAmount]% Strength for enemy [combatantFilter] units in adjacent [tileFilter] tiles", UniqueTarget.Unit),
     StrengthWhenStacked("[relativeAmount]% Strength when stacked with [mapUnitFilter]", UniqueTarget.Unit),  // candidate for conditional!
+    StrengthBonusInRadius("[relativeAmount]% Strength bonus for [mapUnitFilter] units within [amount] tiles", UniqueTarget.Unit),
 
     AdditionalAttacks("[amount] additional attacks per turn", UniqueTarget.Unit, UniqueTarget.Global),
     Movement("[amount] Movement", UniqueTarget.Unit, UniqueTarget.Global),
@@ -529,8 +530,6 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
     RemoveOtherReligions("Removes other religions when spreading religion", UniqueTarget.Unit),
 
     CanActionSeveralTimes("Can [action] [amount] times", UniqueTarget.Unit),
-    // TODO needs to be more general
-    BonusForUnitsInRadius("Bonus for units in 2 tile radius 15%", UniqueTarget.Unit),
 
     CanSpeedupConstruction("Can speed up construction of a building", UniqueTarget.Unit),
     CanHurryResearch("Can hurry technology research", UniqueTarget.Unit),
@@ -761,6 +760,8 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
 
     // region DEPRECATED AND REMOVED
 
+    @Deprecated("as of 4.1.0", ReplaceWith("[+15]% Strength bonus for [Military] units within [2] tiles"), DeprecationLevel.ERROR)
+    BonusForUnitsInRadius("Bonus for units in 2 tile radius 15%", UniqueTarget.Unit),
     @Deprecated("as of 4.0.15", ReplaceWith("Irremovable"), DeprecationLevel.ERROR)
     Indestructible("Indestructible", UniqueTarget.Improvement),
     

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

@@ -577,7 +577,8 @@ class BaseUnit : RulesetObject(), INonPerpetualConstruction {
         }
     }
 
-    fun isGreatPerson() = hasUnique(UniqueType.GreatPerson)
+    fun isGreatPerson() = getMatchingUniques(UniqueType.GreatPerson).any()
+    fun isGreatPersonOfType(type: String) = getMatchingUniques(UniqueType.GreatPerson).any { it.params[0] == type }
 
     fun isNuclearWeapon() = hasUnique(UniqueType.NuclearWeapon)
 

docs/Modders/uniques.md

@@ -458,7 +458,7 @@ Simple unique parameters are explained by mouseover. Complex parameters are expl
 	Applicable to: Global
 
 ??? example  "Great General provides double combat bonus"
-	Applicable to: Global
+	Applicable to: Global, Unit
 
 ??? example  "Receive a tech boost when scientific buildings/wonders are built in capital"
 	Applicable to: Global
@@ -998,6 +998,11 @@ Simple unique parameters are explained by mouseover. Complex parameters are expl
 
 	Applicable to: Unit
 
+??? example  "[relativeAmount]% Strength bonus for [mapUnitFilter] units in [amount] tiles"
+	Example: "[+20]% Strength bonus for [Wounded] units in [3] tiles"
+
+	Applicable to: Unit
+
 ??? example  "May found a religion"
 	Applicable to: Unit
 
@@ -1214,9 +1219,6 @@ Simple unique parameters are explained by mouseover. Complex parameters are expl
 
 	Applicable to: Unit
 
-??? example  "Bonus for units in 2 tile radius 15%"
-	Applicable to: Unit
-
 ??? example  "Can speed up construction of a building"
 	Applicable to: Unit