Fix popups (#1784)

* Fix community popup not opening

Fixes a regression in b95844d2f4601f726eedac93df3c12d7394f5793. This commit refactored popups and it was thought that the "screen has popup -> don't show popup" was correct for all popups. That assumption was incorrect, the community popup was not opening anymore as well as the game menu popups (editor and normal game) could not be opened over other popups anymore.

This commit fixes that by introducing a queue for popups. When you try to open a popup and one is already open, the popup you tried to open only gets shown when the popup that was already open is closed. This can be manually overridden with a calling the `open` method with a `(force = true)` argument.

Also, all popups are now and should be opened and closed only with their `open()` and `close()` methods to ensure this behavior works.

* Refactor: Remove all open() methods from popup constructors

While it may be a little less to type, it should be up to the caller to decide to open a popup over other popups (via the `force = true` parameter) or not. This is not possible if a popup is opened automatically within its constructor, which is why that is the wrong place to open the popup-

core/src/com/unciv/ui/cityscreen/CityInfoTable.kt

@@ -95,7 +95,7 @@ class CityInfoTable(private val cityScreen: CityScreen) : Table(CameraStageBaseS
                                     cityScreen.city.sellBuilding(building.name)
                                     cityScreen.city.cityStats.update()
                                     cityScreen.update()
-                                }, cityScreen)
+                                }, cityScreen).open()
                     }
                     if (cityScreen.city.hasSoldBuildingThisTurn || cityScreen.city.isPuppet
                             || !UncivGame.Current.worldScreen.isPlayersTurn)

core/src/com/unciv/ui/cityscreen/ConstructionsTable.kt

@@ -14,7 +14,6 @@ import com.unciv.models.UncivSound
 import com.unciv.models.stats.Stat
 import com.unciv.models.translations.tr
 import com.unciv.ui.utils.*
-import com.unciv.ui.utils.YesNoPopup
 
 class ConstructionsTable(val cityScreen: CityScreen) : Table(CameraStageBaseScreen.skin) {
     /* -2 = Nothing, -1 = current construction, >= 0 queue entry */
@@ -294,7 +293,7 @@ class ConstructionsTable(val cityScreen: CityScreen) : Table(CameraStageBaseScre
                     }
                     if (!construction.shouldBeDisplayed(cityConstructions)) cityScreen.selectedConstruction = null
                     cityScreen.update()
-                }, cityScreen)
+                }, cityScreen).open()
             }
 
             if (constructionGoldCost > city.civInfo.gold)

core/src/com/unciv/ui/mapeditor/LoadMapScreen.kt

@@ -12,7 +12,6 @@ import com.unciv.models.translations.tr
 import com.unciv.ui.pickerscreens.PickerScreen
 import com.unciv.ui.saves.Gzip
 import com.unciv.ui.utils.*
-import com.unciv.ui.utils.YesNoPopup
 
 class LoadMapScreen(previousMap: TileMap?) : PickerScreen(){
     var chosenMap = ""
@@ -43,7 +42,7 @@ class LoadMapScreen(previousMap: TileMap?) : PickerScreen(){
 
         val downloadMapButton = TextButton("Download map".tr(), skin)
         downloadMapButton.onClick {
-            MapDownloadPopup(this)
+            MapDownloadPopup(this).open()
         }
         rightSideTable.add(downloadMapButton).row()
 
@@ -70,7 +69,7 @@ class LoadMapScreen(previousMap: TileMap?) : PickerScreen(){
             YesNoPopup("Are you sure you want to delete this map?", {
                 MapSaver().deleteMap(chosenMap)
                 UncivGame.Current.setScreen(LoadMapScreen(previousMap))
-            }, this)
+            }, this).open()
         }
         deleteMapButton.disable()
         deleteMapButton.color = Color.RED

core/src/com/unciv/ui/mapeditor/MapDownloadPopup.kt

@@ -8,9 +8,9 @@ import com.unciv.UncivGame
 import com.unciv.logic.MapSaver
 import com.unciv.ui.saves.Gzip
 import com.unciv.ui.utils.CameraStageBaseScreen
