City construct menu (#9961)

* Some preparation refactoring

* Some preparation API extension

* Initial constructions context menu

* More CityConstructions API clarification

* Templates and KeyBindings

* Fix quirks and prettify highlighting issues

android/assets/jsons/translations/template.properties

@@ -1236,6 +1236,12 @@ Default Focus =
 [stat] Focus = 
 Please enter a new name for your city = 
 Please select a tile for this building's [improvement] = 
+Move to the top of the queue = 
+Move to the end of the queue = 
+Add to the top of the queue = 
+Add to the queue in all cities = 
+Add or move to the top in all cities = 
+Remove from the queue in all cities = 
 
 # Specialized Popups - Ask for text or numbers, file picker
 

core/src/com/unciv/logic/city/CityConstructions.kt

@@ -142,13 +142,16 @@ class CityConstructions : IsPartOfGameInfoSerialization {
         return result
     }
 
-    /** @constructionName needs to be a non-perpetual construction, else an empty string is returned */
+    /** @param constructionName needs to be a non-perpetual construction, else an empty string is returned */
-    internal fun getTurnsToConstructionString(constructionName: String, useStoredProduction:Boolean = true): String {
+    internal fun getTurnsToConstructionString(constructionName: String, useStoredProduction:Boolean = true) =
-        val construction = getConstruction(constructionName)
+        getTurnsToConstructionString(getConstruction(constructionName), useStoredProduction)
+
+    /** @param construction needs to be a non-perpetual construction, else an empty string is returned */
+    internal fun getTurnsToConstructionString(construction: IConstruction, useStoredProduction:Boolean = true): String {
         if (construction !is INonPerpetualConstruction) return ""   // shouldn't happen
         val cost = construction.getProductionCost(city.civ)
-        val turnsToConstruction = turnsToConstruction(constructionName, useStoredProduction)
+        val turnsToConstruction = turnsToConstruction(construction.name, useStoredProduction)
-        val currentProgress = if (useStoredProduction) getWorkDone(constructionName) else 0
+        val currentProgress = if (useStoredProduction) getWorkDone(construction.name) else 0
         val lines = ArrayList<String>()
         val buildable = !construction.getMatchingUniques(UniqueType.Unbuildable)
             .any { it.conditionalsApply(StateForConditionals(city.civ, city)) }
@@ -189,12 +192,23 @@ class CityConstructions : IsPartOfGameInfoSerialization {
     fun isAllBuilt(buildingList: List<String>): Boolean = buildingList.all { isBuilt(it) }
 
     fun isBuilt(buildingName: String): Boolean = builtBuildingObjects.any { it.name == buildingName }
-    @Suppress("MemberVisibilityCanBePrivate")
-    fun isBeingConstructed(constructionName: String): Boolean = currentConstructionFromQueue == constructionName
-    fun isEnqueued(constructionName: String): Boolean = constructionQueue.contains(constructionName)
-    fun isBeingConstructedOrEnqueued(constructionName: String): Boolean = isBeingConstructed(constructionName) || isEnqueued(constructionName)
 
-    fun isQueueFull(): Boolean = constructionQueue.size == queueMaxSize
+    // Note: There was a isEnqueued here functionally identical to isBeingConstructedOrEnqueued,
+    // which was calling both isEnqueued and isBeingConstructed - BUT: currentConstructionFromQueue is just a
+    // a wrapper for constructionQueue[0], so that was redundant. Also, isEnqueued was used nowhere,
+    // and isBeingConstructed _only_ redundantly as described above.
+    // `isEnqueuedForLater` is not optimal code as it can iterate the whole list where checking size
+    // and first() would suffice, but the one current use in CityScreenConstructionMenu isn't critical.
+
+    @Suppress("unused", "MemberVisibilityCanBePrivate")  // kept for illustration
+    /** @return `true` if [constructionName] is the top queue entry, the one receiving production points */
+    fun isBeingConstructed(constructionName: String) = currentConstructionFromQueue == constructionName
+    /** @return `true` if [constructionName] is queued but not the top queue entry */
+    fun isEnqueuedForLater(constructionName: String) = constructionQueue.indexOf(constructionName) > 0
+    /** @return `true` if [constructionName] is anywhere in the construction queue - [isBeingConstructed] **or** [isEnqueuedForLater] */
+    fun isBeingConstructedOrEnqueued(constructionName: String) = constructionQueue.contains(constructionName)
+
+    fun isQueueFull(): Boolean = constructionQueue.size >= queueMaxSize
 
     fun isBuildingWonder(): Boolean {
         val currentConstruction = getCurrentConstruction()
@@ -208,8 +222,8 @@ class CityConstructions : IsPartOfGameInfoSerialization {
 
     /** If the city is constructing multiple units of the same type, subsequent units will require the full cost  */
     fun isFirstConstructionOfItsKind(constructionQueueIndex: Int, name: String): Boolean {
-        // if the construction name is the same as the current construction, it isn't the first
+        // Simply compare index of first found [name] with given index
-        return constructionQueueIndex == constructionQueue.indexOfFirst { it == name }
+        return constructionQueueIndex == constructionQueue.indexOf(name)
     }
 
 
@@ -745,25 +759,53 @@ class CityConstructions : IsPartOfGameInfoSerialization {
         newTile.improvementFunctions.markForCreatesOneImprovement(improvement.name)
     }
 
-    fun addToQueue(constructionName: String) {
+    fun canAddToQueue(construction: IConstruction) =
-        if (isQueueFull()) return
+        !isQueueFull() &&
-        val construction = getConstruction(constructionName)
+        construction.isBuildable(this) &&
-        if (!construction.isBuildable(this)) return
+        !(construction is Building && isBeingConstructedOrEnqueued(construction.name))
-        if (construction is Building && isBeingConstructedOrEnqueued(constructionName)) return
+
-        if (currentConstructionFromQueue == "" || currentConstructionFromQueue == "Nothing") {
+    private fun isLastConstructionPerpetual() = constructionQueue.isNotEmpty() &&
-            currentConstructionFromQueue = constructionName
+        PerpetualConstruction.isNamePerpetual(constructionQueue.last())
-        } else if (getConstruction(constructionQueue.last()) is PerpetualConstruction) {
+        // `getConstruction(constructionQueue.last()) is PerpetualConstruction` is clear but more expensive
-            if (construction is PerpetualConstruction) {  // perpetual constructions will replace each other
+
-                constructionQueue.removeAt(constructionQueue.size - 1)
+    /** Add [construction] to the end or top (controlled by [addToTop]) of the queue with all checks (does nothing if not possible)
+     *
+     *  Note: Overload with string parameter `constructionName` exists as well.
+     */
+    fun addToQueue(construction: IConstruction, addToTop: Boolean = false) {
+        if (!canAddToQueue(construction)) return
+        val constructionName = construction.name
+        when {
+            currentConstructionFromQueue.isEmpty() || currentConstructionFromQueue == "Nothing" ->
+                currentConstructionFromQueue = constructionName
+            addToTop && construction is PerpetualConstruction && PerpetualConstruction.isNamePerpetual(currentConstructionFromQueue) ->
+                currentConstructionFromQueue = constructionName // perpetual constructions will replace each other
+            addToTop ->
+                constructionQueue.add(0, constructionName)
+            isLastConstructionPerpetual() -> {
+                // Note this also works if currentConstructionFromQueue is perpetual and the only entry - that var is delegated to the first queue position
+                if (construction is PerpetualConstruction) {
+                    // perpetual constructions will replace each other
+                    constructionQueue.removeLast()
+                    constructionQueue.add(constructionName)
+                } else
+                    constructionQueue.add(constructionQueue.size - 1, constructionName) // insert new construction before perpetual one
+            }
+            else ->
                 constructionQueue.add(constructionName)
-            } else
+        }
-                constructionQueue.add(constructionQueue.size - 1, constructionName) // insert new construction before perpetual one
-        } else
-            constructionQueue.add(constructionName)
         currentConstructionIsUserSet = true
     }
 
-    /** If this was done automatically, we should automatically try to choose a new construction and treat it as such */
+    /** Add a construction named [constructionName] to the end of the queue with all checks
+     *
+     *  Note: Delegates to overload with `construction` parameter.
+     */
+    fun addToQueue(constructionName: String) = addToQueue(getConstruction(constructionName))
+
+    /** Remove one entry from the queue by index.
+     *  @param automatic  If this was done automatically, we should automatically try to choose a new construction and treat it as such
+     */
     fun removeFromQueue(constructionQueueIndex: Int, automatic: Boolean) {
         val constructionName = constructionQueue.removeAt(constructionQueueIndex)
 
@@ -783,6 +825,38 @@ class CityConstructions : IsPartOfGameInfoSerialization {
         } else true // we're just continuing the regular queue
     }
 
+    /** Remove all queue entries for [constructionName].
+     *
+     *  Does nothing if there's no entry of that name in the queue.
+     *  If the queue is emptied, no automatic: getSettings().autoAssignCityProduction is ignored! (parameter to be added when needed)
+     */
+    fun removeAllByName(constructionName: String) {
+        while (true) {
+            val index = constructionQueue.indexOf(constructionName)
+            if (index < 0) return
+            removeFromQueue(index, false)
+        }
+    }
+
+    /** Moves an entry to the queue top by index.
+     *  No-op when index invalid. Must not be called for PerpetualConstruction entries - unchecked! */
+    fun moveEntryToTop(constructionQueueIndex: Int) {
+        if (constructionQueueIndex == 0 || constructionQueueIndex >= constructionQueue.size) return
+        val constructionName = constructionQueue.removeAt(constructionQueueIndex)
+        constructionQueue.add(0, constructionName)
+    }
+
+    /** Moves an entry by index to the end of the queue, or just before a PerpetualConstruction
+     *  (or replacing a PerpetualConstruction if it itself is one and the queue is by happenstance invalid having more than one of those)
+     */
+    fun moveEntryToEnd(constructionQueueIndex: Int) {
+        if (constructionQueueIndex >= constructionQueue.size) return
+        val constructionName = constructionQueue.removeAt(constructionQueueIndex)
+        // Some of the overhead of addToQueue is redundant here, but if the complex "needs to replace or go before a perpetual" logic is needed, then use it anyway
+        if (isLastConstructionPerpetual()) return addToQueue(constructionName)
+        constructionQueue.add(constructionName)
+    }
+
     fun raisePriority(constructionQueueIndex: Int): Int {
         constructionQueue.swap(constructionQueueIndex - 1, constructionQueueIndex)
         return constructionQueueIndex - 1

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

@@ -224,6 +224,9 @@ open class PerpetualConstruction(override var name: String, val description: Str
 
         val perpetualConstructionsMap: Map<String, PerpetualConstruction>
                 = mapOf(science.name to science, gold.name to gold, culture.name to culture, faith.name to faith, idle.name to idle)
+
+        /** @return whether [name] represents a PerpetualConstruction - note "" is translated to Nothing in the queue so `isNamePerpetual("")==true` */
+        fun isNamePerpetual(name: String) = name.isEmpty() || name in perpetualConstructionsMap
     }
 
     override fun isBuildable(cityConstructions: CityConstructions): Boolean =

core/src/com/unciv/ui/components/input/KeyboardBinding.kt

@@ -157,6 +157,12 @@ enum class KeyboardBinding(
     CultureFocus(Category.CityScreen, "[${Stat.Culture.name}] Focus", KeyCharAndCode.ctrl('c')),
     FaithFocus(Category.CityScreen, "[${Stat.Faith.name}] Focus", KeyCharAndCode.UNKNOWN),
 
+    // CityScreenConstructionMenu (not quite cleanly) reuses RaisePriority/LowerPriority, plus:
+    AddConstructionTop(Category.CityScreenConstructionMenu, "Add to the top of the queue", 't'),
+    AddConstructionAll(Category.CityScreenConstructionMenu, "Add to the queue in all cities", KeyCharAndCode.ctrl('a')),
+    AddConstructionAllTop(Category.CityScreenConstructionMenu, "Add or move to the top in all cities", KeyCharAndCode.ctrl('t')),
+    RemoveConstructionAll(Category.CityScreenConstructionMenu, "Remove from the queue in all cities", KeyCharAndCode.ctrl('r')),
+
     // Popups
     Confirm(Category.Popups, "Confirm Dialog", 'y'),
     Cancel(Category.Popups, "Cancel Dialog", 'n'),
@@ -179,6 +185,7 @@ enum class KeyboardBinding(
             override fun checkConflictsIn() = sequenceOf(WorldScreen)
         },
         CityScreen,
+        CityScreenConstructionMenu, // Maybe someday a category hierarchy?
         Popups
         ;
         val label = unCamelCase(name)

core/src/com/unciv/ui/popups/AnimatedMenuPopup.kt

@@ -4,6 +4,7 @@ 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.Actor
 import com.badlogic.gdx.scenes.scene2d.Stage
 import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.actions.Actions
@@ -54,6 +55,13 @@ open class AnimatedMenuPopup(
     var anyButtonWasClicked = false
         private set
 
+    companion object {
+        /** Get stage coords of an [actor]'s right edge center, to help position an [AnimatedMenuPopup].
+         *  Note the Popup will center over this point.
+         */
+        fun getActorTopRight(actor: Actor): Vector2 = actor.localToStageCoordinates(Vector2(actor.width, actor.height / 2))
+    }
+
     /**
      *  Provides the Popup content.
      *
@@ -61,8 +69,11 @@ open class AnimatedMenuPopup(
      *  You can use [getButton], which produces TextButtons slightly smaller than Unciv's default ones.
      *  The content adding functions offered by [Popup] or [Table] won't work.
      *  The content needs to be complete when the method finishes, it will be `pack()`ed and measured immediately.
+     *
+     *  Return `null` to abort the menu creation - nothing will be shown and the instance should be discarded.
+     *  Useful if you need full context first to determine if any entry makes sense.
      */
-    open fun createContentTable() = Table().apply {
+    open fun createContentTable(): Table? = Table().apply {
         defaults().pad(5f, 15f, 5f, 15f).growX()
         background = BaseScreen.skinStrings.getUiBackground("General/AnimatedMenu", BaseScreen.skinStrings.roundedEdgeRectangleShape, Color.DARK_GRAY)
     }
@@ -79,6 +90,7 @@ open class AnimatedMenuPopup(
 
     private fun createAndShow(position: Vector2) {
         val newInnerTable = createContentTable()
+            ?: return  // Special case - we don't want the context menu after all. If cleanup should become necessary in that case, add here.
         newInnerTable.pack()
         container.actor = newInnerTable
         container.touchable = Touchable.childrenOnly

core/src/com/unciv/ui/popups/CityScreenConstructionMenu.kt

@@ -0,0 +1,105 @@
+package com.unciv.ui.popups
+
+import com.badlogic.gdx.scenes.scene2d.Actor
+import com.badlogic.gdx.scenes.scene2d.Stage
+import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.unciv.logic.city.City
+import com.unciv.logic.city.CityConstructions
+import com.unciv.models.ruleset.Building
+import com.unciv.models.ruleset.IConstruction
+import com.unciv.models.ruleset.PerpetualConstruction
+import com.unciv.ui.components.input.KeyboardBinding
+
+//todo Check move/top/end for "place one improvement" buildings
+//todo Check add/remove-all for "place one improvement" buildings
+
+/**
+ *  "Context menu" for City constructions - available by right-clicking (or long-press) in
+ *   City Screen, left side, available constructions or queue entries.
+ *
+ *  @param city The [City] calling us - we need only `cityConstructions`, but future expansion may be easier having the parent
+ *  @param construction The construction that was right-clicked
+ *  @param onButtonClicked Callback if closed due to any action having been chosen - to update CityScreen
+ */
+class CityScreenConstructionMenu(
+    stage: Stage,
+    positionNextTo: Actor,
+    private val city: City,
+    private val construction: IConstruction,
+    private val onButtonClicked: () -> Unit
+) : AnimatedMenuPopup(stage, getActorTopRight(positionNextTo)) {
+
+    // These are only readability shorteners
+    private val cityConstructions = city.cityConstructions
+    private val constructionName = construction.name
+    private val queueSizeWithoutPerpetual get() = // simply remove get() should this be needed more than once
+        cityConstructions.constructionQueue
+        .count { it !in PerpetualConstruction.perpetualConstructionsMap }
+    private val myIndex = cityConstructions.constructionQueue.indexOf(constructionName)
+    private fun anyCity(predicate: (CityConstructions) -> Boolean) =
+        (construction as? Building)?.isAnyWonder() != true &&
+        city.civ.cities.map { it.cityConstructions }.any(predicate)
+    private fun forAllCities(action: (CityConstructions) -> Unit) =
+        city.civ.cities.map { it.cityConstructions }.forEach(action)
+
+    init {
+        closeListeners.add {
+            if (anyButtonWasClicked) onButtonClicked()
+        }
+    }
+
+    override fun createContentTable(): Table? {
+        val table = super.createContentTable()!!
+        if (canMoveQueueTop())
+            table.add(getButton("Move to the top of the queue", KeyboardBinding.RaisePriority, ::moveQueueTop)).row()
+        if (canMoveQueueEnd())
+            table.add(getButton("Move to the end of the queue", KeyboardBinding.LowerPriority, ::moveQueueEnd)).row()
+        if (canAddQueueTop())
+            table.add(getButton("Add to the top of the queue", KeyboardBinding.AddConstructionTop, ::addQueueTop)).row()
+        if (canAddAllQueues())
+            table.add(getButton("Add to the queue in all cities", KeyboardBinding.AddConstructionAll, ::addAllQueues)).row()
+        if (canAddAllQueuesTop())
+            table.add(getButton("Add or move to the top in all cities", KeyboardBinding.AddConstructionAllTop, ::addAllQueuesTop)).row()
+        if (canRemoveAllQueues())
+            table.add(getButton("Remove from the queue in all cities", KeyboardBinding.RemoveConstructionAll, ::removeAllQueues)).row()
+        return table.takeUnless { it.cells.isEmpty }
+    }
+
+    private fun canMoveQueueTop(): Boolean {
+        if (construction is PerpetualConstruction)
+            return false
+        return myIndex > 0
+    }
+    private fun moveQueueTop() = cityConstructions.moveEntryToTop(myIndex)
+
+    private fun canMoveQueueEnd(): Boolean {
+        if (construction is PerpetualConstruction)
+            return false
+        return myIndex in 0 until queueSizeWithoutPerpetual - 1
+    }
+    private fun moveQueueEnd() = cityConstructions.moveEntryToEnd(myIndex)
+
+    private fun canAddQueueTop() = construction !is PerpetualConstruction &&
+        cityConstructions.canAddToQueue(construction)
+    private fun addQueueTop() = cityConstructions.addToQueue(construction, addToTop = true)
+
+    private fun canAddAllQueues() = anyCity {
+        it.canAddToQueue(construction) &&
+        // A Perpetual that is already queued can still be added says canAddToQueue, but here we don't want to count that
+        !(construction is PerpetualConstruction && it.isBeingConstructedOrEnqueued(constructionName))
+    }
+    private fun addAllQueues() = forAllCities { it.addToQueue(construction) }
+
+    private fun canAddAllQueuesTop() = construction !is PerpetualConstruction &&
+        anyCity { it.canAddToQueue(construction) || it.isEnqueuedForLater(constructionName) }
+    private fun addAllQueuesTop() = forAllCities {
+        val index = it.constructionQueue.indexOf(constructionName)
+        if (index > 0)
+            it.moveEntryToTop(index)
+        else
+            it.addToQueue(construction, true)
+    }
+
+    private fun canRemoveAllQueues() = anyCity { it.isBeingConstructedOrEnqueued(constructionName) }
+    private fun removeAllQueues() = forAllCities { it.removeAllByName(constructionName) }
+}

core/src/com/unciv/ui/popups/UnitUpgradeMenu.kt

@@ -1,6 +1,7 @@
 package com.unciv.ui.popups
 
 import com.badlogic.gdx.math.Vector2
+import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.Stage
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.logic.map.mapunit.MapUnit
@@ -30,12 +31,12 @@ import com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsUpgrade
  */
 class UnitUpgradeMenu(
     stage: Stage,
-    position: Vector2,
+    positionNextTo: Actor,
     private val unit: MapUnit,
     private val unitAction: UpgradeUnitAction,
     private val callbackAfterAnimation: Boolean = false,
     private val onButtonClicked: () -> Unit
-) : AnimatedMenuPopup(stage, position) {
+) : AnimatedMenuPopup(stage, getActorTopRight(positionNextTo)) {
 
     private val allUpgradableUnits: Sequence<MapUnit> by lazy {
         unit.civ.units.getCivUnits()

core/src/com/unciv/ui/screens/cityscreen/CityConstructionsTable.kt

@@ -10,14 +10,14 @@ import com.badlogic.gdx.utils.Align
 import com.unciv.Constants
 import com.unciv.logic.city.City
 import com.unciv.logic.city.CityConstructions
+import com.unciv.logic.map.tile.Tile
+import com.unciv.models.UncivSound
+import com.unciv.models.ruleset.Building
 import com.unciv.models.ruleset.IConstruction
 import com.unciv.models.ruleset.INonPerpetualConstruction
 import com.unciv.models.ruleset.PerpetualConstruction
 import com.unciv.models.ruleset.RejectionReason
 import com.unciv.models.ruleset.RejectionReasonType
-import com.unciv.logic.map.tile.Tile
-import com.unciv.models.UncivSound
-import com.unciv.models.ruleset.Building
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.models.stats.Stat
@@ -34,15 +34,17 @@ import com.unciv.ui.components.extensions.darken
 import com.unciv.ui.components.extensions.disable
 import com.unciv.ui.components.extensions.getConsumesAmountString
 import com.unciv.ui.components.extensions.isEnabled
-import com.unciv.ui.components.input.onActivation
-import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.packIfNeeded
 import com.unciv.ui.components.extensions.surroundWithCircle
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
 import com.unciv.ui.components.input.KeyboardBinding
 import com.unciv.ui.components.input.keyShortcuts
+import com.unciv.ui.components.input.onActivation
+import com.unciv.ui.components.input.onClick
+import com.unciv.ui.components.input.onRightClick
 import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.popups.CityScreenConstructionMenu
 import com.unciv.ui.popups.ConfirmPopup
 import com.unciv.ui.popups.Popup
 import com.unciv.ui.popups.closeAllPopups
@@ -206,7 +208,7 @@ class CityConstructionsTable(private val cityScreen: CityScreen) {
         for (entry in constructionsSequence.filter { it.shouldBeDisplayed(cityConstructions) }) {
 
             val useStoredProduction = entry is Building || !cityConstructions.isBeingConstructedOrEnqueued(entry.name)
-            val buttonText = cityConstructions.getTurnsToConstructionString(entry.name, useStoredProduction).trim()
+            val buttonText = cityConstructions.getTurnsToConstructionString(entry, useStoredProduction).trim()
             val resourcesRequired = entry.getResourceRequirementsPerTurn()
             val mostImportantRejection =
                     entry.getRejectionReasons(cityConstructions)
@@ -308,21 +310,16 @@ class CityConstructionsTable(private val cityScreen: CityScreen) {
 
         val table = Table()
         table.align(Align.left).pad(5f)
-        table.background = BaseScreen.skinStrings.getUiBackground("CityScreen/CityConstructionTable/QueueEntry", tintColor = Color.BLACK)
+        highlightQueueEntry(table, constructionQueueIndex == selectedQueueEntry)
-
-        if (constructionQueueIndex == selectedQueueEntry)
-            table.background = BaseScreen.skinStrings.getUiBackground(
-                "CityScreen/CityConstructionTable/QueueEntrySelected",
-                tintColor = Color.GREEN.darken(0.5f)
-            )
 
+        val construction = cityConstructions.getConstruction(constructionName)
         val isFirstConstructionOfItsKind = cityConstructions.isFirstConstructionOfItsKind(constructionQueueIndex, constructionName)
 
         var text = constructionName.tr(true) +
                 if (constructionName in PerpetualConstruction.perpetualConstructionsMap) "\n∞"
-                else cityConstructions.getTurnsToConstructionString(constructionName, isFirstConstructionOfItsKind)
+                else cityConstructions.getTurnsToConstructionString(construction, isFirstConstructionOfItsKind)
 
-        val constructionResource = cityConstructions.getConstruction(constructionName).getResourceRequirementsPerTurn()
+        val constructionResource = construction.getResourceRequirementsPerTurn()
         for ((resourceName, amount) in constructionResource) {
             val resource = cityConstructions.city.getRuleset().tileResources[resourceName] ?: continue
             text += "\n" + resourceName.getConsumesAmountString(amount, resource.isStockpiled()).tr()
@@ -345,15 +342,40 @@ class CityConstructionsTable(private val cityScreen: CityScreen) {
         else table.add().right()
 
         table.touchable = Touchable.enabled
-        table.onClick {
+
+        fun selectQueueEntry(onBeforeUpdate: () -> Unit) {
             cityScreen.selectConstruction(constructionName)
             selectedQueueEntry = constructionQueueIndex
-            cityScreen.update()
+            onBeforeUpdate()
+            cityScreen.update()  // Not before CityScreenConstructionMenu or table will have no parent to get stage coords
             ensureQueueEntryVisible()
         }
+
+        table.onClick { selectQueueEntry {} }
+        if (cityScreen.canCityBeChanged())
+            table.onRightClick { selectQueueEntry {
+                CityScreenConstructionMenu(cityScreen.stage, table, cityScreen.city, construction) {
+                    cityScreen.update()
+                }
+            } }
+
         return table
     }
 
+    private fun highlightQueueEntry(queueEntry: Table, highlight: Boolean) {
+        queueEntry.background =
+            if (highlight)
+                BaseScreen.skinStrings.getUiBackground(
+                    "CityScreen/CityConstructionTable/QueueEntrySelected",
+                    tintColor = Color.GREEN.darken(0.5f)
+                )
+            else
+                BaseScreen.skinStrings.getUiBackground(
+                    "CityScreen/CityConstructionTable/QueueEntry",
+                    tintColor = Color.BLACK
+                )
+    }
+
     private fun getProgressBar(constructionName: String): Group {
         val cityConstructions = cityScreen.city.cityConstructions
         val construction = cityConstructions.getConstruction(constructionName)
@@ -368,22 +390,14 @@ class CityConstructionsTable(private val cityScreen: CityScreen) {
 
     private fun getConstructionButton(constructionButtonDTO: ConstructionButtonDTO): Table {
         val construction = constructionButtonDTO.construction
-        val pickConstructionButton = Table().apply { isTransform = false }
+        val pickConstructionButton = Table().apply {
-
+            isTransform = false
-        pickConstructionButton.align(Align.left).pad(5f)
+            align(Align.left).pad(5f)
-        pickConstructionButton.background = BaseScreen.skinStrings.getUiBackground(
+            touchable = Touchable.enabled
-            "CityScreen/CityConstructionTable/PickConstructionButton",
-            tintColor = Color.BLACK
-        )
-        pickConstructionButton.touchable = Touchable.enabled
-
-        if (!isSelectedQueueEntry() && cityScreen.selectedConstruction == construction) {
-            pickConstructionButton.background = BaseScreen.skinStrings.getUiBackground(
-                "CityScreen/CityConstructionTable/PickConstructionButtonSelected",
-                tintColor = Color.GREEN.darken(0.5f)
-            )
         }
 
+        highlightConstructionButton(pickConstructionButton, !isSelectedQueueEntry() && cityScreen.selectedConstruction == construction)
+
         val icon = ImageGetter.getConstructionPortrait(construction.name, 40f)
         pickConstructionButton.add(getProgressBar(construction.name)).padRight(5f)
         pickConstructionButton.add(icon).padRight(10f)
@@ -444,14 +458,68 @@ class CityConstructionsTable(private val cityScreen: CityScreen) {
                 addConstructionToQueue(construction, cityScreen.city.cityConstructions)
             } else {
                 cityScreen.selectConstruction(construction)
+                highlightConstructionButton(pickConstructionButton, true, true)  // without, will highlight but with visible delay
             }
             selectedQueueEntry = -1
             cityScreen.update()
         }
 
+        if (!cityScreen.canCityBeChanged()) return pickConstructionButton
+
+        pickConstructionButton.onRightClick {
+            if (cityScreen.selectedConstruction != construction) {
+                // Ensure context is visible
+                cityScreen.selectConstruction(construction)
+                highlightConstructionButton(pickConstructionButton, true, true)
+                cityScreen.updateWithoutConstructionAndMap()
+            }
+            CityScreenConstructionMenu(cityScreen.stage, pickConstructionButton, cityScreen.city, construction) {
+                cityScreen.update()
+            }
+        }
         return pickConstructionButton
     }
 
+    private fun highlightConstructionButton(
+        pickConstructionButton: Table,
+        highlight: Boolean,
+        clearOthers: Boolean = false
+    ) {
+        val unselected by lazy {
+            // Lazy because possibly not needed (highlight true, clearOthers false) and slightly costly
+            BaseScreen.skinStrings.getUiBackground(
+                "CityScreen/CityConstructionTable/PickConstructionButton",
+                tintColor = Color.BLACK
+            )
+        }
+
+        pickConstructionButton.background =
+            if (highlight)
+                BaseScreen.skinStrings.getUiBackground(
+                    "CityScreen/CityConstructionTable/PickConstructionButtonSelected",
+                    tintColor = Color.GREEN.darken(0.5f)
+                )
+            else unselected
+
+        if (!clearOthers) return
+        // Using knowledge about Widget hierarchy - Making the Buttons their own class might be a better design.
+        for (categoryExpander in availableConstructionsTable.children.filterIsInstance<ExpanderTab>()) {
+            if (!categoryExpander.isOpen) continue
+            for (button in categoryExpander.innerTable.children.filterIsInstance<Table>()) {
+                if (button == pickConstructionButton) continue
+                button.background = unselected
+            }
+        }
+
+        if (!isSelectedQueueEntry()) return
+        // Same as above but worse - both buttons and headers are typed `Table`
+        for (button in constructionsQueueTable.children.filterIsInstance<Table>()) {
+            if (button.children.size == 1) continue // Skip headers, they only have 1 Label
+            highlightQueueEntry(button, false)
+        }
+        selectedQueueEntry = -1
+    }
+
     private fun isSelectedQueueEntry(): Boolean = selectedQueueEntry >= 0
 
     private fun cannotAddConstructionToQueue(construction: IConstruction, city: City, cityConstructions: CityConstructions): Boolean {

core/src/com/unciv/ui/screens/cityscreen/ConstructionInfoTable.kt

@@ -5,19 +5,19 @@ import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.ui.Label
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.UncivGame
-import com.unciv.models.ruleset.IConstruction
-import com.unciv.models.ruleset.PerpetualConstruction
-import com.unciv.models.ruleset.PerpetualStatConversion
 import com.unciv.models.UncivSound
 import com.unciv.models.ruleset.Building
+import com.unciv.models.ruleset.IConstruction
 import com.unciv.models.ruleset.IRulesetObject
+import com.unciv.models.ruleset.PerpetualConstruction
+import com.unciv.models.ruleset.PerpetualStatConversion
 import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.models.translations.tr
 import com.unciv.ui.components.Fonts
 import com.unciv.ui.components.extensions.darken
 import com.unciv.ui.components.extensions.disable
-import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.components.input.onClick
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.popups.ConfirmPopup
 import com.unciv.ui.popups.closeAllPopups
@@ -74,7 +74,7 @@ class ConstructionInfoTable(val cityScreen: CityScreen): Table() {
             val specialConstruction = PerpetualConstruction.perpetualConstructionsMap[construction.name]
 
             buildingText += specialConstruction?.getProductionTooltip(city)
-                    ?: cityConstructions.getTurnsToConstructionString(construction.name)
+                    ?: cityConstructions.getTurnsToConstructionString(construction)
 
             add(Label(buildingText, BaseScreen.skin)).row()  // already translated
 

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

@@ -6,8 +6,6 @@ import com.badlogic.gdx.scenes.scene2d.Action
 import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.Group
 import com.badlogic.gdx.scenes.scene2d.actions.Actions
-import com.badlogic.gdx.scenes.scene2d.ui.Cell
-import com.badlogic.gdx.scenes.scene2d.ui.Image
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.utils.Align
 import com.unciv.Constants
@@ -267,8 +265,7 @@ class UnitOverviewTab(
                 val upgradeIcon = ImageGetter.getUnitIcon(unitToUpgradeTo.name,
                     if (enable) Color.GREEN else Color.GREEN.darken(0.5f))
                 if (enable) upgradeIcon.onClick {
-                    val pos = upgradeIcon.localToStageCoordinates(Vector2(upgradeIcon.width/2, upgradeIcon.height/2))
+                    UnitUpgradeMenu(overviewScreen.stage, upgradeIcon, unit, unitAction) {
-                    UnitUpgradeMenu(overviewScreen.stage, pos, unit, unitAction) {
                         unitListTable.updateUnitListTable()
                         select(selectKey)
                     }

core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsTable.kt

@@ -1,7 +1,6 @@
 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.unciv.GUI
@@ -10,9 +9,7 @@ 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.UncivTooltip.Companion.addTooltip
 import com.unciv.ui.components.extensions.disable
-import com.unciv.ui.components.input.keyShortcuts
 import com.unciv.ui.components.input.onActivation
 import com.unciv.ui.components.input.onRightClick
 import com.unciv.ui.images.IconTextButton
@@ -29,8 +26,7 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
             val button = getUnitActionButton(unit, unitAction)
             if (unitAction is UpgradeUnitAction) {
                 button.onRightClick {
-                    val pos = button.localToStageCoordinates(Vector2(button.width, button.height))
+                    UnitUpgradeMenu(worldScreen.stage, button, unit, unitAction, callbackAfterAnimation = true) {
-                    UnitUpgradeMenu(worldScreen.stage, pos, unit, unitAction, callbackAfterAnimation = true) {
                         worldScreen.shouldUpdate = true
                     }
                 }