Repository navigation
feat(ui): add game shortcuts to the Ports menu - #9
alexander-clawthorne wants to merge 13 commits into
Conversation
Pressing Select on a library title writes a launcher for it next to GreenOvercast.sh, so the game gets its own Ports entry. The shortcut re-runs the port launcher with the title's product id and GREENOVERCAST_AUTOSTART, and the library starts that title once when it is in the catalog; otherwise the library opens as usual. After the session the library is shown as for any other launch. Pressing Select again removes the shortcut. On muOS the cached artwork is also written to the Ports catalogue as a PNG fitted inside 320x240, matching what PortMaster does for installed ports. Other CFWs get the entry without artwork. Only files carrying the shortcut marker are replaced or removed, so an existing port with the same name is never touched. Select acts on release, so the Select + Start exit chord never creates a shortcut. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
readFileAlloc sizes its buffer with File.stat(), which uses statx and fails on the 4.9 kernels H700 devices ship, so shortcuts were saved without artwork on muOS. Read the cached image with readToEndAlloc, as the settings store does, and log which step fails if a cover cannot be produced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply to a comment to ask cubic a question or push back. It learns from your replies.
Re-trigger cubic
- Treat a generated script as this title's shortcut only when it launches the same product id. Different titles can share a port name, and Select on one must not remove the other's shortcut. - Write through std.fs.AtomicFile, which creates a randomly named temporary file exclusively, instead of truncating <name>.sh.tmp in the shared Ports folder. - Remove the artwork first and best-effort, then the script, so a reported failure always matches what is left on disk. - Stay quiet when a title's artwork is simply not cached yet, and note why cover generation runs on the caller's thread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve the launcher from BASH_SOURCE so the path stays right if a wrapper sources it, and log that game shortcuts are disabled when the path cannot be resolved instead of dropping them silently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
writeScript accepted launcher paths of any length, while state() only reads the first 1024 bytes, so an unusually long Ports path could produce a shortcut that could never be recognised, removed or replaced. Cap the launcher path at 512 bytes and assert at compile time that the longest possible script fits in the read buffer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Producdevity
left a comment
Producdevity
left a comment
There was a problem hiding this comment.
Reviewed 69517dd against feat/geforce-now (455f4b5).
Requesting changes for the shared muOS artwork ownership bug: creating/removing a shortcut can overwrite/delete another port’s PNG. The no-cover deletion case is reproduced against the production module. There is also a smaller relative-launch path bug; both have concrete reproductions and requested fixes inline.
The earlier script-collision, temporary-file, shell-quoting, and maximum-script-length fixes hold up. A generated-script execution test preserved literal shell metacharacters and the exact launcher/product arguments without executing substitutions. The Xbox/GFN selection and input paths passed the focused checks.
I accepted and resolved the synchronous-artwork concern separately. Its bounded, add-only muOS path does not justify forcing a general worker redesign here.
Validation on this commit:
-
tools/zig.sh build test: 318/318 tests passed, 99/99 steps succeeded. -
tools/zig.sh build fmt-check,git diff --check, andbash -n: passed. ShellCheck reports only the same two existing PortMaster-source/context warnings as the base. -
15 additional headless checks of the actual UI/controller code passed: both providers, exact ID matching, one-shot/missing-ID autostart, Select-release toggling, both exit-chord orders, and inactive-controller events.
-
Four filesystem/shell probes confirmed the two artwork failure cases, same-folder foreign-port protection, and exact safe script execution. The failure-case probes assert the observed bug; they are not regression tests of desired behavior.
-
Two supplemental artwork allocation/stride tests passed, and the generated PNG decoded independently with Python/zlib and FFmpeg.
-
tools/zig.sh build release: passed (ARM64 ReleaseSafe). The output is an AArch64 ELF executable, with 0 undefined__aarch64_*symbols.
No physical handheld or live cloud session was exercised in this review. CodeRabbit’s GitHub status is a skipped review; its CLI could not authenticate here (environment_unsupported), so these conclusions come from the independent source review and tests.
| if (artwork) |directory| { | ||
| var cover_buffer: [name_capacity + 8]u8 = undefined; | ||
| const cover_name = try std.fmt.bufPrint(&cover_buffer, "{s}.png", .{name}); | ||
| directory.deleteFile(cover_name) catch |err| switch (err) { |
There was a problem hiding this comment.
P2: Preserve artwork that this shortcut does not own.
The script check only covers this Ports directory, but muOS uses a shared External - Ports/box catalogue. For example, a native Celeste.sh on one card can coexist with GreenOvercast’s shortcut on another card while both use Celeste.png.
I reproduced this against the unchanged module: seed an existing PNG, call add(..., "Celeste", null) after a cache miss, then remove the shortcut. The original PNG survives creation but is deleted here, even though this shortcut never wrote it. With a non-null cover, add() also overwrites the existing PNG at lines 124–129.
Please preserve pre-existing artwork and record whether/which image the shortcut owns, then replace or delete only that owned image. Skipping an existing PNG during creation alone is insufficient because removal still deletes it. Add a regression for an existing PNG plus cover_png == null, and retain the existing same-directory foreign-script protections.
There was a problem hiding this comment.
Thanks, reproduced and fixed in d4842b5.
A shortcut now records ownership in its own script (# GreenOvercast artwork: crc32=… size=…), and only when it created the image. add() writes artwork only if no <Name>.png exists, or if the existing one is still exactly the image this shortcut recorded; otherwise it keeps it and logs "Existing artwork kept". remove() deletes the image only when the record exists and the file still matches its CRC-32 and size, so artwork created or replaced by anything else, such as a native port installed later, is kept. Re-adding without a cached cover carries an existing matching record forward.
Regressions, with your scenario first: existing artwork survives a shortcut added without a cover (existing PNG, cover_png == null, add then remove), existing artwork is neither overwritten nor removed by a shortcut with a cover, a shortcut replaces and removes only the artwork it wrote, and artwork replaced after the shortcut saved it is kept. The same-directory foreign-script tests are unchanged. Your reproduction now keeps native-port-art in both the null and non-null cover cases.
On the RG35XX-H, add → remove → add created the cover, recorded crc32=9688531c size=104952 (matching the file), deleted it on removal and recreated it.
| export GREENOVERCAST_ARTWORK_CACHE_DIR="$artwork_cache_dir" | ||
|
|
||
| # Game shortcuts are written next to this launcher and run it again with a title id. | ||
| if port_launcher=$(realpath "${BASH_SOURCE[0]:-$0}"); then |
There was a problem hiding this comment.
P3: Resolve the launcher path before changing directories.
cd "$GAMEDIR" has already run at line 30. When started from the Ports folder with bash ./GreenOvercast.sh, BASH_SOURCE[0] remains relative, so this resolves to .../ports/greenovercast/GreenOvercast.sh instead of .../ports/GreenOvercast.sh. GNU realpath succeeds when only the final component is missing, so support stays enabled and shortcuts are written inside the data directory with a nonexistent target.
I reproduced this with the unmodified launcher and a harmless mock PortMaster/application environment: absolute invocation exported an existing launcher; relative invocation exported the nonexistent path above. Normal muOS frontend invocation uses an absolute path, so this is specifically a relative/manual-launch bug.
Please capture the absolute launcher path before sourcing helpers or changing directories, verify it is an existing file, and export that saved value here.
There was a problem hiding this comment.
Reproduced and fixed in 5c17dc0. The launcher path is now resolved at the top of the script, before PortMaster's helpers are sourced or cd "$GAMEDIR" runs, and exported only when it names an existing file; otherwise it logs "Game shortcuts disabled". I renamed the variable to greenovercast_launcher, since it's now set before control.txt is sourced.
With a mock PortMaster environment, absolute, relative (bash ./GreenOvercast.sh from the Ports folder) and sourced-wrapper starts all export the existing …/ports/GreenOvercast.sh. ShellCheck output is unchanged from the base (the same SC1090/SC2154/SC1091 PortMaster-source notices).
muOS keeps every port's box art in one shared catalogue, also across both cards, so a same-named PNG can belong to a native port. add() overwrote it when a cover was cached, and remove() deleted it either way. A shortcut now writes its artwork only when no image exists yet, and records the CRC-32 and size of what it wrote in its own script. remove() deletes the image only when that record exists and still matches the file, so artwork created or replaced by anything else is kept. Re-adding a shortcut replaces only its own unchanged image and keeps the record when no new cover is available. The image is read rather than stat'ed, since statx is unavailable on 4.9 kernels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The shortcut launcher path was resolved after `cd "$GAMEDIR"`, so a relative start such as `bash ./GreenOvercast.sh` exported a nonexistent path inside the data directory. Resolve it before PortMaster's helpers run or the directory changes, and export it only when it names an existing file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
View guided diff | Re-trigger cubic
add() wrote the artwork before building and saving the script, so a failed script save left the previous script recording the old image while a new one was on disk, orphaning it. Build and validate the script first, and if saving it fails, delete artwork that was just created or restore the image the shortcut owned from the bytes read when checking ownership. A failed artwork write saves the script with the previous record, so an image that is not on disk is never recorded. New artwork is created exclusively, so a file another writer creates after the existence check is kept instead of replaced. Exclusive creation is used rather than a no-replace rename, which FUSE filesystems such as the muOS catalogue's do not reliably support. remove() still verifies and deletes in separate steps, which POSIX cannot make atomic; that is documented. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
openDirAbsolute() asserts that its path is absolute, so a relative or empty GREENOVERCAST_PORT_LAUNCHER or GREENOVERCAST_PORT_ARTWORK_DIR ended the app on Select in ReleaseSafe builds. Accept only non-empty absolute paths and treat anything else as shortcuts being unavailable. Autostart now requires GREENOVERCAST_AUTOSTART=1 rather than any value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The search keyboard and settings consume Select's release, so a press made before opening them could toggle a shortcut on a later release back in the library. Clear the pending press before either view opens. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… red A shortcut whose game is no longer in the catalog opened the library without explanation. Show SHORTCUT GAME NOT FOUND instead. Notices now carry a kind, and problems (game not found, save or removal failures, name in use, missing folders) draw in a new danger() red so they stand out from ADDED and REMOVED. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
remove() deleted the script, the only record of the artwork it owns, even when deleting that artwork failed, leaving an image nothing could ever remove. Keep the script in that case, and when a configured artwork folder cannot be opened, so a later attempt can clean up. A missing artwork folder still means the system has none, so shortcuts there remain removable. writeScript() also enforces name_capacity and rejects ':' in names, so a name can never imitate the artwork record line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The rollback tests only checked the end state, so they also passed if the artwork was never written. A test-only hook, compiled out of other builds, now records the artwork between its write and the script save, proving each rollback actually undid something. skipAsRoot() uses libc geteuid() off Linux, since host tests also run on macOS. add() documents that it also handles re-adding, which the UI does not do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 participants
Based on
feat/geforce-nowlike #8, and independent of it: the two touch different parts ofhandheld_ui.zigand merge cleanly. Branch name and commits follow thefeat/<kebab-case>and conventional-commit patterns of #2-#4. The fix for a bug found during device testing is a separate commit, as #3 did.What does this change?
Adds game shortcuts: a game from the library gets its own Ports entry that opens straight into that game. The idea comes from the shortcut feature in Kasumi (a GeForce NOW client for the 3DS). No Kasumi code is used, since it is GPL-3.0.
<Title>.shnext toGreenOvercast.sh, and Select again removes it. The prompt bar showsSELECT PORTSwhen shortcuts are available, and a short notice confirms each action in place of the second prompt row.GREENOVERCAST_SERVICEandGREENOVERCAST_AUTOSTART=1, thenexecs the real launcher with the title's product id as$1.execkeeps$0pointing at the real launcher, which PortMaster'scontrol.txtderivesGAMEDIRfrom.argv[1]cursor handling is reused through a new exact-matchlibrary.findTitle. If the title is no longer in the catalog, the library opens as usual. After the session ends, the library is shown, as for any other launch.box/folder. The folder and the "fit inside 320x240, never enlarge" rule are the ones PortMaster'smuos/image_smash.txtuses for installed ports. The launcher exportsGREENOVERCAST_PORT_ARTWORK_DIRonly when that folder exists. Other CFWs get the entry without artwork; their frontends store artwork differently and I can't test them.New code:
src/ui/port_shortcut.zig: file names, script generation, add and remove.src/ui/cover_image.zig: box-filter fit and a small PNG encoder built onstd.compress.zlib. The FFmpeg build has no PNG encoder.Loader.cachedCoverPnginartwork_loader.zig.GREENOVERCAST_PORT_LAUNCHERand, on muOS,GREENOVERCAST_PORT_ARTWORK_DIR.How did you test it?
tools/zig.sh build testandtools/zig.sh build fmt-checkpass. New tests cover:tools/zig.sh build releasepasses, andnm -D --undefined-only webrtc_stream | grep -c __aarch64_is 0.CONTROL Resonant.shto Ports, along withbox/CONTROL Resonant.png(170x240 from the 256-wide portrait art). The entry showed its cover in the muOS Ports list.Starting shortcut title), skipping the library.execs the port launcher. On muOS,realpath "$0"resolves the launcher to/mnt/union/ROMS/PORTS/..., and the shortcut works from that path.Dir.readFileAllocsizes its buffer withFile.stat(), which usesstatx, and that fails on kernel 4.9 (the limitationstat_compat.zigdocuments). The second commit reads the cache withreadToEndAllocinstead.video_pipeline.zig:154callsfile.getEndPos(), which also goes throughFile.stat()/statx. On 4.9 kernels that probably fails and returns 0, so the H.264 bootstrap file may never be read. I haven't verified this; happy to look at it separately.Review follow-up (cubic)
I fixed five of cubic's six findings in two focused commits. The sixth is documented rather than changed; my reasons are in its thread.
b3bbf8efix(ui):std.fs.AtomicFile(random name, exclusive create)3cb0f78fix(portmaster): the launcher resolves itself throughBASH_SOURCEand logs when shortcuts are disabled.Re-verified on the RG35XX-H after these commits: remove and re-add worked, the cover was rewritten, the script is mode 755, and no temporary files were left behind.
Review round 2 (maintainer)
Both requested changes are fixed, one commit each:
d4842b5fix(ui): shortcut artwork is created only when no image exists, and its CRC-32 and size are recorded in the shortcut's own script. It is replaced or removed only while that record still matches the file, so other ports' images in muOS's shared catalogue are never overwritten or deleted. Regression tests cover an existing PNG with and without a cached cover, owned artwork, and artwork replaced after creation.5c17dc0fix(portmaster): the launcher path is resolved before PortMaster's helpers run or the directory changes, and exported only when it is an existing file, sobash ./GreenOvercast.shfrom the Ports folder works.Re-verified:
build test,fmt-checkandbuild release(0__aarch64_imports). ShellCheck is unchanged from the base. Your P2 and P3 reproductions now pass, the P3 mock covering absolute, relative and sourced starts. On the RG35XX-H, add → remove → add created the cover, recorded it, deleted it and recreated it, with no temporary files left behind.Review round 3
3e77e68fix(ui): addresses cubic's two findings.add()now builds the script before touching any file, creates new artwork exclusively (O_EXCL), and rolls the artwork back if saving the script fails.While skimming the full change, I also found and fixed a few more issues:
94550d0: only absolute shortcut paths are accepted from the environment, becauseopenDirAbsolute()asserts. Autostart requiresGREENOVERCAST_AUTOSTART=1.ad94ad9: a pending Select is cancelled when the search keyboard or settings open.5d97941: a shortcut whose game is gone shows "SHORTCUT GAME NOT FOUND", and shortcut problems are shown in red.8823480: a shortcut's script is kept while its artwork can't be removed, so no image is orphaned. Names can't imitate the artwork record.80ef3ee: the rollback tests now prove the rollback ran, and the root check works on macOS hosts.Each commit passes
build testandfmt-checkon its own.build releasepasses with 0__aarch64_imports. Re-tested on the RG35XX-H: add and remove with artwork, the missing-game notice, and Select with search open.AI disclosure
Per CONTRIBUTING: this was written with AI assistance (Claude). I reviewed the diff, built it, and tested it on hardware myself. I'm happy to change the button, the naming, or how other CFWs should be handled.
🤖 Generated with Claude Code
Summary by cubic
Adds game shortcuts to the Ports menu: pressing Select on a library title creates a launcher next to
GreenOvercast.shthat starts that game directly, and pressing Select again removes it. The launcher re-runs the port launcher with the title's product id and autostart, so launching the shortcut skips the library. On muOS, the title's cached artwork is also written to the Ports catalogue as a PNG so the entry shows a cover.Only scripts carrying the shortcut marker are ever replaced or removed, and only when they launch the same product id, so existing ports with the same name are never touched. Writes go through atomic files, the launcher path is resolved from
BASH_SOURCEbefore PortMaster's helpers change directories, and launcher paths are capped at 512 bytes so every generated script fits in the recognition buffer.Bug Fixes
File.stat(), which needsstatxand fails on the 4.9 kernels these handhelds ship; it now reads withreadToEndAlloc.SHORTCUT GAME NOT FOUNDin red instead of opening the library without explanation.Written for commit 80ef3ee. Summary will update on new commits.