+import com.unciv.ui.utils.Popup
 import com.unciv.ui.utils.onClick
 import com.unciv.ui.worldscreen.mainmenu.DropBox
-import com.unciv.ui.utils.Popup
 import kotlin.concurrent.thread
 
 class MapDownloadPopup(loadMapScreen: LoadMapScreen): Popup(loadMapScreen) {
@@ -39,6 +39,7 @@ class MapDownloadPopup(loadMapScreen: LoadMapScreen): Popup(loadMapScreen) {
                                 val couldNotDownloadMapPopup = Popup(screen)
                                 couldNotDownloadMapPopup.addGoodSizedLabel("Could not download map!").row()
                                 couldNotDownloadMapPopup.addCloseButton()
+                                couldNotDownloadMapPopup.open()
                             }
                         }
                     }
@@ -50,6 +51,5 @@ class MapDownloadPopup(loadMapScreen: LoadMapScreen): Popup(loadMapScreen) {
             addGoodSizedLabel("Could not get list of maps!").row()
         }
         addCloseButton()
-        open()
     }
 }

core/src/com/unciv/ui/mapeditor/MapEditorMenuPopup.kt

@@ -11,9 +11,9 @@ import com.unciv.logic.map.MapType
 import com.unciv.logic.map.RoadStatus
 import com.unciv.models.translations.tr
 import com.unciv.ui.saves.Gzip
+import com.unciv.ui.utils.Popup
 import com.unciv.ui.utils.onClick
 import com.unciv.ui.worldscreen.mainmenu.DropBox
-import com.unciv.ui.utils.Popup
 import kotlin.concurrent.thread
 
 class MapEditorMenuPopup(mapEditorScreen: MapEditorScreen): Popup(mapEditorScreen){
@@ -95,7 +95,5 @@ class MapEditorMenuPopup(mapEditorScreen: MapEditorScreen): Popup(mapEditorScree
         val closeOptionsButton = TextButton("Close".tr(), skin)
         closeOptionsButton.onClick { close() }
         add(closeOptionsButton).row()
-
-        open()
     }
 }

core/src/com/unciv/ui/mapeditor/MapEditorScreen.kt

@@ -13,6 +13,7 @@ import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.translations.tr
 import com.unciv.ui.utils.CameraStageBaseScreen
 import com.unciv.ui.utils.onClick
+import com.unciv.ui.utils.popups
 import com.unciv.ui.utils.setFontSize
 
 class MapEditorScreen(): CameraStageBaseScreen() {
@@ -76,9 +77,9 @@ class MapEditorScreen(): CameraStageBaseScreen() {
 
         val optionsMenuButton = TextButton("Menu".tr(), skin)
         optionsMenuButton.onClick {
-            if(stage.actors.any { it is MapEditorMenuPopup })
+            if(popups.any { it is MapEditorMenuPopup })
                 return@onClick // already open
-            MapEditorMenuPopup(this)
+            MapEditorMenuPopup(this).open(force = true)
         }
         optionsMenuButton.label.setFontSize(24)
         optionsMenuButton.labelCell.pad(20f)

core/src/com/unciv/ui/trade/DiplomacyScreen.kt

@@ -158,7 +158,7 @@ class DiplomacyScreen(val viewingCiv:CivilizationInfo):CameraStageBaseScreen() {
                     tradeLogic.currentTrade.theirOffers.add(TradeOffer(Constants.peaceTreaty, TradeType.Treaty, 30))
                     tradeLogic.acceptTrade()
                     updateLeftSideTable()
-                }, this)
+                }, this).open()
             }
             diplomacyTable.add(peaceButton).row()
             if(isNotPlayersTurn()) peaceButton.disable()
@@ -373,7 +373,7 @@ class DiplomacyScreen(val viewingCiv:CivilizationInfo):CameraStageBaseScreen() {
                 diplomacyManager.declareWar()
                 setRightSideFlavorText(otherCiv, otherCiv.getTranslatedNation().attacked, "Very well.")
                 updateLeftSideTable()
-            }, this)
+            }, this).open()
         }
         return declareWarButton
     }

core/src/com/unciv/ui/utils/CameraStageBaseScreen.kt

