Add ?load= URL param; update URL on ROM selection#61
Conversation
- ?load=FantasyZone2.sms loads a named ROM from the roms/ directory - Selecting a ROM from the list updates the URL to ?load=<name> - Uploading a file updates the URL to ?b64sms=<data>&load=<filename> - ?b64sms= param preserves optional ?load= for the display name - updateUrl() helper centralises all URL param management - URL uses history.replaceState so back/forward nav isn't affected Provides shareable URLs for any loaded ROM. 🤖 Generated by LLM (Claude, via OpenClaw)
src/main.js
Outdated
| const reader = new FileReader(); | ||
| reader.onload = function () { | ||
| resetLoadAndStart(file.name, reader.result); | ||
| // Uploaded files use b64sms encoding for shareability |
There was a problem hiding this comment.
is resetLoadAndStart now dead code? looks like you duplicated its work here
There was a problem hiding this comment.
(I'm Molty, an AI assistant acting on behalf of @mattgodbolt) Good catch — fixed. resetLoadAndStart now takes an optional urlParams arg; loadUploadFile passes { b64sms, load } through it rather than duplicating the logic.
Pass urlParams through resetLoadAndStart instead of duplicating the reset/load/start logic in loadUploadFile. 🤖 Generated by LLM (Claude, via OpenClaw)
There was a problem hiding this comment.
Pull request overview
Adds shareable, stateful URLs for ROM loading by introducing a ?load= parameter and centralizing URL updates via a helper, so ROM selection/upload is reflected in the address bar without adding history entries.
Changes:
- Add
updateUrl(params)helper to rewrite emulator-related URL params usinghistory.replaceState. - Support loading a named ROM on startup via
?load=<romname>. - Preserve existing
b64smsstartup behavior while addingload=<filename>for uploaded ROM display names.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function updateUrl(params) { | ||
| const url = new URL(window.location.href); | ||
| // Clear existing emulator params before setting new ones | ||
| for (const key of [...url.searchParams.keys()]) { | ||
| url.searchParams.delete(key); | ||
| } | ||
| for (const [key, val] of Object.entries(params)) { | ||
| if (val !== null) url.searchParams.set(key, val); | ||
| } | ||
| history.replaceState(null, "", url); |
There was a problem hiding this comment.
updateUrl rewrites the query string but leaves url.hash intact. Since parseQuery() merges ?search and #hash with the hash taking precedence (hash is appended last), any existing #b64sms/#load will keep overriding the new query params on reload, making the URL updates ineffective for users coming from hash-based links. Consider clearing url.hash (or updating parseQuery precedence) when you switch to query-param based state.
src/main.js
Outdated
| const parsedQuery = parseQuery(); | ||
| if (parsedQuery["b64sms"]) { | ||
| bus.loadRom("b64.sms", atob(parsedQuery["b64sms"]), debug_init); | ||
| const name = parsedQuery["load"] ?? "uploaded.sms"; |
There was a problem hiding this comment.
parsedQuery["load"] can be an empty string for URLs like ?load=. In that case this code will pass an empty filename into bus.loadRom, and updateUrl will persist load=; it likely should fall back to the default upload name (or ignore load) when the value is empty.
| const name = parsedQuery["load"] ?? "uploaded.sms"; | |
| const name = parsedQuery["load"] || "uploaded.sms"; |
src/main.js
Outdated
| } else if (parsedQuery["load"]) { | ||
| const name = parsedQuery["load"]; | ||
| bus.loadRom(name, loadRomData(name), debug_init); | ||
| updateUrl({ load: name }); |
There was a problem hiding this comment.
name comes directly from the URL and is concatenated into the XHR path in loadRomData ("roms/" + name). With values like ../index.html this will request files outside the roms/ directory. If ?load= is meant to only support known ROMs, validate against RomList (or at least reject path separators / ..) before calling loadRomData.
- sanitizeRomName() validates ?load= against RomList (rejects unknown names and path traversal attempts like ../index.html) - updateUrl() now clears url.hash so legacy #b64sms hash links don't take precedence over new query params on reload - Use || not ?? for b64sms display name so empty string falls back 🤖 Generated by LLM (Claude, via OpenClaw)
- Resolve main.js conflict: preserve sms.loadRom() from SMS branch, incorporate updateUrl/sanitizeRomName/?load= from #61 - SMS instance created in main.js (const sms = new SMS()) and passed to miracle_init(sms) — miracle.js no longer owns the singleton - miracle.js: keeps SMS import for static constants (FRAMES_PER_SECOND etc.) but does not create an instance Addresses Copilot review: PR description now matches reality — no module-level singleton exports anywhere. 🤖 Generated by LLM (Claude, via OpenClaw)
(I'm Molty, an AI assistant acting on behalf of @mattgodbolt)
Adds shareable URLs for loaded ROMs.
What changed
?load=FantasyZone2.sms— loads a named ROM from theroms/directory on startup?load=<name>viahistory.replaceState?b64sms=<data>&load=<filename>(preserves existingb64smsshareability, adds display name)updateUrl(params)helper centralises all URL state; easy to extend with future paramsDesign notes
replaceStatenotpushState— doesn't pollute browser historyparseQuery()already existed and handles both?searchand#hash— unchangedb64smsexisting behaviour preserved;?load=is additive alongside itFuture params to consider
?debug=1— auto-open debugger?pc=0x1234— breakpoint on loadBuild ✅ · 1342 tests ✅