Repository navigation
feat(builder): a UI overhaul — the launcher page, and one design for every page - #1759
isaiahbenj94-star wants to merge 8 commits into
Conversation
A player who double-clicks Hookshot in their game folder lands on a text list that hides what the folder already is: a built, playable game. The mockup (see the PR) shows the same facts at a glance - the hookshot and the version on the green rail, Play as the disc over the hero, and the version chain and update offer along the top and foot. The hero is read from the player's own hookshot_hero.png when the folder keeps one, so no picture of the game ships with the builder (the new check guards that), and a calm gradient covers the rest.
CONTRIBUTING.md asks for screenshots when a change shows on screen. These are rendered offscreen (QT_QPA_PLATFORM=offscreen), so they hold the UI and the placeholder hero only: no picture of the game is in them, and the one non-gradient hero shot uses a generated stand-in, not game footage.
The AppImage builder (builder/linux) is the launcher a Linux player's game folder starts, and it still showed the plain text list. It gets the mockup layout too: green rail, hero with the Play disc, the version chain and the "Hookshot: ..." strip. Like the Qt page its hero is read from the player's own folder, never from a picture the builder ships, and the extended check guards both builders. Verified by running the AppImage's own python with --launch and looking at the window.
The rail's caption was wider than the rail, and Tk reads a positive font size as points, so the launcher's text came out too big on some screens: the caption now splits over two lines and the launcher's fonts are sized in pixels, the way this file's apply_scale expects. Screenshots of both states, rendered with the AppImage's own Python.
ghost
left a comment
There was a problem hiding this comment.
Looks good. I read through launch_page.py, the window.py wiring and the Tk page; the hero is only ever read from <folder>/hookshot_hero.png, the preview folder is made-up file names only, paths.write_small / self.hook / game_ready all exist, and the new test is picked up by run_all's test_*.py discovery. The screenshots use a generated placeholder, so nothing from the game ships.
Nits, none blocking:
- Tk page:
tk.PhotoImage(file=picture)raises TclError ifhookshot_hero.pngis corrupt or not a PNG/GIF, and that would break the launcher page. A try/except falling back to the gradient would be safer (the Qt side already handles a null pixmap). - Tk page: the hero is only zoomed up, never scaled down, so a normal screenshot (1080p/4K) is just centre-cropped to the 650x560 canvas instead of cover-fit as the description says. Fine for a first cut.
- Qt:
set_color(self.launch_version, "White")on the update chain is never reset to InkSoft if the launcher is shown again for another folder in the same session. Only matters in preview/dev. - The tests are source-string checks, so they guard the rule but not behaviour. That's fine for the no-game-art rule.
CI hasn't run on this branch (no checks reported), so a Windows run of --launch <folder> as you suggested is still worth one look before merging.
|
Can you fix the screenshot links |
|
I've withdrawn my approval for now, sorry for the back and forth. The page itself still looks good to me. The issue is the screenshots:
Could you drag the images into the PR description instead (GitHub hosts them then) and drop the |
|
Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks! |
The launcher page set the design; the rest of the Linux builder stayed gray ttk. Now the form and the progress pages open on the same dark page with the green rail (the builder's tag on it, updated when GitHub answers), grouped GAME FILES / OPTIONS cards, dark entries, log and combo, and a footer strip that carries the status line and the one green action. Fixed 900x560 like the launcher page. The checks that grep this file (check_version, build, apply_scale...) still match: same names, same flow, new paint.
The launcher page set the design; the other pages kept the old look. Both builders now open every page on the same frame: the green hookshot rail at the left (with the builder's tag on it) and a dark strip along the foot that carries each page's action row — Build/Play, Show log/Cancel, Open/Play, Back/Repair — so the window reads as one product from first screen to last. The Qt chrome stands down only for the launch page, which draws its own pair inside the hero. Offscreen previews of all five Qt screens and a live run of the two Tk pages were checked against the mockups.
main took the Linux builder to its own Slate theme: dark cards, Inter, the header's step pill, Play's green. That is the look this PR came out of, so the merge takes main's form, progress and theme whole, and keeps only what the PR adds there: the mockup launcher page (its rail, hero, disc and "Hookshot: ..." strip) on top of it, with its few new colours named beside main's palette. load_logo grows into logo(size, over) so the rail can draw the same averaged hookshot, big, over its green. The Qt side, which main did not theme, keeps this PR's rail and footer chrome.
…creenshots of the merged builder The chrome shrinks the body to make room for its rail; the launch page draws a rail of its own, so inside the shrunk body it was squeezed and the status line clipped at "Update available: v". show_page now gives the body its full width whenever the chrome stands down. Screenshots of every page, re-rendered from the merged tree; the Linux launcher ones are live runs of the merged Tk builder with its own Python.
|
Screenshots posted, and code cleaned! |
ghost
left a comment
There was a problem hiding this comment.
Thanks for adding the screenshots to the description, they render now. The PNGs are still in the PR though, so I can't re-approve yet.
The PR's files still list nine PNGs:
docs/launcher/:hero_image.png,linux_tk_up_to_date.png,linux_tk_update_available.png,up_to_date.png,update_available.pngdocs/overhaul/:qt_build_page.png,qt_files_page.png,qt_problem_page.png,qt_ready_page.png
The latest commit only modified the docs/launcher/ ones and removed two from docs/overhaul/; the files are still there, and they live in the earlier commits' history too. Nothing links to them now that the description uses GitHub-hosted images, so they only add weight to the repo. Please delete all nine (docs/launcher/*.png and docs/overhaul/*.png). Because history would still hold them if merged as separate commits, either squash the branch, or rebuild it without the screenshot commits (5ad96253, e701e369 and the overhaul commit 70db3016 add them) so no commit adds a PNG. If this gets squash-merged, deleting the files in a final commit is enough.
The code changes since my approval look fine: the Tk rail now lays the logo over the gradient with subsample and falls back to the flattened copy on an older Tk, and the Qt launch page takes the full body width back while the chrome is hidden. No new issues there. The nits from my first review (corrupt hero PNG in Tk, hero only zoomed up) are unchanged and still non-blocking.
No CI checks have been reported on this branch; the PR is mergeable. Once the PNGs are out I'll approve.
Joey-Magnus
left a comment
There was a problem hiding this comment.
The nine screenshots are still in the PR: five in docs/launcher/*.png and four in docs/overhaul/*.png, added by 5ad9625, e701e36 and 70db301. Screenshots belong in the description, which you've already done, not in the repo. Please rebuild the branch without those commits, or squash it, so no commit adds a PNG.
Other points:
- builder/linux/hookshot_builder.py conflicts with main. Several Linux builder fixes landed in that file since (#2485, #2406, #2279, #2150), and the Tk launcher page is the Linux Play path, so this needs a careful merge.
- hookshot_builder.py:930:
tk.PhotoImage(file=picture)still has no try/except. A corrupt or renamedhookshot_hero.pngraises TclError and takes the launcher page down. Falling back to the gradient would fix it. - The Windows release ships the .NET WPF builder (
dotnet publishin windows-builder.yml), and no workflow builds the Qt builder. So "one design for every page" doesn't reach Windows players; it only reaches Linux and the Qt port. Whether the WPF builder should follow is a maintainer call.
|
This now conflicts with main in |
Why
The community drew a mockup of what the game folder's launcher should look like (green hookshot rail, hero with a Play disc,
"Build & Update", the version chain, and a status line that says what just happened). This PR builds that page for both
launcher implementations — the Qt builder (Windows) and the Tk builder in the Linux AppImage — and gives the rest of the Qt
builder the launcher's frame too, so the whole window reads as one design.
(On the Linux side, main already took the Tk builder to its own dark Slate theme — dark cards, Inter, Play's green. This PR
now sits on top of that: it keeps main's theme for the form and progress pages, and adds the mockup launcher page to it. The
branch includes a merge of main.)
What changes
--launch <game folder>) in both builders: green rail with the hookshot and the game's version, thehero, the Play disc over it, "Build & Update" with the version chain over a scrim, and the "Hookshot: …" strip along the
foot with the mockup's sentences.
The hero picture is the player's own file, never ours:
<game folder>/hookshot_hero.pngwhen the folder has one, agenerated dusk-green gradient otherwise. No game art ships in the builder (CONTRIBUTING, "the rule with no exceptions").
each page's action row — Build/Play, Show log/Cancel, Open/Play, Back/Repair. The chrome stands down for the launch page,
which draws its own pair inside, and the body takes the whole width back for it.
tests/test_builder_launch_hero.py, text-based like the other builder tests): the hero is read from thegame folder only; no hero art ships anywhere under
builder/; both builders draw the mockup's words; the Qt pages share thechrome; the preview fakes folder names, not game data.
Screenshots
The launcher page as the mockup draws it (Qt offscreen previews; the chain shows the preview's made-up tags):
With the player's own hero picture (a generated placeholder — no game data):
The rest of the Qt builder, same frame (offscreen previews):
The Linux (Tk) launcher, live under Xwayland with the AppImage's own Python, both states:
Tests
python -I -B tests/test_builder_launch_hero.py— new, passes.python -I -B tests/run_all.py builder— the existing builder checks (jobs, scaling, status lines...) still pass; theirgreps still match.
Play, Update, the version chain and a
hookshot_hero.pngfrom the folder all behave.