@@ -7,21 +7,8 @@ import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.graphics.GL20
 import com.badlogic.gdx.graphics.g2d.Batch
 import com.badlogic.gdx.graphics.g2d.SpriteBatch
-import com.badlogic.gdx.scenes.scene2d.Actor
+import com.badlogic.gdx.scenes.scene2d.*
-import com.badlogic.gdx.scenes.scene2d.InputEvent
+import com.badlogic.gdx.scenes.scene2d.ui.*
-import com.badlogic.gdx.scenes.scene2d.InputListener
-import com.badlogic.gdx.scenes.scene2d.Stage
-import com.badlogic.gdx.scenes.scene2d.Touchable
-import com.badlogic.gdx.scenes.scene2d.ui.Button
-import com.badlogic.gdx.scenes.scene2d.ui.Cell
-import com.badlogic.gdx.scenes.scene2d.ui.CheckBox
-import com.badlogic.gdx.scenes.scene2d.ui.Image
-import com.badlogic.gdx.scenes.scene2d.ui.Label
-import com.badlogic.gdx.scenes.scene2d.ui.SelectBox
-import com.badlogic.gdx.scenes.scene2d.ui.Skin
-import com.badlogic.gdx.scenes.scene2d.ui.Table
-import com.badlogic.gdx.scenes.scene2d.ui.TextButton
-import com.badlogic.gdx.scenes.scene2d.ui.TextField
 import com.badlogic.gdx.scenes.scene2d.utils.ClickListener
 import com.badlogic.gdx.utils.viewport.ExtendViewport
 import com.unciv.JsonParser
@@ -83,9 +70,6 @@ open class CameraStageBaseScreen : Screen {
         tutorialController.showTutorial(tutorial)
     }
 
-    fun hasOpenPopups(): Boolean =
-            stage.actors.any { it is Popup }
-
     companion object {
         var skin = Skin(Gdx.files.internal("skin/flat-earth-ui.json"))
 

core/src/com/unciv/ui/utils/Popup.kt

@@ -18,20 +18,30 @@ open class Popup(val screen: CameraStageBaseScreen): Table(CameraStageBaseScreen
 
         this.pad(20f)
         this.defaults().pad(5f)
+
+        this.isVisible = false
     }
 
     /**
-     * Displays the Popup on the screen. Will not open the popup if another one is already open.
+     * Displays the Popup on the screen. If another popup is already open, this one will display after the other has
+     * closed. Use [force] = true if you want to open this popup above the other one anyway.
      */
-    fun open() {
+    fun open(force: Boolean = false) {
-        if (screen.hasOpenPopups()) return;
+        if (force || !screen.hasOpenPopups()) {
+            this.isVisible = true
+        }
+
+        screen.stage.addActor(this)
         pack()
         center(screen.stage)
-        screen.stage.addActor(this)
+
     }
 
     open fun close() {
         remove()
+        if (screen.popups.isNotEmpty()) {
+            screen.popups[0].isVisible = true;
+        }
     }
 
     fun addGoodSizedLabel(text: String, size:Int=18): Cell<Label> {
@@ -58,3 +68,8 @@ open class Popup(val screen: CameraStageBaseScreen): Table(CameraStageBaseScreen
     fun addCloseButton() = addButton("Close") { close() }
 }
 
+fun CameraStageBaseScreen.hasOpenPopups(): Boolean = stage.actors.any { it is Popup && it.isVisible }
+
+val CameraStageBaseScreen.popups: List<Popup>
+    get() = stage.actors.filter{ it is Popup }.map{ it as Popup }
+

core/src/com/unciv/ui/utils/YesNoPopup.kt

@@ -10,6 +10,5 @@ class YesNoPopup(question:String, action:()->Unit,
         add(question.toLabel()).colspan(2).row()
         add(TextButton("No".tr(), skin).onClick { close(); restoredefault() })
         add(TextButton("Yes".tr(), skin).onClick { close(); action() })
-        open()
     }
 }

core/src/com/unciv/ui/worldscreen/AlertPopup.kt

@@ -178,7 +178,6 @@ class AlertPopup(val worldScreen: WorldScreen, val popupAlert: PopupAlert): Popu
                 add(getCloseButton("Close"))
             }
         }
