Fix screens not correctly being dispose()d (#6876)

core/src/com/unciv/UncivGame.kt

@@ -67,6 +67,7 @@ class UncivGame(parameters: UncivGameParameters) : Game() {
     val alertBattle = false
 
     lateinit var worldScreen: WorldScreen
+        private set
     fun getWorldScreenOrNull() = if (this::worldScreen.isInitialized) worldScreen else null
 
     var isInitialized = false
@@ -155,18 +156,30 @@ class UncivGame(parameters: UncivGameParameters) : Game() {
         if (gameInfo.civilizations.count { it.playerType == PlayerType.Human } > 1 && !gameInfo.gameParameters.isOnlineMultiplayer)
             setScreen(PlayerReadyScreen(gameInfo, gameInfo.getPlayerToViewAs()))
         else {
-            worldScreen = WorldScreen(gameInfo, gameInfo.getPlayerToViewAs())
+            resetToWorldScreen(WorldScreen(gameInfo, gameInfo.getPlayerToViewAs()))
-            setWorldScreen()
         }
     }
 
     fun setScreen(screen: BaseScreen) {
+        val oldScreen = getScreen()
         Gdx.input.inputProcessor = screen.stage
         super.setScreen(screen)
+        if (oldScreen != getWorldScreenOrNull()) oldScreen.dispose()
     }
 
-    fun setWorldScreen() {
+    /**
-        if (screen != null && screen != worldScreen) screen.dispose()
+     * If called with null [newWorldScreen], disposes of the current screen and sets it to the current stored world screen. 
+     * If the current screen is already the world screen, the only thing that happens is that the world screen updates.
+     */
+    fun resetToWorldScreen(newWorldScreen: WorldScreen? = null) {
+        if (newWorldScreen != null) {
+            val oldWorldScreen = getWorldScreenOrNull()
+            worldScreen = newWorldScreen
+            // setScreen disposes the current screen, but the old world screen is not the current screen, so need to dispose here
+            if (screen != oldWorldScreen) {
+                oldWorldScreen?.dispose()
+            }
+        }
         setScreen(worldScreen)
         worldScreen.shouldUpdate = true // This can set the screen to the policy picker or tech picker screen, so the input processor must come before
         Gdx.graphics.requestRendering()

core/src/com/unciv/logic/civilization/QuestManager.kt

@@ -918,13 +918,13 @@ class AssignedQuest(val questName: String = "",
 
         when (questName) {
             QuestName.ClearBarbarianCamp.value -> {
-                game.setWorldScreen()
+                game.resetToWorldScreen()
                 game.worldScreen.mapHolder.setCenterPosition(Vector2(data1.toFloat(), data2.toFloat()), selectUnit = false)
             }
             QuestName.Route.value -> {
-                game.setWorldScreen()
+                game.resetToWorldScreen()
                 game.worldScreen.mapHolder.setCenterPosition(gameInfo.getCivilization(assigner).getCapital().location, selectUnit = false)
             }
         }
     }
-}
+}

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

@@ -111,7 +111,7 @@ class CityScreen(
     private val nextTileToOwn = city.expansion.chooseNewTileToOwn()
 
     init {
-        onBackButtonClicked { game.setWorldScreen() }
+        onBackButtonClicked { game.resetToWorldScreen() }
         UncivGame.Current.settings.addCompletedTutorialTask("Enter city screen")
 
         addTiles()
@@ -435,7 +435,7 @@ class CityScreen(
     }
 
     fun exit() {
-        game.setWorldScreen()
+        game.resetToWorldScreen()
         game.worldScreen.mapHolder.setCenterPosition(city.location)
         game.worldScreen.bottomUnitTable.selectUnit()
     }

core/src/com/unciv/ui/options/OptionsPopup.kt

@@ -120,8 +120,7 @@ class OptionsPopup(
     private fun reloadWorldAndOptions() {
         settings.save()
         if (screen is WorldScreen) {
-            screen.game.worldScreen = WorldScreen(screen.gameInfo, screen.viewingCiv)
+            screen.game.resetToWorldScreen(WorldScreen(screen.gameInfo, screen.viewingCiv))
-            screen.game.setWorldScreen()
         } else if (screen is MainMenuScreen) {
             screen.game.setScreen(MainMenuScreen())
         }

core/src/com/unciv/ui/overviewscreen/EmpireOverviewScreen.kt

@@ -44,7 +44,7 @@ class EmpireOverviewScreen(
             else game.settings.lastOverviewPage
         val iconSize = Constants.defaultFontSize.toFloat()
 
-        onBackButtonClicked { game.setWorldScreen() }
+        onBackButtonClicked { game.resetToWorldScreen() }
 
         tabbedPager = TabbedPager(
             stage.width, stage.width,
@@ -54,7 +54,7 @@ class EmpireOverviewScreen(
             capacity = EmpireOverviewCategories.values().size)
 
         tabbedPager.bindArrowKeys()
-        tabbedPager.addClosePage { game.setWorldScreen() }
+        tabbedPager.addClosePage { game.resetToWorldScreen() }
 
         for (category in EmpireOverviewCategories.values()) {
             val tabState = category.stateTester(viewingPlayer)

core/src/com/unciv/ui/overviewscreen/ResourcesOverviewTable.kt

@@ -74,7 +74,7 @@ class ResourcesOverviewTab(
         ImageGetter.getResourceImage(name, iconSize).apply {
             onClick {
                 if (viewingPlayer.gameInfo.notifyExploredResources(viewingPlayer, name, 0, true))
-                    overviewScreen.game.setWorldScreen()
+                    overviewScreen.game.resetToWorldScreen()
             }
         }
     private fun TileResource.getLabel() = name.toLabel().apply {

core/src/com/unciv/ui/overviewscreen/UnitOverviewTable.kt

@@ -68,7 +68,7 @@ class UnitOverviewTab(
 
     private fun showWorldScreenAt(position: Vector2, unit: MapUnit?) {
         val game = overviewScreen.game
-        game.setWorldScreen()
+        game.resetToWorldScreen()
         game.worldScreen.mapHolder.setCenterPosition(position, forceSelectUnit = unit)
     }
     private fun showWorldScreenAt(unit: MapUnit) = showWorldScreenAt(unit.currentTile.position, unit)

core/src/com/unciv/ui/overviewscreen/WonderOverviewTable.kt

@@ -238,7 +238,7 @@ class WonderOverviewTab(
                 val locationLabel = locationText.toLabel()
                 if (wonder.location != null)
                     locationLabel.onClick{
-                        UncivGame.Current.setWorldScreen()
+                        UncivGame.Current.resetToWorldScreen()
                         UncivGame.Current.worldScreen.mapHolder.setCenterPosition(wonder.location.position)
                     }
                 add(locationLabel).fillY()

core/src/com/unciv/ui/pickerscreens/DiplomaticVotePickerScreen.kt

@@ -30,7 +30,7 @@ class DiplomaticVotePickerScreen(private val votingCiv: CivilizationInfo) : Pick
 
         rightSideButton.onClick(UncivSound.Chimes) {
             votingCiv.diplomaticVoteForCiv(chosenCiv!!)
-            UncivGame.Current.setWorldScreen()
+            UncivGame.Current.resetToWorldScreen()
         }
 
     }

core/src/com/unciv/ui/pickerscreens/DiplomaticVoteResultScreen.kt

@@ -25,7 +25,7 @@ class DiplomaticVoteResultScreen(val votesCast: HashMap<String, String>, val vie
         
         rightSideButton.onClick(UncivSound.Click) {
             viewingCiv.addFlag(CivFlags.ShowDiplomaticVotingResults.name, -1)
-            UncivGame.Current.setWorldScreen()
+            UncivGame.Current.resetToWorldScreen()
         }
         rightSideButton.enable()
         rightSideButton.setText("Continue".tr())
@@ -52,4 +52,4 @@ class DiplomaticVoteResultScreen(val votesCast: HashMap<String, String>, val vie
         topTable.add(ImageGetter.getNationIndicator(votedCiv.nation, 30f)).pad(10f)
         topTable.add(votedCiv.civName.toLabel()).row()
     }
-}
+}

core/src/com/unciv/ui/pickerscreens/GreatPersonPickerScreen.kt

@@ -37,7 +37,7 @@ class GreatPersonPickerScreen(val civInfo:CivilizationInfo) : PickerScreen() {
                 civInfo.greatPeople.mayaLimitedFreeGP--
                 civInfo.greatPeople.longCountGPPool.remove(theChosenOne!!.name)
             }
-            UncivGame.Current.setWorldScreen()
+            UncivGame.Current.resetToWorldScreen()
         }
 
     }

core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt

@@ -44,13 +44,13 @@ class ImprovementPickerScreen(
             unit.action = null // this is to "wake up" the worker if it's sleeping
             onAccept()
         }
-        game.setWorldScreen()
+        game.resetToWorldScreen()
         dispose()
     }
 
     init {
         setDefaultCloseAction()
-        onBackButtonClicked { UncivGame.Current.setWorldScreen() }
+        onBackButtonClicked { UncivGame.Current.resetToWorldScreen() }
 
         rightSideButton.setText("Pick improvement".tr())
         rightSideButton.onClick {

core/src/com/unciv/ui/pickerscreens/PickerScreen.kt

@@ -42,7 +42,7 @@ open class PickerScreen(disableScroll: Boolean = false) : BaseScreen() {
     fun setDefaultCloseAction(previousScreen: BaseScreen?=null) {
         val closeAction = {
             if (previousScreen != null) game.setScreen(previousScreen)
-            else game.setWorldScreen()
+            else game.resetToWorldScreen()
             dispose()
         }
         pickerPane.closeButton.onClick(closeAction)

core/src/com/unciv/ui/pickerscreens/PolicyPickerScreen.kt

@@ -40,7 +40,7 @@ class PolicyPickerScreen(val worldScreen: WorldScreen, civInfo: CivilizationInfo
         if (policies.freePolicies > 0 && policies.canAdoptPolicy())
             closeButton.disable()
         else
-            onBackButtonClicked { UncivGame.Current.setWorldScreen() }
+            onBackButtonClicked { UncivGame.Current.resetToWorldScreen() }
 
         rightSideButton.onClick(UncivSound.Policy) {
             val policy = pickedPolicy!!
@@ -52,7 +52,7 @@ class PolicyPickerScreen(val worldScreen: WorldScreen, civInfo: CivilizationInfo
 
             // If we've moved to another screen in the meantime (great person pick, victory screen) ignore this
             if (game.screen !is PolicyPickerScreen || !policies.canAdoptPolicy()) {
-                game.setWorldScreen()
+                game.resetToWorldScreen()
                 dispose()
             } else {
                 val policyScreen = PolicyPickerScreen(worldScreen)

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

@@ -25,13 +25,13 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen() {
         if (unit.promotions.canBePromoted())
             game.setScreen(PromotionPickerScreen(unit).setScrollY(scrollPane.scrollY))
         else
-            game.setWorldScreen()
+            game.resetToWorldScreen()
         dispose()
         game.worldScreen.shouldUpdate = true
     }
 
     init {
-        onBackButtonClicked { UncivGame.Current.setWorldScreen() }
+        onBackButtonClicked { UncivGame.Current.resetToWorldScreen() }
         setDefaultCloseAction()
 
         rightSideButton.setText("Pick promotion".tr())

core/src/com/unciv/ui/pickerscreens/ReligionPickerScreenCommon.kt

@@ -61,7 +61,7 @@ abstract class ReligionPickerScreenCommon(
         rightSideButton.setText(buttonText.tr())
         rightSideButton.onClick(UncivSound.Choir) {
             choosingCiv.religionManager.action()
-            UncivGame.Current.setWorldScreen()
+            UncivGame.Current.resetToWorldScreen()
             dispose()
         }
     }

core/src/com/unciv/ui/pickerscreens/TechPickerScreen.kt

@@ -57,7 +57,7 @@ class TechPickerScreen(
 
     init {
         setDefaultCloseAction()
-        onBackButtonClicked { UncivGame.Current.setWorldScreen() }
+        onBackButtonClicked { UncivGame.Current.resetToWorldScreen() }
         scrollPane.setOverscroll(false, false)
 
         descriptionLabel.onClick {
@@ -83,7 +83,7 @@ class TechPickerScreen(
 
             game.settings.addCompletedTutorialTask("Pick technology")
 
-            game.setWorldScreen()
+            game.resetToWorldScreen()
             game.worldScreen.shouldUpdate = true
             dispose()
         }

core/src/com/unciv/ui/saves/SaveGameScreen.kt

@@ -63,7 +63,7 @@ class SaveGameScreen(val gameInfo: GameInfo) : PickerScreen(disableScroll = true
                 launchCrashHandling("SaveGame", runAsDaemon = false) {
                     GameSaver.saveGameToCustomLocation(gameInfo, gameNameTextField.text) { e ->
                         if (e == null) {
-                            postCrashHandlingRunnable { game.setWorldScreen() }
+                            postCrashHandlingRunnable { game.resetToWorldScreen() }
                         } else if (e !is CancellationException) {
                             errorLabel.setText("Could not save game to custom location!".tr())
                             e.printStackTrace()
@@ -101,7 +101,7 @@ class SaveGameScreen(val gameInfo: GameInfo) : PickerScreen(disableScroll = true
             GameSaver.saveGame(gameInfo, gameNameTextField.text) {
                 postCrashHandlingRunnable {
                     if (it != null) ToastPopup("Could not save game!", this@SaveGameScreen)
-                    else UncivGame.Current.setWorldScreen()
+                    else UncivGame.Current.resetToWorldScreen()
                 }
             }
         }

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

@@ -59,7 +59,7 @@ class DiplomacyScreen(
     private fun isNotPlayersTurn() = !UncivGame.Current.worldScreen.canChangeState
 
     init {
-        onBackButtonClicked { UncivGame.Current.setWorldScreen() }
+        onBackButtonClicked { UncivGame.Current.resetToWorldScreen() }
         val splitPane = SplitPane(leftSideScroll, rightSideTable, false, skin)
         splitPane.splitAmount = 0.2f
 
@@ -68,7 +68,7 @@ class DiplomacyScreen(
         splitPane.setFillParent(true)
         stage.addActor(splitPane)
 
-        closeButton.onClick { UncivGame.Current.setWorldScreen() }
+        closeButton.onClick { UncivGame.Current.resetToWorldScreen() }
         closeButton.label.setFontSize(Constants.headingFontSize)
         closeButton.labelCell.pad(10f)
         closeButton.pack()
@@ -457,7 +457,7 @@ class DiplomacyScreen(
         diplomaticMarriageButton.onClick {
             val newCities = otherCiv.cities
             otherCiv.cityStateFunctions.diplomaticMarriage(viewingCiv)
-            UncivGame.Current.setWorldScreen() // The other civ will no longer exist
+            UncivGame.Current.resetToWorldScreen() // The other civ will no longer exist
             for (city in newCities)
                 viewingCiv.popupAlerts.add(PopupAlert(AlertType.DiplomaticMarriage, city.id))   // Player gets to choose between annex and puppet
         }
@@ -937,7 +937,7 @@ class DiplomacyScreen(
     private fun getGoToOnMapButton(civilization: CivilizationInfo): TextButton {
         val goToOnMapButton = "Go to on map".toTextButton()
         goToOnMapButton.onClick {
-            UncivGame.Current.setWorldScreen()
+            UncivGame.Current.resetToWorldScreen()
             UncivGame.Current.worldScreen.mapHolder.setCenterPosition(civilization.getCapital().location, selectUnit = false)
         }
         return goToOnMapButton

core/src/com/unciv/ui/victoryscreen/VictoryScreen.kt

@@ -67,7 +67,7 @@ class VictoryScreen(val worldScreen: WorldScreen) : PickerScreen() {
             wonOrLost("", null, false)
         } else if (!someoneHasWon) {
             setDefaultCloseAction()
-            onBackButtonClicked { game.setWorldScreen() }
+            onBackButtonClicked { game.resetToWorldScreen() }
         }
     }
 
@@ -95,7 +95,7 @@ class VictoryScreen(val worldScreen: WorldScreen) : PickerScreen() {
         closeButton.setText("One more turn...!".tr())
         closeButton.onClick {
             gameInfo.oneMoreTurnMode = true
-            game.setWorldScreen()
+            game.resetToWorldScreen()
         }
     }
 

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

@@ -19,8 +19,7 @@ class PlayerReadyScreen(gameInfo: GameInfo, currentPlayerCiv: CivilizationInfo)
 
         table.onClick {
             postCrashHandlingRunnable { // To avoid ANRs on Android when the creation of the worldscreen takes more than 500ms
-                game.worldScreen = WorldScreen(gameInfo, currentPlayerCiv)
+                game.resetToWorldScreen(WorldScreen(gameInfo, currentPlayerCiv))
-                game.setWorldScreen()
             }
         }
         table.setFillParent(true)

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

@@ -651,8 +651,7 @@ class WorldScreen(val gameInfo: GameInfo, val viewingCiv:CivilizationInfo) : Bas
         newWorldScreen.selectedCiv = gameInfo.getCivilization(selectedCiv.civName)
         newWorldScreen.fogOfWar = fogOfWar
 
-        game.worldScreen = newWorldScreen
+        game.resetToWorldScreen(newWorldScreen)
-        game.setWorldScreen()
     }
 
     fun nextTurn() {