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

@@ -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()
 

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<String>
+) : UncivShowableException("Missing mods: [${shorten(missingMods)}]") {
+    companion object {
+        private fun shorten(missingMods: Iterable<String>) = missingMods.joinToString(limit = 5) { it }
+    }
+}

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)
     }
 

core/src/com/unciv/logic/map/tile/Tile.kt

@@ -821,7 +821,7 @@ open class Tile : IsPartOfGameInfoSerialization {
             else probability
     }
 
-    fun setTerrainFeatures(terrainFeatureList:List<String>) {
+    fun setTerrainFeatures(terrainFeatureList: List<String>) {
         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

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()) {

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<String> = 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()