From 7132c4a32624107b7843067374ee2e78517fd133 Mon Sep 17 00:00:00 2001 From: Sylpheed port agent Date: Sat, 29 Aug 2026 13:41:00 +0000 Subject: [PATCH] port: implement MODDING rule 4, and withdraw a red flag that was my own bad measurement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MODDING.md calls base-and-overrides "a design constraint on the exporter today, not a milestone to add later". Nothing read `data/mods/` at all -- the directory has existed since the monorepo merge with a .gitkeep and no code path anywhere. Eight milestones shipped past it. ExportTree.resolve() now shadows by path, and every read goes through it: screens, sprites, cues, the music bed, movies. MenuAudio was reading tree.root directly and would otherwise have made audio the one asset kind a mod could not touch, for no reason a modder could have guessed. No manifest, no registration step -- the path IS the registration, which is the whole of the rule. One tree, not a stack: layering needs a load order and nobody has asked for one, so data/mods/README.md says that rather than inventing it. Every shadowed file is printed as it is read. The first version summarised in _ready, before any asset had been read, so it always said "nothing shadowed yet" -- a report structurally incapable of reporting anything, which is worse than none because it looks like an answer. data/mods/ was NOT gitignored, and that is a hole in a hard rule: a mod is usually an edited game asset, and this was the one directory a user is invited to put modified sprites in and git would have taken them. Now excluded except the README. Gate: a synthetic 203x43 magenta PNG (nothing disc-derived) at data/mods/sprites/title/main_menu/ptbtn01.png changes 8501 pixels in a bounding box of exactly 203x43 at the button's position, and `check` still passes. RAISED, NOT RESOLVED: MODDING.md says the tree is data/base/, PORT-MISSION.md §3 and the exporter and .gitignore say export/. Both are mission files and only the human changes a mission. REFUTATION on Q3's paint-order key: 2 of 16 screens did not match a stable sort by layer key -- but that was my test. pgloading_eff00.prm carries NO layer key (layer_source "none"): a primitive with no sprite header and no implied-name fallback. I sorted keyless first; the decoders put it last, which is right, since it is the full-screen black quad and HANDOFF's own sentence is that the fade quad paints last. Completing the rule to "keyless last" gives 16 of 16. SURVIVES. Recorded because the published claim does not say where a keyless element goes and there is one in the archive. Separately the tie-break's reach looks understated: 105 elements share a layer key across 12 of 16 screens, where HANDOFF characterises the cost as "one element's blend on one screen". WITHDRAWN, and it was mine: I filed "the runtime mix has no headroom" in red twice, off a peak reading. Measured properly it is 43 samples at full scale in 5.9 s and 24 in 98.5 s, longest run 0.25 ms -- the disc's own confirm cue on a transient, possibly only in the 16-bit save. Nothing changed, deliberately: attenuating would be an unmeasured level decision of the kind I refused for the loop point. A peak reading is not a clipping measurement. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WM5XL4HfrHuxz8RiMWdCMC --- .gitignore | 7 ++ data/mods/README.md | 57 ++++++++++++++ docs/port/BLOCKED.md | 3 + docs/port/DECISIONS.md | 143 ++++++++++++++++++++++++++++++++++++ port/scripts/boot.gd | 7 ++ port/scripts/export_tree.gd | 69 ++++++++++++++++- port/scripts/menu_audio.gd | 6 +- 7 files changed, 288 insertions(+), 4 deletions(-) create mode 100644 data/mods/README.md diff --git a/.gitignore b/.gitignore index b6afbe4d..dd234bde 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,10 @@ __pycache__/ !/exchange/.gitkeep .godot/ port/.godot/ + +# A mod is usually an EDITED GAME ASSET, and this repository never holds game +# assets. `data/mods/` is the user's own directory -- the exporter never touches +# it and neither does git, except for the README that explains the rule. +/data/mods/* +!/data/mods/README.md +!/data/mods/.gitkeep diff --git a/data/mods/README.md b/data/mods/README.md new file mode 100644 index 00000000..db373bd3 --- /dev/null +++ b/data/mods/README.md @@ -0,0 +1,57 @@ +# Your mods go here + +A mod **replaces a file by shadowing its path**. There is no manifest, no +registration and no load order: if a file exists here at the same relative path +it has in the export tree, the game reads yours instead. + +``` +export/sprites/title/main_menu/ptbtn01.png <- what the exporter wrote +data/mods/sprites/title/main_menu/ptbtn01.png <- what the game will use +``` + +That works for **every** asset kind the port reads — a screen's JSON, a sprite +PNG, a sound cue, the music bed, a movie — because every read goes through one +resolver (`port/scripts/export_tree.gd`, `ExportTree.resolve`). + +Nothing under `export/` is ever touched, so **re-exporting from your disc is +always safe**, and *"did I break it?"* is answered by moving your file out of +this directory. + +Point the game somewhere else with `SYLPHEED_MODS=/path/to/tree`. + +## The game tells you what you changed + +Every file a mod replaces is printed the first time it is read: + +``` +mod: sprites/title/main_menu/ptbtn01.png <- /work/data/mods/sprites/title/main_menu/ptbtn01.png +``` + +A modded run that looked identical to an unmodded one in the log would leave you +with exactly one debugging tool — delete the mod and try again. + +## Try it in ten seconds + +Replace the `NEW GAME` label with a magenta block. The size is the original's, +`203x43`, and nothing here is derived from the disc: + +```bash +mkdir -p data/mods/sprites/title/main_menu +ffmpeg -f lavfi -i "color=c=0xff00c8:s=203x43" -frames:v 1 -pix_fmt rgba \ + data/mods/sprites/title/main_menu/ptbtn01.png +godot --path port -- --menu +``` + +Delete the file to put it back. + +## Nothing in here is committed + +`.gitignore` excludes everything in this directory except this README. That is +deliberate: a mod is usually an *edited game asset*, and this repository never +holds game assets — not in `export/`, and not here either. + +## One tree, not a stack + +Several mods layering over each other would need a load order, and a load order +needs a rule nobody has asked for yet. Today there is one override tree. If you +want more, say so rather than assuming the port has an answer. diff --git a/docs/port/BLOCKED.md b/docs/port/BLOCKED.md index 46f89d92..ab57b9e5 100644 --- a/docs/port/BLOCKED.md +++ b/docs/port/BLOCKED.md @@ -107,6 +107,9 @@ git log -1 --format=%h -- docs/port/HANDOFF.md # newer than 9ca1eb5? re-reconc | P7 — what fills the 4.5 s before `S00A` | **is the LOADING screen what appears between the save slot and the new-game movie?** | Q4 + Q9 | ❔ **not observed, and the port has not assumed it.** Q9 measures `S00A.wmv` starting ~4.5 s after Ⓐ on the save slot. The run that would have shown what is on screen for those 4.5 s hit the documented `sub_823070B0` cache crash after `SELECT DATA`. `GP_TITLE` carries a loading screen (row above) and 4.5 s is about the right shape for one, and that is **exactly why it is written here and not in `flow.json`**. Settled by one run that reaches the movie without crashing. | | P3 — a second `rest.t` casualty | **the loading screen's fade quad rests OPAQUE BLACK** | — | 🔴 **noted, not fixed.** `pgloading_eff00.prm` on entries 12/15 is a 1280×720 black quad whose group is `0xff000000` at t=38, `0xff000000` at t=48, then `0x00000000` on the untimed final — black, held, *then* clear. Its `rest.t` is **38**, where it is fully opaque. So a port that draws this screen at its `rest` draws **a black rectangle over the whole loading screen**. This is the same `settle_time()`/`rest.t` problem as the row below, in a form where it hides the entire screen rather than dimming it — and it will bite whoever first draws a loading screen. | +| ~~P6 — runtime headroom~~ | ~~the Master bus clips~~ | — | 🟢 **withdrawn by the port, 2026-08-29 — it was my own overstatement.** Filed 🔴 twice on a peak reading of 0.0 dBFS. Measured properly: **43 samples at full scale in 5.9 s and 24 in 98.5 s, longest clamped run 0.25 ms** — the disc's own `confirm` cue touching the ceiling on a transient, possibly only in the recording's 16-bit conversion since Godot mixes in float. Not a defect, and nothing is changed: attenuating to buy headroom would be an unmeasured level decision of exactly the kind this port refused for the BGM loop point. **A peak reading is not a clipping measurement** — one sample at 0 dBFS and two seconds of square wave give the same number. | +| Modding — rule 4 | ~~base-and-overrides is unimplemented~~ | — | ✅ **implemented 2026-08-29, and it was not blocked on anybody.** `MODDING.md` calls it a constraint on the exporter *today*; nothing read `data/mods/` for eight milestones. `ExportTree.resolve` now shadows by path for every asset kind, each replacement is logged as it is read, and `.gitignore` excludes the directory's contents — a mod is usually an edited game asset, and that directory was the one place git would have taken one. ⚠️ The `export/` vs `data/base/` naming split between `PORT-MISSION.md` §3 and `MODDING.md` is **raised, not resolved**: only the human changes a mission. | + ## Answered since this file was last written — no longer blocking Q1 (keyframe time unit — linear ramp, 2 units per rendered frame, 1 unit = 1/60 s diff --git a/docs/port/DECISIONS.md b/docs/port/DECISIONS.md index 20d08874..56bd351b 100644 --- a/docs/port/DECISIONS.md +++ b/docs/port/DECISIONS.md @@ -2095,3 +2095,146 @@ down rather than shrugging off: **the discipline protects a reader who opens the path, and it does not protect a reader who stats it at the wrong moment.** Size on disk is not a measurement of a file somebody else is still writing. Ask the decoder, not the directory entry. + +## Modding — rule 4 was never implemented, 2026-08-29 + +`docs/port/MODDING.md` is explicit that modding is *"a design constraint on the +exporter today — not a milestone to add later"*, and its rule 4 is base-and- +overrides: a mod replaces a file by **shadowing its path**, so a modder edits +nothing under the derived tree and re-exporting is always safe. + +**Nothing read `data/mods/` at all.** The directory has existed since the +monorepo merge with a `.gitkeep` in it and no code path anywhere — exporter or +runtime — that looked at it. Eight milestones shipped past that. + +### One resolver, and every read goes through it + +`ExportTree.resolve(rel)` returns the mod tree's copy when one exists and the +derived tree's otherwise. `read_json`, `texture`, `video` and `MenuAudio` all +call it, so a mod can replace **a screen's JSON, a sprite, a cue, the music bed +or a movie** — every asset kind the port reads. + +`MenuAudio` was reading `tree.root.path_join(...)` directly and had to be +changed. Left alone it would have made audio the one asset kind a mod could not +touch, for no reason a modder could have guessed — which is the failure mode +rule 4 exists to prevent. + +There is deliberately no manifest of what a mod contains and no registration +step: **the path is the registration**, which is the whole of the rule. + +⚠️ **One tree, not a stack.** Several mods layering over each other needs a load +order, and a load order needs a rule nobody has asked for. Said out loud in +`data/mods/README.md` rather than answered. + +### A modded run must not look like an unmodded one + +Every shadowed file is printed the first time it is read: + +``` +mod: sprites/title/main_menu/ptbtn01.png <- /work/data/mods/sprites/…/ptbtn01.png +``` + +MODDING says *"did I break it?"* is answered by disabling a mod. That is a fine +last resort and a poor only resort, so the log names the replacement instead. + +**The first version of this got it wrong in an instructive way**: it printed a +summary in `_ready`, before a single asset had been read, and so always said +`(nothing shadowed yet)`. A report structurally incapable of reporting anything +is worse than no report, because it looks like an answer. It now announces each +shadow at the moment it happens. + +### Gate + +A synthetic 203×43 magenta PNG — nothing disc-derived — dropped at +`data/mods/sprites/title/main_menu/ptbtn01.png`: + +| | | +|---|---| +| pixels changed between the two renders | **8 501** of 921 600 (0.92 %) | +| bounding box of the change | x 542…744, y 162…204 — **203×43**, the sprite's own size | +| `sylpheed-export check export` afterwards | 16 screens still validate | + +The changed region is exactly the sprite and nothing else moved. + +### `data/mods/` was not gitignored, and that is a hole in a hard rule + +*"Never commit game assets"* has been enforced on `export/` and `data/base/` +since P0. But **a mod is usually an edited game asset**, and `data/mods/` was +fully tracked — so the one directory a user is invited to put modified sprites in +was the one directory git would happily take them from. + +`.gitignore` now excludes everything under it except the README. + +### The naming split is not mine to resolve + +`MODDING.md` describes the tree as `data/base/`; `PORT-MISSION.md` §3, the +exporter, `ExportTree` and `.gitignore` all say `export/`. Both are mission +files, and PROTOCOL is clear that **only the human changes a mission**, so this +is raised rather than picked. `.gitignore` has ignored both names on purpose +since P0. + +It matters here for one concrete reason: MODDING's layout has `base/` and `mods/` +as **siblings**, and today they are not — the tree is `export/` at the repo root +while mods are `data/mods/`. The resolver takes `SYLPHEED_MODS` or defaults to +`data/mods/`, which is what exists; if the tree is ever renamed to `data/base/` +the sibling rule becomes natural and that default can go. + +## Refutation — the paint-order key, and the reach of its tie-break + +**The claim** (HANDOFF Q3): paint order is *"a `u16` layer key at `+0x0A`, +**decoded**"*, with the tie-break filed 🟡 as *"eight candidates refuted; costs +one element's blend on one screen"*. + +**First pass: 2 of 16 screens did not match** a stable sort by layer key — both +loading screens, `build_12` and `build_15`. + +**That was my test, not the claim.** `pgloading_eff00.prm` carries **no layer key +at all** — `layer: null`, `layer_source: "none"`: it is a primitive with no +sprite header, and the exporter's implied-name fallback produces nothing either. +My sort put a keyless element first; the decoders put it **last**. + +Completing the rule as *"stable sort by layer key, elements with no key last"* +gives **16 of 16**. And last is right: `pgloading_eff00` is the full-screen black +quad, and HANDOFF's own sentence is that the fade quad paints last. + +**Verdict: survives, with the rule completed.** Worth recording because the +published statement does not say where a keyless element goes, and there is at +least one in the archive. + +🟡 **But the tie-break's reach looks understated.** Census over this export: + +``` +elements sharing a layer key with another element: 105, across 12 of 16 screens +``` + +HANDOFF characterises the cost as *"one element's blend on one screen"*. 105 +elements on 12 screens is a much larger surface than that. Most of those ties are +probably invisible — two elements that share a key and never overlap cannot show +a difference — but *probably* is doing the work in that sentence, and nothing has +measured which. The port is unaffected either way: it draws +`ui_layout::derived_paint_order` verbatim and derives no order of its own. + +## Correction — the runtime "clipping" I flagged 🔴 twice was overstated + +P6 and P7 both filed 🔴 *"the runtime mix has no headroom"* on the strength of a +peak reading of 0.0 dBFS off the Master bus. Measured properly: + +| | samples at full scale | of total | longest clamped run | +|---|---|---|---| +| P6 walk (5.944 s) | 43 | 0.0082 % | 10 samples — **0.23 ms** | +| P7 new-game run (98.453 s) | 24 | 0.00028 % | 11 samples — **0.25 ms** | + +That is not a headroom defect. It is the disc's own `confirm` cue, mastered near +full scale (+0.18 dBFS after a lossy decode), touching the ceiling for a quarter +of a millisecond on a transient — and possibly only in the recording's 16-bit +conversion, since Godot mixes in float and `AudioEffectRecord` saves `s16`. + +**Nothing is changed, and that is the point.** Attenuating the mix to buy +headroom would be an unmeasured decision about level — the same class of thing +this port refused for the BGM loop point and the stem balance. Refusing it there +and taking it here would be inconsistent, and it would trade an inaudible +0.25 ms clamp for an audible change nobody measured. + +**A peak reading is not a clipping measurement.** One sample at 0 dBFS and two +seconds of square wave give the same number, and I reported the first as though +it were the second — twice, in red, in two milestones' write-ups. diff --git a/port/scripts/boot.gd b/port/scripts/boot.gd index 324da239..b572004d 100644 --- a/port/scripts/boot.gd +++ b/port/scripts/boot.gd @@ -82,6 +82,13 @@ func _ready() -> void: get_tree().quit(2) return + # Say it before anything is drawn. A modded run that looked identical to an + # unmodded one in the log would leave a modder with exactly one debugging + # tool -- delete the mod and try again. + var mods := export_tree.mod_report() + if mods != "": + print(mods) + _flow = export_tree.authored("flow.json") if _flow == null and (args.has("boot") or args.has("menu")): push_error(export_tree.error) diff --git a/port/scripts/export_tree.gd b/port/scripts/export_tree.gd index ea3d141f..7312d17d 100644 --- a/port/scripts/export_tree.gd +++ b/port/scripts/export_tree.gd @@ -11,6 +11,15 @@ const FORMAT_SCREEN := "sylpheed.screen/3" const FORMAT_MANIFEST := "sylpheed.manifest/1" var root: String = "" +## The override tree, or "" when there is none. MODDING rule 4: a mod replaces a +## file by SHADOWING ITS PATH, so `mods/screens/title/main_menu.json` stands in +## for `/screens/title/main_menu.json` and nothing under the derived tree +## is touched. That is what makes re-exporting always safe. +var mods: String = "" +## Relative paths a mod actually replaced this run, in the order they were first +## read. Recorded because MODDING says "did I break it?" is answered by disabling +## a mod -- which only works if a modded run does not look like an unmodded one. +var shadowed: Array[String] = [] var error: String = "" @@ -27,9 +36,50 @@ static func locate() -> ExportTree: t.error = "no manifest.json under %s -- run `sylpheed-export` first" % candidate return t t.root = candidate + + # The override tree. `SYLPHEED_MODS` wins for the same reason + # `SYLPHEED_EXPORT` does; otherwise `data/mods/`, which is the directory + # MODDING.md's own layout diagram names and the one this repository ships. + # + # Absent is normal and silent: an unmodded run is the common case, and a + # warning about a directory nobody created would be noise. + var m := OS.get_environment("SYLPHEED_MODS") + if m == "": + m = ProjectSettings.globalize_path("res://").path_join("../data/mods").simplify_path() + if DirAccess.dir_exists_absolute(m): + t.mods = m return t +## Where a relative path actually comes from: the mod tree if it has one, else +## the derived tree. +## +## Every read in this class goes through here, so a mod can replace a screen's +## JSON, a sprite, a cue, a music bed or a movie by dropping a file at the same +## relative path. There is deliberately no manifest of what a mod contains and no +## registration step -- the path IS the registration, which is the whole of +## MODDING rule 4. +## +## ⚠️ One tree, not a stack. Several mods layering over each other needs an +## order, and an order needs a rule nobody has asked for yet. Say so rather than +## invent one. +func resolve(rel: String) -> String: + if mods != "": + var over := mods.path_join(rel) + if FileAccess.file_exists(over): + if not shadowed.has(rel): + shadowed.append(rel) + # Announced the moment it happens, not summarised at startup. + # The first version printed a summary in `_ready`, before a + # single asset had been read, so it always said "nothing + # shadowed yet" -- a report that is structurally incapable of + # reporting anything is worse than none, because it looks like + # an answer. + print("mod: %s <- %s" % [rel, over]) + return over + return root.path_join(rel) + + # `authored/` sits beside `export/`, never inside it: it is hand-written and # committed, and a re-export must not be able to touch it. func authored(name: String) -> Variant: @@ -42,7 +92,7 @@ func authored(name: String) -> Variant: func read_json(rel: String) -> Variant: - var path := root.path_join(rel) + var path := resolve(rel) var text := FileAccess.get_file_as_string(path) if text == "": error = "cannot read %s" % path @@ -88,7 +138,7 @@ func screen(name: String) -> Dictionary: func video(name: String) -> Dictionary: for entry: Dictionary in manifest().get("videos", []): if entry.get("name") == name: - var path := root.path_join(entry["file"]) + var path := resolve(String(entry["file"])) if not FileAccess.file_exists(path): error = "manifest lists %s but %s is not there" % [name, path] return {} @@ -109,7 +159,7 @@ func screen_names() -> PackedStringArray: # the disc's own texels and several elements are drawn at 200 %, where a # bilinear filter would invent detail the disc does not have. func texture(rel: String) -> Texture2D: - var bytes := FileAccess.get_file_as_bytes(root.path_join(rel)) + var bytes := FileAccess.get_file_as_bytes(resolve(rel)) if bytes.is_empty(): error = "cannot read sprite %s" % rel return null @@ -118,3 +168,16 @@ func texture(rel: String) -> Texture2D: error = "%s is not a PNG" % rel return null return ImageTexture.create_from_image(img) + + +## One line naming what a mod replaced, or "" when nothing did. +## +## Printed by every run that loads a tree. A modded run that looked identical to +## an unmodded one in the log would make "disable the mod and see" the only +## debugging tool a modder has; this makes it the second one. +func mod_report() -> String: + if mods == "": + return "" + if shadowed.is_empty(): + return "mods: %s is present; each file it replaces is logged as it is read" % mods + return "mods: %s -- %d file(s) shadowed: %s" % [mods, shadowed.size(), ", ".join(shadowed)] diff --git a/port/scripts/menu_audio.gd b/port/scripts/menu_audio.gd index 27d8d655..cf91065c 100644 --- a/port/scripts/menu_audio.gd +++ b/port/scripts/menu_audio.gd @@ -45,7 +45,11 @@ func configure(tree: ExportTree) -> bool: error = tree.error return false for entry: Dictionary in manifest.get("audio", []): - var path := tree.root.path_join(String(entry.get("file", ""))) + # Through the resolver, so a mod can replace a cue or the music bed by + # dropping a file at the same relative path (MODDING rule 4). Reading + # `tree.root` directly here would have made audio the one asset kind a + # mod could not touch, for no reason a modder could have guessed. + var path := tree.resolve(String(entry.get("file", ""))) var stream := AudioStreamOggVorbis.load_from_file(path) if stream == null: error = "manifest lists audio %s but %s is not a readable Ogg Vorbis file" \