UnitTable: show a summary when no unit is selected (#12832)

* Show a summary when no unit is selected. Allows to start cycling units any time. Refactor UnitTable to have separate Presenters for units, cities, spies and no selection.

* fix SummaryPresenter hiding close button

---------

Co-authored-by: M. Rittweger <m.rittweger@mvolution.de>

android/assets/jsons/translations/template.properties

@@ -1173,7 +1173,8 @@ turns =
 turn = 
 Next unit = 
 [amount] units idle = 
-[amount] units due = 
+[idleCount] idle = 
+[waitingCount] waiting = 
 Fog of War = 
 Pick a policy = 
 Move Spies = 

core/src/com/unciv/ui/components/widgets/ExpanderTab.kt

@@ -55,8 +55,14 @@ class ExpanderTab(
         val persistedStates = HashMap<String, Boolean>()
     }
 
-    val header = Table(skin)  // Header with label and icon, touchable to show/hide
+    /** Header with label, [headerContent] and icon, touchable to show/hide.
+     *  This internal container is public to allow e.g. alignment changes.
+     */
+    val header = Table(skin)
+
+    /** Additional elements can be added to the `ExpanderTab`'s header using this container, empty by default. */
     val headerContent = Table()
+    
     private val headerLabel = title.toLabel(fontSize = fontSize, hideIcons = true)
     private val headerIcon = ImageGetter.getImage(arrowImage)
     private val contentWrapper = Table()  // Wrapper for innerTable, this is what will be shown/hidden

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

@@ -210,7 +210,7 @@ class CityConstructionsTable(private val cityScreen: CityScreen) {
             constructionsQueueTable.add(getQueueEntry(0, currentConstruction))
                     .expandX().fillX().row()
         else
-            constructionsQueueTable.add("Pick a construction".toLabel()).pad(2f).row()
+            constructionsQueueTable.add("Pick a construction".toLabel()).height(50f).pad(2f).row()
 
         // always show queue expander, even when empty, in order to keep lowerTable at constant position
         queueExpander.innerTable.clear()

core/src/com/unciv/ui/screens/worldscreen/WorldScreen.kt

@@ -48,7 +48,6 @@ import com.unciv.ui.screens.savescreens.LoadGameScreen
 import com.unciv.ui.screens.savescreens.QuickSave
 import com.unciv.ui.screens.savescreens.SaveGameScreen
 import com.unciv.ui.screens.victoryscreen.VictoryScreen
-import com.unciv.ui.screens.worldscreen.worldmap.WorldMapTileUpdater.updateTiles
 import com.unciv.ui.screens.worldscreen.bottombar.BattleTable
 import com.unciv.ui.screens.worldscreen.bottombar.TileInfoTable
 import com.unciv.ui.screens.worldscreen.mainmenu.WorldScreenMusicPopup
@@ -63,6 +62,7 @@ import com.unciv.ui.screens.worldscreen.unit.AutoPlay
 import com.unciv.ui.screens.worldscreen.unit.UnitTable
 import com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsTable
 import com.unciv.ui.screens.worldscreen.worldmap.WorldMapHolder
+import com.unciv.ui.screens.worldscreen.worldmap.WorldMapTileUpdater.updateTiles
 import com.unciv.utils.Concurrency
 import com.unciv.utils.debug
 import com.unciv.utils.launchOnGLThread
@@ -764,15 +764,13 @@ class WorldScreen(
         // Deselect Unit
         if (bottomUnitTable.selectedUnit != null) {
             bottomUnitTable.selectUnit()
-            bottomUnitTable.isVisible = false
             shouldUpdate = true
             return
         }
 
         // Deselect city
         if (bottomUnitTable.selectedCity != null) {
-            bottomUnitTable.selectedCity = null
-            bottomUnitTable.isVisible = false
+            bottomUnitTable.selectUnit()
             shouldUpdate = true
             return
         }

core/src/com/unciv/ui/screens/worldscreen/status/NextTurnAction.kt

@@ -217,10 +217,7 @@ enum class NextTurnAction(protected val text: String, val color: Color) {
         private fun getIdleUnitsText(worldScreen: WorldScreen): String? {
             val count = worldScreen.viewingCiv.units.getDueUnits().count()
             if (count > 0) {
-                return if (worldScreen.game.settings.checkForDueUnitsCycles)
-                    "[$count] units idle"
-                else
-                    "[$count] units due"
+                return "[$count] units idle"
             }
             return null
         }

core/src/com/unciv/ui/screens/worldscreen/unit/IdleUnitButton.kt

@@ -6,19 +6,18 @@ import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.utils.Align
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.ui.components.UncivTooltip.Companion.addTooltip
-import com.unciv.ui.images.ImageGetter
-import com.unciv.ui.screens.worldscreen.worldmap.WorldMapHolder
-import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.pad
 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.images.ImageGetter
+import com.unciv.ui.screens.worldscreen.worldmap.WorldMapHolder
 
 class IdleUnitButton (
-    internal val unitTable: UnitTable,
+    private val unitTable: UnitTable,
     private val tileMapHolder: WorldMapHolder,
     val previous: Boolean,
-    private val keyShortcutBind: KeyboardBinding
+    keyShortcutBind: KeyboardBinding
 ) : Table() {
 
     val image = ImageGetter.getImage("OtherIcons/BackArrow")

core/src/com/unciv/ui/screens/worldscreen/unit/UnitTable.kt

@@ -1,92 +1,103 @@
 package com.unciv.ui.screens.worldscreen.unit
 
 import com.badlogic.gdx.Gdx
-import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.math.Vector2
 import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.Touchable
 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.logic.battle.CityCombatant
 import com.unciv.logic.city.City
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.logic.map.tile.Tile
 import com.unciv.models.Spy
-import com.unciv.models.translations.tr
-import com.unciv.ui.components.extensions.*
-import com.unciv.ui.components.fonts.Fonts
+import com.unciv.ui.components.extensions.addRoundCloseButton
+import com.unciv.ui.components.extensions.addSeparator
+import com.unciv.ui.components.extensions.center
+import com.unciv.ui.components.extensions.darken
+import com.unciv.ui.components.extensions.isShiftKeyPressed
+import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.input.KeyboardBinding
 import com.unciv.ui.components.input.keyShortcuts
 import com.unciv.ui.components.input.onClick
-import com.unciv.ui.components.widgets.UnitIconGroup
-import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.images.padTopDescent
 import com.unciv.ui.screens.basescreen.BaseScreen
-import com.unciv.ui.screens.pickerscreens.CityRenamePopup
-import com.unciv.ui.screens.pickerscreens.PromotionPickerScreen
-import com.unciv.ui.screens.pickerscreens.UnitRenamePopup
 import com.unciv.ui.screens.worldscreen.WorldScreen
+import com.unciv.ui.screens.worldscreen.unit.presenter.CityPresenter
+import com.unciv.ui.screens.worldscreen.unit.presenter.SpyPresenter
+import com.unciv.ui.screens.worldscreen.unit.presenter.SummaryPresenter
+import com.unciv.ui.screens.worldscreen.unit.presenter.UnitPresenter
+import java.awt.Label
 
 class UnitTable(val worldScreen: WorldScreen) : Table() {
     private val prevIdleUnitButton =
         IdleUnitButton(this, worldScreen.mapHolder, true, KeyboardBinding.PrevIdleButton)
     private val nextIdleUnitButton =
         IdleUnitButton(this, worldScreen.mapHolder, false, KeyboardBinding.NextIdleButton)
-    private val unitIconHolder = Table()
-    private val unitNameLabel = "".toLabel(fontSize = 24)
-    private val unitIconNameGroup = Table()
-    private val promotionsTable = Table().apply { defaults().padRight(5f) }
-    private val unitDescriptionTable = Table(BaseScreen.skin)
-    private val deselectUnitButton: Actor
+    internal val unitIconHolder = Table()
+    internal val unitNameLabel = "".toLabel(fontSize = 24).apply { setAlignment(Label.CENTER) }
+    internal val unitIconNameGroup = Table()
+    internal val promotionsTable = Table().apply { defaults().padRight(5f) }
+    internal val descriptionTable = Table(BaseScreen.skin)
+    internal val closeButton: Actor
+    internal val separator: Actor
 
-    val selectedUnit : MapUnit?
-        get() = selectedUnits.firstOrNull()
-    /** This is in preparation for multi-select and multi-move  */
-    val selectedUnits = ArrayList<MapUnit>()
+    /**
+     * The unit table shows infos of selected units, cities, spies, and a summary if none of these
+     * are selected. Each one of them have their own presenters to show their data.
+     */
+    private var presenter: Presenter
 
-    // Whether the (first) selected unit is in unit-swapping mode
-    var selectedUnitIsSwapping = false
+    private val unitPresenter = UnitPresenter(this, worldScreen)
+    private val cityPresenter = CityPresenter(this, unitPresenter)
+    private val spyPresenter = SpyPresenter(this)
+    private val summaryPresenter = SummaryPresenter(this)
 
-    // Whether the (first) selected unit is in road-connecting mode
-    var selectedUnitIsConnectingRoad = false
-
-    /** Sending no unit clears the selected units entirely */
-    fun selectUnit(unit: MapUnit? = null, append: Boolean = false) {
-        if (!append) selectedUnits.clear()
-        selectedCity = null
-        if (unit != null) {
-            selectedUnits.add(unit)
-            unit.actionsOnDeselect()
-        }
-        selectedUnitIsSwapping = false
-        selectedUnitIsConnectingRoad = false
-        selectedSpy = null
-    }
-
-    var selectedCity : City? = null
 
     // This is so that not on every update(), we will update the unit table.
     // Most of the time it's the same unit with the same stats so why waste precious time?
-    var selectedUnitHasChanged = false
-    val separator: Actor
-
-    var selectedSpy: Spy? = null
-
-    fun selectSpy(spy: Spy?) {
-        selectedSpy = spy
-        selectedCity = null
-        selectedUnits.clear()
-        selectedUnitIsSwapping = false
-        selectedUnitIsConnectingRoad = false
-    }
+    var shouldUpdate = false
 
     private var bg = Image(
-        BaseScreen.skinStrings.getUiBackground("WorldScreen/UnitTable",
-        BaseScreen.skinStrings.roundedEdgeRectangleMidShape,
-        BaseScreen.skinStrings.skinConfig.baseColor.darken(0.5f)))
+        BaseScreen.skinStrings.getUiBackground(
+            "WorldScreen/UnitTable",
+            BaseScreen.skinStrings.roundedEdgeRectangleMidShape,
+            BaseScreen.skinStrings.skinConfig.baseColor.darken(0.5f)
+        )
+    )
 
+    val selectedUnit: MapUnit?
+        get() = presenter.let { if (it is UnitPresenter) it.selectedUnit else null }
+    
+    val selectedCity: City?
+        get() = presenter.let { if (it is CityPresenter) it.selectedCity else null }
+
+    val selectedSpy: Spy?
+        get() = presenter.let { if (it is SpyPresenter) it.selectedSpy else null }
+
+    val selectedUnits: List<MapUnit>
+        get() = unitPresenter.selectedUnits
+
+    var selectedUnitIsSwapping: Boolean
+        get() = unitPresenter.selectedUnitIsSwapping
+        set(value) { unitPresenter.selectedUnitIsSwapping = value }
+
+    var selectedUnitIsConnectingRoad: Boolean
+        get() = unitPresenter.selectedUnitIsConnectingRoad
+        set(value) { unitPresenter.selectedUnitIsConnectingRoad = value }
+
+    var nameLabelText: String
+        get() = unitNameLabel.text.toString()
+        set(value) {
+            if (nameLabelText != value) {
+                unitNameLabel.setText(value)
+                // We need to reload the health bar of the unit in the icon - happens e.g. when picking the Heal Instantly promotion
+                shouldUpdate = true
+            }
+        }
 
     init {
+        presenter = summaryPresenter
+
         pad(5f)
         touchable = Touchable.enabled
         background = BaseScreen.skinStrings.getUiBackground(
@@ -96,12 +107,11 @@ class UnitTable(val worldScreen: WorldScreen) : Table() {
 
         promotionsTable.touchable = Touchable.enabled
 
-        deselectUnitButton = addRoundCloseButton(this) {
+        closeButton = addRoundCloseButton(this) {
             selectUnit()
             worldScreen.shouldUpdate = true
-            this@UnitTable.isVisible = false
         }
-        deselectUnitButton.keyShortcuts.clear() // This is the only place we don't want the BACK keyshortcut getCloseButton assigns
+        closeButton.keyShortcuts.clear() // This is the only place we don't want the BACK keyshortcut getCloseButton assigns
 
         add(Table().apply {
             val moveBetweenUnitsTable = Table().apply {
@@ -117,222 +127,81 @@ class UnitTable(val worldScreen: WorldScreen) : Table() {
 
             separator = addSeparator().padBottom(5f).actor!!
             add(promotionsTable).row()
-            add(unitDescriptionTable)
+            add(descriptionTable)
             touchable = Touchable.enabled
             onClick {
-                val position = selectedUnit?.currentTile?.position
-                    ?: selectedCity?.location
-                if (position != null)
-                    worldScreen.mapHolder.setCenterPosition(position, immediately = false, selectUnit = false)
+                presenter.position?.let {
+                    worldScreen.mapHolder.setCenterPosition(
+                        it,
+                        immediately = false,
+                        selectUnit = false
+                    )
+                }
             }
         }).expand()
 
     }
 
-    fun update() {
-        if (selectedUnit != null) {
-            isVisible = true
-            if (selectedUnit!!.civ != worldScreen.viewingCiv && !worldScreen.viewingCiv.isSpectator()) { // The unit that was selected, was captured. It exists but is no longer ours.
-                selectUnit()
-                selectedUnitHasChanged = true
-            } else if (selectedUnit!! !in selectedUnit!!.getTile().getUnits()) { // The unit that was there no longer exists
-                selectUnit()
-                selectedUnitHasChanged = true
-            }
-        }
 
-        if (worldScreen.viewingCiv.units.getIdleUnits().any()) { // more efficient to do this check once for both
+    /** Sending no unit clears the selected units entirely */
+    fun selectUnit(unit: MapUnit? = null, append: Boolean = false) {
+        presenter = if (unit != null) unitPresenter else summaryPresenter
+        unitPresenter.selectUnit(unit, append)
+        resetUnitTable()
+    }
+
+    fun selectSpy(spy: Spy?) {
+        presenter = spyPresenter
+        spyPresenter.selectSpy(spy)
+        resetUnitTable()
+    }
+
+    fun citySelected(city: City): Boolean {
+        presenter = cityPresenter
+        return cityPresenter.selectCity(city).also {
+            resetUnitTable()
+            worldScreen.shouldUpdate = true
+        }
+    }
+
+
+    fun update() {
+        closeButton.isVisible = true
+        presenter.update()
+
+        // more efficient to do this check once for both
+        if (worldScreen.viewingCiv.units.getIdleUnits().any()) {
             prevIdleUnitButton.enable()
             nextIdleUnitButton.enable()
         } else {
             prevIdleUnitButton.disable()
             nextIdleUnitButton.disable()
         }
+        
+        if (!shouldUpdate) return
 
-        if (selectedUnit != null) { // set texts - this is valid even when it's the same unit, because movement points and health change
-            if (selectedUnits.size == 1) { //single selected unit
-                separator.isVisible = true
-                val unit = selectedUnit!!
-                val nameLabelText = buildNameLabelText(unit)
-                if (nameLabelText != unitNameLabel.text.toString()) {
-                    unitNameLabel.setText(nameLabelText)
-                    selectedUnitHasChanged = true // We need to reload the health bar of the unit in the icon - happens e.g. when picking the Heal Instantly promotion
-                }
+        resetUnitTable()
 
-                unitNameLabel.clearListeners()
-                unitNameLabel.onClick {
-                    if (!worldScreen.canChangeState) return@onClick
-                    UnitRenamePopup(
-                        screen = worldScreen,
-                        unit = unit,
-                        actionOnClose = {
-                            unitNameLabel.setText(buildNameLabelText(unit))
-                            selectedUnitHasChanged = true
-                        }
-                    )
-                }
-
-                unitDescriptionTable.clear()
-                unitDescriptionTable.defaults().pad(2f)
-                unitDescriptionTable.add(Fonts.movement + unit.getMovementString()).padRight(10f)
-
-                if (!unit.isCivilian())
-                    unitDescriptionTable.add(Fonts.strength + unit.baseUnit.strength.tr()).padRight(10f)
-
-                if (unit.baseUnit.rangedStrength != 0)
-                    unitDescriptionTable.add(Fonts.rangedStrength + unit.baseUnit.rangedStrength.tr()).padRight(10f)
-
-                if (unit.baseUnit.isRanged())
-                    unitDescriptionTable.add(Fonts.range + unit.getRange().tr()).padRight(10f)
-
-                val interceptionRange = unit.getInterceptionRange()
-                if (interceptionRange > 0) {
-                    unitDescriptionTable.add(ImageGetter.getStatIcon("InterceptRange")).size(20f)
-                    unitDescriptionTable.add(interceptionRange.tr()).padRight(10f)
-                }
-
-                if (!unit.isCivilian()) {
-                    unitDescriptionTable.add("XP".toLabel().apply {
-                        onClick {
-                            if (selectedUnit == null) return@onClick
-                            worldScreen.game.pushScreen(PromotionPickerScreen(unit))
-                        }
-                    })
-                    unitDescriptionTable.add(unit.promotions.XP.tr() + "/" + unit.promotions.xpForNextPromotion().tr())
-                }
-
-                if (unit.baseUnit.religiousStrength > 0) {
-                    unitDescriptionTable.add(ImageGetter.getStatIcon("ReligiousStrength")).size(20f)
-                    unitDescriptionTable.add((unit.baseUnit.religiousStrength - unit.religiousStrengthLost).tr())
-                }
-
-                if (unit.promotions.promotions.size != promotionsTable.children.size) // The unit has been promoted! Reload promotions!
-                    selectedUnitHasChanged = true
-            } else { // multiple selected units
-                unitNameLabel.setText("")
-                unitDescriptionTable.clear()
-            }
-        } else if (selectedCity != null) {
-            isVisible = true
-            separator.isVisible = true
-            val city = selectedCity!!
-            var nameLabelText = city.name.tr()
-            if (city.health < city.getMaxHealth()) nameLabelText += " (${city.health.tr()})"
-            unitNameLabel.setText(nameLabelText)
-
-            unitNameLabel.clearListeners()
-            unitNameLabel.onClick {
-                if (!worldScreen.canChangeState) return@onClick
-                CityRenamePopup(
-                    screen = worldScreen,
-                    city = city,
-                    actionOnClose = {
-                        unitNameLabel.setText(city.name.tr())
-                        worldScreen.shouldUpdate = true
-                    })
-            }
-
-            unitDescriptionTable.clear()
-            unitDescriptionTable.defaults().pad(2f).padRight(5f)
-            unitDescriptionTable.add("Strength".tr())
-            unitDescriptionTable.add(CityCombatant(city).getDefendingStrength().tr()).row()
-            unitDescriptionTable.add("Bombard strength".tr())
-            unitDescriptionTable.add(CityCombatant(city).getAttackingStrength().tr()).row()
-
-            selectedUnitHasChanged = true
-        } else if (selectedSpy != null) {
-            val spy = selectedSpy!!
-            isVisible = true
-            unitNameLabel.clearListeners()
-            unitNameLabel.setText(spy.name)
-            unitDescriptionTable.clear()
-
-            unitIconHolder.clear()
-            unitIconHolder.add (ImageGetter.getImage("OtherIcons/Spy_White").apply {
-                color = Color.WHITE
-            }).size(30f)
-
-            separator.isVisible = true
-            val color = when(spy.rank) {
-                1 -> Color.BROWN
-                2 -> Color.LIGHT_GRAY
-                3 -> Color.GOLD
-                else -> ImageGetter.CHARCOAL
-            }
-            repeat(spy.rank) {
-                val star = ImageGetter.getImage("OtherIcons/Star")
-                star.color = color
-                unitDescriptionTable.add(star).size(20f).pad(1f)
-            }
-        } else {
-            isVisible = false
-        }
-
-        if (!selectedUnitHasChanged) return
+        presenter.updateWhenNeeded()
+        
+        pack()
+        closeButton.setPosition(
+            width - closeButton.width * 3 / 4,
+            height - closeButton.height * 3 / 4
+        )
+        closeButton.toFront()
+        bg.setSize(width - 3f, height - 3f)
+        bg.center(this)
+        shouldUpdate = false
+    }
 
+    private fun resetUnitTable() {
         unitIconHolder.clear()
         promotionsTable.clear()
-        unitDescriptionTable.clearListeners()
-        separator.width = 0f  // ImageWithCustomSize remembers width and returns if when Table asks for prefWidth
-
-        if (selectedUnit != null) {
-            if (selectedUnits.size == 1) { // single selected unit
-                unitIconHolder.add(UnitIconGroup(selectedUnit!!, 30f)).pad(5f)
-
-                for (promotion in selectedUnit!!.promotions.getPromotions(true))
-                    promotionsTable.add(ImageGetter.getPromotionPortrait(promotion.name, 20f)).padBottom(2f)
-                
-                for (status in selectedUnit!!.statuses) {
-                    val group = ImageGetter.getPromotionPortrait(status.name)
-                    val turnsLeft = "${status.turnsLeft}${Fonts.turn}".toLabel(fontSize = 8).surroundWithCircle(15f, color = ImageGetter.CHARCOAL)
-                    group.addActor(turnsLeft)
-                    turnsLeft.setPosition(group.width, 0f, Align.bottomRight)
-                    promotionsTable.add(group).padBottom(2f)
-                }
-
-                // Since Clear also clears the listeners, we need to re-add them every time
-                promotionsTable.onClick {
-                    if (selectedUnit == null || selectedUnit!!.promotions.promotions.isEmpty()) return@onClick
-                    worldScreen.game.pushScreen(PromotionPickerScreen(selectedUnit!!))
-                }
-
-                unitIconHolder.onClick {
-                    worldScreen.openCivilopedia(selectedUnit!!.baseUnit.makeLink())
-                }
-            } else { // multiple selected units
-                for (unit in selectedUnits)
-                    unitIconHolder.add(UnitIconGroup(unit, 30f)).pad(5f)
-            }
-        }
-
-        pack()
-        deselectUnitButton.setPosition(width - deselectUnitButton.width*3/4, height - deselectUnitButton.height*3/4)
-        deselectUnitButton.toFront()
-        bg.setSize(width-3f, height-3f)
-        bg.center(this)
-        selectedUnitHasChanged = false
-    }
-
-    private fun buildNameLabelText(unit: MapUnit) : String {
-        var nameLabelText = unit.displayName().tr(true)
-        if (unit.health < 100) nameLabelText += " (${unit.health.tr()})"
-
-        return nameLabelText
-    }
-
-    fun citySelected(city: City) : Boolean {
-        // If the last selected unit connecting a road, keep it selected. Otherwise, clear.
-        if (selectedUnitIsConnectingRoad) {
-            selectUnit(selectedUnits[0])
-            selectedUnitIsConnectingRoad = true // selectUnit resets this
-        } else {
-            selectUnit()
-        }
-        if (city == selectedCity) return false
-        selectedCity = city
-        selectedUnitHasChanged = true
-        worldScreen.shouldUpdate = true
-        return true
+        descriptionTable.clearListeners()
+        // ImageWithCustomSize remembers width and returns if when Table asks for prefWidth
+        separator.width = 0f
+        shouldUpdate = true
     }
 
     fun tileSelected(selectedTile: Tile, forceSelectUnit: MapUnit? = null) {
@@ -374,28 +243,37 @@ class UnitTable(val worldScreen: WorldScreen) : Table() {
         }
 
         nextUnit = when {
-                curUnit == null -> priorityUnit
-                curUnit == civUnit && milUnit != null && milUnit.isEligible() -> null
-                curUnit == milUnit && civUnit != null && civUnit.isEligible() -> civUnit
-                else -> priorityUnit
-            }
+            curUnit == null -> priorityUnit
+            curUnit == civUnit && milUnit != null && milUnit.isEligible() -> null
+            curUnit == milUnit && civUnit != null && civUnit.isEligible() -> civUnit
+            else -> priorityUnit
+        }
 
 
+        val isCitySelected = selectedTile.isCityCenter()
+            && (selectedTile.getOwner() == worldScreen.viewingCiv || worldScreen.viewingCiv.isSpectator())
+            && !selectedUnitIsConnectingRoad
         when {
-            forceSelectUnit != null ->
-                selectUnit(forceSelectUnit)
-            selectedTile.isCityCenter() &&
-                    (selectedTile.getOwner() == worldScreen.viewingCiv || worldScreen.viewingCiv.isSpectator()) ->
-                citySelected(selectedTile.getCity()!!)
+            forceSelectUnit != null -> selectUnit(forceSelectUnit)
+            isCitySelected -> citySelected(selectedTile.getCity()!!)
             nextUnit != null -> selectUnit(nextUnit, Gdx.input.isShiftKeyPressed())
+            // toggle selection if same unit is clicked again by player
             selectedTile == previouslySelectedUnit?.currentTile -> {
                 selectUnit()
-                isVisible = false
+                shouldUpdate = true
             }
         }
 
         if (selectedUnit != previouslySelectedUnit || selectedUnits.size != previousNumberOfSelectedUnits)
-            selectedUnitHasChanged = true
+            shouldUpdate = true
     }
 
+    interface Presenter {
+        /** map position of the selected entity */
+        val position: Vector2?
+        /** called every time [WorldScreen] is updated */
+        fun update() {}
+        /** only called when [UnitTable.shouldUpdate] is true */
+        fun updateWhenNeeded() {}
+    }
 }

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

@@ -382,12 +382,13 @@ object UnitActions {
             type = UnitActionType.Wait,
             useFrequency = 65f, // Preferably have this on the first page
             action = {
+                unit.due = !unit.due
                 // If it's on, skips to next unit due to worldScreen.switchToNextUnit() in activateAction
                 // We don't want to switch twice since then we skip units :)
-                if (!UncivGame.Current.settings.autoUnitCycle)
+                if (!unit.due && !UncivGame.Current.settings.autoUnitCycle)
                     GUI.getWorldScreen().switchToNextUnit()
-                unit.due = false 
-            }
+            }.takeIf { unit.hasMovement() },
+            isCurrentAction = !unit.due
         ))
     }
 

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

@@ -183,6 +183,6 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() {
         if (unit.isDestroyed || 
             unitAction.type.isSkippingToNextUnit && (!unit.isMoving() || !unit.hasMovement()))
             worldScreen.switchToNextUnit()
-        else worldScreen.bottomUnitTable.selectedUnitHasChanged = true
+        else worldScreen.bottomUnitTable.shouldUpdate = true
     }
 }

core/src/com/unciv/ui/screens/worldscreen/unit/presenter/CityPresenter.kt

@@ -0,0 +1,62 @@
+package com.unciv.ui.screens.worldscreen.unit.presenter
+
+import com.badlogic.gdx.math.Vector2
+import com.unciv.logic.battle.CityCombatant
+import com.unciv.logic.city.City
+import com.unciv.models.translations.tr
+import com.unciv.ui.components.input.onClick
+import com.unciv.ui.screens.pickerscreens.CityRenamePopup
+import com.unciv.ui.screens.worldscreen.unit.UnitTable
+
+class CityPresenter(private val unitTable: UnitTable, private val unitPresenter: UnitPresenter) : UnitTable.Presenter {
+
+    var selectedCity : City? = null
+
+    override val position: Vector2?
+        get() = selectedCity?.location
+
+    fun selectCity(city: City) : Boolean {
+        // If the last selected unit connecting a road, keep it selected. Otherwise, clear.
+        unitPresenter.apply {
+            if (selectedUnitIsConnectingRoad) {
+                selectUnit(selectedUnits[0])
+                selectedUnitIsConnectingRoad = true // selectUnit resets this
+            } else {
+                selectUnit()
+            }
+        }
+        if (city == selectedCity) return false
+        selectedCity = city
+        return true
+    }
+
+    override fun updateWhenNeeded() = with(unitTable) {
+        separator.isVisible = true
+        val city = selectedCity!!
+        var nameLabelText = city.name.tr()
+        if (city.health < city.getMaxHealth()) nameLabelText += " (${city.health.tr()})"
+        unitNameLabel.setText(nameLabelText)
+
+        unitNameLabel.clearListeners()
+        unitNameLabel.onClick {
+            if (!worldScreen.canChangeState) return@onClick
+            CityRenamePopup(
+                screen = worldScreen,
+                city = city,
+                actionOnClose = {
+                    unitNameLabel.setText(city.name.tr())
+                    worldScreen.shouldUpdate = true
+                })
+        }
+
+        descriptionTable.clear()
+        descriptionTable.defaults().pad(2f).padRight(5f)
+        descriptionTable.add("Strength".tr())
+        descriptionTable.add(CityCombatant(city).getDefendingStrength().tr()).row()
+        descriptionTable.add("Bombard strength".tr())
+        descriptionTable.add(CityCombatant(city).getAttackingStrength().tr()).row()
+
+        shouldUpdate = true
+
+    }
+}

core/src/com/unciv/ui/screens/worldscreen/unit/presenter/SpyPresenter.kt

@@ -0,0 +1,44 @@
+package com.unciv.ui.screens.worldscreen.unit.presenter
+
+import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.math.Vector2
+import com.unciv.models.Spy
+import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.screens.worldscreen.unit.UnitTable
+
+class SpyPresenter(private val unitTable: UnitTable) : UnitTable.Presenter {
+
+    var selectedSpy: Spy? = null
+
+    override val position: Vector2?
+        get() = selectedSpy?.getCityOrNull()?.location
+    
+    fun selectSpy(spy: Spy?) {
+        selectedSpy = spy
+    }
+
+    override fun updateWhenNeeded() = with(unitTable) {
+        val spy = selectedSpy!!
+        unitNameLabel.clearListeners()
+        unitNameLabel.setText(spy.name)
+        descriptionTable.clear()
+
+        unitIconHolder.clear()
+        unitIconHolder.add(ImageGetter.getImage("OtherIcons/Spy_White").apply {
+            color = Color.WHITE
+        }).size(30f)
+
+        separator.isVisible = true
+        val color = when (spy.rank) {
+            1 -> Color.BROWN
+            2 -> Color.LIGHT_GRAY
+            3 -> Color.GOLD
+            else -> ImageGetter.CHARCOAL
+        }
+        repeat(spy.rank) {
+            val star = ImageGetter.getImage("OtherIcons/Star")
+            star.color = color
+            descriptionTable.add(star).size(20f).pad(1f)
+        }
+    }
+}

core/src/com/unciv/ui/screens/worldscreen/unit/presenter/SummaryPresenter.kt

@@ -0,0 +1,37 @@
+package com.unciv.ui.screens.worldscreen.unit.presenter
+
+import com.badlogic.gdx.math.Vector2
+import com.unciv.models.translations.tr
+import com.unciv.ui.screens.worldscreen.unit.UnitTable
+
+class SummaryPresenter(private val unitTable: UnitTable) : UnitTable.Presenter {
+    
+    override val position: Vector2? = null
+
+    override fun update() {
+        unitTable.closeButton.isVisible = false
+    }
+    
+    override fun updateWhenNeeded() {
+        unitTable.apply {
+            descriptionTable.clear()
+         
+            unitNameLabel.setText("Units".tr())
+
+            val idleCount = worldScreen.viewingCiv.units.getIdleUnits().count { it.due }
+            val waitingCount = worldScreen.viewingCiv.units.getIdleUnits().count { !it.due }
+            
+            val subText = mutableListOf<String>().apply {
+                if (idleCount > 0) add("[$idleCount] idle".tr())
+                if (waitingCount > 0) add("[$waitingCount] waiting".tr())
+            }.joinToString(", ")
+            
+            if(subText!="") {
+                separator.isVisible = true
+                descriptionTable.add(subText)
+            } else {
+                separator.isVisible = false
+            }
+        }
+    }
+}

core/src/com/unciv/ui/screens/worldscreen/unit/presenter/UnitPresenter.kt

@@ -0,0 +1,158 @@
+package com.unciv.ui.screens.worldscreen.unit.presenter
+
+import com.badlogic.gdx.math.Vector2
+import com.badlogic.gdx.utils.Align
+import com.unciv.logic.map.mapunit.MapUnit
+import com.unciv.models.translations.tr
+import com.unciv.ui.components.extensions.surroundWithCircle
+import com.unciv.ui.components.extensions.toLabel
+import com.unciv.ui.components.fonts.Fonts
+import com.unciv.ui.components.input.onClick
+import com.unciv.ui.components.widgets.UnitIconGroup
+import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.screens.pickerscreens.PromotionPickerScreen
+import com.unciv.ui.screens.pickerscreens.UnitRenamePopup
+import com.unciv.ui.screens.worldscreen.WorldScreen
+import com.unciv.ui.screens.worldscreen.unit.UnitTable
+
+class UnitPresenter(private val unitTable: UnitTable, private val worldScreen: WorldScreen) : UnitTable.Presenter {
+
+    val selectedUnit : MapUnit?
+        get() = selectedUnits.firstOrNull()
+    
+    /** This is in preparation for multi-select and multi-move  */
+    val selectedUnits = ArrayList<MapUnit>()
+
+    // Whether the (first) selected unit is in unit-swapping mode
+    var selectedUnitIsSwapping = false
+
+    // Whether the (first) selected unit is in road-connecting mode
+    var selectedUnitIsConnectingRoad = false
+
+    override val position: Vector2?
+        get() = selectedUnit?.currentTile?.position
+
+    fun selectUnit(unit: MapUnit? = null, append: Boolean = false) {
+        if (!append) selectedUnits.clear()
+        if (unit != null) {
+            selectedUnits.add(unit)
+            unit.actionsOnDeselect()
+        }
+        selectedUnitIsSwapping = false
+        selectedUnitIsConnectingRoad = false
+    }
+
+    override fun update() = selectedUnit?.let { unit ->
+        // The unit that was selected, was captured. It exists but is no longer ours.
+        val captured = unit.civ != worldScreen.viewingCiv && !worldScreen.viewingCiv.isSpectator()
+        // The unit that was there no longer exists
+        val disappeared = unit !in unit.getTile().getUnits()
+        if (captured || disappeared) {
+            unitTable.selectUnit()
+            worldScreen.shouldUpdate = true
+            return
+        }
+
+        // set texts - this is valid even when it's the same unit, because movement points and health change
+        // single selected unit
+        if (selectedUnits.size == 1) with(unitTable) { 
+            separator.isVisible = true
+            nameLabelText = buildNameLabelText(unit)
+            unitNameLabel.clearListeners()
+            unitNameLabel.onClick {
+                if (!worldScreen.canChangeState) return@onClick
+                UnitRenamePopup(
+                    screen = worldScreen,
+                    unit = unit,
+                    actionOnClose = {
+                        unitNameLabel.setText(buildNameLabelText(unit))
+                        shouldUpdate = true
+                    }
+                )
+            }
+
+            descriptionTable.clear()
+            descriptionTable.defaults().pad(2f)
+            descriptionTable.add(Fonts.movement + unit.getMovementString()).padRight(10f)
+
+            if (!unit.isCivilian())
+                descriptionTable.add(Fonts.strength + unit.baseUnit.strength.tr()).padRight(10f)
+
+            if (unit.baseUnit.rangedStrength != 0)
+                descriptionTable.add(Fonts.rangedStrength + unit.baseUnit.rangedStrength.tr()).padRight(10f)
+
+            if (unit.baseUnit.isRanged())
+                descriptionTable.add(Fonts.range + unit.getRange().tr()).padRight(10f)
+
+            val interceptionRange = unit.getInterceptionRange()
+            if (interceptionRange > 0) {
+                descriptionTable.add(ImageGetter.getStatIcon("InterceptRange")).size(20f)
+                descriptionTable.add(interceptionRange.tr()).padRight(10f)
+            }
+
+            if (!unit.isCivilian()) {
+                descriptionTable.add("XP".toLabel().apply {
+                    onClick {
+                        if (selectedUnit == null) return@onClick
+                        worldScreen.game.pushScreen(PromotionPickerScreen(unit))
+                    }
+                })
+                descriptionTable.add(unit.promotions.XP.tr() + "/" + unit.promotions.xpForNextPromotion().tr())
+            }
+
+            if (unit.baseUnit.religiousStrength > 0) {
+                descriptionTable.add(ImageGetter.getStatIcon("ReligiousStrength")).size(20f)
+                descriptionTable.add((unit.baseUnit.religiousStrength - unit.religiousStrengthLost).tr())
+            }
+
+            if (unit.promotions.promotions.size != promotionsTable.children.size) // The unit has been promoted! Reload promotions!
+                shouldUpdate = true
+        } else with(unitTable) { // multiple selected units
+            nameLabelText = ""
+            descriptionTable.clear()
+        }
+
+    } ?: Unit
+
+    override fun updateWhenNeeded() = selectedUnit?.let { unit ->
+        // single selected unit
+        if (selectedUnits.size == 1) with(unitTable) {
+            
+            unitIconHolder.add(UnitIconGroup(unit, 30f)).pad(5f)
+
+            for (promotion in unit.promotions.getPromotions(true))
+                promotionsTable.add(ImageGetter.getPromotionPortrait(promotion.name, 20f))
+                    .padBottom(2f)
+
+            for (status in unit.statuses) {
+                val group = ImageGetter.getPromotionPortrait(status.name)
+                val turnsLeft = "${status.turnsLeft}${Fonts.turn}".toLabel(fontSize = 8)
+                    .surroundWithCircle(15f, color = ImageGetter.CHARCOAL)
+                group.addActor(turnsLeft)
+                turnsLeft.setPosition(group.width, 0f, Align.bottomRight)
+                promotionsTable.add(group).padBottom(2f)
+            }
+
+            // Since Clear also clears the listeners, we need to re-add them every time
+            promotionsTable.onClick {
+                if (selectedUnit == null || unit.promotions.promotions.isEmpty()) return@onClick
+                worldScreen.game.pushScreen(PromotionPickerScreen(unit))
+            }
+
+            unitIconHolder.onClick {
+                worldScreen.openCivilopedia(unit.baseUnit.makeLink())
+            }
+        } else { // multiple selected units
+            for (selectedUnit in selectedUnits)
+                unitTable.unitIconHolder.add(UnitIconGroup(selectedUnit, 30f)).pad(5f)
+        }
+        Unit
+    } ?: Unit
+
+    private fun buildNameLabelText(unit: MapUnit) : String {
+        var nameLabelText = unit.displayName().tr(true)
+        if (unit.health < 100) nameLabelText += " (${unit.health.tr()})"
+        return nameLabelText
+    }
+
+}

core/src/com/unciv/ui/screens/worldscreen/worldmap/OverlayButtonData.kt

@@ -186,7 +186,7 @@ class MoveSpyOverlayButtonData(val spy: Spy, val city: City?) : OverlayButtonDat
                 worldScreen.game.pushScreen(EspionageOverviewScreen(worldScreen.selectedCiv, worldScreen))
             } else {
                 worldScreen.game.pushScreen(EspionageOverviewScreen(worldScreen.selectedCiv, worldScreen))
-                worldScreen.bottomUnitTable.selectedSpy = null
+                worldScreen.bottomUnitTable.selectSpy(null)
             }
             worldMapHolder.removeUnitActionOverlay()
             worldMapHolder.selectedTile = null