Mod name defense attempt II (#9645)

* Improve Load game error label readability

* Fix threading on load game screen

* Miscellaneous tweaks

* Compatibility with Mods using trailing dashes on Windows

android/src/com/unciv/app/AndroidLauncher.kt

@@ -61,7 +61,8 @@ open class AndroidLauncher : AndroidApplication() {
         val internalModsDir = File("${filesDir.path}/mods")
 
         // Mod directory in the shared app data (where the user can see and modify)
-        val externalModsDir = File("${getExternalFilesDir(null)?.path}/mods")
+        val externalPath = getExternalFilesDir(null)?.path ?: return
+        val externalModsDir = File("$externalPath/mods")
 
         // Copy external mod directory (with data user put in it) to internal (where it can be read)
         if (!externalModsDir.exists()) externalModsDir.mkdirs() // this can fail sometimes, which is why we check if it exists again in the next line

core/src/com/unciv/logic/GameInfo.kt

@@ -37,6 +37,7 @@ import com.unciv.models.ruleset.nation.Difficulty
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.ui.audio.MusicMood
 import com.unciv.ui.audio.MusicTrackChooserFlags
+import com.unciv.ui.screens.pickerscreens.Github.repoNameToFolderName
 import com.unciv.ui.screens.savescreens.Gzip
 import com.unciv.ui.screens.worldscreen.status.NextTurnProgress
 import com.unciv.utils.DebugUtils
@@ -540,6 +541,14 @@ class GameInfo : IsPartOfGameInfoSerialization, HasGameInfoSerializationVersion
         // [TEMPORARY] Convert old saves to newer ones by moving base rulesets from the mod list to the base ruleset field
         convertOldSavesToNewSaves()
 
+        // Cater for the mad modder using trailing '-' in their repo name - convert the mods list so
+        // it requires our new, Windows-safe local name (no trailing blanks)
+        for ((oldName, newName) in gameParameters.mods.map { it to it.repoNameToFolderName() }) {
+            if (newName == oldName) continue
+            gameParameters.mods.remove(oldName)
+            gameParameters.mods.add(newName)
+        }
+
         ruleset = RulesetCache.getComplexRuleset(gameParameters)
 
         // any mod the saved game lists that is currently not installed causes null pointer

core/src/com/unciv/ui/screens/pickerscreens/GitHub.kt

@@ -94,7 +94,7 @@ object Github {
 
         val innerFolder = unzipDestination.list().first()
         // innerFolder should now be "$tempName/$repoName-$defaultBranch/" - use this to get mod name
-        val finalDestinationName = innerFolder.name().replace("-$defaultBranch", "").replace('-', ' ')
+        val finalDestinationName = innerFolder.name().replace("-$defaultBranch", "").repoNameToFolderName()
         // finalDestinationName is now the mod name as we display it. Folder name needs to be identical.
         val finalDestination = folderFileHandle.child(finalDestinationName)
 
@@ -453,6 +453,32 @@ object Github {
         modOptions.updateDeprecations()
         json().toJson(modOptions, modOptionsFile)
     }
+
+    private const val outerBlankReplacement = '='
+    // Github disallows **any** special chars and replaces them with '-' - so use something ascii the
+    // OS accepts but still is recognizable as non-original, to avoid confusion
+
+    /** Convert a [Repo] name to a local name for both display and folder name
+     *
+     *  Replaces '-' with blanks but ensures no leading or trailing blanks.
+     *  As mad modders know no limits, trailing "-" did indeed happen, causing things to break due to trailing blanks on a folder name.
+     *  As "test-" and "test" are different allowed repository names, trimmed blanks are replaced with one overscore per side.
+     */
+    fun String.repoNameToFolderName(): String {
+        var result = replace('-', ' ')
+        if (result.endsWith(' ')) result = result.trimEnd() + outerBlankReplacement
+        if (result.startsWith(' ')) result = outerBlankReplacement + result.trimStart()
+        return result
+    }
+
+    /** Inverse of [repoNameToFolderName] */
+    // As of this writing, only used for loadMissingMods
+    fun String.folderNameToRepoName(): String {
+        var result = replace(' ', '-')
+        if (result.endsWith(outerBlankReplacement)) result = result.trimEnd(outerBlankReplacement) + '-'
+        if (result.startsWith(outerBlankReplacement)) result = '-' + result.trimStart(outerBlankReplacement)
+        return result
+    }
 }
 
 /** Utility - extract Zip archives

core/src/com/unciv/ui/screens/pickerscreens/ModManagementScreen.kt

@@ -45,6 +45,7 @@ import com.unciv.ui.popups.ToastPopup
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.basescreen.RecreateOnResize
 import com.unciv.ui.screens.mainmenuscreen.MainMenuScreen
+import com.unciv.ui.screens.pickerscreens.Github.repoNameToFolderName
 import com.unciv.ui.screens.pickerscreens.ModManagementOptions.SortType
 import com.unciv.utils.Concurrency
 import com.unciv.utils.Log
@@ -258,7 +259,7 @@ class ModManagementScreen(
 
         for (repo in repoSearch.items) {
             if (stopBackgroundTasks) return
-            repo.name = repo.name.replace('-', ' ')
+            repo.name = repo.name.repoNameToFolderName()
 
             if (onlineModInfo.containsKey(repo.name))
                 continue // we already got this mod in a previous download, since one has been added in between

core/src/com/unciv/ui/screens/savescreens/LoadGameScreen.kt

@@ -26,6 +26,7 @@ import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
 import com.unciv.ui.popups.LoadingPopup
+import com.unciv.ui.screens.pickerscreens.Github.folderNameToRepoName
 import com.unciv.utils.Log
 import com.unciv.utils.Concurrency
 import com.unciv.utils.launchOnGLThread
@@ -33,7 +34,7 @@ import java.io.FileNotFoundException
 
 class LoadGameScreen : LoadOrSaveScreen() {
     private val copySavedGameToClipboardButton = getCopyExistingSaveToClipboardButton()
-    private val errorLabel = "".toLabel(Color.RED).apply { isVisible = false }
+    private val errorLabel = "".toLabel(Color.RED)
     private val loadMissingModsButton = getLoadMissingModsButton()
     private var missingModsToLoad: Iterable<String> = emptyList()
 
@@ -78,6 +79,9 @@ class LoadGameScreen : LoadOrSaveScreen() {
     }
 
     init {
+        errorLabel.isVisible = false
+        errorLabel.wrap = true
+
         setDefaultCloseAction()
         rightSideTable.initRightSideTable()
         rightSideButton.onActivation { onLoadGame() }
@@ -108,7 +112,7 @@ class LoadGameScreen : LoadOrSaveScreen() {
     private fun Table.initRightSideTable() {
         add(getLoadFromClipboardButton()).row()
         addLoadFromCustomLocationButton()
-        add(errorLabel).row()
+        add(errorLabel).width(stage.width / 2).row()
         add(loadMissingModsButton).row()
         add(deleteSaveButton).row()
         add(copySavedGameToClipboardButton).row()
@@ -141,6 +145,7 @@ class LoadGameScreen : LoadOrSaveScreen() {
     private fun getLoadFromClipboardButton(): TextButton {
         val pasteButton = loadFromClipboard.toTextButton()
         pasteButton.onActivation {
+            if (!Gdx.app.clipboard.hasContents()) return@onActivation
             pasteButton.setText(Constants.working.tr())
             pasteButton.disable()
             Concurrency.run(loadFromClipboard) {
@@ -220,17 +225,17 @@ class LoadGameScreen : LoadOrSaveScreen() {
         Log.error("Error while loading game", ex)
         val (errorText, isUserFixable) = getLoadExceptionMessage(ex, primaryText)
 
-        if (!isUserFixable) {
-            val cantLoadGamePopup = Popup(this@LoadGameScreen)
-            cantLoadGamePopup.addGoodSizedLabel("It looks like your saved game can't be loaded!").row()
-            cantLoadGamePopup.addGoodSizedLabel("If you could copy your game data (\"Copy saved game to clipboard\" - ").row()
-            cantLoadGamePopup.addGoodSizedLabel("  paste into an email to yairm210@hotmail.com)").row()
-            cantLoadGamePopup.addGoodSizedLabel("I could maybe help you figure out what went wrong, since this isn't supposed to happen!").row()
-            cantLoadGamePopup.addCloseButton()
-            cantLoadGamePopup.open()
-        }
-
         Concurrency.runOnGLThread {
+            if (!isUserFixable) {
+                val cantLoadGamePopup = Popup(this@LoadGameScreen)
+                cantLoadGamePopup.addGoodSizedLabel("It looks like your saved game can't be loaded!").row()
+                cantLoadGamePopup.addGoodSizedLabel("If you could copy your game data (\"Copy saved game to clipboard\" - ").row()
+                cantLoadGamePopup.addGoodSizedLabel("  paste into an email to yairm210@hotmail.com)").row()
+                cantLoadGamePopup.addGoodSizedLabel("I could maybe help you figure out what went wrong, since this isn't supposed to happen!").row()
+                cantLoadGamePopup.addCloseButton()
+                cantLoadGamePopup.open()
+            }
+
             errorLabel.setText(errorText)
             errorLabel.isVisible = true
             if (ex is MissingModsException) {
@@ -246,7 +251,7 @@ class LoadGameScreen : LoadOrSaveScreen() {
         Concurrency.runOnNonDaemonThreadPool(downloadMissingMods) {
             try {
                 for (rawName in missingModsToLoad) {
-                    val modName = rawName.replace(' ', '-').lowercase()
+                    val modName = rawName.folderNameToRepoName().lowercase()
                     val repos = Github.tryGetGithubReposWithTopic(10, 1, modName)
                         ?: throw UncivShowableException("Could not download mod list.")
                     val repo = repos.items.firstOrNull { it.name.lowercase() == modName }
@@ -255,7 +260,7 @@ class LoadGameScreen : LoadOrSaveScreen() {
                         repo,
                         Gdx.files.local("mods")
                     )
-                        ?: throw Exception("downloadAndExtract returns null for 404 errors and the like") // downloadAndExtract returns null for 404 errors and the like -> display something!
+                        ?: throw Exception("Unexpected 404 error") // downloadAndExtract returns null for 404 errors and the like -> display something!
                     Github.rewriteModOptions(repo, modFolder)
                     val labelText = descriptionLabel.text // Surprise - a StringBuilder
                     labelText.appendLine()
@@ -273,8 +278,10 @@ class LoadGameScreen : LoadOrSaveScreen() {
             } catch (ex: Exception) {
                 handleLoadGameException(ex, "Could not load the missing mods!")
             } finally {
-                loadMissingModsButton.isEnabled = true
+                launchOnGLThread {
-                descriptionLabel.setText("")
+                    loadMissingModsButton.isEnabled = true
+                    descriptionLabel.setText("")
+                }
             }
         }
     }