-        open()
     }
 
     override fun close(){

core/src/com/unciv/ui/worldscreen/TradePopup.kt

@@ -9,17 +9,17 @@ import com.unciv.logic.trade.TradeLogic
 import com.unciv.logic.trade.TradeType
 import com.unciv.models.translations.tr
 import com.unciv.ui.trade.DiplomacyScreen
+import com.unciv.ui.utils.Popup
 import com.unciv.ui.utils.addSeparator
 import com.unciv.ui.utils.toLabel
-import com.unciv.ui.utils.Popup
 import kotlin.math.max
 import kotlin.math.min
 
 class TradePopup(worldScreen: WorldScreen): Popup(worldScreen){
-    init{
+    val viewingCiv = worldScreen.viewingCiv
-        val viewingCiv = worldScreen.viewingCiv
+    val tradeRequest = viewingCiv.tradeRequests.first()
-        val tradeRequest = viewingCiv.tradeRequests.first()
 
+    init{
         val requestingCiv = worldScreen.gameInfo.getCivilization(tradeRequest.requestingCiv)
         val translatedNation = requestingCiv.getTranslatedNation()
         val otherCivLeaderName = "[${translatedNation.leaderName}] of [${translatedNation.getNameTranslation()}]".tr()
@@ -50,7 +50,6 @@ class TradePopup(worldScreen: WorldScreen): Popup(worldScreen){
             val tradeLogic = TradeLogic(viewingCiv, requestingCiv)
             tradeLogic.currentTrade.set(trade)
             tradeLogic.acceptTrade()
-            viewingCiv.tradeRequests.remove(tradeRequest)
             close()
             Popup(worldScreen).apply {
                 add(otherCivLeaderName.toLabel()).colspan(2)
@@ -67,8 +66,6 @@ class TradePopup(worldScreen: WorldScreen): Popup(worldScreen){
             requestingCiv.addNotification("[${viewingCiv.civName}] has accepted your trade request", Color.GOLD)
         }
         addButton("Not this time.".tr()){
-            viewingCiv.tradeRequests.remove(tradeRequest)
-
             val diplomacyManager = requestingCiv.getDiplomacyManager(viewingCiv)
             if(trade.ourOffers.all { it.type==TradeType.Luxury_Resource } && trade.theirOffers.all { it.type==TradeType.Luxury_Resource })
                 diplomacyManager.setFlag(DiplomacyFlags.DeclinedLuxExchange,20) // offer again in 20 turns
@@ -82,7 +79,6 @@ class TradePopup(worldScreen: WorldScreen): Popup(worldScreen){
             worldScreen.shouldUpdate=true
         }
         addButton("How about something else...".tr()){
-            viewingCiv.tradeRequests.remove(tradeRequest)
             close()
 
             val diplomacyScreen= DiplomacyScreen(viewingCiv)
@@ -92,6 +88,10 @@ class TradePopup(worldScreen: WorldScreen): Popup(worldScreen){
             worldScreen.game.setScreen(diplomacyScreen)
             worldScreen.shouldUpdate=true
         }
-        open()
+    }
+
+    override fun close() {
+        viewingCiv.tradeRequests.remove(tradeRequest)
+        super.close()
     }
 }

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

@@ -209,8 +209,8 @@ class WorldScreen(val viewingCiv:CivilizationInfo) : CameraStageBaseScreen() {
                 !gameInfo.oneMoreTurnMode && gameInfo.civilizations.any { it.victoryManager.hasWon() } -> game.setScreen(VictoryScreen())
                 viewingCiv.policies.freePolicies > 0 && viewingCiv.policies.canAdoptPolicy() -> game.setScreen(PolicyPickerScreen(this))
                 viewingCiv.greatPeople.freeGreatPeople > 0 -> game.setScreen(GreatPersonPickerScreen(viewingCiv))
-                viewingCiv.popupAlerts.any() -> AlertPopup(this, viewingCiv.popupAlerts.first())
+                viewingCiv.popupAlerts.any() -> AlertPopup(this, viewingCiv.popupAlerts.first()).open()
-                viewingCiv.tradeRequests.isNotEmpty() -> TradePopup(this)
+                viewingCiv.tradeRequests.isNotEmpty() -> TradePopup(this).open()
             }
         }
         updateNextTurnButton(hasOpenPopups()) // This must be before the notifications update, since its position is based on it

core/src/com/unciv/ui/worldscreen/WorldScreenTopBar.kt

@@ -95,8 +95,8 @@ class WorldScreenTopBar(val worldScreen: WorldScreen) : Table() {
                 .apply { setSize(50f, 50f) }
         menuButton.color = Color.WHITE
         menuButton.onClick {
-            if(worldScreen.stage.actors.none { it is WorldScreenMenuPopup })
+            if(worldScreen.popups.none { it is WorldScreenMenuPopup })
-                WorldScreenMenuPopup(worldScreen)
+                WorldScreenMenuPopup(worldScreen).open(force = true)
         }
         menuButton.centerY(this)
         menuButton.x = menuButton.y

core/src/com/unciv/ui/worldscreen/mainmenu/WorldScreenMenuPopup.kt

@@ -23,25 +23,25 @@ class WorldScreenMenuPopup(val worldScreen: WorldScreen) : Popup(worldScreen) {
         val height = 30f
         addSquareButton("Map editor".tr()){
             openMapEditorPopup()
-            remove()
+            close()
         }.size(width,height)
         addSeparator()
 
         addSquareButton("Civilopedia".tr()){
             UncivGame.Current.setScreen(CivilopediaScreen(worldScreen.gameInfo.ruleSet))
-            remove()
+            close()
         }.size(width,height)
         addSeparator()
 
         addSquareButton("Load game".tr()){
             UncivGame.Current.setScreen(LoadGameScreen())
-            remove()
+            close()
         }.size(width,height)
         addSeparator()
 
         addSquareButton("Save game".tr()) {
             UncivGame.Current.setScreen(SaveGameScreen())
-            remove()
+            close()
         }.size(width,height)
         addSeparator()
 
@@ -56,22 +56,20 @@ class WorldScreenMenuPopup(val worldScreen: WorldScreen) : Popup(worldScreen) {
         addSeparator()
 
         addSquareButton("Options".tr()){
-            UncivGame.Current.worldScreen.stage.addActor(WorldScreenOptionsPopup(worldScreen))
+            WorldScreenOptionsPopup(worldScreen).open()
-            remove()
+            close()
         }.size(width,height)
         addSeparator()
 
         addSquareButton("Community"){
-            WorldScreenCommunityPopup(worldScreen)
+            WorldScreenCommunityPopup(worldScreen).open()
-            remove()
+            close()
         }.size(width,height)
         addSeparator()
 
         addSquareButton("Close"){
             close()
         }.size(width,height)
-
-        open()
     }
 
 
@@ -159,16 +157,14 @@ class WorldScreenCommunityPopup(val worldScreen: WorldScreen) : Popup(worldScree
     init{
         addButton("Discord"){
             Gdx.net.openURI("https://discord.gg/bjrB4Xw")
-            remove()
+            close()
         }
 
         addButton("Github"){
             Gdx.net.openURI("https://github.com/yairm210/UnCiv")
-            remove()
+            close()
         }
 
         addCloseButton()
-
-        open()
     }
 }

core/src/com/unciv/ui/worldscreen/mainmenu/WorldScreenOptionsPopup.kt

@@ -26,7 +26,6 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
     init {
         UncivGame.Current.settings.addCompletedTutorialTask("Open the options table")
         update()
-        open()
     }
 
 
@@ -35,44 +34,43 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
         settings.save()
         clear()
 
-        val innerTable = Popup(screen) // cheating, to get the old code to fit inside a Scroll =)
+        val innerTable = Table(CameraStageBaseScreen.skin)
-        innerTable.background = null
 
         innerTable.add("Display options".toLabel(fontSize = 24)).colspan(2).row()
 
         innerTable.add("Show worked tiles".toLabel())
-        innerTable.addButton(if (settings.showWorkedTiles) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.showWorkedTiles) "Yes".tr() else "No".tr()) {
             settings.showWorkedTiles= !settings.showWorkedTiles
             update()
         }
 
         innerTable.add("Show resources and improvements".toLabel())
-        innerTable.addButton(if (settings.showResourcesAndImprovements) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.showResourcesAndImprovements) "Yes".tr() else "No".tr()) {
             settings.showResourcesAndImprovements = !settings.showResourcesAndImprovements
             update()
         }
 
 
         innerTable.add("Show tutorials".toLabel())
-        innerTable.addButton(if (settings.showTutorials) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.showTutorials) "Yes".tr() else "No".tr()) {
             settings.showTutorials = !settings.showTutorials
             update()
         }
 
         innerTable.add("Show minimap".toLabel())
-        innerTable.addButton(if (settings.showMinimap) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.showMinimap) "Yes".tr() else "No".tr()) {
             settings.showMinimap = !settings.showMinimap
             update()
         }
 
         innerTable.add("Show pixel units".toLabel())
-        innerTable.addButton(if (settings.showPixelUnits) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.showPixelUnits) "Yes".tr() else "No".tr()) {
             settings.showPixelUnits = !settings.showPixelUnits
             update()
         }
 
         innerTable.add("Show pixel improvements".toLabel())
-        innerTable.addButton(if (settings.showPixelImprovements) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.showPixelImprovements) "Yes".tr() else "No".tr()) {
             settings.showPixelImprovements = !settings.showPixelImprovements
             update()
         }
@@ -85,7 +83,7 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
 
         // Do not add to template.properties yet please.
         innerTable.add("Continuous rendering\n(HIGHLY EXPERIMENTAL)".toLabel())
-        innerTable.addButton(if (settings.continuousRendering) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.continuousRendering) "Yes".tr() else "No".tr()) {
             settings.continuousRendering = !settings.continuousRendering
             Gdx.graphics.isContinuousRendering = settings.continuousRendering
             update()
@@ -95,32 +93,32 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
 
 
         innerTable.add("Check for idle units".toLabel())
-        innerTable.addButton(if (settings.checkForDueUnits) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.checkForDueUnits) "Yes".tr() else "No".tr()) {
             settings.checkForDueUnits = !settings.checkForDueUnits
             update()
         }
 
         innerTable.add("Move units with a single tap".toLabel())
