Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spruced up ModManagementScreen - phase 1 #3983

Merged
merged 2 commits into from
Jun 1, 2021

Conversation

SomeTroglodyte
Copy link
Collaborator

@SomeTroglodyte SomeTroglodyte commented May 22, 2021

This is the trimmed down version of - and will resolve #3912 (even if you don't like this we'll close that one). Merging bits into a fresh branch was hopeless, diff matched and grouped too wildly off, both AS and Meld. So I took the other way round and trimmed away - which must serve as an excuse for the fact this is still quite voluminous.

Features

  • The Next button annoyed me - gone - it now requests next page automatically. I saw no reason against - the API response is ridiculously bloated repeating redundant url parts 50 times, but it's still kilobytes not megabytes...
  • A "..." indicator wandering down the list until done - yes I tested with pagesize=1.
  • Handling of the API's rate limit - waiting until we can proceed, fetching github's idea of count/window from response headers, the works. Also tested with pagesize=1 - it does finish the list in several minutes.
  • More information and a few more fields persisted locally - mostly to reduce the difference between looking at an installed version vs looking in the middle column: author and size. Size can help an old-phone user decide, and author name - well, promote pride in one's work and increase interest that way... Also, might help clarify that that stuff is not Unciv if some copyright hound finds nonfree items in a confiscated phone's Unciv internal folder.
  • An 'update available' mini-icon on the installed mod button - like the visual indicator.
  • Nicer text on the download button when updating a mod.
  • Installed mods sorted alphabetically.
  • Sort by stars passed explicitly in the GitHub query.
  • Lists synchronize (without resorting) - finding and updating mods can be a lot easier. Yes I tested with most available mods installed.

Bugfixes

  • Due to time passing between GitHub search calls and probably effects like people starring / unstarring / changing unciv-mod topic..., the list had duplicates from time to time. Fixed the hard way with a dupe check after the end of the scan.
  • For the same reasons a mod might be skipped -> for this I did a detector only and users get a label at the end of the list (clickable to retry)
    (both effects aren't so rare - one or the other hits at least 33% of the time in my experience, way too much for the offered explanations, but I can't come up with better ones)
  • The next button left gaps. It's gone but the "..." indicator does not leave the same gaps.
  • Extraction after updating an existing mod would merge folders, possibly leaving stuff removed by the author. Pre-existing tempZip folder was not checked - granted shouldn't happen normally but since the extractor uses no content disposition on opening its files, it would be able to, say, find a game.png of 200k in the old folder, then write a game.png of 100k into that, overwriting 100k but leaving the rest... I decided to guard against that too.
  • File timestamps were left in arbitrary order (FileHandle.list() guarantees no ordering), every single file getting a fresh lastModifiedTime, and most of the time the atlas is touched last. Thus, desktop would most of the time re-pack on next launch. Fixed by moving atlas files into mod root last.
  • My old code to join local and online information was a fluke allowing pretty garbled combinations at times, fixed.

Maybe-bugfixes

  • Date formatting in mod manager and load game was wildly different, so I centralized the concept (and put the result in ExtensionFunctions - not deeply thought out). If you concur, any new usecase will use that, and formatting will be consistent, even if we choose to change the format later...
  • A little Layout cleanup - the two scrollpanes had different padding and a few more
  • When the repo zip contains another zip, it gets unpacked recursively. Before, destination was mod root and the zip remained after unpacking. Since nobody is using the feature ATM and it's afaik undocumented, I changed it to unzip relative to the sub-zip's location and delete the sub-zip afterwards. Open to discussion, result should be documented on some modder instructions page.

Unsuccessfully tried

Could we check whether an author has updated its images but forgotten to repack his atlas or push it? My investigations showed - none of the file operations kept lastModified, which doesn't matter because a GitHub zip contains timestamps that have notching to do with the age of the contained files - it generates a fresh zip every time setting all timestamps to the request time. So, a check for atlas validity would involve several GitHub api calls - sorry I stopped there.

Features Removed for this step

  • Indicators for 'Mod is used in savegames': They work nicely, background thread so you can watch them pop in lazily. A page or two of extra code.
  • Mod splash screens right side top, defaulting to author avatar. Some deco sometimes goes a long way to make a product more attractive I thought... Complete with lazy downloader and images even popping in while you look at the page where they're outstanding - and caching beyond session lifetime. Some leeway for improvement in queue priority on the todo list. A bit larger than the other feature...

And a few instances of linting - some you might not like, I probably didn't myself, but sometimes it's easier to accept an elvis operator than silencing Android Studio in other ways. As always, criticism welcome.

This did a few rounds of testing, most desktop, but also including an Oreo AVD and my Pie-level physical tablet.

@yairm210
Copy link
Owner

Since you've added about 200 lines of Github-related code, please move all Github functions out of Dropbox file and into a Github file

@yairm210 yairm210 merged commit 9ed73d0 into yairm210:master Jun 1, 2021
@SomeTroglodyte SomeTroglodyte deleted the ModManager-phase1 branch June 1, 2021 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inviting testers for spruced-up Mod manager screen
2 participants