From fe3ba3054e83a9ee81ccf4b76facc7a6e87fff3e Mon Sep 17 00:00:00 2001 From: Zwitschi Date: Tue, 3 Nov 2026 11:52:19 +0100 Subject: [PATCH] Gate texel density and review scorecard coverage --- MEMORY.md | 6 ++++ TODO.md | 12 ++++++-- tools/test_blender_assets.py | 16 +++++++++++ tools/test_model_review.py | 56 ++++++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 tools/test_model_review.py diff --git a/MEMORY.md b/MEMORY.md index aa00345..2eedc78 100644 --- a/MEMORY.md +++ b/MEMORY.md @@ -4,6 +4,12 @@ Session memory for durable decisions, state, and gotchas. Keep concise, remove o ## Decisions +- **3D review gates (2026-11-03):** `tools/test_blender_assets.py` now gates texel density + (all five character slots present, 40–400 px/m authored, ≥250 px/m effective fabric), and + `tools/test_model_review.py` gates scorecard coverage (every `CHARACTERS`/`GEAR`/`PROPS` entry + needs a row in `docs/3d-model-review.md`, characters need a verdict). Python tooling is 116 + tests. Collider-proxy coverage is still an open gate. Remember CI only runs the Godot suite; + the Python tests are local (`cd tools && python3 -m unittest discover -p 'test_*.py'`). - **Pose capture is blocked (2026-11-03):** `src/test/capture_npc_pose.gd` stages idle/walk/sit poses for the pose/weight review, but the saved frames show the rest pose while the same run logs `rig_y=-0.339`, `leg0=-1.357` and a rotated `HipL` bone quaternion at capture time. diff --git a/TODO.md b/TODO.md index 35f9a4b..65d058c 100644 --- a/TODO.md +++ b/TODO.md @@ -116,9 +116,17 @@ except the rebuilt assets themselves. ### E. Gates and documentation -- [ ] Add the measurable checks the review needs (texel density, weight/pose sanity, collider +- [x] Add the measurable checks the review needs (texel density, weight/pose sanity, collider coverage, LOD/impostor check) as stdlib or GDScript gates where possible; CI stays offline. -- [ ] Add a coverage test so every catalogue asset has a scorecard row and verdict. + (Texel-density gate in `tools/test_blender_assets.py` checks all five slots per character + and the 40–400 px/m authored band plus the 250 px/m fabric floor; the pose→bone contract is + covered by `test_pose_reaches_the_skeleton_bones`; LOD/impostor already has + `test_avatar_mesh_budget_and_lod_swap`. Collider coverage is still open — split below.) +- [ ] Add a collider/proxy coverage check: every library prop that blocks walking has a + `StaticBody3D` + `NavigationObstacle3D`, checked against the catalogue. +- [x] Add a coverage test so every catalogue asset has a scorecard row and verdict. + (`tools/test_model_review.py`: every `CHARACTERS`/`GEAR`/`PROPS` entry needs a row in + `docs/3d-model-review.md`, and character rows need a non-empty verdict.) - [ ] Update `docs/blender-assets.md`, `docs/npc-visual-review.md`, `docs/3d-model-review.md`, `docs/architecture.md` and `AGENTS.md` with the skill routes and the new gates. - [ ] Record accepted limits and deferred rebuilds in `docs/known-limitations.md` and `MEMORY.md`. diff --git a/tools/test_blender_assets.py b/tools/test_blender_assets.py index 44a91c3..6f7d7d7 100644 --- a/tools/test_blender_assets.py +++ b/tools/test_blender_assets.py @@ -118,6 +118,22 @@ class TexelDensityTests(unittest.TestCase): skin = report["slots"]["skin"] self.assertEqual(skin["td_px_per_m"], skin["td_runtime_px_per_m"]) + def test_every_character_reports_a_plausible_density_for_every_slot(self): + # Gate for the 3D review: a new or rebuilt character must keep all five + # tint slots measurable and inside the documented stylized band. + for name in ba.CHARACTERS: + report = ba.texel_density(name) + self.assertIsNotNone(report, name) + for slot in ba.SLOTS: + self.assertIn(slot, report["slots"], f"{name}:{slot}") + td = report["slots"][slot]["td_px_per_m"] + self.assertGreaterEqual(td, 40.0, f"{name}/{slot} below 40 px/m") + self.assertLessEqual(td, 400.0, f"{name}/{slot} above 400 px/m") + for slot in ba.TEXEL_RUNTIME_UV_SCALE: + if slot in report["slots"]: + self.assertGreaterEqual(report["slots"][slot]["td_runtime_px_per_m"], + 250.0, f"{name}/{slot} fabric below 250 px/m") + def test_base_fallback_is_measurable_from_its_own_path(self): from pathlib import Path path = ba.ROOT / "assets" / "models" / "generated" / "npc_base.glb" diff --git a/tools/test_model_review.py b/tools/test_model_review.py new file mode 100644 index 0000000..da8e1b1 --- /dev/null +++ b/tools/test_model_review.py @@ -0,0 +1,56 @@ +"""Keep the 3D model review scorecard honest. + +Every catalogue asset (characters, gear, props) must have a row in +`docs/3d-model-review.md`, and the character rows must carry a verdict. This does +not judge the art; it only fails when an asset is added or renamed without being +reviewed, which is how a scorecard silently rots. +""" + +import re +import unittest +from pathlib import Path + +import blender_assets as model + +ROOT = Path(__file__).resolve().parent.parent +REVIEW = ROOT / "docs" / "3d-model-review.md" + + +def catalogue_names() -> set[str]: + return set(model.CHARACTERS) | set(model.GEAR) | set(model.PROPS) + + +def reviewed_rows(text: str) -> dict[str, list[str]]: + """Map asset name -> table cells for rows written as ``| `name` | ... |``.""" + rows = {} + for line in text.splitlines(): + match = re.match(r"^\| `([a-z0-9_]+)` \|(.*)\|\s*$", line) + if match: + rows[match.group(1)] = [cell.strip() for cell in match.group(2).split("|")] + return rows + + +class ModelReviewTests(unittest.TestCase): + def test_every_catalogue_asset_has_a_scorecard_row(self): + text = REVIEW.read_text(encoding="utf-8") + rows = reviewed_rows(text) + missing = sorted(catalogue_names() - set(rows)) + self.assertEqual(missing, [], f"no scorecard row for: {', '.join(missing)}") + + def test_character_rows_carry_a_verdict(self): + rows = reviewed_rows(REVIEW.read_text(encoding="utf-8")) + for character in model.CHARACTERS: + self.assertIn(character, rows, character) + verdict = rows[character][-1] + self.assertTrue(verdict, f"{character} has no verdict") + self.assertNotEqual(verdict, "—", f"{character} verdict not filled in") + + def test_gear_and_prop_rows_have_a_verdict_column(self): + rows = reviewed_rows(REVIEW.read_text(encoding="utf-8")) + for prop in model.PROPS: + self.assertIn(prop, rows, prop) + self.assertTrue(rows[prop][-1], f"{prop} has no verdict column") + + +if __name__ == "__main__": + unittest.main()