-        innerTable.addButton(if (settings.singleTapMove) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.singleTapMove) "Yes".tr() else "No".tr()) {
             settings.singleTapMove = !settings.singleTapMove
             update()
         }
 
         innerTable.add("Auto-assign city production".toLabel())
-        innerTable.addButton(if (settings.autoAssignCityProduction) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.autoAssignCityProduction) "Yes".tr() else "No".tr()) {
             settings.autoAssignCityProduction = !settings.autoAssignCityProduction
             update()
         }
 
         innerTable.add("Auto-build roads".toLabel())
-        innerTable.addButton(if (settings.autoBuildingRoads) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.autoBuildingRoads) "Yes".tr() else "No".tr()) {
             settings.autoBuildingRoads = !settings.autoBuildingRoads
             update()
         }
 
 
         innerTable.add("Enable nuclear weapons".toLabel())
-        innerTable.addButton(if (settings.nuclearWeaponEnabled) "Yes".tr() else "No".tr()) {
+        addButton(innerTable, if (settings.nuclearWeaponEnabled) "Yes".tr() else "No".tr()) {
             settings.nuclearWeaponEnabled = !settings.nuclearWeaponEnabled
             update()
         }
@@ -150,8 +148,14 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
         UncivGame.Current.worldScreen.shouldUpdate = true
     }
 
