From 1a6d8d72bbbd29719f7952f5ecce71fc0a7bd4d1 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Sun, 25 Jun 2023 08:46:00 +0200 Subject: [PATCH] Fix ruleset error crash (#9666) * Harden NewGameScreen against exceptions from getComplexRuleset * Miscellaneous tweaks to NewGameScreen - e.g. allow it to show last used custom map --- .../jsons/translations/template.properties | 1 + .../newgamescreen/MapFileSelectTable.kt | 40 ++++++++++++---- .../screens/newgamescreen/MapOptionsTable.kt | 18 +++---- .../ui/screens/newgamescreen/NewGameScreen.kt | 48 +++++++++++++++---- 4 files changed, 81 insertions(+), 26 deletions(-) diff --git a/android/assets/jsons/translations/template.properties b/android/assets/jsons/translations/template.properties index 55b0402279..5ae6d53675 100644 --- a/android/assets/jsons/translations/template.properties +++ b/android/assets/jsons/translations/template.properties @@ -360,6 +360,7 @@ Domination = Cultural = Diplomatic = Time = +Your previous options needed to be reset to defaults. = # Used for random nation indicator in empire selector and unknown nation icons in various overview screens. # Should be a single character, or at least visually square. diff --git a/core/src/com/unciv/ui/screens/newgamescreen/MapFileSelectTable.kt b/core/src/com/unciv/ui/screens/newgamescreen/MapFileSelectTable.kt index 8abd87f529..e34cc8a0eb 100644 --- a/core/src/com/unciv/ui/screens/newgamescreen/MapFileSelectTable.kt +++ b/core/src/com/unciv/ui/screens/newgamescreen/MapFileSelectTable.kt @@ -10,9 +10,9 @@ import com.unciv.UncivGame import com.unciv.logic.files.MapSaver import com.unciv.logic.map.MapParameters import com.unciv.models.ruleset.RulesetCache -import com.unciv.ui.components.input.onChange import com.unciv.ui.components.extensions.pad import com.unciv.ui.components.extensions.toLabel +import com.unciv.ui.components.input.onChange import com.unciv.ui.screens.basescreen.BaseScreen import com.unciv.ui.screens.victoryscreen.LoadMapPreview import com.unciv.utils.Concurrency @@ -32,7 +32,7 @@ class MapFileSelectTable( // The SelectBox auto displays the text a object.toString(), which on the FileHandle itself includes the folder path. // So we wrap it in another object with a custom toString() private class MapWrapper(val fileHandle: FileHandle, val mapParameters: MapParameters) { - override fun toString(): String = mapParameters.baseRuleset + " | "+ fileHandle.name() + override fun toString(): String = mapParameters.baseRuleset + " | " + fileHandle.name() } private val mapWrappers= ArrayList() @@ -44,10 +44,11 @@ class MapFileSelectTable( add(mapFileLabel).left() add(mapFileSelectBox) // because SOME people gotta give the hugest names to their maps - .width(columnWidth * 2 / 3) + .width(columnWidth - 40f - mapFileLabel.prefWidth) .right().row() add(miniMapWrapper) .pad(15f) + .maxWidth(columnWidth - 20f) .colspan(2).center().row() mapFileSelectBox.onChange { onSelectBoxChange() } @@ -86,7 +87,6 @@ class MapFileSelectTable( .sortedWith(compareBy(UncivGame.Current.settings.getCollatorFromLocale()) { it.toString() }) .sortedByDescending { it.mapParameters.baseRuleset == newGameScreen.gameSetupInfo.gameParameters.baseRuleset } .forEach { mapFiles.add(it) } - mapFileSelectBox.items = mapFiles // Pre-select: a) map saved within last 15min or b) map named in mapParameters or c) alphabetically first // This is a kludge - the better way would be to have a "play this map now" menu button in the editor @@ -97,9 +97,21 @@ class MapFileSelectTable( ?: mapFiles.firstOrNull { it.fileHandle.name() == mapParameters.name } ?: mapFiles.firstOrNull() ?: return + + // since mapFileSelectBox.selection.setProgrammaticChangeEvents() defaults to true, this would + // kill the mapParameters.name we would like to look for when determining what to pre-select - + // so do it ***after*** getting selectedItem - but control programmaticChangeEvents anyway + // to avoid the overhead of doing a full updateRuleset + updateTables + startMapPreview + // (all expensive) for something that will be overthrown momentarily + mapFileSelectBox.selection.setProgrammaticChangeEvents(false) + mapFileSelectBox.items = mapFiles + + // Now, we want this ON because the event removes map selections which are pulling mods + // that trip updateRuleset - so that code should still be active for the pre-selection + mapFileSelectBox.selection.setProgrammaticChangeEvents(true) mapFileSelectBox.selected = selectedItem - mapParameters.name = selectedItem.toString() - newGameScreen.gameSetupInfo.mapFile = selectedItem.fileHandle + // In the event described above, we now have: mapFileSelectBox.selected != selectedItem + // Do NOT try to put back the "bad" preselection! } private fun onSelectBoxChange() { @@ -111,10 +123,20 @@ class MapFileSelectTable( newGameScreen.gameSetupInfo.gameParameters.mods = LinkedHashSet(mapMods.second) newGameScreen.gameSetupInfo.gameParameters.baseRuleset = mapMods.first.firstOrNull() ?: mapFileSelectBox.selected.mapParameters.baseRuleset - newGameScreen.updateRuleset() + val success = newGameScreen.updateRuleset() newGameScreen.updateTables() hideMiniMap() - startMapPreview(mapFile) + if (success) { + startMapPreview(mapFile) + } else { + // Mod error - the options have been reset by updateRuleset + // Note SelectBox doesn't react sensibly to _any_ clear - Group, Selection or items + val items = mapFileSelectBox.items + items.removeIndex(mapFileSelectBox.selectedIndex) + // Changing the array itself is not enough, SelectBox gets out of sync, need to call setItems() + mapFileSelectBox.items = items + // Note - this will have triggered a nested onSelectBoxChange()! + } } private fun startMapPreview(mapFile: FileHandle) { @@ -124,7 +146,7 @@ class MapFileSelectTable( if (!isActive) return@run map.setTransients(newGameScreen.ruleset, false) if (!isActive) return@run - // ReplyMap still paints outside its bounds - so we subtract padding and a little extra + // ReplayMap still paints outside its bounds - so we subtract padding and a little extra val size = (columnWidth - 40f).coerceAtMost(500f) val miniMap = LoadMapPreview(map, size, size) if (!isActive) return@run diff --git a/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt b/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt index 3950fbd3ff..5075a94a00 100644 --- a/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt +++ b/core/src/com/unciv/ui/screens/newgamescreen/MapOptionsTable.kt @@ -2,11 +2,11 @@ package com.unciv.ui.screens.newgamescreen import com.badlogic.gdx.scenes.scene2d.ui.Table import com.unciv.logic.map.MapGeneratedMainType -import com.unciv.ui.components.input.onChange import com.unciv.ui.components.extensions.toLabel +import com.unciv.ui.components.input.onChange import com.unciv.ui.screens.basescreen.BaseScreen -class MapOptionsTable(private val newGameScreen: NewGameScreen): Table() { +class MapOptionsTable(private val newGameScreen: NewGameScreen, isReset: Boolean = true): Table() { private val mapParameters = newGameScreen.gameSetupInfo.mapParameters private var mapTypeSpecificTable = Table() @@ -22,7 +22,14 @@ class MapOptionsTable(private val newGameScreen: NewGameScreen): Table() { val mapTypes = arrayListOf(MapGeneratedMainType.generated, MapGeneratedMainType.randomGenerated) if (savedMapOptionsTable.isNotEmpty()) mapTypes.add(MapGeneratedMainType.custom) - mapTypeSelectBox = TranslatedSelectBox(mapTypes, "Generated", BaseScreen.skin) + + // Pre-select custom if any map saved within last 15 minutes + val chooseCustom = !isReset && ( + savedMapOptionsTable.recentlySavedMapExists() || + savedMapOptionsTable.isNotEmpty() && mapParameters.type == MapGeneratedMainType.custom && mapParameters.name.isNotEmpty() + ) + val mapTypeDefault = if (chooseCustom) MapGeneratedMainType.custom else MapGeneratedMainType.generated + mapTypeSelectBox = TranslatedSelectBox(mapTypes, mapTypeDefault, BaseScreen.skin) fun updateOnMapTypeChange() { mapTypeSpecificTable.clear() @@ -50,11 +57,6 @@ class MapOptionsTable(private val newGameScreen: NewGameScreen): Table() { newGameScreen.updateTables() } - // Pre-select custom if any map saved within last 15 minutes - if (savedMapOptionsTable.recentlySavedMapExists()) - mapTypeSelectBox.selected = - TranslatedSelectBox.TranslatedString(MapGeneratedMainType.custom) - // activate once, so when we had a file map before we'll have the right things set for another one updateOnMapTypeChange() diff --git a/core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt b/core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt index b295a21c7c..43d778c036 100644 --- a/core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt +++ b/core/src/com/unciv/ui/screens/newgamescreen/NewGameScreen.kt @@ -11,12 +11,15 @@ import com.unciv.UncivGame import com.unciv.logic.GameInfo import com.unciv.logic.GameStarter import com.unciv.logic.IdChecker +import com.unciv.logic.UncivShowableException import com.unciv.logic.civilization.PlayerType import com.unciv.logic.files.MapSaver import com.unciv.logic.map.MapGeneratedMainType import com.unciv.logic.multiplayer.OnlineMultiplayer import com.unciv.logic.multiplayer.storage.FileStorageRateLimitReached +import com.unciv.models.metadata.BaseRuleset import com.unciv.models.metadata.GameSetupInfo +import com.unciv.models.ruleset.Ruleset import com.unciv.models.ruleset.RulesetCache import com.unciv.models.translations.tr import com.unciv.ui.components.ExpanderTab @@ -44,14 +47,16 @@ import com.unciv.utils.launchOnGLThread import kotlinx.coroutines.coroutineScope import java.net.URL import java.util.UUID +import kotlin.math.floor import com.unciv.ui.components.AutoScrollPane as ScrollPane class NewGameScreen( - _gameSetupInfo: GameSetupInfo? = null + defaultGameSetupInfo: GameSetupInfo? = null, + isReset: Boolean = false ): IPreviousScreen, PickerScreen(), RecreateOnResize { - override val gameSetupInfo = _gameSetupInfo ?: GameSetupInfo.fromSettings() - override var ruleset = RulesetCache.getComplexRuleset(gameSetupInfo.gameParameters) // needs to be set because the GameOptionsTable etc. depend on this + override var gameSetupInfo = defaultGameSetupInfo ?: GameSetupInfo.fromSettings() + override val ruleset = Ruleset() // updateRuleset will clear and add private val newGameOptionsTable: GameOptionsTable private val playerPickerTable: PlayerPickerTable private val mapOptionsTable: MapOptionsTable @@ -60,7 +65,6 @@ class NewGameScreen( val isPortrait = isNarrowerThan4to3() updateRuleset() // must come before playerPickerTable so mod nations from fromSettings - // Has to be initialized before the mapOptionsTable, since the mapOptionsTable refers to it on init // remove the victory types which are not in the rule set (e.g. were in the recently disabled mod) gameSetupInfo.gameParameters.victoryTypes.removeAll { it !in ruleset.victories.keys } @@ -78,7 +82,7 @@ class NewGameScreen( updatePlayerPickerTable = { desiredCiv -> playerPickerTable.update(desiredCiv) }, updatePlayerPickerRandomLabel = { playerPickerTable.updateRandomNumberLabel() } ) - mapOptionsTable = MapOptionsTable(this) + mapOptionsTable = MapOptionsTable(this, isReset) closeButton.onActivation { mapOptionsTable.cancelBackgroundJobs() game.popScreen() @@ -101,7 +105,7 @@ class NewGameScreen( "Are you sure you want to reset all game options to defaults?", "Reset to defaults", ) { - game.replaceCurrentScreen(NewGameScreen(GameSetupInfo())) + game.replaceCurrentScreen(NewGameScreen(GameSetupInfo(), isReset = true)) }.open(true) } } @@ -231,7 +235,7 @@ class NewGameScreen( /** Subtables may need an upper limit to their width - they can ask this function. */ // In sync with isPortrait in init, here so UI details need not know about 3-column vs 1-column layout - internal fun getColumnWidth() = stage.width / (if (isNarrowerThan4to3()) 1 else 3) + internal fun getColumnWidth() = floor(stage.width / (if (isNarrowerThan4to3()) 1 else 3)) private fun initLandscape() { scrollPane.setScrollingDisabled(true,true) @@ -354,11 +358,37 @@ class NewGameScreen( } } - fun updateRuleset() { + /** Updates our local [ruleset] from [gameSetupInfo], guarding against exceptions. + * + * Note: The options reset on failure is not propagated automatically to the Widgets - + * the caller must ensure that. + * + * @return Success - failure means gameSetupInfo was reset to defaults and the Ruleset was reverted to G&K + */ + fun updateRuleset(): Boolean { + var success = true + fun handleFailure(message: String): Ruleset { + success = false + gameSetupInfo = GameSetupInfo() + ToastPopup(message, this, 5000) + return RulesetCache[BaseRuleset.Civ_V_GnK.fullName]!! + } + + val newRuleset = try { + // this can throw with non-default gameSetupInfo, e.g. when Mods change or we change the impact of Mod errors + RulesetCache.getComplexRuleset(gameSetupInfo.gameParameters) + } catch (ex: UncivShowableException) { + handleFailure("«YELLOW»{Your previous options needed to be reset to defaults.}«»\n\n${ex.localizedMessage}") + } catch (ex: Throwable) { + Log.debug("updateRuleset failed", ex) + handleFailure("«RED»{Your previous options needed to be reset to defaults.}«»") + } + ruleset.clear() - ruleset.add(RulesetCache.getComplexRuleset(gameSetupInfo.gameParameters)) + ruleset.add(newRuleset) ImageGetter.setNewRuleset(ruleset) game.musicController.setModList(gameSetupInfo.gameParameters.getModsAndBaseRuleset()) + return success } fun lockTables() {