Fix Pixel unit nation colors after combat (#7033)

* Fix Pixel Unit national colors reverting to white after battle

* A little linting on top

A little linting on top

core/src/com/unciv/ui/tilegroups/TileGroup.kt

@@ -34,7 +34,7 @@ open class TileGroup(
     /*
         Layers (reordered in TileGroupMap):
         Base image (+ overlay)
-        Terrain Feature overlay (including roads and pixel units)
+        Terrain Feature overlay (including roads)
         Misc: improvements, resources, yields, worked, resources, border, arrows, and starting locations in editor
         Pixel Units
         Highlight, Fog, Crosshair layer (in that order)

core/src/com/unciv/ui/worldscreen/bottombar/BattleTable.kt

@@ -1,13 +1,7 @@
 package com.unciv.ui.worldscreen.bottombar
 
 import com.badlogic.gdx.graphics.Color
-import com.badlogic.gdx.math.Interpolation
-import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.Touchable
-import com.badlogic.gdx.scenes.scene2d.actions.Actions
-import com.badlogic.gdx.scenes.scene2d.actions.FloatAction
-import com.badlogic.gdx.scenes.scene2d.actions.RepeatAction
-import com.badlogic.gdx.scenes.scene2d.ui.Image
 import com.badlogic.gdx.scenes.scene2d.ui.Label
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.UncivGame
@@ -23,6 +17,8 @@ import com.unciv.ui.audio.Sounds
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.utils.*
 import com.unciv.ui.worldscreen.WorldScreen
+import com.unciv.ui.worldscreen.bottombar.BattleTableHelpers.flashWoundedCombatants
+import com.unciv.ui.worldscreen.bottombar.BattleTableHelpers.getHealthBar
 import kotlin.math.max
 
 class BattleTable(val worldScreen: WorldScreen): Table() {
@@ -30,7 +26,7 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
     init {
         isVisible = false
         skin = BaseScreen.skin
-        background = ImageGetter.getBackground(ImageGetter.getBlue().apply { a=0.8f })
+        background = ImageGetter.getBackground(ImageGetter.getBlue().apply { a = 0.8f })
 
         defaults().pad(5f)
         pad(5f)
@@ -132,114 +128,94 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
         add(defender.getDefendingStrength(attacker.isRanged()).toString() + defenceIcon).row()
 
 
-        val quarterScreen = worldScreen.stage.width/4
+        val quarterScreen = worldScreen.stage.width / 4
 
+        fun getModifierTable(key: String, value: Int) = Table().apply {
+            val description = if (key.startsWith("vs "))
+                ("vs [" + key.drop(3) + "]").tr()
+                else key.tr()
+            val percentage = (if (value > 0) "+" else "") + value + "%"
+            val upOrDownLabel = if (value > 0f) "⬆".toLabel(Color.GREEN)
+                else "⬇".toLabel(Color.RED)
+
+            add(upOrDownLabel)
+            val modifierLabel = "$percentage $description".toLabel(fontSize = 14).apply { wrap = true }
+            add(modifierLabel).width(quarterScreen - upOrDownLabel.minWidth)
+        }
         val attackerModifiers =
                 BattleDamage.getAttackModifiers(attacker, defender).map {
-                    val description = if (it.key.startsWith("vs "))
+                    getModifierTable(it.key, it.value)
-                        ("vs [" + it.key.replace("vs ", "") + "]").tr()
-                    else it.key.tr()
-                    val percentage = (if (it.value > 0) "+" else "") + it.value + "%"
-
-                    val upOrDownLabel = if (it.value > 0f) "⬆".toLabel(Color.GREEN) else "⬇".toLabel(
-                        Color.RED)
-                    val modifierTable = Table()
-                    modifierTable.add(upOrDownLabel)
-                    val modifierLabel = "$percentage $description".toLabel(fontSize = 14).apply { wrap=true }
-                    modifierTable.add(modifierLabel).width(quarterScreen)
-                    modifierTable
                 }
         val defenderModifiers =
                 if (defender is MapUnitCombatant)
                     BattleDamage.getDefenceModifiers(attacker, defender).map {
-                        val description = if(it.key.startsWith("vs ")) ("vs ["+it.key.replace("vs ","")+"]").tr() else it.key.tr()
+                        getModifierTable(it.key, it.value)
-                        val percentage = (if(it.value>0)"+" else "")+ it.value +"%"
-                        val upOrDownLabel = if (it.value > 0f) "⬆".toLabel(Color.GREEN) else "⬇".toLabel(
-                            Color.RED)
-                        val modifierTable = Table()
-                        modifierTable.add(upOrDownLabel)
-                        val modifierLabel = "$percentage $description".toLabel(fontSize = 14).apply { wrap=true }
-                        modifierTable.add(modifierLabel).width(quarterScreen)
-                        modifierTable
                     }
                 else listOf()
 
-        for (i in 0..max(attackerModifiers.size,defenderModifiers.size)) {
+        for (i in 0..max(attackerModifiers.size, defenderModifiers.size)) {
-            if (attackerModifiers.size > i)
+            if (i < attackerModifiers.size) add(attackerModifiers[i]) else add()
-                add(attackerModifiers[i])
+            if (i < defenderModifiers.size) add(defenderModifiers[i]) else add()
-            else add().width(quarterScreen)
-            if (defenderModifiers.size > i)
-                add(defenderModifiers[i])
-            else add().width(quarterScreen)
             row().pad(2f)
         }
 
         // from Battle.addXp(), check for can't gain more XP from Barbarians
-        val modConstants = attacker.getCivInfo().gameInfo.ruleSet.modOptions.constants
+        val maxXPFromBarbarians = attacker.getCivInfo().gameInfo.ruleSet.modOptions.constants.maxXPfromBarbarians
-        if (attacker is MapUnitCombatant && attacker.unit.promotions.totalXpProduced() >= modConstants.maxXPfromBarbarians
+        if (attacker is MapUnitCombatant && attacker.unit.promotions.totalXpProduced() >= maxXPFromBarbarians
                 && defender.getCivInfo().isBarbarian()){
-            add("Cannot gain more XP from Barbarians".tr().toLabel(fontSize = 16).apply { wrap=true }).width(quarterScreen)
+            add("Cannot gain more XP from Barbarians".toLabel(fontSize = 16).apply { wrap = true }).width(quarterScreen)
             row()
         }
 
         var damageToDefender = BattleDamage.calculateDamageToDefender(attacker, defender, true)
         var damageToAttacker = BattleDamage.calculateDamageToAttacker(attacker, defender, true)
 
-
+        if (damageToAttacker > attacker.getHealth() && damageToDefender > defender.getHealth()) {
-        if (damageToAttacker>attacker.getHealth() && damageToDefender>defender.getHealth()) // when damage exceeds health, we don't want to show negative health numbers
+            // when damage exceeds health, we don't want to show negative health numbers
-        // Also if both parties are supposed to die it's not indicative of who is more likely to win
+            // Also if both parties are supposed to die it's not indicative of who is more likely to win
-        // So we "normalize" the damages until one dies
+            // So we "normalize" the damages until one dies
-        {
+            if (damageToDefender * attacker.getHealth() > damageToAttacker * defender.getHealth()) { // defender dies quicker ie first
-            if(damageToDefender/defender.getHealth().toFloat() > damageToAttacker/attacker.getHealth().toFloat()) // defender dies quicker ie first
-            {
                 // Both damages *= (defender.health/damageToDefender)
                 damageToDefender = defender.getHealth()
-                damageToAttacker *= (defender.getHealth()/damageToDefender.toFloat()).toInt()
+                damageToAttacker *= (defender.getHealth() / damageToDefender.toFloat()).toInt()
-            } else{ // attacker dies first
+            } else { // attacker dies first
                 // Both damages *= (attacker.health/damageToAttacker)
                 damageToAttacker = attacker.getHealth()
-                damageToDefender *= (attacker.getHealth()/damageToAttacker.toFloat()).toInt()
+                damageToDefender *= (attacker.getHealth() / damageToAttacker.toFloat()).toInt()
             }
         }
-        else if (damageToAttacker>attacker.getHealth()) damageToAttacker=attacker.getHealth()
+        else if (damageToAttacker > attacker.getHealth()) damageToAttacker = attacker.getHealth()
-        else if (damageToDefender>defender.getHealth()) damageToDefender=defender.getHealth()
+        else if (damageToDefender > defender.getHealth()) damageToDefender = defender.getHealth()
 
-
+        if (attacker.isMelee() &&
-        if(attacker.isMelee() && (defender.isCivilian()
+                (defender.isCivilian() || defender is CityCombatant && defender.isDefeated())) {
-                        || defender is CityCombatant && defender.isDefeated())) {
+            add()
-            add("")
+            val defeatedText = when {
-            add(
+                !defender.isCivilian() -> "Occupied!"
-                if (defender.isCivilian()
+                (defender as MapUnitCombatant).unit.hasUnique(UniqueType.Uncapturable) -> ""
-                    && (defender as MapUnitCombatant).unit.hasUnique(UniqueType.Uncapturable)
+                else -> "Captured!"
-                ) ""
+            }
-                else if (defender.isCivilian()) "Captured!".tr()
+            add(defeatedText.toLabel())
-                else "Occupied!".tr()
+        } else {
-            )
-        }
-
-
-        else {
             add(getHealthBar(attacker.getHealth(), attacker.getMaxHealth(), damageToAttacker))
             add(getHealthBar(defender.getHealth(), defender.getMaxHealth(), damageToDefender))
         }
 
         row().pad(5f)
-        val attackText : String = when (attacker) {
+        val attackText: String = when (attacker) {
             is CityCombatant -> "Bombard"
             else -> "Attack"
         }
         val attackButton = attackText.toTextButton().apply { color= Color.RED }
 
-        var attackableTile : AttackableTile? = null
+        var attackableTile: AttackableTile? = null
 
         if (attacker.canAttack()) {
             if (attacker is MapUnitCombatant) {
                 attackableTile = BattleHelper
                         .getAttackableEnemies(attacker.unit, attacker.unit.movement.getDistanceToTiles())
                         .firstOrNull{ it.tileToAttack == defender.getTile()}
-            }
+            } else if (attacker is CityCombatant) {
-            else if (attacker is CityCombatant)
-            {
                 val canBombard = UnitAutomation.getBombardTargets(attacker.city).contains(defender.getTile())
                 if (canBombard) {
                     attackableTile = AttackableTile(attacker.getTile(), defender.getTile(), 0f)
@@ -250,9 +226,7 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
         if (!worldScreen.isPlayersTurn || attackableTile == null) {
             attackButton.disable()
             attackButton.label.color = Color.GRAY
-        }
+        } else {
-
-        else {
             attackButton.onClick(UncivSound.Silent) {  // onAttackButtonClicked will do the sound
                 onAttackButtonClicked(attacker, defender, attackableTile, damageToAttacker, damageToDefender)
             }
@@ -262,7 +236,7 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
 
         pack()
 
-        setPosition(worldScreen.stage.width/2-width/2, 5f)
+        setPosition(worldScreen.stage.width / 2 - width / 2, 5f)
     }
 
     private fun onAttackButtonClicked(
@@ -283,46 +257,9 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
         Sounds.play(attacker.getAttackSound())
         Battle.attackOrNuke(attacker, attackableTile)
 
-        val actorsToFlashRed = arrayListOf<Actor>()
+        worldScreen.flashWoundedCombatants(attacker, damageToAttacker, defender, damageToDefender)
-
-        if (damageToDefender != 0)
-            actorsToFlashRed.addAll(getMapActorsForCombatant(defender))
-        if (damageToAttacker != 0)
-            actorsToFlashRed.addAll(getMapActorsForCombatant(attacker))
-        fun updateRedPercent(percent: Float) {
-            for (actor in actorsToFlashRed)
-                actor.color = Color.WHITE.cpy().lerp(Color.RED, percent)
-        }
-        worldScreen.stage.addAction(Actions.sequence(
-            object : FloatAction(0f, 1f, 0.3f, Interpolation.sine) {
-                override fun update(percent: Float) = updateRedPercent(percent)
-            },
-            object : FloatAction(0f, 1f, 0.3f, Interpolation.sine) {
-                override fun update(percent: Float) = updateRedPercent(1 - percent)
-            }
-        ))
     }
 
-    fun getMapActorsForCombatant(combatant: ICombatant):Sequence<Actor> =
-        sequence {
-            val tilegroups =
-                worldScreen.mapHolder.tileGroups[combatant.getTile()]!!
-            when {
-                combatant.isCity() -> yieldAll(tilegroups.mapNotNull { it.icons.improvementIcon })
-                combatant.isCivilian() -> {
-                    for (tileGroup in tilegroups) {
-                        tileGroup.icons.civilianUnitIcon?.let { yield(it) }
-                        yieldAll(tileGroup.pixelCivilianUnitGroup.children)
-                    }
-                }
-                else -> {
-                    for (tileGroup in tilegroups) {
-                        tileGroup.icons.militaryUnitIcon?.let { yield(it) }
-                        yieldAll(tileGroup.pixelMilitaryUnitGroup.children)
-                    }
-                }
-            }
-        }
 
     private fun simulateNuke(attacker: MapUnitCombatant, targetTile: TileInfo){
         clear()
@@ -372,37 +309,6 @@ class BattleTable(val worldScreen: WorldScreen): Table() {
 
         pack()
 
-        setPosition(worldScreen.stage.width/2-width/2, 5f)
+        setPosition(worldScreen.stage.width / 2 - width / 2, 5f)
     }
-
-    private fun getHealthBar(currentHealth: Int, maxHealth: Int, expectedDamage:Int): Table {
-        val healthBar = Table()
-        val totalWidth = 100f
-        fun addHealthToBar(image: Image, amount:Int) {
-            val width = totalWidth * amount/maxHealth
-            healthBar.add(image).size(width.coerceIn(0f,totalWidth),3f)
-        }
-        addHealthToBar(ImageGetter.getDot(Color.BLACK), maxHealth-currentHealth)
-
-        val damagedHealth = ImageGetter.getDot(Color.FIREBRICK)
-        if (UncivGame.Current.settings.continuousRendering) {
-            damagedHealth.addAction(Actions.repeat(RepeatAction.FOREVER, Actions.sequence(
-                    Actions.color(Color.BLACK,0.7f),
-                    Actions.color(Color.FIREBRICK,0.7f)
-            ))) }
-        addHealthToBar(damagedHealth,expectedDamage)
-
-        val remainingHealth = currentHealth-expectedDamage
-        val remainingHealthDot = ImageGetter.getWhiteDot()
-        remainingHealthDot.color = when {
-            remainingHealth / maxHealth.toFloat() > 2 / 3f -> Color.GREEN
-            remainingHealth / maxHealth.toFloat() > 1 / 3f -> Color.ORANGE
-            else -> Color.RED
-        }
-        addHealthToBar(remainingHealthDot ,remainingHealth)
-
-        healthBar.pack()
-        return healthBar
-    }
-
 }

core/src/com/unciv/ui/worldscreen/bottombar/BattleTableHelpers.kt

@@ -0,0 +1,94 @@
+package com.unciv.ui.worldscreen.bottombar
+
+import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.math.Interpolation
+import com.badlogic.gdx.scenes.scene2d.Actor
+import com.badlogic.gdx.scenes.scene2d.actions.Actions
+import com.badlogic.gdx.scenes.scene2d.actions.FloatAction
+import com.badlogic.gdx.scenes.scene2d.actions.RepeatAction
+import com.badlogic.gdx.scenes.scene2d.ui.Image
+import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.unciv.UncivGame
+import com.unciv.logic.battle.ICombatant
+import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.worldscreen.WorldScreen
+
+object BattleTableHelpers {
+
+    fun WorldScreen.flashWoundedCombatants(
+        attacker: ICombatant, damageToAttacker: Int,
+        defender: ICombatant, damageToDefender: Int
+    ) {
+        fun getMapActorsForCombatant(combatant: ICombatant):Sequence<Actor> =
+                sequence {
+                    val tilegroups = mapHolder.tileGroups[combatant.getTile()]!!
+                    when {
+                        combatant.isCity() -> yieldAll(tilegroups.mapNotNull { it.icons.improvementIcon })
+                        combatant.isCivilian() -> {
+                            for (tileGroup in tilegroups) {
+                                tileGroup.icons.civilianUnitIcon?.let { yield(it) }
+                                yieldAll(tileGroup.pixelCivilianUnitGroup.children)
+                            }
+                        }
+                        else -> {
+                            for (tileGroup in tilegroups) {
+                                tileGroup.icons.militaryUnitIcon?.let { yield(it) }
+                                yieldAll(tileGroup.pixelMilitaryUnitGroup.children)
+                            }
+                        }
+                    }
+                }
+
+        val actorsToFlashRed =
+                sequence {
+                    if (damageToDefender != 0) yieldAll(getMapActorsForCombatant(defender))
+                    if (damageToAttacker != 0) yieldAll(getMapActorsForCombatant(attacker))
+                }.mapTo(arrayListOf()) { it to it.color.cpy() }
+
+        fun updateRedPercent(percent: Float) {
+            for ((actor, color) in actorsToFlashRed)
+                actor.color = color.cpy().lerp(Color.RED, percent)
+        }
+
+        stage.addAction(
+            Actions.sequence(
+            object : FloatAction(0f, 1f, 0.3f, Interpolation.sine) {
+                override fun update(percent: Float) = updateRedPercent(percent)
+            },
+            object : FloatAction(0f, 1f, 0.3f, Interpolation.sine) {
+                override fun update(percent: Float) = updateRedPercent(1 - percent)
+            }
+        ))
+    }
+
+    fun getHealthBar(currentHealth: Int, maxHealth: Int, expectedDamage: Int): Table {
+        val healthBar = Table()
+        val totalWidth = 100f
+        fun addHealthToBar(image: Image, amount:Int) {
+            val width = totalWidth * amount / maxHealth
+            healthBar.add(image).size(width.coerceIn(0f, totalWidth),3f)
+        }
+        addHealthToBar(ImageGetter.getDot(Color.BLACK), maxHealth - currentHealth)
+
+        val damagedHealth = ImageGetter.getDot(Color.FIREBRICK)
+        if (UncivGame.Current.settings.continuousRendering) {
+            damagedHealth.addAction(Actions.repeat(
+                RepeatAction.FOREVER, Actions.sequence(
+                Actions.color(Color.BLACK, 0.7f),
+                Actions.color(Color.FIREBRICK, 0.7f)
+            ))) }
+        addHealthToBar(damagedHealth,expectedDamage)
+
+        val remainingHealth = currentHealth - expectedDamage
+        val remainingHealthDot = ImageGetter.getWhiteDot()
+        remainingHealthDot.color = when {
+            remainingHealth / maxHealth.toFloat() > 2 / 3f -> Color.GREEN
+            remainingHealth / maxHealth.toFloat() > 1 / 3f -> Color.ORANGE
+            else -> Color.RED
+        }
+        addHealthToBar(remainingHealthDot ,remainingHealth)
+
+        healthBar.pack()
+        return healthBar
+    }
+}