docker-dosbox-novnc/TODO.md

# TODO

- ~~Reduce image size of `jayfield/dosbox-novnc` (currently ~2.3GB).~~ Done in two steps:
  1. Merged the game download/extract steps and the unzip/smbclient install into a single
     layer (so the downloaded zips and the packages only needed to extract them no longer
     persist in the final image). 2.31GB -> 1.6GB.
  2. Stopped baking the games into the image at all (927MB of that was just t7g's two CD
     ISOs). `run.sh` mounts a named volume (`dosbox-games`) so installed games persist
     across `--rm` restarts. 1.6GB -> ~690MB. Container now needs network access to
     `vlda-01` at runtime (not just build time) to install games.
  3. Dropped the auto-download at container startup entirely. `scripts/setup_server.py` is
     a small stdlib-only HTTP server that serves an HTML page on `${SETUP_PORT}`
     (`70${DISPLAY_NUM}`, published as `7099` by `run.sh`) listing every `*.zip` on the SMB
     share with an Installed/Install status per game; the user clicks "Install" to
     fetch+extract a specific game into `${GAMES_HOME}` on demand.

- **Manifest-driven games + Start/Stop/Uninstall**: games are no longer discovered from raw
  `*.zip` files on the share. `setup_server.py` lists `*.json` manifests instead (one per
  game, in the `Games\dosbox\` catalog directory), each declaring
  `name`/`title`/`zip_path`/`start_cmd`. The web page grew Start/Stop/Uninstall buttons
  alongside Install, driven entirely by that manifest — nothing about a game's identity or
  how to run it is hardcoded in the image anymore. Started as a `stuntcar`-only pilot, now
  extended to 6 games (see the SCUMM item below). `t7g` still has no manifest and won't show
  up in the setup page until one is added (`t7g.json` with a `start_cmd` for its
  `dosbox-0.74-3.conf`). `game.sh`/`scripts/start_game.sh` (the old `docker exec`-based
  launch path, with its hardcoded `DOSBOX_VERSION`/`run.bat` convention) were intentionally
  left untouched during the pilot and still overlap with the new Start button — worth
  reconciling (or removing) once every remaining game has a manifest.

- ~~Create manifests for SCUMM engine games~~ Done: `scummvm` added to the Dockerfile
  (`/usr/games/scummvm`, not on `$PATH` by default under a non-login shell — manifests use
  the absolute path). Manifests live in the usual `Games\dosbox\*.json` catalog directory on
  the share, but their `zip_path` now points at wherever the game's zip actually already
  lived (`Games\Monkey Island\...`, `Games\Indiana Jones\...`, etc.) — this required
  generalizing the manifest schema from a bare `zip` filename (implicitly under
  `Games\dosbox\`) to a full `zip_path` relative to the share root, since these games weren't
  colocated with dosbox's. Added and verified end-to-end (install/start/stop/uninstall) via
  `monkey2`:
  - `monkey` — The Secret of Monkey Island
  - `monkey2` — Monkey Island 2: LeChuck's Revenge
  - `atlantis` — Indiana Jones and the Fate of Atlantis
  - `indy3` — Indiana Jones and the Last Crusade
  - `tentacle` — Day of the Tentacle

  **Real bug found later (all 5 of the above), reported as "Indiana Jones and the Fate of
  Atlantis doesn't start properly"**: every one of these manifests' command was
  `["/usr/games/scummvm", "-p", ".", "-f", "atlantis"]`-shaped, i.e. `-f <target>`. `-f` is
  ScummVM's `--fullscreen` flag, not "select this game" — there's no flag that takes a bare
  game-id positionally either; passing one is rejected outright ("Stray argument 'atlantis'"),
  and ScummVM exits near-instantly with a nonzero status. Because the exit happens faster than
  the setup page's ~3s poll interval, `is_running()`'s lazy cleanup silently reaped it before
  anyone saw a "Running" status or an error message — clicking Start just looked like nothing
  happened, no error surfaced anywhere. (This also means the "verified end-to-end" claim above,
  from when these were first added, was wrong — Stop/Uninstall working afterward isn't
  evidence the game itself ran; it only proves `is_running()` correctly saw it wasn't.)
  Root-caused via `docker exec` running the exact command by hand and reading ScummVM's own
  stderr, rather than through the setup page. Fixed by switching every one of these 5
  manifests' command to `["/usr/games/scummvm", "-p", ".", "--auto-detect"]` — ScummVM's own
  flag for "scan the given path and start whatever single game it finds there", rather than
  fighting its target/config-file model, which fits this container's one-game-per-directory
  layout exactly. Verified for real this time: manually launched under a live Xvnc, screenshotted,
  and confirmed the actual Indiana Jones and the Fate of Atlantis title screen renders (not
  just "process didn't crash") - then re-verified through the real setup page end-to-end
  (`POST /start` -> "Running (Start)" -> `POST /stop`).

  Skipped the German CD release of Day of the Tentacle on the share (`Day Of The Tentacle
  (CD DOS, German).zip`) — unlike every other zip here, it doesn't wrap its contents in a
  single top-level folder (loose files at the zip root plus a stray `MANIAC/` dir for the
  embedded Maniac Mansion easter egg), so it doesn't fit the current "zip's top-level folder
  == install name" extraction convention. Also skipped Curse of Monkey Island (much larger,
  untested) — could be added the same way if wanted.

  ScummVM's own game-target IDs (`monkey`, `monkey2`, `atlantis`, `indy3`, `tentacle`)
  happened to exactly match each zip's existing top-level folder name, so no change was
  needed to the install/extraction logic itself, only to where zips are looked up from.

- ~~Handle multiple games running at once~~ Done: each running game now gets its own
  ephemeral X session (`Xvnc`+`fluxbox`+`websockify`, its own display `:90`-`:99` and noVNC
  port `8090`-`8099`) spun up by `start_game()` and torn down by `stop_game()` (or lazily on
  the next page load, if the game exited/crashed on its own) — instead of every game sharing
  one screen and fighting over focus/audio. Capped at `MAX_CONCURRENT_GAMES = 10`; starting
  an 11th game while all 10 slots are in use is refused with an error shown on its row rather
  than silently doing nothing. There is no more a single shared "default" desktop or global
  noVNC link — `server.sh` no longer starts Xvnc/fluxbox/websockify at container startup at
  all, only `setup_server.py` itself; each game's own "Open Screen" link appears on its row
  only while it's running, built with a little client-side JS (the noVNC port differs from
  the setup port and can't be known server-side without knowing which hostname the browser
  used to reach the container). `run.sh` publishes the whole `8090-8099` range up front,
  since Docker can't add port mappings to an already-running container. Verified end-to-end
  with two different games running concurrently (separate processes, separate displays,
  separate noVNC ports, independent stop/teardown) and the 10-slot cap logic.

- ~~Get game sound actually audible during play~~ Done: PulseAudio + a hand-rolled stdlib
  WebSocket bridge (`pcm_ws_bridge.py`, in `docker-common` — reusable, not dosbox-specific,
  per the earlier plan for this item) carries raw PCM to the browser, played back via a
  Web Audio `AudioWorkletNode` (`pcm-worklet.js`, also in `docker-common`). Verified real
  end-to-end audio: DOSBox → ALSA (default device, routed through Pulse via
  `/etc/asound.conf`) → PulseAudio's one default sink → `parec` → the bridge → a raw
  WebSocket client, measured RMS ≈9292 while `stuntcar` was actually playing (vs. silence
  before advancing past its title screen) — not just a plumbing check, actual game audio.

  **Audio is a single shared mix, not per-game** — deliberately simplified from an
  earlier per-slot-isolated design (considered and dropped as unnecessary complexity):
  every running game's audio mixes into Pulse's one default sink, and every `/screen/<name>`
  page connects to the same single `AUDIO_PORT` (`71${DISPLAY_NUM}`, `7199` by default).
  Trade-off: with more than one game running, you can't tell their audio apart.

  **Real bug found and fixed along the way**: `parec --device=@DEFAULT_SINK@.monitor`
  (the seemingly-obvious macro syntax) fails with "Stream error: Invalid argument" —
  PulseAudio has a *separate* single-token macro, `@DEFAULT_MONITOR@`, for exactly this;
  concatenating `@DEFAULT_SINK@` with a literal `.monitor` suffix isn't valid.

  `server.sh` now starts `pulseaudio --start --exit-idle-time=-1` and the bridge
  (`pcm_ws_bridge.py 7199 parec --device=@DEFAULT_MONITOR@ ...`) once, before `exec`-ing
  `setup_server.py` — both live for the container's whole lifetime, independent of any
  game's start/stop.

  **No iframe, no per-game screen page** — tried an `<iframe>`-based `/screen/<name>` wrapper
  first (embedding noVNC + an Enable Sound button on one page), but Firefox refuses to load
  iframe content signed by a certificate whose warning hasn't been accepted at the top level,
  with no way to click through *inside* the iframe (Chrome is more lenient, which is why it
  briefly looked like it worked). Settled on: "Open Screen" is a plain top-level link straight
  to noVNC's own URL (opens in a new tab — normal "Accept the Risk and Continue" applies
  there), and the one "Enable Sound" toggle lives on the main setup page instead, meant to be
  left open in its own tab for as long as you're playing. Also regenerated the self-signed
  cert with `CN=localhost` + SAN (`DNS:localhost`, `IP:127.0.0.1`) instead of the old
  meaningless `CN=NY` — Firefox validates the WebSocket-Secure connection's cert against the
  hostname independently of the page load and was rejecting the CN mismatch there even after
  the page-level warning was accepted. Only fixes access via `localhost`/`127.0.0.1`; a LAN IP
  or other hostname would hit the same mismatch again, since a static cert can't cover every
  possible address in advance.

  **A second, worse cert bug turned up right after**: adding `-addext` without also pinning
  `basicConstraints=CA:FALSE` left `openssl req -x509` defaulting to `CA:TRUE` — i.e. the
  generated cert claimed to be a Certificate Authority, not a normal server certificate.
  Firefox hard-refuses a CA-flagged cert used as a TLS server cert (not an overridable
  "Advanced -> Accept the Risk" warning, just a dead end), which is exactly why "the warning
  appears but won't let me proceed" even after the CN fix above. Chrome is lenient about this
  too, which is why it kept looking fine there. Fixed by explicitly setting
  `basicConstraints=critical,CA:FALSE`, `keyUsage=critical,digitalSignature,keyEncipherment`,
  and `extendedKeyUsage=serverAuth` in the same `openssl req -x509 -addext ...` call.

  Because the setup page now hosts a persistent `AudioContext`/`WebSocket`, its old
  `<meta http-equiv="refresh">` auto-refresh was narrowed to only fire while an install was
  actively in progress — it used to also refresh continuously whenever any game was running
  (to catch a crashed game's slot being freed), which would have torn down the live audio
  connection every 3 seconds. **Superseded entirely below** by a JS-driven refresh that
  doesn't navigate the page at all.

  **Removed the "Enable Sound" button — audio is on by default now.** The `AudioContext`/
  `AudioWorkletNode`/`WebSocket` all connect eagerly on page load; the only thing still
  gated on a user gesture (unavoidable browser autoplay policy) is `ctx.resume()`, which
  now piggybacks on the page's very first click or keypress — whatever the user was already
  doing, e.g. clicking Start on a game — rather than requiring a dedicated audio-only click.

  **Added a per-game volume slider.** Despite audio being one shared mix, this is a real,
  independent control: every running game is still its own distinct PulseAudio sink-input
  even though they all feed the same sink, and `pactl set-sink-input-volume` adjusts one
  sink-input without touching the others. `_sink_input_index()` finds the right one by
  matching `pactl -f json list sink-inputs`' `application.process.id` against the game's own
  PID (confirmed exact via `ps`/`pactl` cross-check). The slider POSTs to a new `/volume`
  route via `fetch()` on every tick (not a form submit — no page navigation per drag step).

  **Real bug found right after removing the button**: audio still never actually played.
  Install/Start/Stop/Uninstall were still plain `<form method="post">` submits — every click
  caused a full page navigation (via the server's `303` redirect back to `/`), which tears
  down whatever `AudioContext` was just connected. The fresh page after that reload creates
  a brand-new *suspended* context with no further gesture to unlock it (the reload itself
  doesn't count), so in completely normal usage — load the page, click Start — audio never
  actually starts. Symptom that nailed it down: no speaker icon ever appeared on the Chrome
  tab. Fixed by converting the whole page away from form-based navigation entirely: every
  button is now a bare `<button onclick="doAction(...)">`, `do_POST` returns a plain `204`
  instead of a `303` redirect, and client-side `doAction()`/`refresh()` `fetch()` the action
  and the updated page, then swap just `#content`'s `innerHTML` in place — the page itself
  never navigates, so the audio connection now survives every install/start/stop/uninstall
  click, and `setInterval(refresh, 3000)` replaces the old `<meta refresh>` for keeping
  status current (crashed-game slot cleanup, install progress) without that risk at all.

  **Third real bug, found once audio was actually reaching the speakers**: ~2s of latency,
  visibly getting *worse* the longer a game ran (lip sync drifting further out over time),
  and volume-slider changes taking a couple seconds to actually be heard. Root cause:
  `pcm-worklet.js`'s playback queue (`docker-common/scripts/pcm-worklet.js`, copied into
  this project) had no size cap — every incoming WebSocket message just got `push()`ed on
  regardless of how fast the network delivered it relative to real-time playback. On a fast
  local connection the browser routinely receives data faster than 48kHz real-time consumes
  it, so the backlog only ever grew, never shrank; `set-sink-input-volume` changes the volume
  at the PulseAudio *source*, so anything already sitting in that ever-growing client queue
  still played at the old volume until it drained, i.e. the whole visible backlog's worth of
  delay before a slider change was audible. Fixed by capping the queue at ~100ms
  (`maxQueuedFrames`) and dropping the *oldest* excess data whenever a new chunk would push
  it over that cap — verified directly in Node (stubbing `AudioWorkletProcessor`/
  `sampleRate`/`registerProcessor`) that force-feeding a simulated 4.3s burst leaves only
  ~85ms actually queued afterward, instead of growing unbounded. Propagated to both the
  `docker-common` canonical copy and this project's own copy, per the usual convention.

