From fcd309781df9969831d66c08ed9f41f2046cf8ac Mon Sep 17 00:00:00 2001
From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com>
Date: Wed, 31 May 2023 17:41:57 +0200
Subject: [PATCH] Upgrading from Unit overview improved (#9485)

* Unit upgrade tooltip in overview

* Unit upgrade tooltip in action table

* Unit upgrade tooltip in action table - colored Key

* Unit upgrade in Overview - reselect

* Fix merge problems and FormattedLine color markup ability

* Relax MarkupRenderer.render lines parameter type

* Skin has a getColor shortcut - use it

* Unit overview upgrade icons now open a menu instead of upgrading immediately

* Unit Overview upgrade - "Mid" buttons

* Unit Overview upgrade - reorg
---
 .../jsons/translations/template.properties    |   2 +
 core/src/com/unciv/models/UnitAction.kt       |  41 +++-
 .../com/unciv/models/ruleset/nation/Nation.kt |  41 +---
 .../unciv/ui/components/ColorMarkupLabel.kt   |  49 ++--
 .../extensions/Scene2dExtensions.kt           |   2 +-
 .../BaseUnitDescriptions.kt                   |  89 ++++++++
 .../com/unciv/ui/popups/options/AboutTab.kt   |   2 +-
 .../cityscreen/CitizenManagementTable.kt      |   4 +-
 .../ui/screens/cityscreen/CityStatsTable.kt   |   2 +-
 .../cityscreen/SpecialistAllocationTable.kt   |   4 +-
 .../civilopediascreen/FormattedLine.kt        |   9 +-
 .../civilopediascreen/MarkupRenderer.kt       |   2 +-
 .../mapeditorscreen/MapEditorToolsDrawer.kt   |   2 +-
 .../tabs/MapEditorEditSubTabs.kt              |  16 +-
 .../mapeditorscreen/tabs/MapEditorViewTab.kt  |   2 +-
 .../screens/overviewscreen/UnitOverviewTab.kt |  27 ++-
 .../screens/overviewscreen/UnitUpgradeMenu.kt | 209 ++++++++++++++++++
 .../unit/actions/UnitActionsTable.kt          |  21 +-
 .../unit/actions/UnitActionsUpgrade.kt        |  32 ++-
 docs/Modders/Creating-a-UI-skin.md            |   1 +
 docs/Other/Miscellaneous-JSON-files.md        |   3 +
 21 files changed, 462 insertions(+), 98 deletions(-)
 create mode 100644 core/src/com/unciv/ui/screens/overviewscreen/UnitUpgradeMenu.kt

diff --git a/android/assets/jsons/translations/template.properties b/android/assets/jsons/translations/template.properties
index 03aa39cc3a..a072ce77e7 100644
--- a/android/assets/jsons/translations/template.properties
+++ b/android/assets/jsons/translations/template.properties
@@ -299,6 +299,8 @@ Non-existent city =
 Lost ability = 
 National ability = 
 [firstValue] vs [secondValue] = 
+Gained = 
+Lost = 
 
 
 # New game screen
diff --git a/core/src/com/unciv/models/UnitAction.kt b/core/src/com/unciv/models/UnitAction.kt
index cfa8feaa44..1c3ef4d129 100644
--- a/core/src/com/unciv/models/UnitAction.kt
+++ b/core/src/com/unciv/models/UnitAction.kt
@@ -3,6 +3,7 @@ package com.unciv.models
 import com.badlogic.gdx.Input
 import com.badlogic.gdx.scenes.scene2d.Actor
 import com.unciv.Constants
+import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.models.translations.getPlaceholderParameters
 import com.unciv.ui.components.Fonts
 import com.unciv.ui.components.KeyCharAndCode
@@ -11,9 +12,9 @@ import com.unciv.ui.images.ImageGetter
 
 /** Unit Actions - class - carries dynamic data and actual execution.
  * Static properties are in [UnitActionType].
- * Note this is for the buttons offering actions, not the ongoing action stored with a [MapUnit][com.unciv.logic.map.MapUnit]
+ * Note this is for the buttons offering actions, not the ongoing action stored with a [MapUnit][com.unciv.logic.map.mapunit.MapUnit]
  */
-data class UnitAction(
+open class UnitAction(
     val type: UnitActionType,
     val title: String = type.value,
     val isCurrentAction: Boolean = false,
@@ -38,8 +39,44 @@ data class UnitAction(
             else -> ImageGetter.getUnitActionPortrait("Star")
         }
     }
+
+    //TODO remove once sure they're unused
+    override fun equals(other: Any?): Boolean {
+        if (this === other) return true
+        if (other !is UnitAction) return false
+
+        if (type != other.type) return false
+        if (isCurrentAction != other.isCurrentAction) return false
+        if (action != other.action) return false
+
+        return true
+    }
+
+    override fun hashCode(): Int {
+        var result = type.hashCode()
+        result = 31 * result + isCurrentAction.hashCode()
+        result = 31 * result + (action?.hashCode() ?: 0)
+        return result
+    }
+
+    override fun toString(): String {
+        return "UnitAction(type=$type, title='$title', isCurrentAction=$isCurrentAction)"
+    }
 }
 
+/** Specialized [UnitAction] for upgrades
+ *
+ *  Transports [unitToUpgradeTo] from [creation][com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsUpgrade.getUpgradeAction]
+ *  to [UI][com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsTable.update]
+ */
+class UpgradeUnitAction(
+    title: String,
+    val unitToUpgradeTo: BaseUnit,
+    val goldCostOfUpgrade: Int,
+    val newResourceRequirements: Counter<String>,
+    action: (() -> Unit)?
+) : UnitAction(UnitActionType.Upgrade, title, action = action)
+
 /** Unit Actions - generic enum with static properties
  *
  * @param value         _default_ label to display, can be overridden in UnitAction instantiation
diff --git a/core/src/com/unciv/models/ruleset/nation/Nation.kt b/core/src/com/unciv/models/ruleset/nation/Nation.kt
index 6a6b0b20e7..f2542c2781 100644
--- a/core/src/com/unciv/models/ruleset/nation/Nation.kt
+++ b/core/src/com/unciv/models/ruleset/nation/Nation.kt
@@ -13,6 +13,7 @@ import com.unciv.ui.components.Fonts
 import com.unciv.ui.components.extensions.colorFromRGB
 import com.unciv.ui.screens.civilopediascreen.CivilopediaScreen.Companion.showReligionInCivilopedia
 import com.unciv.ui.screens.civilopediascreen.FormattedLine
+import com.unciv.ui.objectdescriptions.BaseUnitDescriptions
 import kotlin.math.pow
 
 class Nation : RulesetObject() {
@@ -230,42 +231,10 @@ class Nation : RulesetObject() {
                 yield(FormattedLine("Replaces [${originalUnit.name}]", link="Unit/${originalUnit.name}", indent=1))
                 if (unit.cost != originalUnit.cost)
                     yield(FormattedLine("{Cost} ".tr() + "[${unit.cost}] vs [${originalUnit.cost}]".tr(), indent=1))
-                if (unit.strength != originalUnit.strength)
-                    yield(FormattedLine("${Fonts.strength} " + "[${unit.strength}] vs [${originalUnit.strength}]".tr(), indent=1))
-                if (unit.rangedStrength != originalUnit.rangedStrength)
-                    yield(FormattedLine("${Fonts.rangedStrength} " + "[${unit.rangedStrength}] vs [${originalUnit.rangedStrength}]".tr(), indent=1))
-                if (unit.range != originalUnit.range)
-                    yield(FormattedLine("${Fonts.range} " + "[${unit.range}] vs [${originalUnit.range}]".tr(), indent=1))
-                if (unit.movement != originalUnit.movement)
-                    yield(FormattedLine("${Fonts.movement} " + "[${unit.movement}] vs [${originalUnit.movement}]".tr(), indent=1))
-                for (resource in originalUnit.getResourceRequirementsPerTurn().keys)
-                    if (!unit.getResourceRequirementsPerTurn().containsKey(resource)) {
-                        yield(FormattedLine("[$resource] not required", link="Resource/$resource", indent=1))
-                    }
-                // This does not use the auto-linking FormattedLine(Unique) for two reasons:
-                // would look a little chaotic as unit uniques unlike most uniques are a HashSet and thus do not preserve order
-                // No .copy() factory on FormattedLine and no FormattedLine(Unique, all other val's) constructor either
-                if (unit.replacementTextForUniques.isNotEmpty()) {
-                    yield(FormattedLine(unit.replacementTextForUniques))
-                }
-                else for (unique in unit.uniqueObjects.filterNot { it.text in originalUnit.uniques || it.hasFlag(UniqueFlag.HiddenToUsers) }) {
-                    yield(FormattedLine(unique.text.tr(), indent = 1))
-                }
-                for (unique in originalUnit.uniqueObjects.filterNot { it.text in unit.uniques || it.hasFlag(UniqueFlag.HiddenToUsers) }) {
-                    yield(
-                        FormattedLine("Lost ability".tr() + " (" + "vs [${originalUnit.name}]".tr() + "): " +
-                            unique.text.tr(), indent = 1)
-                    )
-                }
-                for (promotion in unit.promotions.filter { it !in originalUnit.promotions }) {
-                    val effect = ruleset.unitPromotions[promotion]!!.uniques
-                    // "{$promotion} ({$effect})" won't work as effect may contain [] and tr() does not support that kind of nesting
-                    yield(
-                        FormattedLine(
-                        "${promotion.tr(true)} (${effect.joinToString(",") { it.tr() }})",
-                        link = "Promotion/$promotion", indent = 1 )
-                    )
-                }
+                yieldAll(
+                    BaseUnitDescriptions.getDifferences(ruleset, originalUnit, unit)
+                    .map { (text, link) -> FormattedLine(text, link = link ?: "", indent = 1) }
+                )
             } else if (unit.replaces != null) {
                 yield(FormattedLine("Replaces [${unit.replaces}], which is not found in the ruleset!", indent = 1))
             } else {
diff --git a/core/src/com/unciv/ui/components/ColorMarkupLabel.kt b/core/src/com/unciv/ui/components/ColorMarkupLabel.kt
index 72cc1dcdb2..079e39910c 100644
--- a/core/src/com/unciv/ui/components/ColorMarkupLabel.kt
+++ b/core/src/com/unciv/ui/components/ColorMarkupLabel.kt
@@ -12,7 +12,8 @@ import com.unciv.ui.screens.basescreen.BaseScreen
 
 /** A Label allowing Gdx markup
  *
- * See also [Color Markup Language](https://libgdx.com/wiki/graphics/2d/fonts/color-markup-language)
+ *  This constructor does _not_ auto-translate or otherwise preprocess [text]
+ *  See also [Color Markup Language](https://libgdx.com/wiki/graphics/2d/fonts/color-markup-language)
  */
 class ColorMarkupLabel private constructor(
     fontSize: Int,  // inverted order so it can be differentiated from the translating constructor
@@ -22,18 +23,31 @@ class ColorMarkupLabel private constructor(
     /** A Label allowing Gdx markup, auto-translated.
      *
      *  Since Gdx markup markers are interpreted and removed by translation, use «» instead.
+     *
+     *  @param defaultColor the color text starts with - will be converted to markup, not actor tint
+     *  @param hideIcons passed to translation to prevent auto-insertion of symbols for gameplay names
      */
-    constructor(text: String, fontSize: Int = Constants.defaultFontSize)
-        : this(fontSize, mapMarkup(text))
+    constructor(
+        text: String,
+        fontSize: Int = Constants.defaultFontSize,
+        defaultColor: Color = Color.WHITE,
+        hideIcons: Boolean = false
+    ) : this(fontSize, mapMarkup(text, defaultColor, hideIcons))
 
-    /** A Label automatically applying Gdx markup colors to symbols and rest of text separately
-     *  - _after_ translating [text].
+    /** A Label automatically applying Gdx markup colors to symbols and rest of text separately -
+     *  _**after**_ translating [text].
+     *
+     *  Use to easily color text without also coloring the icons which translation inserts as
+     *  characters for recognized gameplay names.
+     *
+     *  @see Fonts.charToRulesetImageActor
      */
-    constructor(text: String,
-                textColor: Color,
-                symbolColor: Color = Color.WHITE,
-                fontSize: Int = Constants.defaultFontSize)
-        : this (fontSize, prepareText(text, textColor, symbolColor))
+    constructor(
+        text: String,
+        textColor: Color,
+        symbolColor: Color = Color.WHITE,
+        fontSize: Int = Constants.defaultFontSize
+    ) : this (fontSize, prepareText(text, textColor, symbolColor))
 
     /** Only if wrap was turned on, this is the prefWidth before.
      *  Used for getMaxWidth as better estimate than the default 0. */
@@ -88,12 +102,6 @@ class ColorMarkupLabel private constructor(
     override fun getMaxWidth() = unwrappedPrefWidth  // If unwrapped, we return 0 same as super
 
     companion object {
-        private fun mapMarkup(text: String): String {
-            val translated = text.tr()
-            if ('«' !in translated) return translated
-            return translated.replace('«', '[').replace('»', ']')
-        }
-
         private val inverseColorMap = Colors.getColors().associate { it.value to it.key }
         private fun Color.toMarkup(): String {
             val mapEntry = inverseColorMap[this]
@@ -102,9 +110,16 @@ class ColorMarkupLabel private constructor(
             return "#" + toString().substring(0,6)
         }
 
+        private fun mapMarkup(text: String, defaultColor: Color, hideIcons: Boolean): String {
+            val translated = if (defaultColor == Color.WHITE) text.tr(hideIcons)
+                else "[${defaultColor.toMarkup()}]${text.tr(hideIcons)}[]"
+            if ('«' !in translated) return translated
+            return translated.replace('«', '[').replace('»', ']')
+        }
+
         private fun prepareText(text: String, textColor: Color, symbolColor: Color): String {
             val translated = text.tr()
-            if (textColor == Color.WHITE && symbolColor == Color.WHITE || translated.isBlank())
+            if ((textColor == Color.WHITE && symbolColor == Color.WHITE) || translated.isBlank())
                 return translated
             val tc = textColor.toMarkup()
             if (textColor == symbolColor)
diff --git a/core/src/com/unciv/ui/components/extensions/Scene2dExtensions.kt b/core/src/com/unciv/ui/components/extensions/Scene2dExtensions.kt
index b3024e09f5..680995ecd8 100644
--- a/core/src/com/unciv/ui/components/extensions/Scene2dExtensions.kt
+++ b/core/src/com/unciv/ui/components/extensions/Scene2dExtensions.kt
@@ -429,7 +429,7 @@ fun Group.addBorderAllowOpacity(size:Float, color: Color): Group {
 
 /** get background Image for a new separator */
 private fun getSeparatorImage(color: Color) = ImageGetter.getDot(
-    if (color.a != 0f) color else BaseScreen.skin.get("color", Color::class.java) //0x334d80
+    if (color.a != 0f) color else BaseScreen.skin.getColor("color") //0x334d80
 )
 
 /**
diff --git a/core/src/com/unciv/ui/objectdescriptions/BaseUnitDescriptions.kt b/core/src/com/unciv/ui/objectdescriptions/BaseUnitDescriptions.kt
index 7f35a06dda..ba3af8eb88 100644
--- a/core/src/com/unciv/ui/objectdescriptions/BaseUnitDescriptions.kt
+++ b/core/src/com/unciv/ui/objectdescriptions/BaseUnitDescriptions.kt
@@ -1,7 +1,13 @@
 package com.unciv.ui.objectdescriptions
 
+import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.scenes.scene2d.Touchable
+import com.badlogic.gdx.scenes.scene2d.ui.Container
+import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.scenes.scene2d.ui.WidgetGroup
 import com.unciv.logic.city.City
 import com.unciv.models.ruleset.Ruleset
+import com.unciv.models.ruleset.unique.Unique
 import com.unciv.models.ruleset.unique.UniqueFlag
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.ruleset.unit.BaseUnit
@@ -13,6 +19,8 @@ import com.unciv.ui.components.Fonts
 import com.unciv.ui.components.extensions.getConsumesAmountString
 import com.unciv.ui.components.extensions.toPercent
 import com.unciv.ui.screens.civilopediascreen.FormattedLine
+import com.unciv.ui.screens.basescreen.BaseScreen
+import com.unciv.ui.screens.civilopediascreen.MarkupRenderer
 import kotlin.math.pow
 
 object BaseUnitDescriptions {
@@ -196,6 +204,7 @@ object BaseUnitDescriptions {
         return textList
     }
 
+    @Suppress("RemoveExplicitTypeArguments")  // for faster IDE - inferring sequence types can be slow
     fun UnitType.getUnitTypeCivilopediaTextLines(ruleset: Ruleset): List<FormattedLine> {
         fun getDomainLines() = sequence<FormattedLine> {
             yield(FormattedLine("{Unit types}:", header = 4))
@@ -231,4 +240,84 @@ object BaseUnitDescriptions {
         }
         return (if (name.startsWith("Domain: ")) getDomainLines() else getUnitTypeLines()).toList()
     }
+
+    /**
+     * Lists differences e.g. for help on an upgrade, or how a nation-unique compares to its replacement.
+     *
+     * Cost is **not** included.
+     * Result lines are **not** translated.
+     *
+     * @param originalUnit The "older" unit
+     * @param betterUnit The "newer" unit
+     * @return Sequence of Pairs - first is the actual text, second is an optional link for Civilopedia use
+     */
+    fun getDifferences(ruleset: Ruleset, originalUnit: BaseUnit, betterUnit: BaseUnit):
+            Sequence<Pair<String, String?>> = sequence {
+        if (betterUnit.strength != originalUnit.strength)
+            yield("${Fonts.strength} {[${betterUnit.strength}] vs [${originalUnit.strength}]}" to null)
+
+        if (betterUnit.rangedStrength > 0 && originalUnit.rangedStrength == 0)
+            yield("[Gained] ${Fonts.rangedStrength} [${betterUnit.rangedStrength}] ${Fonts.range} [${betterUnit.range}]" to null)
+        else if (betterUnit.rangedStrength == 0 && originalUnit.rangedStrength > 0)
+            yield("[Lost] ${Fonts.rangedStrength} [${originalUnit.rangedStrength}] ${Fonts.range} [${originalUnit.range}]" to null)
+        else {
+            if (betterUnit.rangedStrength != originalUnit.rangedStrength)
+                yield("${Fonts.rangedStrength} " + "[${betterUnit.rangedStrength}] vs [${originalUnit.rangedStrength}]" to null)
+            if (betterUnit.range != originalUnit.range)
+                yield("${Fonts.range} {[${betterUnit.range}] vs [${originalUnit.range}]}" to null)
+        }
+
+        if (betterUnit.movement != originalUnit.movement)
+            yield("${Fonts.movement} {[${betterUnit.movement}] vs [${originalUnit.movement}]}" to null)
+
+        for (resource in originalUnit.getResourceRequirementsPerTurn().keys)
+            if (!betterUnit.getResourceRequirementsPerTurn().containsKey(resource)) {
+                yield("[$resource] not required" to "Resource/$resource")
+            }
+        // We return the unique text directly, so Nation.getUniqueUnitsText will not use the
+        // auto-linking FormattedLine(Unique) - two reasons in favor:
+        // would look a little chaotic as unit uniques unlike most uniques are a HashSet and thus do not preserve order
+        // No .copy() factory on FormattedLine and no (Unique, all other val's) constructor either
+        if (betterUnit.replacementTextForUniques.isNotEmpty()) {
+            yield(betterUnit.replacementTextForUniques to null)
+        } else {
+            val newAbilityPredicate: (Unique)->Boolean = { it.text in originalUnit.uniques || it.hasFlag(UniqueFlag.HiddenToUsers) }
+            for (unique in betterUnit.uniqueObjects.filterNot(newAbilityPredicate))
+                yield(unique.text to null)
+        }
+
+        val lostAbilityPredicate: (Unique)->Boolean = { it.text in betterUnit.uniques || it.hasFlag(UniqueFlag.HiddenToUsers) }
+        for (unique in originalUnit.uniqueObjects.filterNot(lostAbilityPredicate)) {
+            yield("Lost ability (vs [${originalUnit.name}]): [${unique.text}]" to null)
+        }
+        for (promotion in betterUnit.promotions.filter { it !in originalUnit.promotions }) {
+            val effects = ruleset.unitPromotions[promotion]!!.uniques
+                .joinToString(",") { "{$it}" }  // {} for individual translations, default separator would have extra blank
+            yield("{$promotion} ($effects)" to "Promotion/$promotion")
+        }
+    }
+
+    /** Prepares a WidgetGroup for display as tooltip to an upgrade
+     *  Specialized to the WorldScreen UnitAction button and Unit Overview upgrade icon -
+     *  in both cases the [UnitAction][com.unciv.models.UnitAction] and [unitToUpgradeTo] have already been evaluated.
+     */
+    fun getUpgradeTooltipActor(title: String, unitUpgrading: BaseUnit, unitToUpgradeTo: BaseUnit) =
+        getUpgradeInfoTable(title, unitUpgrading, unitToUpgradeTo).wrapScaled(0.667f)
+
+    fun getUpgradeInfoTable(title: String, unitUpgrading: BaseUnit, unitToUpgradeTo: BaseUnit): Table {
+        val ruleset = unitToUpgradeTo.ruleset
+        val info = sequenceOf(FormattedLine(title, color = "#FDA", icon = unitToUpgradeTo.makeLink(), header = 5)) +
+            getDifferences(ruleset, unitUpgrading, unitToUpgradeTo)
+                .map { FormattedLine(it.first, icon = it.second ?: "") }
+        val infoTable = MarkupRenderer.render(info.asIterable(), 400f)
+        infoTable.background = BaseScreen.skinStrings.getUiBackground("General/Tooltip", BaseScreen.skinStrings.roundedEdgeRectangleShape, Color.DARK_GRAY)
+        return infoTable
+    }
+
+    private fun Table.wrapScaled(scale: Float): WidgetGroup =
+        Container(this).apply {
+            touchable = Touchable.disabled
+            isTransform = true
+            setScale(scale)
+        }
 }
diff --git a/core/src/com/unciv/ui/popups/options/AboutTab.kt b/core/src/com/unciv/ui/popups/options/AboutTab.kt
index 57a19dcf45..46396b1f10 100644
--- a/core/src/com/unciv/ui/popups/options/AboutTab.kt
+++ b/core/src/com/unciv/ui/popups/options/AboutTab.kt
@@ -14,5 +14,5 @@ fun aboutTab(): Table {
         yield(FormattedLine("See online Readme", link = "https://github.com/yairm210/Unciv/blob/master/README.md#unciv---foss-civ-v-for-androiddesktop"))
         yield(FormattedLine("Visit repository", link = "https://github.com/yairm210/Unciv"))
     }
-    return MarkupRenderer.render(lines.toList()).pad(20f)
+    return MarkupRenderer.render(lines.asIterable()).pad(20f)
 }
diff --git a/core/src/com/unciv/ui/screens/cityscreen/CitizenManagementTable.kt b/core/src/com/unciv/ui/screens/cityscreen/CitizenManagementTable.kt
index 6e457df201..229b13d958 100644
--- a/core/src/com/unciv/ui/screens/cityscreen/CitizenManagementTable.kt
+++ b/core/src/com/unciv/ui/screens/cityscreen/CitizenManagementTable.kt
@@ -16,8 +16,8 @@ class CitizenManagementTable(val cityScreen: CityScreen) : Table(BaseScreen.skin
     fun update() {
         clear()
 
-        val colorSelected = BaseScreen.skin.get("selection", Color::class.java)
-        val colorButton = BaseScreen.skin.get("color", Color::class.java)
+        val colorSelected = BaseScreen.skin.getColor("selection")
+        val colorButton = BaseScreen.skin.getColor("color")
         // effectively a button, but didn't want to rewrite TextButton style
         // and much more compact and can control backgrounds easily based on settings
         val resetLabel = "Reset Citizens".toLabel()
diff --git a/core/src/com/unciv/ui/screens/cityscreen/CityStatsTable.kt b/core/src/com/unciv/ui/screens/cityscreen/CityStatsTable.kt
index 81bfdb84ff..ca33305dda 100644
--- a/core/src/com/unciv/ui/screens/cityscreen/CityStatsTable.kt
+++ b/core/src/com/unciv/ui/screens/cityscreen/CityStatsTable.kt
@@ -77,7 +77,7 @@ class CityStatsTable(private val cityScreen: CityScreen): Table() {
         lowerTable.clear()
 
         val miniStatsTable = Table()
-        val selected = BaseScreen.skin.get("selection", Color::class.java)
+        val selected = BaseScreen.skin.getColor("selection")
         for ((stat, amount) in cityInfo.cityStats.currentCityStats) {
             if (stat == Stat.Faith && !cityInfo.civ.gameInfo.isReligionEnabled()) continue
             val icon = Table()
diff --git a/core/src/com/unciv/ui/screens/cityscreen/SpecialistAllocationTable.kt b/core/src/com/unciv/ui/screens/cityscreen/SpecialistAllocationTable.kt
index 9f7456a49f..65d4c6aff9 100644
--- a/core/src/com/unciv/ui/screens/cityscreen/SpecialistAllocationTable.kt
+++ b/core/src/com/unciv/ui/screens/cityscreen/SpecialistAllocationTable.kt
@@ -27,7 +27,7 @@ class SpecialistAllocationTable(private val cityScreen: CityScreen) : Table(Base
         if (cityScreen.canCityBeChanged()) {
             if (cityInfo.manualSpecialists) {
                 val manualSpecialists = "Manual Specialists".toLabel()
-                    .addBorder(5f, BaseScreen.skin.get("color", Color::class.java))
+                    .addBorder(5f, BaseScreen.skin.getColor("color"))
                 manualSpecialists.onClick {
                     cityInfo.manualSpecialists = false
                     cityInfo.reassignPopulation(); cityScreen.update()
@@ -35,7 +35,7 @@ class SpecialistAllocationTable(private val cityScreen: CityScreen) : Table(Base
                 add(manualSpecialists).colspan(5).row()
             } else {
                 val autoSpecialists = "Auto Specialists".toLabel()
-                    .addBorder(5f, BaseScreen.skin.get("color", Color::class.java))
+                    .addBorder(5f, BaseScreen.skin.getColor("color"))
                 autoSpecialists.onClick { cityInfo.manualSpecialists = true; update() }
                 add(autoSpecialists).colspan(5).row()
             }
diff --git a/core/src/com/unciv/ui/screens/civilopediascreen/FormattedLine.kt b/core/src/com/unciv/ui/screens/civilopediascreen/FormattedLine.kt
index 43b0490ffc..b81aae1812 100644
--- a/core/src/com/unciv/ui/screens/civilopediascreen/FormattedLine.kt
+++ b/core/src/com/unciv/ui/screens/civilopediascreen/FormattedLine.kt
@@ -11,9 +11,11 @@ import com.unciv.models.metadata.BaseRuleset
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.ruleset.unique.Unique
+import com.unciv.ui.components.ColorMarkupLabel
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.screens.basescreen.BaseScreen
+import com.unciv.ui.components.extensions.toLabel
 import com.unciv.utils.Log
 import kotlin.math.max
 
@@ -258,7 +260,7 @@ class FormattedLine (
             size == Int.MIN_VALUE -> Constants.defaultFontSize
             else -> size
         }
-        val labelColor = if(starred) defaultColor else displayColor
+        val labelColor = if (starred) defaultColor else displayColor
 
         val table = Table(BaseScreen.skin)
         var iconCount = 0
@@ -285,7 +287,10 @@ class FormattedLine (
                 else -> (indent-1) * indentPad +
                         indentOneAtNumIcons * (minIconSize + iconPad) + iconPad - usedWidth
             }
-            val label = textToDisplay.toLabel(labelColor, fontSize, hideIcons = iconCount!=0)
+            val label = if ('«' in textToDisplay)
+                ColorMarkupLabel(textToDisplay, fontSize, hideIcons = iconCount != 0)
+            else
+                textToDisplay.toLabel(labelColor, fontSize, hideIcons = iconCount != 0)
             label.wrap = !centered && labelWidth > 0f
             label.setAlignment(align)
             if (labelWidth == 0f)
diff --git a/core/src/com/unciv/ui/screens/civilopediascreen/MarkupRenderer.kt b/core/src/com/unciv/ui/screens/civilopediascreen/MarkupRenderer.kt
index 65da72dee4..19e7d6b2b4 100644
--- a/core/src/com/unciv/ui/screens/civilopediascreen/MarkupRenderer.kt
+++ b/core/src/com/unciv/ui/screens/civilopediascreen/MarkupRenderer.kt
@@ -28,7 +28,7 @@ object MarkupRenderer {
      *  @param linkAction       Delegate to call for internal links. Leave null to suppress linking.
      */
     fun render(
-        lines: Collection<FormattedLine>,
+        lines: Iterable<FormattedLine>,
         labelWidth: Float = 0f,
         padding: Float = defaultPadding,
         iconDisplay: FormattedLine.IconDisplay = FormattedLine.IconDisplay.All,
diff --git a/core/src/com/unciv/ui/screens/mapeditorscreen/MapEditorToolsDrawer.kt b/core/src/com/unciv/ui/screens/mapeditorscreen/MapEditorToolsDrawer.kt
index 4e03ee6c82..f8f012dee2 100644
--- a/core/src/com/unciv/ui/screens/mapeditorscreen/MapEditorToolsDrawer.kt
+++ b/core/src/com/unciv/ui/screens/mapeditorscreen/MapEditorToolsDrawer.kt
@@ -49,7 +49,7 @@ class MapEditorToolsDrawer(
         add(arrowWrapper).align(Align.center).width(handleWidth).fillY().apply {  // the "handle"
             background = BaseScreen.skinStrings.getUiBackground(
                 "MapEditor/MapEditorToolsDrawer/Handle",
-                tintColor = BaseScreen.skin.get("color", Color::class.java)
+                tintColor = BaseScreen.skin.getColor("color")
             )
         }
 
diff --git a/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorEditSubTabs.kt b/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorEditSubTabs.kt
index 2b8a2e25bc..cac9ba58ca 100644
--- a/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorEditSubTabs.kt
+++ b/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorEditSubTabs.kt
@@ -59,7 +59,7 @@ class MapEditorEditTerrainTab(
         .filter { it.type.isBaseTerrain }
     private fun getTerrains() = allTerrains()
         .map { FormattedLine(it.name, it.name, "Terrain/${it.name}", size = 32) }
-        .toList()
+        .asIterable()
 
     override fun isDisabled() = false // allTerrains().none() // wanna see _that_ mod...
 }
@@ -100,7 +100,7 @@ class MapEditorEditFeaturesTab(
         .filter { it.type == TerrainType.TerrainFeature }
     private fun getFeatures() = allowedFeatures()
         .map { FormattedLine(it.name, it.name, "Terrain/${it.name}", size = 32) }
-        .toList()
+        .asIterable()
 
     override fun isDisabled() = allowedFeatures().none()
 }
@@ -133,7 +133,7 @@ class MapEditorEditWondersTab(
         .filter { it.type == TerrainType.NaturalWonder }
     private fun getWonders() = allowedWonders()
         .map { FormattedLine(it.name, it.name, "Terrain/${it.name}", size = 32) }
-        .toList()
+        .asIterable()
 
     override fun isDisabled() = allowedWonders().none()
 }
@@ -176,7 +176,7 @@ class MapEditorEditResourcesTab(
 
     private fun allowedResources() = ruleset.tileResources.values.asSequence()
         .filter { !it.hasUnique(UniqueType.CityStateOnlyResource) }
-    private fun getResources(): List<FormattedLine> = sequence {
+    private fun getResources(): Iterable<FormattedLine> = sequence {
         var lastGroup = ResourceType.Bonus
         for (resource in allowedResources()) {
             val name = resource.name
@@ -186,7 +186,7 @@ class MapEditorEditResourcesTab(
             }
             yield (FormattedLine(name, name, "Resource/$name", size = 32))
         }
-    }.toList()
+    }.asIterable()
 
     override fun isDisabled() = allowedResources().none()
 }
@@ -233,7 +233,7 @@ class MapEditorEditImprovementsTab(
         .filter { improvement ->
             disallowImprovements.none { improvement.name.startsWith(it) }
         }
-    private fun getImprovements(): List<FormattedLine> = sequence {
+    private fun getImprovements(): Iterable<FormattedLine> = sequence {
         var lastGroup = 0
         for (improvement in allowedImprovements()) {
             val name = improvement.name
@@ -244,7 +244,7 @@ class MapEditorEditImprovementsTab(
             }
             yield (FormattedLine(name, name, "Improvement/$name", size = 32))
         }
-    }.toList()
+    }.asIterable()
 
     override fun isDisabled() = allowedImprovements().none()
 
@@ -306,7 +306,7 @@ class MapEditorEditStartsTab(
     private fun getNations() = allowedNations()
         .sortedWith(compareBy<Nation>{ it.isCityState }.thenBy(collator) { it.name.tr() })
         .map { FormattedLine("[${it.name}] starting location", it.name, "Nation/${it.name}", size = 24) }
-        .toList()
+        .asIterable()
 
     override fun isDisabled() = allowedNations().none()
 
diff --git a/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorViewTab.kt b/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorViewTab.kt
index e916728c79..e460dd42c0 100644
--- a/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorViewTab.kt
+++ b/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorViewTab.kt
@@ -138,7 +138,7 @@ class MapEditorViewTab(
                 startsOutOpened = false,
                 headerPad = 5f
             ) {
-                it.add(MarkupRenderer.render(lines.toList(), iconDisplay = IconDisplay.NoLink) { name ->
+                it.add(MarkupRenderer.render(lines.asIterable(), iconDisplay = IconDisplay.NoLink) { name ->
                     scrollToStartOfNation(name)
                 })
             }).row()
diff --git a/core/src/com/unciv/ui/screens/overviewscreen/UnitOverviewTab.kt b/core/src/com/unciv/ui/screens/overviewscreen/UnitOverviewTab.kt
index 5cdf421538..4ae017019c 100644
--- a/core/src/com/unciv/ui/screens/overviewscreen/UnitOverviewTab.kt
+++ b/core/src/com/unciv/ui/screens/overviewscreen/UnitOverviewTab.kt
@@ -14,7 +14,8 @@ import com.unciv.logic.civilization.Civilization
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.logic.map.tile.Tile
 import com.unciv.models.UnitActionType
-import com.unciv.ui.audio.SoundPlayer
+import com.unciv.models.UpgradeUnitAction
+import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.ui.components.ExpanderTab
 import com.unciv.ui.components.Fonts
 import com.unciv.ui.components.TabbedPager
@@ -273,16 +274,20 @@ class UnitOverviewTab(
             add(promotionsTable)
 
             // Upgrade column
-            if (unit.upgrade.canUpgrade()) {
-                val unitAction = UnitActionsUpgrade.getUpgradeAction(unit)
-                val enable = unitAction?.action != null && viewingPlayer.isCurrentPlayer() &&
+            val unitAction = UnitActionsUpgrade.getUpgradeActionAnywhere(unit)
+            if (unitAction != null) {
+                val enable = unitAction.action != null && viewingPlayer.isCurrentPlayer() &&
                     GUI.isAllowedChangeState()
-                val upgradeIcon = ImageGetter.getUnitIcon(unit.upgrade.getUnitToUpgradeTo().name,
+                val unitToUpgradeTo = (unitAction as UpgradeUnitAction).unitToUpgradeTo
+                val selectKey = getUnitIdentifier(unit, unitToUpgradeTo)
+                val upgradeIcon = ImageGetter.getUnitIcon(unitToUpgradeTo.name,
                     if (enable) Color.GREEN else Color.GREEN.darken(0.5f))
                 if (enable) upgradeIcon.onClick {
-                    SoundPlayer.play(unitAction!!.uncivSound)
-                    unitAction.action!!()
-                    unitListTable.updateUnitListTable()
+                    val pos = upgradeIcon.localToStageCoordinates(Vector2(upgradeIcon.width/2, upgradeIcon.height/2))
+                    UnitUpgradeMenu(overviewScreen.stage, pos, unit, unitAction) {
+                        unitListTable.updateUnitListTable()
+                        select(selectKey)
+                    }
                 }
                 add(upgradeIcon).size(28f)
             } else add()
@@ -295,7 +300,10 @@ class UnitOverviewTab(
     }
 
     companion object {
-        fun getUnitIdentifier(unit: MapUnit) = unit.run { "$name@${getTile().position.toPrettyString()}" }
+        fun getUnitIdentifier(unit: MapUnit, unitToUpgradeTo: BaseUnit? = null): String {
+            val name = unitToUpgradeTo?.name ?: unit.name
+            return "$name@${unit.getTile().position.toPrettyString()}"
+        }
     }
 
     override fun select(selection: String): Float? {
@@ -316,4 +324,5 @@ class UnitOverviewTab(
         button.addAction(blinkAction)
         return scrollY
     }
+
 }
diff --git a/core/src/com/unciv/ui/screens/overviewscreen/UnitUpgradeMenu.kt b/core/src/com/unciv/ui/screens/overviewscreen/UnitUpgradeMenu.kt
new file mode 100644
index 0000000000..4b6a1ff658
--- /dev/null
+++ b/core/src/com/unciv/ui/screens/overviewscreen/UnitUpgradeMenu.kt
@@ -0,0 +1,209 @@
+package com.unciv.ui.screens.overviewscreen
+
+import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.graphics.g2d.NinePatch
+import com.badlogic.gdx.math.Interpolation
+import com.badlogic.gdx.math.Vector2
+import com.badlogic.gdx.scenes.scene2d.Stage
+import com.badlogic.gdx.scenes.scene2d.Touchable
+import com.badlogic.gdx.scenes.scene2d.actions.Actions
+import com.badlogic.gdx.scenes.scene2d.ui.Container
+import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.scenes.scene2d.ui.TextButton
+import com.badlogic.gdx.scenes.scene2d.utils.NinePatchDrawable
+import com.unciv.logic.map.mapunit.MapUnit
+import com.unciv.models.UpgradeUnitAction
+import com.unciv.ui.audio.SoundPlayer
+import com.unciv.ui.components.KeyCharAndCode
+import com.unciv.ui.components.KeyboardBinding
+import com.unciv.ui.components.extensions.keyShortcuts
+import com.unciv.ui.components.extensions.onActivation
+import com.unciv.ui.components.extensions.pad
+import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.objectdescriptions.BaseUnitDescriptions
+import com.unciv.ui.popups.Popup
+import com.unciv.ui.screens.basescreen.BaseScreen
+import com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsUpgrade
+
+//TODO When this gets reused. e.g. from UnitActionsTable, move to another package.
+
+/**
+ *  A popup menu showing info about an Unit upgrade, with buttons to upgrade "this" unit or _all_
+ *  similar units.
+ *
+ *  Meant to animate "in" at a given position - unlike other [Popup]s which are always stage-centered.
+ *  No close button - use "click-behind".
+ *  The "click-behind" semi-transparent covering of the rest of the stage is much darker than a normal
+ *  Popup (geve the impression to take away illumination and spotlight the menu) and fades in together
+ *  with the UnitUpgradeMenu itself. Closing the menu in any of the four ways will fade out everything
+ *  inverting the fade-and-scale-in.
+ *
+ *  @param stage The stage this will be shown on, passed to Popup and used for clamping **`position`**
+ *  @param position stage coortinates to show this centered over - clamped so that nothing is clipped outside the [stage]
+ *  @param unit Who is ready to upgrade?
+ *  @param unitAction Holds pre-calculated info like unitToUpgradeTo, cost or resource requirements. Its action is mapped to the Upgrade button.
+ *  @param onButtonClicked A callback after one or several upgrades have been performed (and the menu is about to close)
+ */
+class UnitUpgradeMenu(
+    stage: Stage,
+    position: Vector2,
+    private val unit: MapUnit,
+    private val unitAction: UpgradeUnitAction,
+    private val onButtonClicked: () -> Unit
+) : Popup(stage, scrollable = false) {
+    private val container: Container<Table>
+    private val allUpgradableUnits: Sequence<MapUnit>
+    private val animationDuration = 0.33f
+    private val backgroundColor = (background as NinePatchDrawable).patch.color
+
+    init {
+        innerTable.remove()
+
+        // Note: getUpgradeInfoTable skins this as General/Tooltip, roundedEdgeRectangle, DARK_GRAY
+        // TODO - own skinnable path, possibly when tooltip use of getUpgradeInfoTable gets replaced
+        val newInnerTable = BaseUnitDescriptions.getUpgradeInfoTable(
+            unitAction.title, unit.baseUnit, unitAction.unitToUpgradeTo
+        )
+
+        newInnerTable.row()
+        val smallButtonStyle = SmallButtonStyle()
+        val upgradeButton = "Upgrade".toTextButton(smallButtonStyle)
+        upgradeButton.onActivation(::doUpgrade)
+        upgradeButton.keyShortcuts.add(KeyboardBinding.Confirm)
+        newInnerTable.add(upgradeButton).pad(15f, 15f, 5f, 15f).growX().row()
+
+        allUpgradableUnits = unit.civ.units.getCivUnits()
+            .filter {
+                it.baseUnit.name == unit.baseUnit.name
+                    && it.currentMovement > 0f
+                    && it.currentTile.getOwner() == unit.civ
+                    && !it.isEmbarked()
+                    && it.upgrade.canUpgrade(unitAction.unitToUpgradeTo, ignoreResources = true)
+            }
+        newInnerTable.tryAddUpgradeAllUnitsButton(smallButtonStyle)
+
+        clickBehindToClose = true
+        keyShortcuts.add(KeyCharAndCode.BACK) { close() }
+
+        newInnerTable.pack()
+        container = Container(newInnerTable)
+        container.touchable = Touchable.childrenOnly
+        container.isTransform = true
+        container.setScale(0.05f)
+        container.color.a = 0f
+
+        open(true)  // this only does the screen-covering "click-behind" portion
+
+        container.setPosition(
+            position.x.coerceAtMost(stage.width - newInnerTable.width / 2),
+            position.y.coerceAtLeast(newInnerTable.height / 2)
+        )
+        addActor(container)
+
+        container.addAction(
+            Actions.parallel(
+            Actions.scaleTo(1f, 1f, animationDuration, Interpolation.fade),
+            Actions.fadeIn(animationDuration, Interpolation.fade)
+        ))
+
+        backgroundColor.set(0)
+        addAction(Actions.alpha(0.35f, animationDuration, Interpolation.fade).apply {
+            color = backgroundColor
+        })
+    }
+
+    private fun Table.tryAddUpgradeAllUnitsButton(buttonStyle: TextButton.TextButtonStyle) {
+        val allCount = allUpgradableUnits.count()
+        if (allCount <= 1) return
+        // Note - all same-baseunit units cost the same to upgrade? What if a mod says e.g. 50% discount on Oasis?
+        // - As far as I can see the rest of the upgrading code doesn't support such conditions at the moment.
+        val allCost = unitAction.goldCostOfUpgrade * allCount
+        val allResources = unitAction.newResourceRequirements * allCount
+        val upgradeAllButton = "Upgrade all [$allCount] [${unit.name}] ([$allCost] gold)"
+            .toTextButton(buttonStyle)
+        upgradeAllButton.isDisabled = unit.civ.gold < allCost ||
+            allResources.isNotEmpty() &&
+            unit.civ.getCivResourcesByName().run {
+                allResources.any {
+                    it.value > (this[it.key] ?: 0)
+                }
+            }
+        upgradeAllButton.onActivation(::doAllUpgrade)
+        add(upgradeAllButton).pad(2f, 15f).growX().row()
+    }
+
+    private fun doUpgrade() {
+        SoundPlayer.play(unitAction.uncivSound)
+        unitAction.action!!()
+        onButtonClicked()
+        close()
+    }
+
+    private fun doAllUpgrade() {
+        stage.addAction(
+            Actions.sequence(
+            Actions.run { SoundPlayer.play(unitAction.uncivSound) },
+            Actions.delay(0.2f),
+            Actions.run { SoundPlayer.play(unitAction.uncivSound) }
+        ))
+        for (unit in allUpgradableUnits) {
+            val otherAction = UnitActionsUpgrade.getUpgradeAction(unit)
+            otherAction?.action?.invoke()
+        }
+        onButtonClicked()
+        close()
+    }
+
+    override fun close() {
+        addAction(Actions.alpha(0f, animationDuration, Interpolation.fade).apply {
+            color = backgroundColor
+        })
+        container.addAction(
+            Actions.sequence(
+            Actions.parallel(
+                Actions.scaleTo(0.05f, 0.05f, animationDuration, Interpolation.fade),
+                Actions.fadeOut(animationDuration, Interpolation.fade)
+            ),
+            Actions.run {
+                container.remove()
+                super.close()
+            }
+        ))
+    }
+
+    class SmallButtonStyle : TextButton.TextButtonStyle(BaseScreen.skin[TextButton.TextButtonStyle::class.java]) {
+        /** Modify NinePatch geometry so the roundedEdgeRectangleMidShape button is 38f high instead of 48f,
+         *  Otherwise this excercise would be futile - normal roundedEdgeRectangleShape based buttons are 50f high.
+         */
+        private fun NinePatchDrawable.reduce(): NinePatchDrawable {
+            val patch = NinePatch(this.patch)
+            patch.padTop = 10f
+            patch.padBottom = 10f
+            patch.topHeight = 10f
+            patch.bottomHeight = 10f
+            return NinePatchDrawable(this).also { it.patch = patch }
+        }
+
+        init {
+            val upColor = BaseScreen.skin.getColor("color")
+            val downColor = BaseScreen.skin.getColor("pressed")
+            val overColor = BaseScreen.skin.getColor("highlight")
+            val disabledColor = BaseScreen.skin.getColor("disabled")
+            // UiElementDocsWriter inspects source, which is why this isn't prettified better
+            val shape = BaseScreen.run {
+                // Let's use _one_ skinnable background lookup but with different tints
+                val skinned = skinStrings.getUiBackground("UnitUpgradeMenu/Button", skinStrings.roundedEdgeRectangleMidShape)
+                // Reduce height only if not skinned
+                val default = ImageGetter.getNinePatch(skinStrings.roundedEdgeRectangleMidShape)
+                if (skinned === default) default.reduce() else skinned
+            }
+            // Now get the tinted variants
+            up = shape.tint(upColor)
+            down = shape.tint(downColor)
+            over = shape.tint(overColor)
+            disabled = shape.tint(disabledColor)
+            disabledFontColor = Color.GRAY
+        }
+    }
+}
diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsTable.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsTable.kt
index 59f4ccee87..90842021fa 100644
--- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsTable.kt
+++ b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsTable.kt
@@ -1,19 +1,25 @@
 package com.unciv.ui.screens.worldscreen.unit.actions
 
 import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.math.Vector2
 import com.badlogic.gdx.scenes.scene2d.ui.Button
 import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.utils.Align
 import com.unciv.GUI
 import com.unciv.UncivGame
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.models.UnitAction
 import com.unciv.models.UnitActionType
+import com.unciv.models.UpgradeUnitAction
 import com.unciv.ui.components.KeyCharAndCode
+import com.unciv.ui.components.UncivTooltip
 import com.unciv.ui.components.UncivTooltip.Companion.addTooltip
 import com.unciv.ui.components.extensions.disable
 import com.unciv.ui.components.extensions.keyShortcuts
 import com.unciv.ui.components.extensions.onActivation
+import com.unciv.ui.components.extensions.packIfNeeded
 import com.unciv.ui.images.IconTextButton
+import com.unciv.ui.objectdescriptions.BaseUnitDescriptions
 import com.unciv.ui.screens.worldscreen.WorldScreen
 
 class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
@@ -22,9 +28,17 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
         clear()
         if (unit == null) return
         if (!worldScreen.canChangeState) return // No actions when it's not your turn or spectator!
-        for (button in UnitActions.getUnitActions(unit)
-            .map { getUnitActionButton(unit, it) })
+        for (unitAction in UnitActions.getUnitActions(unit)) {
+            val button = getUnitActionButton(unit, unitAction)
+            if (unitAction is UpgradeUnitAction) {
+                val tipTitle = "«RED»${unitAction.type.key}«»: {Upgrade}"
+                val tipActor = BaseUnitDescriptions.getUpgradeTooltipActor(tipTitle, unit.baseUnit, unitAction.unitToUpgradeTo)
+                button.addListener(UncivTooltip(button, tipActor
+                    , offset = Vector2(0f, tipActor.packIfNeeded().height * 0.333f) // scaling fails to express size in parent coordinates
+                    , tipAlign = Align.topLeft, targetAlign = Align.topRight))
+            }
             add(button).left().padBottom(2f).row()
+        }
         pack()
     }
 
@@ -40,7 +54,8 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
         if (unitAction.type == UnitActionType.Promote && unitAction.action != null)
             actionButton.color = Color.GREEN.cpy().lerp(Color.WHITE, 0.5f)
 
-        actionButton.addTooltip(key)
+        if (unitAction !is UpgradeUnitAction)  // Does its own toolTip
+            actionButton.addTooltip(key)
         actionButton.pack()
         if (unitAction.action == null) {
             actionButton.disable()
diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt
index 19913752da..683ca23219 100644
--- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt
+++ b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt
@@ -3,11 +3,11 @@ package com.unciv.ui.screens.worldscreen.unit.actions
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.models.Counter
 import com.unciv.models.UnitAction
-import com.unciv.models.UnitActionType
+import com.unciv.models.UpgradeUnitAction
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.translations.tr
 
-object UnitActionsUpgrade{
+object UnitActionsUpgrade {
 
     internal fun addUnitUpgradeAction(
         unit: MapUnit,
@@ -21,13 +21,14 @@ object UnitActionsUpgrade{
     private fun getUpgradeAction(
         unit: MapUnit,
         isFree: Boolean,
-        isSpecial: Boolean
+        isSpecial: Boolean,
+        isAnywhere: Boolean
     ): UnitAction? {
         val specialUpgradesTo = unit.baseUnit().getMatchingUniques(UniqueType.RuinsUpgrade).map { it.params[0] }.firstOrNull()
         if (unit.baseUnit().upgradesTo == null && specialUpgradesTo == null) return null // can't upgrade to anything
         val unitTile = unit.getTile()
         val civInfo = unit.civ
-        if (!isFree && unitTile.getOwner() != civInfo) return null
+        if (!isAnywhere && unitTile.getOwner() != civInfo) return null
 
         val upgradesTo = unit.baseUnit().upgradesTo
         val upgradedUnit = when {
@@ -46,8 +47,9 @@ object UnitActionsUpgrade{
             resourceRequirementsDelta.add(resource, -amount)
         for ((resource, amount) in upgradedUnit.getResourceRequirementsPerTurn())
             resourceRequirementsDelta.add(resource, amount)
+        for ((resource, _) in resourceRequirementsDelta.filter { it.value < 0 })  // filter copies, so no CCM
+            resourceRequirementsDelta[resource] = 0
         val newResourceRequirementsString = resourceRequirementsDelta.entries
-            .filter { it.value > 0 }
             .joinToString { "${it.value} {${it.key}}".tr() }
 
         val goldCostOfUpgrade = if (isFree) 0 else unit.upgrade.getCostOfUpgrade(upgradedUnit)
@@ -58,13 +60,19 @@ object UnitActionsUpgrade{
             "Upgrade to [${upgradedUnit.name}] ([$goldCostOfUpgrade] gold)"
         else "Upgrade to [${upgradedUnit.name}]\n([$goldCostOfUpgrade] gold, [$newResourceRequirementsString])"
 
-        return UnitAction(
-            UnitActionType.Upgrade,
+        return UpgradeUnitAction(
             title = title,
+            unitToUpgradeTo = upgradedUnit,
+            goldCostOfUpgrade = goldCostOfUpgrade,
+            newResourceRequirements = resourceRequirementsDelta,
             action = {
                 unit.destroy(destroyTransportedUnit = false)
                 val newUnit = civInfo.units.placeUnitNearTile(unitTile.position, upgradedUnit.name)
 
+                /** We were UNABLE to place the new unit, which means that the unit failed to upgrade!
+                 * The only known cause of this currently is "land units upgrading to water units" which fail to be placed.
+                 */
+
                 /** We were UNABLE to place the new unit, which means that the unit failed to upgrade!
                  * The only known cause of this currently is "land units upgrading to water units" which fail to be placed.
                  */
@@ -80,6 +88,7 @@ object UnitActionsUpgrade{
                 isFree || (
                         unit.civ.gold >= goldCostOfUpgrade
                                 && unit.currentMovement > 0
+                                && unitTile.getOwner() == civInfo
                                 && !unit.isEmbarked()
                                 && unit.upgrade.canUpgrade(unitToUpgradeTo = upgradedUnit)
                         )
@@ -88,10 +97,11 @@ object UnitActionsUpgrade{
     }
 
     fun getUpgradeAction(unit: MapUnit) =
-            getUpgradeAction(unit, isFree = false, isSpecial = false)
+            getUpgradeAction(unit, isFree = false, isSpecial = false, isAnywhere = false)
     fun getFreeUpgradeAction(unit: MapUnit) =
-            getUpgradeAction(unit,  isFree = true, isSpecial = false)
+            getUpgradeAction(unit, isFree = true, isSpecial = false, isAnywhere = true)
     fun getAncientRuinsUpgradeAction(unit: MapUnit) =
-            getUpgradeAction(unit,  isFree = true, isSpecial = true)
-
+            getUpgradeAction(unit, isFree = true, isSpecial = true, isAnywhere = true)
+    fun getUpgradeActionAnywhere(unit: MapUnit) =
+            getUpgradeAction(unit, isFree = false, isSpecial = false, isAnywhere = true)
 }
diff --git a/docs/Modders/Creating-a-UI-skin.md b/docs/Modders/Creating-a-UI-skin.md
index d8bd8ebddf..8e61e62250 100644
--- a/docs/Modders/Creating-a-UI-skin.md
+++ b/docs/Modders/Creating-a-UI-skin.md
@@ -96,6 +96,7 @@ These shapes are used all over Unciv and can be replaced to make a lot of UI ele
 | TechPickerScreen/ | ResearchedFutureTechColor | 127, 50, 0 | |
 | TechPickerScreen/ | ResearchedTechColor | 255, 215, 0 | |
 | TechPickerScreen/ | TechButtonIconsOutline | roundedEdgeRectangleSmall | |
+| UnitUpgradeMenu/ | Button | roundedEdgeRectangleMid | |
 | VictoryScreen/ | CivGroup | roundedEdgeRectangle | |
 | WorldScreen/ | AirUnitTable | null | |
 | WorldScreen/ | BattleTable | null | |
diff --git a/docs/Other/Miscellaneous-JSON-files.md b/docs/Other/Miscellaneous-JSON-files.md
index 579e12e04d..03f217e990 100644
--- a/docs/Other/Miscellaneous-JSON-files.md
+++ b/docs/Other/Miscellaneous-JSON-files.md
@@ -294,3 +294,6 @@ List of attributes - note not all combinations are valid:
 |`centered`|Boolean|Centers the line (and turns off automatic wrap).|
 
 The lines from json will 'surround' the automatically generated lines such that the latter are inserted just above the first json line carrying a link, if any. If no json lines have links, they will be inserted between the automatic title and the automatic info. This method may, however, change in the future.
+
+Note: `text` now also supports inline color markup. Insert `«color»` to start coloring text, `«»` to stop. `color` can be a name or 6/8-digit hex notation like `#ffa040` (different from the `color` attribute notation only by not allowing 3-digit codes, but allowing the alpha channel).
+Effectively, the `«»` markers are replaced with `[]` _after_ translation and then passed to [Gdx markup language](https://libgdx.com/wiki/graphics/2d/fonts/color-markup-language).