From afefe4df1dcd9510486e6af08ee983011610c027 Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Sun, 12 Jul 2026 09:23:43 +0200 Subject: [PATCH] docs: extend assets builder spec with mdl/dupe integrity tools Fold check-ascii-mdl, fix-mdl-model-names, check-duplicate-names, and clean-duplicate-names out of sow-assets-manifest into `crucible assets` (seven commands total), via a shared internal/assets/mdl port of mdl-name-lib.sh + mdl-scan.awk. Consumer keeps the awk libs for import.sh's hot-path gate; moving import.sh into `crucible depot` is a decomposed follow-up spec. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...26-07-12-crucible-assets-builder-design.md | 172 ++++++++++++++++-- 1 file changed, 159 insertions(+), 13 deletions(-) diff --git a/docs/superpowers/specs/2026-07-12-crucible-assets-builder-design.md b/docs/superpowers/specs/2026-07-12-crucible-assets-builder-design.md index 3328bb5..806be48 100644 --- a/docs/superpowers/specs/2026-07-12-crucible-assets-builder-design.md +++ b/docs/superpowers/specs/2026-07-12-crucible-assets-builder-design.md @@ -5,12 +5,19 @@ Status: approved, pre-implementation ## Goal -Fold three NWN:EE asset tools out of the standalone Python toolkit -(`nwnee-asset-processing`) and into Crucible as a new self-contained builder, -`crucible assets`. The Python scripts are references only; they are broken as-is -and carry a heavy config/staging/report layer that we drop. +Fold the NWN:EE asset tools out of two standalone toolkits into Crucible as a +new self-contained builder, `crucible assets`: -Three commands: +- from the Python toolkit (`nwnee-asset-processing`) — model compile, texture + convert, texture upscale. Those scripts are references only; broken as-is and + carrying a heavy config/staging/report layer that we drop. +- from `sow-assets-manifest/scripts/` — the model-name and duplicate-name + integrity tools (`check-ascii-mdl.sh`, `fix-mdl-model-names.sh`, + `check-duplicate-names.sh`, `clean-duplicate-names.sh`). These are ported into + Go and the shell drivers removed; `sow-assets-manifest` calls + `./crucible.sh assets …` through its bootstrap wrapper instead. + +Seven commands: - `crucible assets compile` — compile every ASCII `.mdl` in a directory to binary, in place. @@ -19,6 +26,14 @@ Three commands: always upside-down relative to its source. - `crucible assets upscale` — upscale every texture in a directory through whatever upscaling backend is installed. +- `crucible assets check-mdl` — report uncompiled ASCII `.mdl` files and + model-name mismatches (read-only). +- `crucible assets fix-mdl` — lowercase `.mdl` filenames and rewrite ASCII + model/root names to match each file's stem. +- `crucible assets check-dupes` — report runtime-name (basename) collisions in + directories, or hak-scoped collisions across `assets/*.yml` manifests. +- `crucible assets clean-dupes` — delete files from a "clean" tree whose + basename collides with anything in a "primary" tree. ## Principles @@ -42,19 +57,21 @@ A new self-contained builder that follows the existing `depot` pattern exactly - `internal/assets/run.go` — `func Run(args []string, stdout, stderr io.Writer, getenv func(string) string) int`. Parses the subcommand - (`compile|convert|upscale`) and dispatches. Returns a sysexits-style code. + (`compile|convert|upscale|check-mdl|fix-mdl|check-dupes|clean-dupes`) and + dispatches. Returns a sysexits-style code. - `internal/assets/` — one file per command plus small shared helpers (recursion/file-selection, external-command runner, backend discovery). - `cmd/crucible-assets/main.go` — one-line shim: `func main() { os.Exit(dispatch.RunBuilder("assets", os.Args[1:])) }`. - `internal/dispatch`: - - one `Registry` entry for `assets` (`Wired: true`) listing the three + - one `Registry` entry for `assets` (`Wired: true`) listing the seven commands with `Usage`/`Options`; - extend the direct-delegate branch that today reads `if b.Name == "depot" && b.Wired` so `assets` also routes to `assets.Run(...)` instead of the legacy `app.Run`. -- `docs/command-surface.md` — three visible-command rows. -- `wrappers/consumers.txt` / wrappers are unchanged (no new consumer). +- `docs/command-surface.md` — seven visible-command rows. +- `wrappers/consumers.txt` / wrappers are unchanged (no new consumer; the + consumers listed already vendor the wrapper). ### Shared helpers (`internal/assets`) @@ -68,6 +85,28 @@ A new self-contained builder that follows the existing `depot` pattern exactly first path that exists from a supplied list of candidates. Used by every backend discovery. +### Shared MDL name module (`internal/assets/mdl`) + +A pure-Go port of `mdl-name-lib.sh` + `mdl-scan.awk`, no external tools. One +place three commands share (`compile`'s pre-check, `check-mdl`, `fix-mdl`): + +- `IsASCII(path)` — NUL in the first 256 bytes → binary; else the leading + keyword must be `#`/`newmodel`/`node`/`setsupermodel`. Mirrors `mdl_is_ascii`. +- `ExpectedName(path)` — the file stem (sans `.mdl`). +- `CheckNames(path) []Mismatch` — for an ASCII model, report header-token + mismatches (`newmodel`, `setsupermodel`, `beginmodelgeom`, `endmodelgeom`, + `donemodel`, `newanim`, `doneanim` — case-insensitive vs the stem) and the + geometry **base node** mismatch (the node inside `beginmodelgeom` whose parent + is `null`; empty if none). Mirrors `mdl_check_model_name` + `mdl_base_name`. +- `FixNames(in) (out []byte, changed bool)` — rewrite only the model identity: + the header tokens above, plus the base node's declaration / any `parent` / + `animroot` pointing at the *old* base name → the expected name. Animation bone + names and supermodel animroots are left untouched so inheritance keeps working. + Mirrors `mdl_fix_model_name` byte-for-byte in intent. + +Detection must stay identical to the awk classifier so behavior does not drift +while `import.sh` still runs the awk version (see Consumer migration). + ## `crucible assets compile …` Usage: `crucible assets compile [--nwn ] [--non-recursive] …` @@ -108,9 +147,11 @@ engine log (`~/.local/share/Neverwinter Nights/logs/nwengineLog.txt`) as advisory diagnostics. Dropped vs reference: internal-name-mismatch pre-checks and the -`Model Names Differ` fail rule are kept (cheap, prevents silent wrong output); -the discard-manifest, dry-run/report layer, and `processing/in|out` staging are -dropped. +`Model Names Differ` fail rule are kept (cheap, prevents silent wrong output), +using the shared `internal/assets/mdl` module — `compile` aborts a model whose +names differ and tells the user to run `crucible assets fix-mdl`. It does **not** +auto-fix (kept separate, by decision). The discard-manifest, dry-run/report +layer, and `processing/in|out` staging are dropped. ## `crucible assets convert …` @@ -165,12 +206,109 @@ Recursively upscale textures in place. Dropped vs reference: min-dimension small-texture staging, the separate `--dds-backend` plumbing (it reuses convert), configured backend-path table. +## `crucible assets check-mdl …` + +Port of `check-ascii-mdl.sh`. Read-only. Each `` is a file or a directory +(recursed for `*.mdl`). For every model, using `internal/assets/mdl`: + +- report an **uncompiled ASCII** `.mdl` (a binary/compiled one is fine, skipped); +- report every model-name mismatch (header tokens and geometry base node) with + file, line, the offending token, and the expected stem. + +Exit non-zero if any ASCII model or mismatch is found; zero and an `OK` line +otherwise. No mutation. + +## `crucible assets fix-mdl [--dry-run] …` + +Port of `fix-mdl-model-names.sh`. For each `*.mdl` under the paths: + +1. **Lowercase the filename** if it is not already lowercase (`mv`; abort that + file on a case-collision with an existing target). +2. For ASCII models with a name mismatch, rewrite the model identity in place + via `mdl.FixNames` (binary models are skipped — the engine owns those). + +`--dry-run` reports every "would lowercase" / "would fix" without touching disk. +Prints a per-file log and a final `no broken mdl model names found` when clean. + +Selection optimization from the reference is preserved: a file is a fix +candidate only if it needs a content fix (name mismatch) or a filename +lowercase, so the expensive per-file read/rewrite runs only on the few that +matter, not the whole tree. + +## `crucible assets check-dupes [--manifests] …` + +Port of `check-duplicate-names.sh`. Read-only. NWN packs hak entries by +basename, not path, so files at different paths with the same basename +(case-insensitively) silently clobber on pack. This flags them. + +- Directory mode (default): across all `` args, report any two files whose + lowercased basename matches. +- `--manifests`: additionally read every `assets/*.yml` in the current repo and + report basename collisions **within the same hak** (`.assets[].path` grouped + by `.assets[].hak`), reusing Crucible's existing `gopkg.in/yaml.v3` parsing. + Requires no external `yq`. + +Exit non-zero if any collision is found. + +## `crucible assets clean-dupes [--dry-run] ` + +Port of `clean-duplicate-names.sh`. Deletes files from the `` tree whose +basename collides (case-insensitively) with any file in the `` tree. +Mutates only ``. Same rule as `check-dupes` directory mode, but resolves +the collision by removal instead of reporting. + +- Refuses to run if `` and `` overlap (either contains the + other), matching the script's guard. +- `--dry-run` reports every "would delete" without removing. + +Exit zero on success with a count of removed (or would-remove) files. + +## Consumer migration (`sow-assets-manifest`) + +`sow-assets-manifest` already ships the `crucible` bootstrap wrapper +(`crucible.sh`) and uses `crucible depot` from its Makefile, so it resolves a +released `crucible` with zero extra setup. + +- **Delete** the four folded driver scripts: `scripts/check-ascii-mdl.sh`, + `scripts/fix-mdl-model-names.sh`, `scripts/check-duplicate-names.sh`, + `scripts/clean-duplicate-names.sh`. +- **Repoint** the Makefile targets to the wrapper: + - `check-mdl` → `./crucible.sh assets check-mdl …` + - `fix-mdl` → `./crucible.sh assets fix-mdl [--dry-run] …` + - `check-dupes` → `./crucible.sh assets check-dupes …` and + `./crucible.sh assets check-dupes --manifests` + - `clean-dupes` → `./crucible.sh assets clean-dupes [--dry-run] ` + - the `haks` target's `check-duplicate-names.sh --manifests` guard → + `./crucible.sh assets check-dupes --manifests` +- **Keep for now:** `scripts/mdl-scan.awk` and `scripts/mdl-name-lib.sh`. They + are still sourced by `import.sh` for its batched pre-import gate (~28× faster + than per-file, on a hot path). They retire in the follow-up below. Because + both the awk classifier and the Go `mdl` module must agree, the port keeps + detection byte-for-byte identical (a shared fixture set checks this). + +Nothing in `sow-assets-manifest` is committed by this project's plan except the +script deletions and Makefile edits; that repo's change lands as its own commit +once the new `crucible` release with `assets` is available. + +## Out of scope — follow-up spec + +Moving the **depot-interacting pipeline** (`import.sh`, `sync-assets.sh`, +`export.sh`, and the inline mdl gate) into `crucible depot` is a separate +project with its own spec. When `import.sh` becomes `crucible depot import`, its +gate uses the in-process `internal/assets/mdl` module and +`scripts/mdl-scan.awk` + `scripts/mdl-name-lib.sh` are deleted. This spec +deliberately stops at the standalone integrity tools to keep one focused plan. + ## Error handling - Missing external tool (engine, `magick`, upscaler) → non-zero exit, message - names the tool and how to get it. Never a faked artifact. + names the tool and how to get it. Never a faked artifact. The pure-Go + integrity commands (`check-mdl`, `fix-mdl`, `check-dupes`, `clean-dupes`) need + no external tool. - Per-file failures are collected and printed as a summary line at the end; the command exits non-zero if any file failed but still processes the rest. +- Report commands (`check-mdl`, `check-dupes`) exit non-zero when they *find* + problems — that is their contract as CI/pre-import gates, not a tool error. - Unknown subcommand / bad flags → usage error, exit 64. ## Testing @@ -184,6 +322,14 @@ Go tests in `internal/assets`: - one real convert round-trip using `magick` (present in the dev shell): encode a small PNG to DDS and back, asserting the pixels are vertically flipped after a single conversion and restored after the round trip. +- `internal/assets/mdl`: table-driven fixtures — ASCII vs binary detection, + each header-token mismatch, the base-node mismatch, and `FixNames` producing + a model that then passes `CheckNames`. A parity fixture set shared in intent + with `mdl-scan.awk` so the Go port and the still-present awk agree. +- `check-dupes`/`clean-dupes`: temp trees asserting basename-collision + detection (case-insensitive), the `--manifests` hak-scoped path against a + small `assets/*.yml` fixture, the overlap guard, and `--dry-run` mutating + nothing. External engine/upscaler calls are exercised through the `run` indirection with a stub in tests; they are not invoked for real in CI.