diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index efc494d6..ad3113ac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,10 @@ jobs: run: uv run --with mypy mypy --config-file apps/blender/pyproject.toml - name: Mypy strict (packages/validator) run: uv run --with mypy mypy --config-file packages/validator/pyproject.toml + - name: Mypy strict (packages/models) + run: uv run --with mypy mypy --config-file packages/models/pyproject.toml + - name: Mypy strict (packages/codegen) + run: uv run --with mypy mypy --config-file packages/codegen/pyproject.toml - name: Pytest (validation unit tests, no Blender) run: uv run pytest tests/ @@ -47,6 +51,9 @@ jobs: - name: Typecheck working-directory: apps/photoshop run: pnpm run typecheck + - name: Lint + working-directory: apps/photoshop + run: pnpm run lint - name: Unit tests working-directory: apps/photoshop run: pnpm test diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a5099bf6..536986ef 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -6,6 +6,23 @@ on: - "blender-v*" - "godot-v*" - "photoshop-v*" + # Tagless dry-run: exercise the package steps without uploading a + # release. The photoshop `.jsx` copy rotted unnoticed precisely because + # this job only ever ran on tags; a dispatch path keeps it exercisable. + workflow_dispatch: + inputs: + component: + description: "Component to package (dry-run, no release upload)" + required: true + type: choice + options: + - blender + - godot + - photoshop + version: + description: "Version string for the artifact name" + required: false + default: "0.0.0-dry-run" permissions: contents: write @@ -18,32 +35,78 @@ jobs: - name: Determine component and version id: meta + # Route the dispatch inputs through env vars rather than expanding + # `${{ inputs.* }}` straight into the script body: `version` is a + # free-text input, so a direct expansion is a shell-injection vector. + env: + INPUT_COMPONENT: ${{ inputs.component }} + INPUT_VERSION: ${{ inputs.version }} run: | - tag="${GITHUB_REF#refs/tags/}" - component="${tag%-v*}" - version="${tag##*-v}" + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + component="$INPUT_COMPONENT" + version="$INPUT_VERSION" + else + tag="${GITHUB_REF#refs/tags/}" + component="${tag%-v*}" + version="${tag##*-v}" + fi echo "name=$component" >> "$GITHUB_OUTPUT" echo "version=$version" >> "$GITHUB_OUTPUT" + # The photoshop artifact is the built UXP bundle, not a source file. + # `apps/photoshop/dist/` is gitignored, so the job builds it here. + - name: Set up Node (photoshop) + if: steps.meta.outputs.name == 'photoshop' + uses: actions/setup-node@v4 + with: + node-version: "20" + - name: Set up pnpm (photoshop) + if: steps.meta.outputs.name == 'photoshop' + uses: pnpm/action-setup@v4 + with: + version: 9 + - name: Build the UXP plugin (photoshop) + if: steps.meta.outputs.name == 'photoshop' + working-directory: apps/photoshop + run: | + pnpm install --frozen-lockfile + pnpm run build + - name: Build artifact + # Same defense as the meta step: the version rides an env var, never + # a `${{ }}` expansion inside the script body. + env: + COMPONENT: ${{ steps.meta.outputs.name }} + VERSION: ${{ steps.meta.outputs.version }} run: | mkdir -p dist - case "${{ steps.meta.outputs.name }}" in + case "$COMPONENT" in blender) - (cd apps/blender && zip -r "../../dist/proscenio-blender-${{ steps.meta.outputs.version }}.zip" . -x "tests/*" "pyproject.toml" "README.md") + (cd apps/blender && zip -r "../../dist/proscenio-blender-$VERSION.zip" . -x "tests/*" "pyproject.toml" "README.md") ;; godot) - (cd apps/godot && zip -r "../../dist/proscenio-godot-${{ steps.meta.outputs.version }}.zip" addons/proscenio) + (cd apps/godot && zip -r "../../dist/proscenio-godot-$VERSION.zip" addons/proscenio) ;; photoshop) - cp apps/photoshop/proscenio_export.jsx "dist/proscenio-photoshop-${{ steps.meta.outputs.version }}.jsx" + # The UXP plugin root is dist/ itself - manifest.json lands + # there after the webpack build - so archive its contents, + # keeping manifest.json at the zip root for the host loader. + (cd apps/photoshop/dist && zip -r "../../../dist/proscenio-photoshop-$VERSION.zip" .) ;; *) - echo "Unknown component: ${{ steps.meta.outputs.name }}" + echo "Unknown component: $COMPONENT" exit 1 ;; esac + - name: List artifact contents + run: | + for f in dist/*.zip; do + echo "== $f ==" + unzip -l "$f" + done + - uses: softprops/action-gh-release@v2 + if: github.event_name == 'push' with: files: dist/* diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7d152891..9f3ddfa3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -49,6 +49,33 @@ repos: entry: uv run --project apps/blender --quiet mypy --config-file packages/validator/pyproject.toml files: ^packages/validator/.*\.py$ pass_filenames: false + - id: mypy-models + name: mypy (packages/models) + language: system + entry: uv run --with mypy --quiet mypy --config-file packages/models/pyproject.toml + files: ^packages/models/.*\.py$ + pass_filenames: false + - id: mypy-codegen + name: mypy (packages/codegen) + language: system + entry: uv run --with mypy --quiet mypy --config-file packages/codegen/pyproject.toml + # Fires on models edits too: codegen's typing depends on the + # models it imports, so a models change can break codegen mypy. + files: ^packages/(codegen|models)/.*\.py$ + pass_filenames: false + + # eslint runs the strictTypeChecked + stylisticTypeChecked config + # (the no-unsafe-* family that tsc cannot see) over the UXP plugin. + # `language: system` so it uses the local pnpm; pass_filenames is off + # because the config globs `src` itself. + - repo: local + hooks: + - id: eslint-photoshop + name: eslint (apps/photoshop) + language: system + entry: pnpm --dir apps/photoshop run lint + files: ^apps/photoshop/src/.*\.tsx?$ + pass_filenames: false - repo: https://github.com/Scony/godot-gdscript-toolkit rev: 4.3.3 diff --git a/apps/godot/tests/test_importer.gd b/apps/godot/tests/test_importer.gd index c80acd94..3c3f912c 100644 --- a/apps/godot/tests/test_importer.gd +++ b/apps/godot/tests/test_importer.gd @@ -77,6 +77,7 @@ func _run_dummy_checks() -> void: _assert_true(anim.length > 0.0, "dummy: animation length > 0") _run_idempotency_check(data, character) + _assert_saved_scene_has_no_scripts(character, "dummy") character.free() @@ -124,6 +125,8 @@ func _run_effect_checks() -> void: ) _assert_eq(anim.track_get_key_count(0), 4, "effect: frame key count") + _assert_saved_scene_has_no_scripts(character, "effect") + character.free() @@ -164,6 +167,8 @@ func _run_skinned_checks() -> void: _assert_eq(torso.modulate, Color(0.25, 0.5, 0.75, 1.0), "skinned: modulate stamped") _assert_eq(torso.z_index, -3, "skinned: z_index stamped") + _assert_saved_scene_has_no_scripts(character, "skinned") + character.free() @@ -238,6 +243,8 @@ func _run_slot_checks() -> void: "slots: track %d uses NEAREST" % i ) + _assert_saved_scene_has_no_scripts(character, "slots") + character.free() @@ -320,6 +327,56 @@ func _collect_descendants_of_type(node: Node, type_name: String) -> Array: return out +# The shipped .scn must carry no addon script references: consumers run with +# the plugin disabled (it is editor-only), so a baked script path would break +# the scene at load. Mirror importer.gd's owner-set + pack + save, reload from +# disk, and assert every node's get_script() is null. +func _assert_saved_scene_has_no_scripts(character: Node2D, label: String) -> void: + _set_owner_recursive(character, character) + var packed := PackedScene.new() + var pack_err := packed.pack(character) + if pack_err != OK: + _fail("%s: PackedScene.pack failed (error %d)" % [label, pack_err]) + return + var path := "user://proscenio_saved_scene_check.scn" + var save_err := ResourceSaver.save(packed, path) + if save_err != OK: + _fail("%s: ResourceSaver.save failed (error %d)" % [label, save_err]) + return + # CACHE_MODE_REPLACE so each fixture reloads its own bytes off disk, not a + # cached PackedScene left by the previous call to this shared path. + var reloaded := ( + ResourceLoader.load(path, "PackedScene", ResourceLoader.CACHE_MODE_REPLACE) as PackedScene + ) + if reloaded == null: + _fail("%s: reload of saved scene returned null" % label) + return + var instance := reloaded.instantiate() + var scripted := _collect_scripted_nodes(instance) + _assert_eq( + scripted.size(), + 0, + "%s: saved scene free of addon script refs [%s]" % [label, ", ".join(scripted)] + ) + instance.free() + + +func _collect_scripted_nodes(node: Node) -> PackedStringArray: + var out: PackedStringArray = [] + if node.get_script() != null: + out.append(String(node.name)) + for child: Node in node.get_children(): + out.append_array(_collect_scripted_nodes(child)) + return out + + +func _set_owner_recursive(node: Node, owner: Node) -> void: + for child: Node in node.get_children(): + if child != owner: + child.owner = owner + _set_owner_recursive(child, owner) + + func _assert_eq(actual: Variant, expected: Variant, label: String) -> void: if actual == expected: _passes += 1 diff --git a/packages/codegen/pyproject.toml b/packages/codegen/pyproject.toml index 2619796c..b7468a69 100644 --- a/packages/codegen/pyproject.toml +++ b/packages/codegen/pyproject.toml @@ -18,3 +18,27 @@ build-backend = "hatchling.build" [tool.hatch.build.targets.wheel] packages = ["src/proscenio_codegen"] + +[tool.mypy] +# 3.11 floor: the codegen CLI runs under a standalone interpreter +# (`python -m proscenio_codegen`), not Blender's bundled 3.13. +python_version = "3.11" +# The pydantic plugin teaches mypy the model surface the emitter walks. +plugins = ["pydantic.mypy"] +strict = true +warn_unused_ignores = true +warn_unreachable = true +warn_return_any = true +extra_checks = true +strict_equality = true +# disallow_any_explicit (kept in the bpy-bound profiles) is dropped here: +# the emitter reasons over typing internals (get_args / get_origin / +# Union / Annotated) whose payloads are Any by the stdlib contract, and +# pydantic's synthesized model methods carry explicit Any besides. +disallow_any_decorated = true +disallow_any_unimported = true +disallow_subclassing_any = true +explicit_package_bases = true +namespace_packages = true +mypy_path = "${MYPY_CONFIG_FILE_DIR}/src" +files = ["${MYPY_CONFIG_FILE_DIR}/src/proscenio_codegen"] diff --git a/packages/codegen/src/proscenio_codegen/godot_emit.py b/packages/codegen/src/proscenio_codegen/godot_emit.py index 17cd21cd..b74ad439 100644 --- a/packages/codegen/src/proscenio_codegen/godot_emit.py +++ b/packages/codegen/src/proscenio_codegen/godot_emit.py @@ -121,9 +121,15 @@ def _union_dispatcher_name(item: Any) -> str | None: from proscenio_models.proscenio import MeshElement, SpriteElement from proscenio_models.psd_manifest import MeshLayer, SpriteLayer - if variant_models == {MeshElement, SpriteElement}: + # Typed as the same set element type as `variant_models` so the literal + # joins to `type[BaseModel]` rather than the per-module `_Strict` base + # (proscenio and psd_manifest each define one; the bare literal would + # join to the wrong one and fail the equality check under strict mypy). + element_variants: set[type[BaseModel]] = {MeshElement, SpriteElement} + layer_variants: set[type[BaseModel]] = {MeshLayer, SpriteLayer} + if variant_models == element_variants: return "ProscenioElement" - if variant_models == {MeshLayer, SpriteLayer}: + if variant_models == layer_variants: return "ProscenioLayer" return None diff --git a/packages/fixtures/simple_psd/build_blend.py b/packages/fixtures/simple_psd/build_blend.py index 7caa7da3..26f014cf 100644 --- a/packages/fixtures/simple_psd/build_blend.py +++ b/packages/fixtures/simple_psd/build_blend.py @@ -45,6 +45,8 @@ def main() -> None: _wipe_blend() _run_importer() _save_blend() + _rewrite_images_to_relpath() + bpy.ops.wm.save_mainfile() print(f"[build_simple_psd] wrote {BLEND_PATH}") @@ -98,6 +100,28 @@ def _save_blend() -> None: bpy.ops.wm.save_as_mainfile(filepath=str(BLEND_PATH), check_existing=False) +def _rewrite_images_to_relpath() -> None: + """After save_as, rewrite each image filepath to a ``//``-relative path. + + The importer loads PNGs by absolute path; ``bpy.path.relpath`` needs the + .blend already on disk (the save_as above sets that base), and the caller + saves again afterward. Keeps the committed .blend machine-independent. + """ + for img in bpy.data.images: + if not img.filepath: + continue + try: + img.filepath = bpy.path.relpath(img.filepath) + except ValueError as exc: + # bpy.path.relpath raises ValueError on Windows when the image + # lives on a different drive letter from the .blend; the absolute + # path still resolves, only the portability promise weakens. + print( + f"[build_simple_psd] keeping absolute path for {img.name} ({exc})", + file=sys.stderr, + ) + + if __name__ == "__main__": try: main() diff --git a/packages/fixtures/slot_cycle/build_blend.py b/packages/fixtures/slot_cycle/build_blend.py index 20dff005..a33bcd93 100644 --- a/packages/fixtures/slot_cycle/build_blend.py +++ b/packages/fixtures/slot_cycle/build_blend.py @@ -54,6 +54,8 @@ def main() -> None: _build_attachment_mesh(name, slot_empty) _build_cycle_action(slot_empty) _save_blend() + _rewrite_images_to_relpath() + bpy.ops.wm.save_mainfile() print(f"[build_slot_cycle] wrote {BLEND_PATH}") @@ -188,6 +190,28 @@ def _save_blend() -> None: bpy.ops.wm.save_as_mainfile(filepath=str(BLEND_PATH), check_existing=False) +def _rewrite_images_to_relpath() -> None: + """After save_as, rewrite each image filepath to a ``//``-relative path. + + ``bpy.path.relpath`` needs the .blend already on disk (the save_as above + sets that base); the caller saves again afterward. Keeps the committed + .blend machine-independent instead of baking a dev's repo root into it. + """ + for img in bpy.data.images: + if not img.filepath: + continue + try: + img.filepath = bpy.path.relpath(img.filepath) + except ValueError as exc: + # bpy.path.relpath raises ValueError on Windows when the image + # lives on a different drive letter from the .blend; the absolute + # path still resolves, only the portability promise weakens. + print( + f"[build_slot_cycle] keeping absolute path for {img.name} ({exc})", + file=sys.stderr, + ) + + if __name__ == "__main__": try: main() diff --git a/packages/models/pyproject.toml b/packages/models/pyproject.toml index 91ed3681..6a1d5c26 100644 --- a/packages/models/pyproject.toml +++ b/packages/models/pyproject.toml @@ -14,3 +14,30 @@ build-backend = "hatchling.build" [tool.hatch.build.targets.wheel] packages = ["src/proscenio_models"] + +[tool.mypy] +# 3.11 floor: this package runs under a standalone interpreter (codegen +# CLI, the validator's pytest), not Blender's bundled 3.13 - so the type +# gate matches its real minimum runtime, unlike the bpy-bound packages. +python_version = "3.11" +# The pydantic plugin teaches mypy the synthesized model __init__ surface, +# which also sharpens the codegen package that constructs and walks these +# models. +plugins = ["pydantic.mypy"] +strict = true +warn_unused_ignores = true +warn_unreachable = true +warn_return_any = true +extra_checks = true +strict_equality = true +# disallow_any_explicit (kept in the bpy-bound profiles) is dropped here: +# pydantic's plugin-synthesized methods carry explicit Any on every +# field-bearing model, so the flag would fire on the model definitions +# themselves rather than on author-written looseness. +disallow_any_decorated = true +disallow_any_unimported = true +disallow_subclassing_any = true +explicit_package_bases = true +namespace_packages = true +mypy_path = "${MYPY_CONFIG_FILE_DIR}/src" +files = ["${MYPY_CONFIG_FILE_DIR}/src/proscenio_models"] diff --git a/specs/035-project-health/TODO.md b/specs/035-project-health/TODO.md index 43326a53..c8689ae0 100644 --- a/specs/035-project-health/TODO.md +++ b/specs/035-project-health/TODO.md @@ -6,34 +6,36 @@ Sequenced from the verdicts in [STUDY.md](STUDY.md): six items land now (the blo ### Repackage the Photoshop release job `[blocking]` -- [ ] Replace the stale `.jsx` copy in [release.yml](../../.github/workflows/release.yml) with a UXP build: setup-node + pnpm, `pnpm install --frozen-lockfile`, `pnpm run build` in `apps/photoshop` (`dist/` is gitignored, so the job must build), then zip `dist/` as `proscenio-photoshop-.zip` (`.ccx` rename optional). -- [ ] Add a `workflow_dispatch` dry-run path that runs the package steps without the gh-release upload, so the job can be exercised tagless - the `.jsx` line rotted precisely because the workflow only runs on tags. -- [ ] Dry-run all three component branches (blender, godot, photoshop) via the dispatch path and check each artifact's content listing. +- [x] Replace the stale `.jsx` copy in [release.yml](../../.github/workflows/release.yml) with a UXP build: setup-node + pnpm, `pnpm install --frozen-lockfile`, `pnpm run build` in `apps/photoshop` (`dist/` is gitignored, so the job must build), then zip `dist/` as `proscenio-photoshop-.zip` (`.ccx` rename optional). +- [x] Add a `workflow_dispatch` dry-run path that runs the package steps without the gh-release upload, so the job can be exercised tagless - the `.jsx` line rotted precisely because the workflow only runs on tags. +- [x] Dry-run all three component branches (blender, godot, photoshop) via the dispatch path and check each artifact's content listing. (Verified: photoshop zip carries `manifest.json` at the root, blender the addon tree, godot `addons/proscenio/`.) ### Turn the existing ESLint config into a gate -- [ ] Run `pnpm run lint` over `apps/photoshop/src`; fix or narrowly scope-justify any findings so the enabling commit lands green. -- [ ] Add the lint step to the `lint-photoshop` job in [ci.yml](../../.github/workflows/ci.yml). -- [ ] Mirror it as a pre-commit local hook (sketch in [backlog-code-quality.md](../backlog-code-quality.md)). +- [x] Run `pnpm run lint` over `apps/photoshop/src`; fix or narrowly scope-justify any findings so the enabling commit lands green. (Tree already passes clean.) +- [x] Add the lint step to the `lint-photoshop` job in [ci.yml](../../.github/workflows/ci.yml). +- [x] Mirror it as a pre-commit local hook (sketch in [backlog-code-quality.md](../backlog-code-quality.md)). ### mypy gate for proscenio-models and proscenio-codegen -- [ ] Add `[tool.mypy]` strict-strict blocks (validator profile, `python_version = "3.11"`) to [packages/models/pyproject.toml](../../packages/models/pyproject.toml) and [packages/codegen/pyproject.toml](../../packages/codegen/pyproject.toml); scope the pydantic discriminator functions' `Any`-by-contract payloads with coded ignores. -- [ ] Add the two mypy steps to `lint-python` in ci.yml and matching pre-commit hooks. -- [ ] Land before the schema-expressiveness wave starts churning the models (cross-spec sequencing; see [EXECUTION_MAP.md](../EXECUTION_MAP.md)). +- [x] Add `[tool.mypy]` strict-strict blocks (validator profile, `python_version = "3.11"`) to [packages/models/pyproject.toml](../../packages/models/pyproject.toml) and [packages/codegen/pyproject.toml](../../packages/codegen/pyproject.toml). The pydantic plugin handles the model surface; `disallow_any_explicit` is dropped here (pydantic's synthesized methods carry explicit `Any` on every model), the rest of the strict profile stays. +- [x] Add the two mypy steps to `lint-python` in ci.yml and matching pre-commit hooks. +- [x] Land before the schema-expressiveness wave starts churning the models (cross-spec sequencing; see [EXECUTION_MAP.md](../EXECUTION_MAP.md)). ### Saved-scene assert for the plugin-uninstall guard -- [ ] In [test_importer.gd](../../apps/godot/tests/test_importer.gd), pack the built character into a `PackedScene`, save, reload, and walk every node asserting `get_script() == null` - no addon script references baked into importer output. -- [ ] Run the assert for all four fixture documents inside the same headless pass (no new CI job). +- [x] In [test_importer.gd](../../apps/godot/tests/test_importer.gd), pack the built character into a `PackedScene`, save, reload, and walk every node asserting `get_script() == null` - no addon script references baked into importer output. +- [x] Run the assert for all four fixture documents inside the same headless pass (no new CI job). ### Fixture portability: strip absolute image paths -- [ ] Apply the blink_eyes `bpy.path.relpath` + re-save pattern to [slot_cycle/build_blend.py](../../packages/fixtures/slot_cycle/build_blend.py) and the simple_psd build path; regenerate both committed `.blend`s. -- [ ] Verify with a strings-scan of both `.blend`s for machine-absolute paths and confirm the goldens still diff clean. +- [x] Apply the blink_eyes `bpy.path.relpath` + re-save pattern to [slot_cycle/build_blend.py](../../packages/fixtures/slot_cycle/build_blend.py) and the simple_psd build path. (Both committed `.blend`s already store relative `//` paths - `save_as_mainfile` remaps by default - so no re-bake was committed; the script change makes the guarantee explicit + cross-drive-safe for the next regeneration.) +- [x] Verify with a strings-scan of both `.blend`s for machine-absolute paths and confirm the goldens still diff clean. (Confirmed via `image.filepath_raw` = `//...` and a clean full `run_tests.py` re-export.) ### End-to-end mixed-feature fixture +Carried to a focused follow-up PR, split from the toolchain/shipping PR (the STUDY flags this as the one heavier now-item). Its precondition is met: the export-correctness writer fixes (picker-first `find_armature`, `MeshElement.polygons`) are already in code. + - [ ] Sequence after the export-correctness blocking writer fixes (armature picker, multi-polygon) so the golden bakes once. - [ ] Author the fixture in the categorization buckets under `examples/generated/`: skinned polygon body + sprite_frame mouth + slot with mixed attachments + packed atlas + Drive-from-Bone + one animation. - [ ] Bake the Blender-to-Godot golden and wire it into the existing `test-blender` re-export diff and `test-godot` smoke; populate the dev project via [sync_fixtures.py](../../scripts/godot/sync_fixtures.py) (never edit the synced copies).