yairm210 / Unciv

Open-source Android/Desktop remake of Civ V
Mozilla Public License 2.0
8.3k stars 1.56k forks source link

Feature request: Ability to import mod from a ZIP file or non-GitHub Repo / Enable download mods on IPv6-only devices #9132

Closed marek22k closed 2 months ago

marek22k commented 1 year ago

Hello,

I would like to import a mod into UnCiv which is not on GitHub. When I enter the URL, "Invalid URL" appears. I can't copy any data to the Android/data directory (I guess because of Android 13). How can I download a mod and import it to UnCiv without it being on GutHub?

Is it possible to import a ZIP file?

(This feature request differs from #9123 in that I want to manually import the mod here.)

SomeTroglodyte commented 1 year ago

Consider this: A modder using github has shown some minimal commitment and is accessible to at least some peer review. Importing any zip opens up avenues for cr... unfathomable quality content or even malware shooting down our game. And people not always lay blame on obvious culprits (themselves in this case)...

If you have the know-how to protect yourself from unsavoury content, then you certainly have the ability to manually unpack a mod where it will be recognized? And don't tell me you can't due to being on mobile, in which case argument 1 applies tenfold.

:wink: technically this request might perhaps be rather easy [^1], and maybe others think that a swell idea, but that's my feeling.

[^1]: Coders: I think maybe only the Author is currently required to be fetched from the URL and not available within the normal repo ZIP, but we could always just insert "Untrustworthy Source" or something there?

marek22k commented 1 year ago

I know by where I would have to pack the mod data, but unfortunately I due to A13 but no authorization for it. The only thing I could do would be to use adb (with root).

Regarding the other argument: Maybe it would be possible to "hide" the option somewhere - like in Android the Developer Settings. Furthermore, I don't see how you could "hack" the game with JSON files only. The only possible thing would be to crash the game. Apart from that, malicious individuals could also publish harmful mods on GitHub.

yairm210 commented 1 year ago

I'm marking this as "won't fix" since it's a deprioritized issue. If within the next year no one starts working on this I'll close the issue

SomeTroglodyte commented 1 year ago

FWIW, I've tried it - just to have it easier debugging another issue - and shelved it as "too unclean/fragile".

As patch ```patch Index: core/src/com/unciv/ui/screens/pickerscreens/ModManagementScreen.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/ui/screens/pickerscreens/ModManagementScreen.kt b/core/src/com/unciv/ui/screens/pickerscreens/ModManagementScreen.kt --- a/core/src/com/unciv/ui/screens/pickerscreens/ModManagementScreen.kt +++ b/core/src/com/unciv/ui/screens/pickerscreens/ModManagementScreen.kt @@ -32,6 +32,7 @@ import com.unciv.ui.components.extensions.isEnabled import com.unciv.ui.components.extensions.keyShortcuts import com.unciv.ui.components.extensions.onActivation +import com.unciv.ui.components.extensions.onChange import com.unciv.ui.components.extensions.onClick import com.unciv.ui.components.extensions.toCheckBox import com.unciv.ui.components.extensions.toLabel @@ -44,6 +45,7 @@ import com.unciv.ui.screens.basescreen.RecreateOnResize import com.unciv.ui.screens.mainmenuscreen.MainMenuScreen import com.unciv.ui.screens.pickerscreens.ModManagementOptions.SortType +import com.unciv.utils.DebugUtils import com.unciv.utils.Log import com.unciv.utils.concurrency.Concurrency import com.unciv.utils.concurrency.launchOnGLThread @@ -413,11 +415,15 @@ actualDownloadButton.setText("Downloading...".tr()) actualDownloadButton.disable() val repo = Github.Repo().parseUrl(textField.text) - if (repo == null) { + if (repo != null) { + downloadMod(repo) { popup.close() } + } else if (DebugUtils.ENABLE_ADVANCED_MOD_DOWNLOAD && + Regex("""^(https?|file)://.*[^/]\.zip$""").matches(textField.text)) { + advancedDownloadFromUrl(popup, textField.text) + } else { ToastPopup("Invalid link!", this@ModManagementScreen) popup.close() // Re-enabling button would be nice, but Toast doesn't work over other Popups - } else - downloadMod(repo) { popup.close() } + } } popup.add(actualDownloadButton).row() popup.addCloseButton() @@ -426,6 +432,46 @@ return downloadButton } + private fun advancedDownloadFromUrl(popup: Popup, url: String) { + popup.reuseWith("Advanced mod download") + popup.addSeparator(Color.DARK_GRAY) + popup.add(Label(url, skin)) + popup.addSeparator(Color.DARK_GRAY) + popup.addGoodSizedLabel("Please enter mod name and author:").row() + + val nameField = UncivTextField.create("Mod name").apply { maxLength = 120 } + val authorField = UncivTextField.create("Mod author").apply { maxLength = 120 } + val actualDownloadButton = "Download".toTextButton() + val nameMatch = Regex("""^.*/([^/]+)\.zip$""").matchEntire(url) + if (nameMatch != null && nameMatch.groups.size > 1) + nameField.text = nameMatch.groups[1]?.value?.replace('.', ' ') ?: "" + + fun check() { + actualDownloadButton.isEnabled = nameField.text.isNotBlank() && authorField.text.isNotBlank() + } + nameField.onChange { check() } + authorField.onChange { check() } + actualDownloadButton.onClick { + actualDownloadButton.setText("Downloading...".tr()) + actualDownloadButton.disable() + val repo = Github.Repo() + repo.html_url = url + repo.name = nameField.text + //repo.default_branch is ""! + repo.owner.login = authorField.text + downloadMod(repo) { success -> + if (success) popup.close() + else check() + } + } + check() + + popup.add(nameField).width(stage.width / 2).row() + popup.add(authorField).width(stage.width / 2).row() + popup.add(actualDownloadButton).row() + popup.addCloseButton() + } + private fun updateModInfo() { if (selectedMod != null) { modActionTable.clear() @@ -477,7 +523,7 @@ } /** Download and install a mod in the background, called both from the right-bottom button and the URL entry popup */ - private fun downloadMod(repo: Github.Repo, postAction: () -> Unit = {}) { + private fun downloadMod(repo: Github.Repo, postAction: (Boolean) -> Unit = {}) { Concurrency.run("DownloadMod") { // to avoid ANRs - we've learnt our lesson from previous download-related actions try { val modFolder = Github.downloadAndExtract( @@ -497,12 +543,13 @@ refreshInstalledModTable() showModDescription(repo.name) unMarkUpdatedMod(repo.name) - postAction() + postAction(true) } } catch (ex: Exception) { + Log.debug("Mod download failed", ex) launchOnGLThread { ToastPopup("Could not download [${repo.name}]", this@ModManagementScreen) - postAction() + postAction(false) } } } Index: core/src/com/unciv/ui/screens/pickerscreens/GitHub.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/ui/screens/pickerscreens/GitHub.kt b/core/src/com/unciv/ui/screens/pickerscreens/GitHub.kt --- a/core/src/com/unciv/ui/screens/pickerscreens/GitHub.kt +++ b/core/src/com/unciv/ui/screens/pickerscreens/GitHub.kt @@ -61,6 +61,7 @@ /** * Download a mod and extract, deleting any pre-existing version. + * @param repo A [GitHub.Repo] filled with at least the html_url * @param folderFileHandle Destination handle of mods folder - also controls Android internal/external * @author **Warning**: This took a long time to get just right, so if you're changing this, ***TEST IT THOROUGHLY*** on _both_ Desktop _and_ Phone * @return FileHandle for the downloaded Mod's folder or null if download failed @@ -72,7 +73,7 @@ val defaultBranch = repo.default_branch val gitRepoUrl = repo.html_url // Initiate download - the helper returns null when it fails - val zipUrl = "$gitRepoUrl/archive/$defaultBranch.zip" + val zipUrl = if (defaultBranch.isEmpty()) gitRepoUrl else "$gitRepoUrl/archive/$defaultBranch.zip" val inputStream = download(zipUrl) ?: return null // Get a mod-specific temp file name @@ -95,6 +96,10 @@ val finalDestinationName = innerFolder.name().replace("-$defaultBranch", "").replace('-', ' ') // finalDestinationName is now the mod name as we display it. Folder name needs to be identical. val finalDestination = folderFileHandle.child(finalDestinationName) + if (repo.default_branch.isEmpty()) { + repo.name = finalDestinationName + repo.html_url = "" + } // prevent mixing new content with old var tempBackup: FileHandle? = null Index: core/src/com/unciv/utils/Debug.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/utils/Debug.kt b/core/src/com/unciv/utils/Debug.kt --- a/core/src/com/unciv/utils/Debug.kt +++ b/core/src/com/unciv/utils/Debug.kt @@ -21,4 +21,6 @@ */ var SIMULATE_UNTIL_TURN: Int = 0 + /** Allows downloading a mod zip from any URL with manual mod name and author entry */ + var ENABLE_ADVANCED_MOD_DOWNLOAD: Boolean = true } Index: core/src/com/unciv/ui/popups/ToastPopup.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/ui/popups/ToastPopup.kt b/core/src/com/unciv/ui/popups/ToastPopup.kt --- a/core/src/com/unciv/ui/popups/ToastPopup.kt +++ b/core/src/com/unciv/ui/popups/ToastPopup.kt @@ -21,7 +21,7 @@ onClick { close() } // or `touchable = Touchable.disabled` so you can operate what's behind addGoodSizedLabel(message) - open() + open(true) //move it to the top so its not in the middle of the screen //have to be done after open() because open() centers the popup y = stageToShowOn.height - (height + 20f) ```

So, even after coding a working POC I really still think it's best to keep it Repo only for now.

marek22k commented 1 year ago

We get Author and mod name from the repo so that would need replacements

Author Name could be taken from the ModOptions.json? And if this is invalid, you could reject the mod.

We trust a zip to contain a subfolder named same as the repo

This could also be checked and if it is not the case, it could be rejected as invalid.

The mod I tested with was badly bugged - came with a modOptions.json that was not loadable - and getting any sanity checks / exception messages back into the UI would need some more kludges

Couldn't you display a message like it is currently for non-GitHub URLs?

SomeTroglodyte commented 1 year ago

Yes (or ask), yes and yes and the resulting card house (which I included) is too ugly and too hard to get right / maintain

SeventhM commented 1 year ago

Since this is a mobile related question only, is it possible to establish a folder at /Unciv/Mods ? I'm sure this will probably require an extra permission, but it'll allow using the same system that Desktop uses to accept local mods.

marek22k commented 1 year ago

Another advantage of having an option to download mods from non-GitHub sources (regardless of whether ZIP or from Codeberg) is that IPv6-only devices can also download mods. Unfortunately, GitHub is IPv4-only. So for people who want to download mods on IPv6-only networks, this is currently impossible. (I thought an extra issue titled "Failed to download mods on IPv6-only device" was a duplicate. Hence my post here)

SomeTroglodyte commented 1 year ago

The problem with the demo I did six weeks a go is basically metadata. An active issue about maps-in-nods is precisely about metadata too. If we get that defined, a workable decision about a data schema, then a "wild" zip will only need to be checked that enough metadata is in ModOptions, and be rejected - E.g. should we declare Author to be mandatory and it's not there.. Much easier.

github-actions[bot] commented 5 months ago

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 15 days.

marek22k commented 5 months ago

Please do not clone. @github-actions

SeventhM commented 5 months ago

Ngl, when I passed by this issue a while ago I somewhat wasn't paying attention and though it was more about accessing a mod outside of the Android/data folder in general. The idea of accepting any zip seems like it could be a pain all on its own, though

As patch

Idk if OP would be open to this suggestion, but what of there was a way to just import zips? Not zip urls. Just zips that were already downloaded. Because that seems like something that could be much safer and easier for validation checking

github-actions[bot] commented 2 months ago

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 15 days.

github-actions[bot] commented 2 months ago

This issue was closed because it has been stalled for 5 days with no activity.

marek22k commented 2 months ago

Bump