feat(config): let the player choose the launcher background - #207
Conversation
The background stops being a fixed asset and becomes a choice with three answers: the scene bundled with the app, a scene downloaded from the repository's backgrounds branch, or a picture the player supplies. The bundled scene stays the default and the offline answer, so a first launch with no network looks exactly as it did before. Closes #192.
Zaldaryon
left a comment
Zaldaryon
left a comment
There was a problem hiding this comment.
Checked out 9f76559 in an isolated worktree and ran every local gate directly rather than trusting the description: typecheck, lint:ci (0 errors, 15 warnings, matching dev), format:check, the full test:coverage suite (118 files, 1368 passed, 2 skipped, coverage 91.8/88.79/90.73/93.41 percent, matching the body's numbers to within rounding), and build:unpack. Confirmed the two claims the description makes about the built output directly: the compiled CSS contains exactly .bg-image-vs{background-image:var(--background-image-image-vs)}, and the build emits one hashed background JPEG asset alongside it.
Traced the security-relevant surface by hand rather than taking the write-up on faith. The allowlist rule ordering is correct: findMatchingRule takes the first covering rule, and the manifest's 32 KB ceiling is listed ahead of the images' 2 MB one, confirmed by the new api-url-ceiling tests including the backgrounds-evil prefix-segment case. Both IPC channels revalidate id and file server-side (isCatalogBackgroundId, isBackgroundFileName) rather than trusting whatever the renderer computed from an already-validated manifest entry, which is the right call at a trust boundary. writeBackground refuses to write over anything that is not a plain file, closing the same symlink-swap class of bug the custom icon flow already guards against. The background: protocol handler mirrors createCacheModImageProtocolHandler's check order exactly (containment, extension, isSafeProtocolFile, then read), and deliberately skips the memory cache cachemodimg: uses, for the right reason: custom.jpg is one name whose bytes change, so a cache keyed on that name would go stale. registerSchemesAsPrivileged replacing rather than appending its list is a real thing to get right, and the diff does: app's registration moved into the same call rather than gaining a second one beside it.
The one thing I'd flag, not against this PR specifically: the JPEG magic-byte check this PR adds (isJpegBytes) is stronger than the extension-only validation the existing custom icon flow (COPY_TO_ICONS, icons:) still uses for PNG. Left a comment on #206 suggesting the same check get backported there for consistency.
No blocking findings. Approving.
# Conflicts: # src/renderer/src/locales/en-US.json
COPY_TO_ICONS decided whether a picked file was a PNG by reading its extension, so a JPEG renamed to .png was copied into the Icons folder and served through the icons: protocol without anything ever looking at its bytes. The background flow stopped doing that in #207, where isJpegBytes confirms the SOI marker before a picture is cached or served. isPngBytes joins isJpegBytes in src/domain/backgrounds.ts and answers the same question about the same kind of file. The handler reads the first eight bytes through a file handle rather than pulling the whole picture into memory, since a custom icon has no size ceiling of its own, and a mismatch comes back as unsupported-format, the reason #206 already gave a file that is not a PNG.
2 participants
Summary
Issue #192 asks for a background the player picks rather than one the installer decides, and it is specific about where the pictures live: not inside the launcher. So there are three answers now, and the config stores which one is in force.
The bundled scene stays the default. Nothing about it changes, and a launch with no network paints it exactly the way it painted before this PR existed. The second answer is the catalog on the new
backgroundsbranch of this repository, eleven scenes plus amanifest.json, fetched when the settings section is opened and never at startup. The third is the player's own JPEG, copied into the same cache under a reserved id, mirroring the custom installation icons flow that already exists.Adding a scene later is a push to
backgrounds. No code, no release, no installer growth.How a choice is applied
bg-image-vscompiles tobackground-image: var(--background-image-image-vs), and three elements use it: the app shell, the loader overlay, andPopupDialogPanel. So applying a background is one line setting that variable on the root element, and none of those three components learns that a choice exists.The variable is set to two layers, the chosen scene over the bundled one. A background layer that fails to load is skipped and the layer under it paints, which is how "an id whose cached file has gone missing renders the default" costs nothing: no existence check at startup, no IPC, no flash of empty window. The section repairs the missing file the next time it is opened.
Every cached file is named after its id, so
village-laneis alwaysvillage-lane.jpgin the cache whatever the manifest called it on the branch. That is what lets the renderer paint a saved choice at startup knowing nothing but the id, which is the whole reason the manifest'sfilefield never reaches the config.One thing that needed care: the custom slot keeps one stable file name, so re-picking writes over
custom.jpgand the id does not change. Without something else moving, the CSS URL is identical and the renderer keeps showing the picture that name used to hold.SET_BACKGROUNDtherefore bumps a session-only_backgroundRevision, which lands in the URL as?r=N. It follows_notifiedModUpdatesInstallations: the normalizer never writes it back to disk.The pictures
Eleven official Vintage Story screenshots from https://www.vintagestory.at/media.html, the same provenance as the background that ships today. Processed with PR #181's pipeline: linear-light Lanczos resize to 2560x1440, 4:4:4 chroma, progressive, metadata stripped.
Where I departed from #181 is the quality number. #181 fixed quality 92 for an asset that is compiled into the installer, where file size is close to free and the real cost is the decoded bitmap. These are downloaded over someone's connection instead, and #181's own matched-size experiment (4:4:4 at quality 70 beating 4:2:0 at quality 82) was run at about 675 KB, which is the band the issue asks for. So I held the pipeline and the 4:4:4 decision and let quality float to whatever lands each file at or under 700 KB. Flat scenes reached 92 and never needed less; the two busiest ones needed 71 and 72.
7,376,651 bytes for the whole catalog, and a player only ever downloads the ones they pick.
Nine of the eleven were already 16:9. Two were not, and both were centre-cropped rather than dropped:
crystal-caverncame in at 2321x1440. Cropped to 2320x1305, which takes 67 pixels off the top and the bottom. The subject is a cluster of salt crystals filling the middle of the frame and it survives untouched.valley-ruinscame in at 2560x1263. Cropped to 2240x1260, 160 pixels off each side. The stone pillars sit centre-right and are well clear of the cut; the pond in the bottom left loses part of its edge and still reads as a pond.Both crops are exact 16:9 integers on purpose, so the resize to 2560x1440 is a clean scale rather than a fit that lands a pixel short. Nothing was dropped from the set.
Boundaries
Two allowlist rules for
raw.githubusercontent.com, both restricted to/StratumServer/RiftLauncher/backgrounds. The manifest rule is listed first so its 32 KB ceiling wins the match, sincefindMatchingRuletakes the first rule that covers the path; images get 2 MB, roughly triple the largest file on the branch and half the generic ceiling they would otherwise inherit.pathPrefixesmatching is already segment-aware, sobackgrounds-evildoes not slip pastbackgrounds.requestBoundedTextgrew a sibling rather than a flag. It now calls a newrequestBoundedBufferand decodes the result, because the utf8 decode was the only thing separating the two functions and a JPEG does not survive it. Ceiling, timeout, and the refusal to follow a redirect are shared, untouched.The
background:protocol handler mirrorscreateCacheModImageProtocolHandlerin order and in shape: containment, extension gate,isSafeProtocolFile, then the read, then a buffered copy into the response. The one deliberate difference is the missing memory cache. Mod icon names are content-addressed so a hit can never be stale, butcustom.jpgis one name whose bytes change, and a cache keyed on it would serve the old picture forever.The scheme is registered non-standard.
registerSchemesAsPrivilegedreplaces its list rather than appending to it, so the existingappregistration moved into the same single call instead of gaining a second one beside it.Both new IPC channels validate on the main side and never trust the id or the file name they are handed. The picker accepts JPEG only, which is what the protocol serves and what the catalog is, and is the same shape
COPY_TO_ICONShas always had with PNG. A comment in the code names what changing that would cost.Type
Checklist
dev, notmain.npm run typecheckpasses.npm run lint:cipasses (0 errors, 15 pre-existingreact-hooks/exhaustive-depswarnings, none in files this PR touches).npm run format:checkpasses.npm run test:coveragepasses, coverage above every floor invitest.config.ts(statements 91.82 vs 87, branches 88.83 vs 85, functions 90.73 vs 85, lines 93.41 vs 89).npm run build:unpackpasses.Testing
118 test files, 1368 passing, 2 skipped, the same two skips
devalready has. 39 of those tests are new: the manifest parser and id rules intests/domain/backgrounds.test.ts, normalization rows intests/ipc/configManager.test.ts, reducer rows intests/renderer/configReducer.test.ts, allowlist and ceiling rows intests/ipc/api-url-ceiling.test.ts, the two IPC channels intests/ipc/backgroundHandlers.test.ts, the protocol handler intests/main/backgroundProtocol.test.ts, and the settings section intests/renderer-dom/configPageBackground.test.tsx.Every one of them was checked by breaking the thing it guards and confirming it went red:
SET_BACKGROUNDstops bumping the revision/StratumServer/RiftLauncher.jpgextension gateisSafeProtocolFileThe extension gate mutation is the one worth calling out. It is the one that survived review on the mod icon handler, and the row that catches it has a real non-JPEG file sitting on disk under the requested name, readable, with nothing but the name check between it and the renderer.
Two checks outside the suite.
electron-vite buildemits one JPEG asset and both the stylesheet and the JS chunk that sets the variable reference the same hashed filename, so the fallback layer points at the real bundled scene and not a dead URL. And the built CSS confirms the mechanism the whole feature rests on:.bg-image-vs{background-image:var(--background-image-image-vs)}, with the theme declaring that variable on:root, which an inline style on the root element overrides.Not covered: I did not run the packaged app and click through the section, so the CSS layer fallback and the
background:scheme are argued from the built stylesheet and the handler tests rather than from having watched a scene download and paint. If a reviewer runs it, the thing worth a second pair of eyes is what the grid looks like on a first visit, since a scene only shows its picture once it has been downloaded and a first visit is therefore a grid of names. That is deliberate, it is the price of not pulling seven megabytes down the moment somebody opens their settings, and a comment in the code says the fix would be small thumbnails on the branch rather than a prefetch.Related issues
Fixes #192