From 86aa3b842bddbb8e9367ea5f339f59af4ba2d36e Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Thu, 8 Jun 2023 11:13:22 +0200 Subject: [PATCH] Only *display* first 5 missing mods but auto-download all (#9543) * Only *display* first 5 missing mods but autodownload all * Fix removeMissingTerrainModReferences * Linting --- core/src/com/unciv/logic/GameInfo.kt | 6 ++-- core/src/com/unciv/logic/UncivExceptions.kt | 12 +++++-- core/src/com/unciv/logic/map/TileMap.kt | 11 +++--- core/src/com/unciv/logic/map/tile/Tile.kt | 14 ++++++-- .../mapeditorscreen/tabs/MapEditorLoadTab.kt | 34 +++++++------------ .../ui/screens/savescreens/LoadGameScreen.kt | 14 ++++---- 6 files changed, 48 insertions(+), 43 deletions(-) diff --git a/core/src/com/unciv/logic/GameInfo.kt b/core/src/com/unciv/logic/GameInfo.kt index 433dedac9e..7f321c0ed4 100644 --- a/core/src/com/unciv/logic/GameInfo.kt +++ b/core/src/com/unciv/logic/GameInfo.kt @@ -544,12 +544,10 @@ class GameInfo : IsPartOfGameInfoSerialization, HasGameInfoSerializationVersion // any mod the saved game lists that is currently not installed causes null pointer // exceptions in this routine unless it contained no new objects or was very simple. // Player's fault, so better complain early: - val missingMods = (gameParameters.mods + gameParameters.baseRuleset) + val missingMods = (listOf(gameParameters.baseRuleset) + gameParameters.mods) .filterNot { it in ruleset.mods } - .joinToString(limit = 5) { it } - if (missingMods.isNotEmpty()) { + if (missingMods.isNotEmpty()) throw MissingModsException(missingMods) - } removeMissingModReferences() diff --git a/core/src/com/unciv/logic/UncivExceptions.kt b/core/src/com/unciv/logic/UncivExceptions.kt index cb4883e8c1..6e855cd9f1 100644 --- a/core/src/com/unciv/logic/UncivExceptions.kt +++ b/core/src/com/unciv/logic/UncivExceptions.kt @@ -19,6 +19,14 @@ open class UncivShowableException( override fun getLocalizedMessage() = message.tr() } +/** An [Exception] indicating a game or map cannot be loaded because [mods][com.unciv.models.metadata.GameParameters.mods] are missing. + * @param missingMods Any [Iterable] or [Collection] of Strings - will be stored entirely, + * but be included in the Exception's message only up to its five first elements. + */ class MissingModsException( - val missingMods: String -) : UncivShowableException("Missing mods: [$missingMods]") + val missingMods: Iterable +) : UncivShowableException("Missing mods: [${shorten(missingMods)}]") { + companion object { + private fun shorten(missingMods: Iterable) = missingMods.joinToString(limit = 5) { it } + } +} diff --git a/core/src/com/unciv/logic/map/TileMap.kt b/core/src/com/unciv/logic/map/TileMap.kt index 92be1ab1db..282ce35f16 100644 --- a/core/src/com/unciv/logic/map/TileMap.kt +++ b/core/src/com/unciv/logic/map/TileMap.kt @@ -464,16 +464,13 @@ class TileMap(initialCapacity: Int = 10) : IsPartOfGameInfoSerialization { } fun removeMissingTerrainModReferences(ruleSet: Ruleset) { + // This will run before setTransients, so do not rely e.g. on Tile.ruleset being available. + // That rules out Tile.removeTerrainFeature, which refreshes object/unique caches for (tile in this.values) { - for (terrainFeature in tile.terrainFeatures.filter { !ruleSet.terrains.containsKey(it) }) - tile.removeTerrainFeature(terrainFeature) - if (tile.resource != null && !ruleSet.tileResources.containsKey(tile.resource!!)) - tile.resource = null - if (tile.improvement != null && !ruleSet.tileImprovements.containsKey(tile.improvement!!)) - tile.changeImprovement(null) + tile.removeMissingTerrainModReferences(ruleSet) } for (startingLocation in startingLocations.toList()) - if (startingLocation.nation !in ruleSet.nations.keys) + if (startingLocation.nation !in ruleSet.nations) startingLocations.remove(startingLocation) } diff --git a/core/src/com/unciv/logic/map/tile/Tile.kt b/core/src/com/unciv/logic/map/tile/Tile.kt index fdfc5b6533..ae15b86863 100644 --- a/core/src/com/unciv/logic/map/tile/Tile.kt +++ b/core/src/com/unciv/logic/map/tile/Tile.kt @@ -821,7 +821,7 @@ open class Tile : IsPartOfGameInfoSerialization { else probability } - fun setTerrainFeatures(terrainFeatureList:List) { + fun setTerrainFeatures(terrainFeatureList: List) { terrainFeatures = terrainFeatureList terrainFeatureObjects = terrainFeatureList.mapNotNull { ruleset.terrains[it] } allTerrains = sequence { @@ -843,7 +843,7 @@ open class Tile : IsPartOfGameInfoSerialization { terrainUniqueMap = newUniqueMap } - fun addTerrainFeature(terrainFeature:String) = + fun addTerrainFeature(terrainFeature: String) = setTerrainFeatures(ArrayList(terrainFeatures).apply { add(terrainFeature) }) fun removeTerrainFeature(terrainFeature: String) = @@ -852,6 +852,16 @@ open class Tile : IsPartOfGameInfoSerialization { fun removeTerrainFeatures() = setTerrainFeatures(listOf()) + /** Clean stuff missing in [ruleset] - called from [TileMap.removeMissingTerrainModReferences] + * Must be able to run before [setTransients] - and does not need to fix transients. + */ + fun removeMissingTerrainModReferences(ruleset: Ruleset) { + terrainFeatures = terrainFeatures.filter { it in ruleset.terrains } + if (resource != null && resource !in ruleset.tileResources) + resource = null + if (improvement != null && improvement !in ruleset.tileImprovements) + changeImprovement(null) + } /** If the unit isn't in the ruleset we can't even know what type of unit this is! So check each place * This works with no transients so can be called from gameInfo.setTransients with no fear diff --git a/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorLoadTab.kt b/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorLoadTab.kt index 970061b6ec..cbae59396c 100644 --- a/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorLoadTab.kt +++ b/core/src/com/unciv/ui/screens/mapeditorscreen/tabs/MapEditorLoadTab.kt @@ -4,7 +4,7 @@ import com.badlogic.gdx.Gdx import com.badlogic.gdx.files.FileHandle import com.badlogic.gdx.graphics.Color import com.badlogic.gdx.scenes.scene2d.ui.Table -import com.unciv.Constants +import com.unciv.logic.MissingModsException import com.unciv.logic.files.MapSaver import com.unciv.logic.UncivShowableException import com.unciv.models.ruleset.RulesetCache @@ -112,29 +112,21 @@ class MapEditorLoadTab( val map = MapSaver.loadMap(chosenMap!!) if (!isActive) return - val missingMods = map.mapParameters.mods.filter { it !in RulesetCache }.toMutableList() - // [TEMPORARY] conversion of old maps with a base ruleset contained in the mods - val newBaseRuleset = map.mapParameters.mods.filter { it !in missingMods }.firstOrNull { RulesetCache[it]!!.modOptions.isBaseRuleset } - if (newBaseRuleset != null) map.mapParameters.baseRuleset = newBaseRuleset - // - if (map.mapParameters.baseRuleset !in RulesetCache) missingMods += map.mapParameters.baseRuleset + // For deprecated maps, set the base ruleset field if it's still saved in the mods field + val modBaseRuleset = map.mapParameters.mods.firstOrNull { RulesetCache[it]?.modOptions?.isBaseRuleset == true } + if (modBaseRuleset != null) { + map.mapParameters.baseRuleset = modBaseRuleset + map.mapParameters.mods -= modBaseRuleset + } - if (missingMods.isNotEmpty()) { - Concurrency.runOnGLThread { - needPopup = false - popup?.close() - ToastPopup("Missing mods: [${missingMods.joinToString()}]", editorScreen) - } - } else Concurrency.runOnGLThread { + val missingMods = (setOf(map.mapParameters.baseRuleset) + map.mapParameters.mods) + .filterNot { it in RulesetCache } + if (missingMods.isNotEmpty()) + throw MissingModsException(missingMods) + + Concurrency.runOnGLThread { Gdx.input.inputProcessor = null // This is to stop ANRs happening here, until the map editor screen sets up. try { - // For deprecated maps, set the base ruleset field if it's still saved in the mods field - val modBaseRuleset = map.mapParameters.mods.firstOrNull { RulesetCache[it]!!.modOptions.isBaseRuleset } - if (modBaseRuleset != null) { - map.mapParameters.baseRuleset = modBaseRuleset - map.mapParameters.mods -= modBaseRuleset - } - val ruleset = RulesetCache.getComplexRuleset(map.mapParameters) val rulesetIncompatibilities = map.getRulesetIncompatibility(ruleset) if (rulesetIncompatibilities.isNotEmpty()) { diff --git a/core/src/com/unciv/ui/screens/savescreens/LoadGameScreen.kt b/core/src/com/unciv/ui/screens/savescreens/LoadGameScreen.kt index 4cae151058..4236c33236 100644 --- a/core/src/com/unciv/ui/screens/savescreens/LoadGameScreen.kt +++ b/core/src/com/unciv/ui/screens/savescreens/LoadGameScreen.kt @@ -35,7 +35,7 @@ class LoadGameScreen : LoadOrSaveScreen() { private val copySavedGameToClipboardButton = getCopyExistingSaveToClipboardButton() private val errorLabel = "".toLabel(Color.RED).apply { isVisible = false } private val loadMissingModsButton = getLoadMissingModsButton() - private var missingModsToLoad = "" + private var missingModsToLoad: Iterable = emptyList() companion object { private const val loadGame = "Load game" @@ -61,11 +61,11 @@ class LoadGameScreen : LoadOrSaveScreen() { isUserFixable = false } is FileNotFoundException -> { - if (ex.cause?.message?.contains("Permission denied") == true) { + isUserFixable = if (ex.cause?.message?.contains("Permission denied") == true) { errorText.append("You do not have sufficient permissions to access the file.".tr()) - isUserFixable = true + true } else { - isUserFixable = false + false } } else -> { @@ -245,8 +245,8 @@ class LoadGameScreen : LoadOrSaveScreen() { descriptionLabel.setText(Constants.loading.tr()) Concurrency.runOnNonDaemonThreadPool(downloadMissingMods) { try { - val mods = missingModsToLoad.replace(' ', '-').lowercase().splitToSequence(",-") - for (modName in mods) { + for (rawName in missingModsToLoad) { + val modName = rawName.replace(' ', '-').lowercase() val repos = Github.tryGetGithubReposWithTopic(10, 1, modName) ?: throw UncivShowableException("Could not download mod list.") val repo = repos.items.firstOrNull { it.name.lowercase() == modName } @@ -264,7 +264,7 @@ class LoadGameScreen : LoadOrSaveScreen() { } launchOnGLThread { RulesetCache.loadRulesets() - missingModsToLoad = "" + missingModsToLoad = emptyList() loadMissingModsButton.isVisible = false errorLabel.isVisible = false rightSideTable.pack()