- Added manifests for `duke` (Duke Nukem 3D, unregistered shareware — confirmed via
  screenshot, "UNREGISTERED SHAREWARE" watermark and the nag screen, despite the folder name)
  and `dn3d` (Duke Nukem 3D: Atomic Edition), both real DOS Build-engine games, both DOSBox
  manifests. Sourced from real played installs at `MiscOS\MSDos\games\duke\`/`dn3d\` on the
  share (found by comparing that directory against `Games\`, which turned up several
  installs — including these — not present as zips anywhere) rather than the existing
  `Games\` zips: these are raw, unzipped folders (with real save games, `duke.rts`, etc.), so
  they were downloaded, re-zipped locally with the usual `<name>/` top-level-folder
  convention, and re-uploaded as `dn3d.zip`/`duke.zip` alongside the source folders on the
  share (manifests, as always, still live in the `Games\dosbox\` catalog regardless).

  A third variant found in the same comparison, `Games\duke3d_w32_bin.zip`, turned out to be
  a **native Windows binary** (`duke3d_w32.exe`, needs `mfc70.dll`/`msvcr70.dll`) rather than
  a DOS game — DOSBox can't run it; would need Wine, a real new dependency. Skipped per
  explicit direction rather than expanding scope.

  Verified end-to-end with screenshots (`xwd`/`imagemagick`, installed temporarily for
  testing only, not part of the image) — both games genuinely launch and render actual
  gameplay (HUD, level geometry, taking hits), not just a menu. **Audio did not play for
  either** despite `pactl` showing dosbox as a normal, uncorked sink-input — tracked down to
  their carried-over `duke3d.cfg` (deleting it entirely breaks the game outright, forcing
  `SETUP.EXE`, so it's required; likely has a stale/mismatched sound-hardware selection from
  whatever machine last ran `SETUP.EXE` for real, rather than DOSBox's emulated SB16). Not a
  bug in the audio pipeline itself — already proven working end-to-end for other games in the
  sound TODO item above. Left as a known gap for these two specifically; fixing it would mean
  either reverse-engineering `duke3d.cfg`'s binary format to patch the stored IRQ/DMA, or
  finding a way to script `SETUP.EXE`'s interactive hardware wizard.

- **Redesigned the manifest schema so a game can expose more than one runnable binary**,
  replacing the single `start_cmd` field with a `programs` list of `{id, label, cmd}` (see
  `setup_server.py`'s module docstring for the full schema). Direct motivation: the
  `duke`/`dn3d` audio gap above — rather than trying to script or reverse-engineer
  `SETUP.EXE`'s hardware wizard from outside, just expose `SETUP.EXE` itself as a second
  "program" alongside the game, so the interactive fix is a button click away. Both
  `duke.json`/`dn3d.json` now list `play` (`duke3d.exe`) and `setup` (`setup.exe`); every
  other existing manifest on the share (`stuntcar`, `monkey`, `monkey2`, `atlantis`, `indy3`,
  `tentacle`) was migrated to a single-entry `programs: [{"id": "start", "label": "Start", ...}]`
  so the "Installed" row still shows one plain "Start" button for them, unchanged in practice.
  `start_game()` now takes a `program_id` and looks up the matching entry; only one program per
  game can run at a time (same one-slot-per-game model as before — starting a second program
  while the first is still running is refused the same way restarting an already-running game
  is). The "Running" status now shows which program is active (e.g. "Running (Setup)") since
  that's no longer implied by the game name alone. Verified end-to-end: built the image,
  installed `duke`, POSTed `/start` with `program=setup`, confirmed via `ps aux` inside the
  container that `dosbox setup.exe` (not `duke3d.exe`) was the process actually running, and
  via a real screenshot that DOSBox's actual "Choose Sound FX Card" screen renders — i.e. the
  interactive fix path this was built for is genuinely reachable now. Did not go further and
  actually reconfigure the sound card/re-verify audio afterward — that's still a manual,
  per-game step left to whoever plays `duke`/`dn3d`, not something the container should do
  unattended.

- **Real bug found via user report ("no audio, no audio icon in browser")**: the page's
  audio-unlock listener (`unlockAudio()`) called `ctx.resume()` once on the page's first
  click/keydown and then unconditionally removed itself, with no `.catch()` on the
  `resume()` promise. Confirmed the server-side pipeline was fine the whole time (PulseAudio
  sink-input active and uncorked, `parec` capturing real non-silent PCM, the raw WebSocket
  bridge on `AUDIO_PORT` handshaking and streaming genuine non-zero frames when tested
  directly) — the browser's own `ctx.state` stayed `"suspended"` no matter how many times the
  user clicked the page. Root cause: if that very first resume attempt silently failed for any
  reason (rejected promise with no handler = no console error, easy to miss), the listener had
  already removed itself and there was no way left to ever retry — permanently stuck
  suspended for the rest of that page load. Confirmed directly: manually attaching a fresh
  one-off click listener that called `ctx.resume()` succeeded immediately (`state=running`),
  proving the `AudioContext` itself was fine and only the one-shot unlock logic was broken.
  Fixed by making `unlockAudio()` idempotent and non-removing: it now calls `ctx.resume()`
  (with a `.catch()` that logs any failure) on *every* click/keydown, and only detaches the
  listeners once `ctx.state` has actually become `"running"` — safe to call repeatedly since
  resuming an already-running context is a no-op. Verified fixed by the user after rebuilding
  and restarting the container: audio icon now appears and sound plays.

- **Added a "Manager" program to all 5 ScummVM manifests** (`monkey`, `monkey2`, `atlantis`,
  `indy3`, `tentacle`), alongside their existing "Start" (`--auto-detect`) - the SCUMM
  equivalent of Duke's "Setup" button. Plain `scummvm -p .` with no game argument opens
  ScummVM's own graphical Launcher (game list, Game Options, Global Options, MT-32/subtitle/
  audio-driver settings, etc.) instead of jumping straight into the game - but confirmed via
  screenshot that `-p .` alone shows an *empty* list ("None" selected, nothing in the game
  panel); the game has to be registered first via `--add` (a separate one-shot command that
  exits after adding, confirmed idempotent - running it again just logs "already been added,
  skipping" and doesn't error or duplicate the config entry) before the Launcher will show it
  pre-selected and ready for "Game Options...". So the "manager" program's command is
  `["bash", "-c", "/usr/games/scummvm -p . --add >/tmp/scummvm-add.log 2>&1; exec /usr/games/scummvm -p ."]`
  - `--add` first (idempotent, safe on every launch), then `exec`'d into the real interactive
  `scummvm -p .` process so the tracked PID (and therefore `_teardown()`'s terminate/kill) is
  the actual ScummVM process, not a wrapper shell with an orphaned child. Verified end-to-end
  through the real setup page: `POST /start` with `program=manager` -> "Running (Manager)" ->
  confirmed via `ps aux` that the real `scummvm -p .` process (not the wrapper) is what's
  running -> screenshot confirms the Launcher opens with "Indiana Jones and the Fate of
  Atlantis" already listed and selectable.