+    private fun addButton(table: Table, text: String, action: () -> Unit): Cell<TextButton> {
+        val button = TextButton(text.tr(), skin).apply { color = ImageGetter.getBlue() }
+        button.onClick(action)
+        return table.add(button).apply { row() }
+    }
 
-    private fun addSoundEffectsVolumeSlider(innerTable: Popup) {
+
+    private fun addSoundEffectsVolumeSlider(innerTable: Table) {
         innerTable.add("Sound effects volume".tr())
 
         val soundEffectsVolumeSlider = Slider(0f, 1.0f, 0.1f, false, skin)
@@ -166,7 +170,7 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
         innerTable.add(soundEffectsVolumeSlider).row()
     }
 
-    private fun addMusicVolumeSlider(innerTable: Popup) {
+    private fun addMusicVolumeSlider(innerTable: Table) {
         val musicLocation =Gdx.files.local(UncivGame.Current.musicLocation)
         if(musicLocation.exists()) {
             innerTable.add("Music volume".tr())
@@ -208,7 +212,7 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
         }
     }
 
-    private fun addResolutionSelectBox(innerTable: Popup) {
+    private fun addResolutionSelectBox(innerTable: Table) {
         innerTable.add("Resolution".toLabel())
 
         val resolutionSelectBox = SelectBox<String>(skin)
@@ -224,12 +228,12 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
                 UncivGame.Current.settings.save()
                 UncivGame.Current.worldScreen = WorldScreen(worldScreen.viewingCiv)
                 UncivGame.Current.setWorldScreen()
-                WorldScreenOptionsPopup(UncivGame.Current.worldScreen)
+                WorldScreenOptionsPopup(UncivGame.Current.worldScreen).open()
             }
         })
     }
 
