Little Promotion UX improvements (#9808)

* Fix regression incorrectly allowing promotions from Unit Overview

* Autoclose PromotionPicker when all promotions chosen in one go

* Fix Unit Overview not showing new promotions when returning from PromotionPicker

core/src/com/unciv/ui/screens/overviewscreen/UnitOverviewTab.kt

@@ -247,35 +247,11 @@ class UnitOverviewTab(
 
             // Promotions column
             val promotionsTable = Table()
-            // getPromotions goes by json order on demand, so this is same sorting as on picker
+            updatePromotionsTable(promotionsTable, unit)
-            val promotions = unit.promotions.getPromotions(true)
-            if (promotions.any()) {
-                val iconCount = promotions.count() + (if (unit.promotions.canBePromoted()) 1 else 0)
-                val numberOfLines = (iconCount - 1) / 8 + 1
-                val promotionsPerLine = (iconCount - 1) / numberOfLines + 1
-                for (linePromotions in promotions.chunked(promotionsPerLine)) {
-                    for (promotion in linePromotions) {
-                        promotionsTable.add(ImageGetter.getPromotionPortrait(promotion.name))
-                    }
-                    if (linePromotions.size == promotionsPerLine) promotionsTable.row()
-                }
-            }
-
-            val canPromoteCell: Cell<Image>? =
-                if (unit.promotions.canBePromoted())
-                    promotionsTable.add(
-                        ImageGetter.getImage("OtherIcons/Star").apply {
-                            color = if (GUI.isAllowedChangeState() && unit.currentMovement > 0f && unit.attacksThisTurn == 0)
-                                    Color.GOLDENROD
-                                else Color.GOLDENROD.darken(0.25f)
-                        }
-                    ).size(24f).padLeft(8f)
-                else null
             promotionsTable.onClick {
                 if (unit.promotions.canBePromoted() || unit.promotions.promotions.isNotEmpty()) {
                     game.pushScreen(PromotionPickerScreen(unit) {
-                        if (canPromoteCell != null && !unit.promotions.canBePromoted())
+                        updatePromotionsTable(promotionsTable, unit)
-                            canPromoteCell.size(0f).pad(0f).setActor(null)
                     })
                 }
             }
@@ -307,6 +283,35 @@ class UnitOverviewTab(
         return this
     }
 
+    private fun updatePromotionsTable(table: Table, unit: MapUnit) {
+        table.clearChildren()
+
+        // getPromotions goes by json order on demand - so this is the same sorting as on UnitTable,
+        // but not same as on PromotionPickerScreen (which e.g. tries to respect prerequisite proximity)
+        val promotions = unit.promotions.getPromotions(true)
+        val showPromoteStar = unit.promotions.canBePromoted()
+        if (promotions.any()) {
+            val iconCount = promotions.count() + (if (showPromoteStar) 1 else 0)
+            val numberOfLines = (iconCount - 1) / 8 + 1  // Int math: -1,/,+1 means divide rounding *up*
+            val promotionsPerLine = (iconCount - 1) / numberOfLines + 1
+            for (linePromotions in promotions.chunked(promotionsPerLine)) {
+                for (promotion in linePromotions) {
+                    table.add(ImageGetter.getPromotionPortrait(promotion.name))
+                }
+                if (linePromotions.size == promotionsPerLine) table.row()
+            }
+        }
+
+        if (!showPromoteStar) return
+        table.add(
+            ImageGetter.getImage("OtherIcons/Star").apply {
+                color = if (GUI.isAllowedChangeState() && unit.currentMovement > 0f && unit.attacksThisTurn == 0)
+                    Color.GOLDENROD
+                else Color.GOLDENROD.darken(0.25f)
+            }
+        ).size(24f).padLeft(8f)
+    }
+
     companion object {
         fun getUnitIdentifier(unit: MapUnit, unitToUpgradeTo: BaseUnit? = null): String {
             val name = unitToUpgradeTo?.name ?: unit.name

core/src/com/unciv/ui/screens/pickerscreens/PromotionPickerScreen.kt

@@ -25,6 +25,7 @@ import kotlin.math.abs
 
 class PromotionPickerScreen(
     val unit: MapUnit,
+    private val closeOnPick: Boolean = true,
     private val onChange: (() -> Unit)? = null
 ) : PickerScreen(), RecreateOnResize {
     // Style stuff
@@ -43,8 +44,8 @@ class PromotionPickerScreen(
 
     // [acceptPromotion] will [recreate] the screen, so these are constant for this picker's lifetime
     private val canChangeState = GUI.isAllowedChangeState()
-    private val canBePromoted = unit.promotions.canBePromoted()
+    private val canPromoteNow = canChangeState &&
-    private val canPromoteNow = canChangeState && canBePromoted &&
+            unit.promotions.canBePromoted() &&
             unit.currentMovement > 0 && unit.attacksThisTurn == 0
 
     // Logic
@@ -100,7 +101,11 @@ class PromotionPickerScreen(
             unit.promotions.addPromotion(promotion.name)
 
         onChange?.invoke()
-        game.replaceCurrentScreen(recreate())
+
+        if (!closeOnPick || unit.promotions.canBePromoted())
+            game.replaceCurrentScreen(recreate(false))
+        else
+            game.popScreen()
     }
 
     private fun fillTable() {
@@ -170,7 +175,9 @@ class PromotionPickerScreen(
     }
 
     private fun getButton(tree: PromotionTree, node: PromotionTree.PromotionNode) : PromotionButton {
-        val isPickable = (!node.pathIsAmbiguous || node.distanceToAdopted == 1) && tree.canBuyUpTo(node.promotion)
+        val isPickable = canPromoteNow &&
+            (!node.pathIsAmbiguous || node.distanceToAdopted == 1) &&
+            tree.canBuyUpTo(node.promotion)
 
         val button = PromotionButton(node, isPickable, promotedLabelStyle, buttonCellMaxWidth - 60f)
 
@@ -319,8 +326,10 @@ class PromotionPickerScreen(
         descriptionLabel.setText("$topLine\n$promotionText")
     }
 
-    override fun recreate(): BaseScreen {
+    override fun recreate() = recreate(closeOnPick)
-        val newScreen = PromotionPickerScreen(unit, onChange)
+
+    fun recreate(closeOnPick: Boolean): BaseScreen {
+        val newScreen = PromotionPickerScreen(unit, closeOnPick, onChange)
         newScreen.setScrollY(scrollPane.scrollY)
         return newScreen
     }