diff --git a/.claude/skills/pixellab-cli-animation/SKILL.md b/.claude/skills/pixellab-cli-animation/SKILL.md index 397c0cf..417844f 100644 --- a/.claude/skills/pixellab-cli-animation/SKILL.md +++ b/.claude/skills/pixellab-cli-animation/SKILL.md @@ -25,16 +25,17 @@ character with a hint about one. ``` 0 inspect which character, which poses it already has free - ---- repeat 1-4 once per motion ---- -1 character state -p the character, in the pose that motion needs 20-40 gen +1 character state -p the idle, once, off the base character 20-40 gen + ---- repeat 2-5 once per motion; step 2 is what takes the idle's id ---- +2 character state -p the pose that motion needs 20-40 gen ---- look at the pose before animating it ---- -2 character enrich the motion, written from that pose ~0.05 gen +3 character enrich the motion, written from that pose ~0.05 gen ---- read the description, edit it ---- -3 character animate one direction per call 1 gen per frame +4 character animate one direction per call 1 gen per frame ---- look at the frames ---- -4 image gif watch the loop before building on it free -5 image flip a mirrored direction, where the set wants one free -6 export atlas what the engine loads free +5 image gif watch the loop before building on it free +6 image flip a mirrored direction, where the set wants one free +7 export atlas what the engine loads free ``` This is the shape of a character that came out right, read back off the account — @@ -76,6 +77,28 @@ Two jobs: - **A variant.** `-p "the same goblin in a red outfit"` — armour, a wound, a Christmas hat, a powered-up form. Same face, same proportions, across every direction. +**Every pose is made from the idle, not from the base character.** Make the idle +first, off the character; make every other pose off the **idle state's id**. A state is +a character with its own id, so it is a valid source, and the colours come across from +whatever it was made from — which is the point. The base character is a neutral +rotation nobody plays; the idle is the character as it stands in the game, and one +ancestor for every pose is what makes two of them match when they play in sequence. A +pose off the base and the next off a state is two ancestors and two slightly different +characters: invisible on one sheet, obvious in motion, and each one a Pro call that +cannot be undone. + +```bash +pixellab-cli --yes character state -p "idle, standing at rest" +pixellab-cli --yes character state -p "mid-stride walking pose" +pixellab-cli --yes character state -p "attack windup, blade raised" +``` + +A costume variant is the one case where the source is a judgement rather than a rule: +off the idle if it is going to be animated, off the base character if it is a sheet of +what the outfit looks like. Either way the record files it under the character all of +it descends from, so `inspect` and `character check` see every pose whichever way it +was made. + **The colours come from the character, and that is the default.** A state drifts a few hues otherwise, which is invisible on its own sheet and obvious the moment two states play in sequence — three states, three palettes, three Pro calls. `--new-colors` turns it diff --git a/docs/notes.md b/docs/notes.md index 33a5d80..a5190a0 100644 --- a/docs/notes.md +++ b/docs/notes.md @@ -98,3 +98,4 @@ over this file answers with the example above as well as with the notes. --> - n-0058 2026-09-20 #gotcha @src/pixellab_cli/pixellab.py — a path parameter is refused in _split_path when it holds a separator or a dot segment, so no route has to remember the check - n-0059 2026-09-20 #gotcha @tests/test_skill.py — the skill tests walk a hand-written CATEGORY_SKILLS tuple, so a packaged skill missing from it is tested by nothing; TestNothingShipsUntested binds it to harness.packaged_skills - n-0060 2026-09-20 #gotcha @src/pixellab_cli/catalog.py — frame_default lives in catalog.py and reads the route's own frame_count, because every animating command multiplies it by the directions to estimate a call +- n-0061 2026-09-20 #gotcha @src/pixellab_cli/subject.py — a state can be made from another state, so the subject record walks the source chain to the root character; a pose filed under nothing is a guardrail that stops seeing it diff --git a/plans/every-pose-comes-off-the-idle.md b/plans/every-pose-comes-off-the-idle.md new file mode 100644 index 0000000..c54158f --- /dev/null +++ b/plans/every-pose-comes-off-the-idle.md @@ -0,0 +1,59 @@ +--- +autonomy: auto +ci: wait +--- + +# Every pose comes off the idle + +A new pose is made with `character state` from the **idle state's id**, not from the +base character's, with the palette carried across. The colour half is already the +default and the skill says so; which character a state is made *from* is not said +anywhere, and the numbered order in the animation skill reads as though every state +comes off the base character. + +## Why + +The base character is a neutral rotation nobody plays. The idle is the character as it +stands in the game, and it is the frame every other pose should continue from: one +common ancestor, one palette carried forward from the same frame, so two poses played +in sequence match. A pose made off the base and the next off a state is two ancestors +and two slightly different characters — invisible on one sheet, obvious in motion, and +each one is a Pro call that cannot be undone. + +`character state ` already works and the palette already carries — +`use_color_palette_from_reference` is on unless `--new-colors` turns it off. The +record did not. A state whose source is another state resolved to no character and +fell into `loose`, so `poses_of`, `check_pose_belongs`, `inspect` and `character +check` stopped seeing every pose after the idle: the guardrails went quiet on exactly +the workflow this plan documents. Writing the instruction without fixing that would +have shipped the failure it is meant to prevent. + +## Paths + +- `src/pixellab_cli/skill/pixellab-cli-animation/SKILL.md` — the workflow and the state section +- `.claude/skills/pixellab-cli-animation/SKILL.md` — the installed copy + +## References + +- `specs/agent-skill/` — R5.4, the category skill carrying the workflow in the order its steps are taken +- `specs/characters-and-animation/` — R1.5 and R1.15, what a state is and where its colours come from + +## Out of scope + +- Any change to `character state` itself. The command already does both halves; what + needed fixing is where the record files what it made. +- Refusing a state made off the base character. It is the right thing for the first + pose of a character that has no idle yet, and a refusal would have to know which + state is the idle, which nothing records. + +## Tasks + +- [x] 1.1 (Unit) Say that a pose is made from the idle, in the order and the section +- [x] 1.2 (TDD) File a state made from a state under its root character + _Reason review found the chained pose orphaned in the record_ + +## Done when + +- The animation skill's numbered order makes the idle first and every later pose from its id +- The state section says which character a state is made from, not only what it costs +- A pose made from the idle is reported under the character by `inspect` diff --git a/specs/asset-workspace/design.md b/specs/asset-workspace/design.md index 49ca04f..4b7f152 100644 --- a/specs/asset-workspace/design.md +++ b/specs/asset-workspace/design.md @@ -123,3 +123,25 @@ A free route is one whose estimate is present, reports zero, and is not a guess `Cost(generations=0.0, source="reported")`, which is how the library routes already declare themselves. An absent estimate still gates: not knowing a price is not the same as knowing it is nothing, and it is the case the gate exists for. + +## A pose made from a pose belongs to the same character + +A state is a character with its own identifier, so a state can be made from one — and +it should be: a pose continues the idle rather than the neutral rotation nobody plays. +The record did not follow that. `_state` keeps the identifier the call was made +against, `build` only makes a top-level entry for a character's own rotations, and a +state whose source is another state therefore resolved to nothing and fell into +`loose`. Reproduced before fixing: a base character, an idle off it, a walk off the +idle, and `poses_of` naming only the idle while `owner_of` answered `None` for the walk. + +Everything that protects a paid call reads that structure — `poses_of`, +`check_pose_belongs`, `check_a_pose_was_made_for_it`, `inspect`, `character check` — +so the effect was that the guardrails quietly stopped seeing every pose after the +first, which is the shape of failure this record exists to prevent. + +The source chain is walked to the character at its root, and the state is recorded +there (R7.6). `of` still names what the state was actually made from, because that is +what happened; where it is *filed* is the character all of it descends from. The walk +is computed from every state run before any is attached, so it does not depend on the +order the runs are read in, and a cycle is stopped by the set of identifiers already +visited rather than trusted not to exist. diff --git a/specs/asset-workspace/requirements.md b/specs/asset-workspace/requirements.md index 81d955c..8c14d44 100644 --- a/specs/asset-workspace/requirements.md +++ b/specs/asset-workspace/requirements.md @@ -1,9 +1,9 @@ --- autonomy: auto ci: wait -branch: feat/the-ui-panel-you-paid-for +branch: docs/every-pose-comes-off-the-idle delivery: in-review -pr: 79 +pr: 84 --- # Asset workspace — requirements @@ -70,6 +70,7 @@ A file nobody can account for is worth less than the same file with a manifest. - **R7.3** Where a call is made from an identifier an earlier call produced, the asset workspace shall record that identifier beside the request, because a request carrying a frame does not say which pose the frame came from. - **R7.4** When asked to describe a subject, the asset workspace shall report it without calling a provider, and shall report the subjects it holds where none is named. - **R7.5** (ADDED) The asset workspace shall gather the animation runs of one character made for one motion into a single animation holding every direction they cover and every file they wrote. +- **R7.6** (ADDED) Where a state was made from another state, the asset workspace shall record it under the character that state belongs to, so that every pose of one character is reported together however it was made. ## Out of scope diff --git a/specs/asset-workspace/tasks.md b/specs/asset-workspace/tasks.md index 98d0a40..e83e0c3 100644 --- a/specs/asset-workspace/tasks.md +++ b/specs/asset-workspace/tasks.md @@ -74,3 +74,8 @@ - [x] 8.1 (Unit) Ask for no agreement where the route is known to cost nothing — R6.5 _Reason a free download asked for --yes, and the suite never saw it_ + +## 9 · A pose made from a pose +- [x] 9.1 (TDD) File a state made from a state under its root character — R7.6 + _Reason the skill now chains poses off the idle_ + diff --git a/src/pixellab_cli/skill/pixellab-cli-animation/SKILL.md b/src/pixellab_cli/skill/pixellab-cli-animation/SKILL.md index 397c0cf..417844f 100644 --- a/src/pixellab_cli/skill/pixellab-cli-animation/SKILL.md +++ b/src/pixellab_cli/skill/pixellab-cli-animation/SKILL.md @@ -25,16 +25,17 @@ character with a hint about one. ``` 0 inspect which character, which poses it already has free - ---- repeat 1-4 once per motion ---- -1 character state -p the character, in the pose that motion needs 20-40 gen +1 character state -p the idle, once, off the base character 20-40 gen + ---- repeat 2-5 once per motion; step 2 is what takes the idle's id ---- +2 character state -p the pose that motion needs 20-40 gen ---- look at the pose before animating it ---- -2 character enrich the motion, written from that pose ~0.05 gen +3 character enrich the motion, written from that pose ~0.05 gen ---- read the description, edit it ---- -3 character animate one direction per call 1 gen per frame +4 character animate one direction per call 1 gen per frame ---- look at the frames ---- -4 image gif watch the loop before building on it free -5 image flip a mirrored direction, where the set wants one free -6 export atlas what the engine loads free +5 image gif watch the loop before building on it free +6 image flip a mirrored direction, where the set wants one free +7 export atlas what the engine loads free ``` This is the shape of a character that came out right, read back off the account — @@ -76,6 +77,28 @@ Two jobs: - **A variant.** `-p "the same goblin in a red outfit"` — armour, a wound, a Christmas hat, a powered-up form. Same face, same proportions, across every direction. +**Every pose is made from the idle, not from the base character.** Make the idle +first, off the character; make every other pose off the **idle state's id**. A state is +a character with its own id, so it is a valid source, and the colours come across from +whatever it was made from — which is the point. The base character is a neutral +rotation nobody plays; the idle is the character as it stands in the game, and one +ancestor for every pose is what makes two of them match when they play in sequence. A +pose off the base and the next off a state is two ancestors and two slightly different +characters: invisible on one sheet, obvious in motion, and each one a Pro call that +cannot be undone. + +```bash +pixellab-cli --yes character state -p "idle, standing at rest" +pixellab-cli --yes character state -p "mid-stride walking pose" +pixellab-cli --yes character state -p "attack windup, blade raised" +``` + +A costume variant is the one case where the source is a judgement rather than a rule: +off the idle if it is going to be animated, off the base character if it is a sheet of +what the outfit looks like. Either way the record files it under the character all of +it descends from, so `inspect` and `character check` see every pose whichever way it +was made. + **The colours come from the character, and that is the default.** A state drifts a few hues otherwise, which is invisible on its own sheet and obvious the moment two states play in sequence — three states, three palettes, three Pro calls. `--new-colors` turns it diff --git a/src/pixellab_cli/subject.py b/src/pixellab_cli/subject.py index f10caae..7c8fcac 100644 --- a/src/pixellab_cli/subject.py +++ b/src/pixellab_cli/subject.py @@ -322,6 +322,29 @@ def _join(animations: list[dict[str, Any]], animation: dict[str, Any]) -> None: held["runs"] = held["runs"] + animation["runs"] +def _root_of(state_id: str | None, sources: dict[str, str]) -> str | None: + """The character a state descends from, however many states are in between. + + A state is a character with its own identifier, so a pose is made from the idle + rather than from the neutral rotation nobody plays — and then its source is not a + character this subject holds, it is another state. Walking the chain is what keeps + every pose of one character filed together, which is what `poses_of` and the + refusals built on it read. + + Computed from every state run before any of them is attached, so the answer does + not depend on the order the runs were read in. A chain that comes back to + something already seen is not a character and is left alone rather than followed. + """ + seen: set[str] = set() + current = state_id + while current in sources: + if current in seen: + return None + seen.add(current) + current = sources[current] + return current + + def build(name: str, workspace: Workspace) -> Subject: """Assemble one subject from the runs under it. @@ -345,12 +368,24 @@ def build(name: str, workspace: Workspace) -> Subject: if run.kind == ROTATIONS and run.character_id and not run.is_state: subject.characters.append(_character(run)) + # Every state's source, before any of them is placed: a state made from a state + # has to reach the character at the root of the chain, and the run that made its + # source may be read after it. + sources = { + str(run.ids.get("character_id")): str( + run.ids.get(SOURCE_ID) or run.links.get("character_id") + ) + for run in runs + if run.is_state and run.ids.get("character_id") + } + for run in runs: if run.kind == ROTATIONS and run.character_id and not run.is_state: continue if run.is_state: state = _state(run) - owner = subject.character(str(state["of"])) if state["of"] else None + root = _root_of(str(state["of"]), sources) if state["of"] else None + owner = subject.character(root) if root else None if owner is not None: owner["states"].append(state) continue diff --git a/tests/test_skill.py b/tests/test_skill.py index fad0240..eb3027d 100644 --- a/tests/test_skill.py +++ b/tests/test_skill.py @@ -320,3 +320,21 @@ def test_every_priced_name_is_a_command_somebody_can_type(self): for where, body in self.sources().items(): for name in sorted(self.priced_names(body)): assert name in commands, f"{where} prices {name!r}, which is not a command" + + +class TestAPoseIsMadeFromTheIdle: + """A state is a character with its own id, so which one a pose is made *from* is a + choice nobody was told how to make: the idle, so every pose has one ancestor and + one palette carried forward from the same frame.""" + + def body(self) -> str: + return skill_body("pixellab-cli-animation") + + def test_the_skill_says_the_pose_comes_off_the_idle(self): + assert "idle-state-id" in self.body() + + def test_the_order_makes_the_idle_before_the_poses(self): + body = self.body() + idle = body.index("the idle, once, off the base character") + pose = body.index("the pose that motion needs") + assert idle < pose diff --git a/tests/test_subject.py b/tests/test_subject.py index 91f99f8..48e7772 100644 --- a/tests/test_subject.py +++ b/tests/test_subject.py @@ -419,3 +419,60 @@ def test_a_direction_animated_twice_is_named_once(self, tmp_path): subject = subjects.load(Workspace(tmp_path), "warrior") assert subject.characters[0]["animations"][0]["directions"] == ["south"] + + +class TestAPoseMadeFromAPose: + """R7.6. A state is a character with its own id, so a pose is made from the idle + rather than from the neutral rotation nobody plays. Everything that guards a paid + call reads this structure, so a pose the record cannot place is a guard that + silently stops seeing it.""" + + IDLE = "char-knight-idle" + + def a_chain(self, tmp_path): + home = tmp_path / "warrior" + a_character(home) + a_state(home, version=2, state_id=self.IDLE, of=CHARACTER, pose="idle, at rest") + a_state(home, version=3, state_id=POSE, of=self.IDLE, pose="mid-stride walking pose") + return subjects.load(Workspace(tmp_path), "warrior") + + def test_the_chained_pose_is_filed_under_the_character(self, tmp_path): + subject = self.a_chain(tmp_path) + + assert [state["id"] for state in subject.characters[0]["states"]] == [self.IDLE, POSE] + + def test_it_is_not_left_loose(self, tmp_path): + subject = self.a_chain(tmp_path) + + assert subject.loose == [] + + def test_the_character_owns_the_pose(self, tmp_path): + """`check_pose_belongs` refuses a pose of another character on this one.""" + subject = self.a_chain(tmp_path) + + assert subject.owner_of(POSE) == CHARACTER + + def test_every_pose_is_offered_for_the_motion(self, tmp_path): + """`check_a_pose_was_made_for_it` names these before a paid animation.""" + subject = self.a_chain(tmp_path) + + assert [name for name, _ in subject.poses_of(CHARACTER)] == [self.IDLE, POSE] + + def test_the_state_still_says_what_it_was_made_from(self, tmp_path): + """Filed under the character it descends from; `of` is still what happened.""" + subject = self.a_chain(tmp_path) + + chained = next(s for s in subject.characters[0]["states"] if s["id"] == POSE) + assert chained["of"] == self.IDLE + + def test_a_chain_that_eats_itself_is_not_followed_forever(self, tmp_path): + """Two states naming each other is not a character; it is loose work.""" + home = tmp_path / "warrior" + a_character(home) + a_state(home, version=2, state_id="a-state", of="b-state", pose="one") + a_state(home, version=3, state_id="b-state", of="a-state", pose="two") + + subject = subjects.load(Workspace(tmp_path), "warrior") + + assert subject.characters[0]["states"] == [] + assert len(subject.loose) == 2