-    private fun addTileSetSelectBox(innerTable: Popup) {
+    private fun addTileSetSelectBox(innerTable: Table) {
         innerTable.add("Tileset".toLabel())
 
         val tileSetSelectBox = SelectBox<String>(skin)
@@ -247,12 +251,12 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
                 UncivGame.Current.settings.save()
                 UncivGame.Current.worldScreen = WorldScreen(worldScreen.viewingCiv)
                 UncivGame.Current.setWorldScreen()
-                WorldScreenOptionsPopup(UncivGame.Current.worldScreen)
+                WorldScreenOptionsPopup(UncivGame.Current.worldScreen).open()
             }
         })
     }
 
-    private fun addAutosaveTurnsSelectBox(innerTable: Popup) {
+    private fun addAutosaveTurnsSelectBox(innerTable: Table) {
         innerTable.add("Turns between autosaves".toLabel())
 
         val autosaveTurnsSelectBox = SelectBox<Int>(skin)
@@ -272,7 +276,7 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
         })
     }
 
-    private fun addLanguageSelectBox(innerTable: Popup) {
+    private fun addLanguageSelectBox(innerTable: Table) {
         val languageSelectBox = SelectBox<Language>(skin)
         val languageArray = Array<Language>()
         UncivGame.Current.translations.percentCompleteOfLanguages
@@ -306,6 +310,6 @@ class WorldScreenOptionsPopup(val worldScreen:WorldScreen) : Popup(worldScreen){
         CameraStageBaseScreen.resetFonts() // to load chinese characters if necessary
         UncivGame.Current.worldScreen = WorldScreen(worldScreen.viewingCiv)
         UncivGame.Current.setWorldScreen()
-        WorldScreenOptionsPopup(UncivGame.Current.worldScreen)
+        WorldScreenOptionsPopup(UncivGame.Current.worldScreen).open()
     }
 }

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

@@ -15,8 +15,8 @@ import com.unciv.models.ruleset.Building
 import com.unciv.models.translations.tr
 import com.unciv.ui.pickerscreens.ImprovementPickerScreen
 import com.unciv.ui.pickerscreens.PromotionPickerScreen
-import com.unciv.ui.worldscreen.WorldScreen
 import com.unciv.ui.utils.YesNoPopup
+import com.unciv.ui.worldscreen.WorldScreen
 
 class UnitActions {
 
@@ -314,7 +314,7 @@ class UnitActions {
                     val disbandText = if (unit.currentTile.getOwner() == unit.civInfo)
                         "Disband this unit for [${unit.baseUnit.getDisbandGold()}] gold?".tr()
                     else "Do you really want to disband this unit?".tr()
-                    YesNoPopup(disbandText, { unit.disband(); worldScreen.shouldUpdate = true })
+                    YesNoPopup(disbandText, { unit.disband(); worldScreen.shouldUpdate = true }).open()
                 })
 
         return actionList