Batch downloading #6
Conversation
|
Hey, thanks for the contribution! I should hopefully be able to take at look at this tomorrow, but I just wanted to let you know that I've seen it, and at a quick glance it looks good! Adding back NUSD's script support to NUSGet has definitely been on my agenda for a bit, but I can only handle working with GUIs in short bursts so I hadn't gotten there yet. I very much appreciate the help with it! |
There was a problem hiding this comment.
Looks pretty good overall!
I do have one concern, which is that if any title listed in the script is invalid, that causes an error when you get to that entry and then the bulk download is aborted. It would probably be good to validate all of the TIDs and version numbers against the database before triggering the bulk download, just to make sure that you won't hit any invalid values during the operation.
I also would maybe consider switching away from NUSD's use of hexadecimal version numbers, or supporting both hex and decimal version numbers, because those unfamiliar with the script format (that would include me, lol) but know that it's TID and then version would probably assume it's the decimal version number since that's all the interface displays, which will either not work or potentially download an unexpected version if the numbers align right.
Feedback on my feedback is definitely welcome if you disagree with anything!
| @@ -3,7 +3,7 @@ | |||
| ################################################################################ | |||
| ## Form generated from reading UI file 'MainMenu.ui' | |||
| ## | |||
| ## Created by: Qt User Interface Compiler version 6.7.2 | |||
| ## Created by: Qt User Interface Compiler version 6.8.0 | |||
There was a problem hiding this comment.
This isn't a review but hey PySide6 being updated to Qt 6.8.0 means that Breeze loads correctly, so that's neat! Looks much better than Fusion tends to.
There was a problem hiding this comment.
I'm on macOS and it seems not to like the large text blocks for some reason, they're cut off, but I don't think this should be fixed as part of this PR tbh
There was a problem hiding this comment.
Ok I think I solved this with some messing around all funny, let me know how you feel about how this looks (the Run Script button was always meant to look small like it does here, I forgot to keep it that way, oops)
There was a problem hiding this comment.
I think that looks good! I'll check it over on Linux and probably Windows as well just to be sure the margins work as expected, but I think it should be fine.
There was a problem hiding this comment.
Update now that I've seen it on Linux: the "vWii Title Settings" gets a little too squished, at least with the Breeze theme. It looked okay under macOS but the way Breeze does padding makes it feel a little claustrophobic. Script button looks good though.
There was a problem hiding this comment.
I can't really figure out how to fix the problem with macOS without making it look this way tbh, for some reason word wrapping just doesn't work
There was a problem hiding this comment.
Hmm... it's especially weird because it is wrapping the vWii checkbox. I wonder if maybe I did something weird when I was setting up word wrapping for the checkbox text, since they have to be separate labels.
There was a problem hiding this comment.
I tried resizing the window to a larger base size and everything fits now, but it looks funny on smaller screens (mostly < 1080p screens, which are arguably uncommon), here's a screenshot of running it on an iPad mini through Sidecar
Name archives properly using the database Fix UI problems on macOS
|
I've decided against supporting decimal numbers in this PR, since the format is pretty old, kinda terrible and not even used very often (this is for pure convenience batch downloading IOSes primarily). In the future, I'd say it'd be better to fully replace NUSD's old script format with something like a table of titles in a JSON file instead. |
|
Yeah that's probably for the best to not try and work with this format more lol. I'm looking to finally push out v1.2.0 soon, because there have been a decent number of improvements that I want to be generally available now, especially with more eyes on NUSGet recently. Most likely I'll give everything a once-over after this PR is merged and then release v1.2.0, and after that I'll look into making a JSON-based format for scripts because yeah that would absolutely be better. |
Checking regions was pretty superfluous anyway, since it only served to *potentially* speed up the lookup.
|
Spent an honestly ridiculous amount of time in Qt Designer on my old 2010 MacBook tinkering with just about every bit of spacing and constraints I could find, and I finally found something that works correctly. This fixes the spacing on macOS (which fixes the text getting cut off) while retaining the original size of everything, and still looks good on Linux (I'm sure it's fine on Windows as well, but if that's not right then oh well, I'll check it out and fix it before v1.2.0). Hopefully this looks good on your end like it did for me, I was working under macOS 12 so it probably should be identical under something newer. Here's the UI file I ended up with: |
|
Yep, works just fine on macOS 15 on my M2 MacBook Air too, it even works fine on an iPad mini now :P |
|
Awesome! In that case, I think that's everything, so I'll go ahead and merge this PR. Thanks! |
|
🫡 |
This PR adds batch downloading using NUS scripts, which is useful for things like WiFi modding Wii Minis.