diff --git a/.claude/skills/pixellab-cli-scenes/SKILL.md b/.claude/skills/pixellab-cli-scenes/SKILL.md index cf1c1fa..fc9184d 100644 --- a/.claude/skills/pixellab-cli-scenes/SKILL.md +++ b/.claude/skills/pixellab-cli-scenes/SKILL.md @@ -86,12 +86,49 @@ so it does not read as pasted from another game. ``` pixellab-cli object new --directions --size --view --style --reference --name pixellab-cli object list +pixellab-cli object show ``` **Pro pricing.** A managed prop with its own id, in one direction or eight. Reach for it when the prop has to be seen from several angles or referred to later; a prop that only has to look right from one angle is `pixellab-cli sprite` for about one generation. +`object show` is free and prints, for each animation the prop has, the identifier that +adds directions to it. + +## A prop that moves, one direction at a time + +``` +pixellab-cli object animate --action/-a --direction/-d --into --frames + --name --enhance --drop-first-frame --again +``` + +**A generation per frame per direction.** The default is eight frames, so one direction +is about eight generations and all eight are about sixty-four. The command names the +directions and the total before it calls; read that line rather than the flags. + +**This is the one animation you can come back to.** A character's animation cannot take +another direction — the provider refuses the field — but an object's can: + +```bash +pixellab-cli --yes object animate -a "the barrel rocks on its base and settles back" +pixellab-cli object show # the animation_group_id is on the line +pixellab-cli --yes object animate --into +``` + +The second call adds **only the directions that animation does not have yet**, inherits +its description, and needs no `--action`. Naming none is the usual way to run it: the +missing set is worked out from the object, and the cost of exactly those directions is +said before anything is sent. `--again` regenerates one it already holds, which is the +only way past the refusal. + +Animate one direction first and look at it. A rocking barrel that reads wrong in south +reads wrong in eight directions, and finding that out costs eight generations instead of +sixty-four. + +A one-direction object animates the direction it has; `--direction` on one is refused, +because the route answers `400` to it. + ## Assembling a map is not a generation problem This tool makes tiles and props. Painting them into a level, auto-tiling while you paint, diff --git a/docs/notes.md b/docs/notes.md index 67343c0..33a5d80 100644 --- a/docs/notes.md +++ b/docs/notes.md @@ -97,3 +97,4 @@ over this file answers with the example above as well as with the notes. --> - n-0057 2026-09-20 #gotcha @src/pixellab_cli/run.py — approve gates on the estimate, not the route, so a reported zero passes and an absent estimate still asks - 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 diff --git a/specs/characters-and-animation/design.md b/specs/characters-and-animation/design.md index 6694e84..cea6677 100644 --- a/specs/characters-and-animation/design.md +++ b/specs/characters-and-animation/design.md @@ -375,3 +375,37 @@ holds behind it, and the subject record folds the runs the same way, so an anima built over two calls reaches `inspect` — and the atlas built from it — as one animation over every direction it covers. The grouping the provider refuses is real everywhere a game engine can see it. + +## An object animation is the one the provider lets you extend + +Everything the character route refuses, the object route offers. `POST +/v2/objects/{object_id}/animations` takes `animation_group_id` — *"pass the +animation_group_id of an existing animation on this object to add more directions to +it"* — returns it on the first call, inherits the description when extending, and +refuses a direction it already holds with `409` unless `replace_existing` says +otherwise. It even computes the missing set: with a group and no `directions`, the +server fills in the cardinals that are not there yet. + +So `object animate` is the shape `character animate` could not be. `--into +` adds to an animation that exists; without it a new one starts and +its identifier is reported, because that identifier is what the next call extends. +`--again` is `replace_existing`, the same word the character side uses for the same +idea. + +The CLI still reads the object first, and that read is free. It is what turns "omit the +directions and let the server decide" into a cost said out loud: `GET /v2/objects/{id}` +names every direction each animation already covers, so the directions this call will +actually generate are known before it is made, and so is the bill — one generation per +frame per direction, which for a default eight-frame animation over eight directions is +sixty-four. Nobody should meet that number on the invoice (R2.43). The same read is what +refuses a direction already animated before the provider's `409` costs a round trip +(R2.44). + +`object show` exists for the same reason (R4.6): the identifier `--into` takes is issued +by the provider and reported nowhere else, so without it the extending flow has no way +to start. It lists the rotations, then each animation with its group identifier, its +description and the directions it holds. + +Only `mode='v3'` is reachable. `pro` is twenty to forty generations *per direction* — +up to three hundred and twenty for eight — and nothing here needs its different look +enough to put that behind a flag that could be typed by accident. diff --git a/specs/characters-and-animation/requirements.md b/specs/characters-and-animation/requirements.md index 9ac146f..8003ecd 100644 --- a/specs/characters-and-animation/requirements.md +++ b/specs/characters-and-animation/requirements.md @@ -1,9 +1,9 @@ --- autonomy: auto ci: wait -branch: feat/one-animation-many-directions +branch: feat/an-object-animation-takes-more-directions delivery: in-review -pr: 78 +pr: 82 --- # Characters and animation — requirements @@ -85,6 +85,11 @@ different endpoint family and no skeleton. - **R2.39** (ADDED) If a direction asked for is already animated on that character for the motion being animated, then the character commands shall refuse the call, name the animation and the directions it already holds, and say that the provider starts a new animation rather than adding a direction to one, before spending anything, unless the caller asks for it to be generated again. - **R2.40** (ADDED) Where a direction is animated for a motion the character already has, the character commands shall report that the frames join that motion in the workspace and not in the animation the provider holds. +- **R2.41** (ADDED) When asked to animate an object, the character commands shall animate it from an action description over the directions asked for. +- **R2.42** (ADDED) Where an animation of that object is named, the character commands shall add to that animation the directions it does not yet hold, rather than starting a second one. +- **R2.43** (ADDED) The character commands shall report, before an object is animated, which directions the call will cover and what they are estimated to cost. +- **R2.44** (ADDED) If a direction asked for is already animated in the animation named, then the character commands shall refuse the call before spending anything, unless the caller asks for it to be generated again. + ## R3 · Rotating @@ -97,6 +102,7 @@ different endpoint family and no skeleton. - **R4.3** When asked for a spritesheet, the character commands shall download the sheet and its layout file and write both to the workspace. - **R4.4** Where a command names an asset that does not exist, the character commands shall say so rather than reporting an empty result. - **R4.5** (ADDED) When showing a character, the character commands shall present the animations it holds for one motion as a single animation over every direction they cover, and shall say how many the provider holds for it. +- **R4.6** (ADDED) The character commands shall show one object's rotations and animations, naming for each animation the identifier that adds directions to it. ## Out of scope diff --git a/specs/characters-and-animation/tasks.md b/specs/characters-and-animation/tasks.md index d8bff1e..aea263f 100644 --- a/specs/characters-and-animation/tasks.md +++ b/specs/characters-and-animation/tasks.md @@ -133,3 +133,19 @@ - [x] 7.4 (Unit) Teach the skill that one call carries every direction — R2.40 _Depends 7.2_ _Reason the provider refuses animation_group_id for characters_ + +## 8 · Animating an object +- [x] 8.1 (Unit) Add the object animation route to the catalogue — R2.41 + _Reason the object route is the one that lets an animation take more directions_ +- [x] 8.2 (Unit) Show an object's rotations and animations with their group ids — R4.6 + _Reason the object route is the one that lets an animation take more directions_ +- [x] 8.3 (Unit) Animate an object, naming directions and cost — R2.41, R2.43 + _Depends 8.1_ + _Reason the object route is the one that lets an animation take more directions_ +- [x] 8.4 (Unit) Add the directions an animation lacks — R2.42, R2.44 + _Depends 8.3_ + _Reason the object route lets an animation take more directions_ +- [x] 8.5 (Unit) Teach the scenes skill to animate an object and extend it — R2.42 + _Depends 8.4_ + _Reason the object route lets an animation take more directions_ + diff --git a/specs/provider-core/design.md b/specs/provider-core/design.md index 750f167..e9f26ff 100644 --- a/specs/provider-core/design.md +++ b/specs/provider-core/design.md @@ -181,3 +181,27 @@ declared length where there is one and by what arrived where there is not (R3.7) Nothing narrower than a size limit is attempted — PixelLab serves assets from more than one host and a host list would fail closed on the day they add another, which for a tool whose whole job is to collect what was paid for is the more expensive failure. + +## Job identifiers inside a list of objects + +`characters/animations` returns `background_job_ids`, a flat list, and `_poll_id` takes +the first. `objects/{object_id}/animations` returns `submissions`: one object per +direction, each carrying its own `background_job_id` and `status`. The rule is the same +— follow the first, and the rest stay in the raw payload where the record keeps them — +so `_poll_id` reads the identifier out of the first entry when the list holds objects +rather than strings (R4.10). A route's own shape is the provider's business; which of +the identifiers is followed is not, and it is one rule for both. + +## The identifier a response chose is still part of a URL + +`_split_path` refuses a path parameter that carries a separator or a dot segment, +because the request built from it carries the bearer token. The job identifier a route +hands back is interpolated into the poll path by the very next request, which carries +the same token — so it is the same problem with the value arriving from the other +direction, and `collect` already said so for the one a caller types. + +One helper now guards both, and the poll loop calls it before every attempt: a +`background_job_id` of `../../v2/characters` is refused rather than followed. That +covers `submissions`, where the object route puts its identifiers, and the flat +`background_job_ids` a character animation returns, which had the same gap one layer +shallower. diff --git a/specs/provider-core/requirements.md b/specs/provider-core/requirements.md index 650bad9..5cafb84 100644 --- a/specs/provider-core/requirements.md +++ b/specs/provider-core/requirements.md @@ -1,9 +1,9 @@ --- autonomy: auto ci: wait -branch: feat/the-ui-panel-you-paid-for +branch: feat/an-object-animation-takes-more-directions delivery: in-review -pr: 79 +pr: 82 --- # Provider core — requirements @@ -47,7 +47,7 @@ looks like have to be decided once. - **R3.3** If a caller supplies a parameter the route does not accept, a value outside an enumerated parameter's set, or a size the route rejects, then the provider core shall fail before the request is sent, naming the parameter and the values or bounds that are allowed. - **R3.4** The route table shall be derived from the vendored provider schemas under `reference/`, and a route whose shape contradicts those schemas shall be reported by the suite rather than at call time. - **R3.5** (ADDED) Where a route requires an image to match one of its size parameters, if the image given does not match it, then the provider core shall fail before the request is sent, naming both sizes. -- **R3.6** (ADDED) If a value that becomes part of a route's address carries a path separator or a dot segment, then the provider core shall refuse the call before anything is sent, because the request that would carry it carries the account's credential. +- **R3.6** (MODIFIED) If a value that becomes part of a route's address carries a path separator or a dot segment, then the provider core shall refuse the call before anything is sent, whether that value came from the caller or from a provider's own response, because the request that would carry it carries the account's credential. - **R3.7** (ADDED) If a download named by a provider's response is larger than the ceiling this tool holds for a generated asset, then the provider core shall refuse it rather than read it. ## R4 · Background jobs @@ -61,6 +61,7 @@ looks like have to be decided once. - **R4.8** (ADDED) If polling stops before a job resolves, then the provider core shall record that call as still running and shall record no cost for it, because a call that has not finished has not reported one. - **R4.7** (MODIFIED) Where a response declares how many images it holds and fewer were found, the provider core shall take them from a field it has seen carry them, and shall leave the count alone otherwise. - **R4.9** (ADDED) Where a completed job reports the address of the image it made rather than its bytes, the provider core shall fetch that address and return the image, because the call has been charged and its result is otherwise never collected. +- **R4.10** (ADDED) Where a route reports its job identifiers inside a list of objects, one per direction, the provider core shall poll the first of them. ## R5 · Results and cost diff --git a/specs/provider-core/tasks.md b/specs/provider-core/tasks.md index 98c1ea4..cc03238 100644 --- a/specs/provider-core/tasks.md +++ b/specs/provider-core/tasks.md @@ -71,3 +71,12 @@ - [x] 7.3 (TDD) Refuse a download past the ceiling for a generated asset — R3.7 _Reason the address is now fetched unasked_ + +## 8 · Job identifiers inside a list of objects +- [x] 8.1 (Unit) Poll the first job id when the list holds objects — R4.10 + _Reason the object animation route returns submissions_ +- [x] 8.2 (Unit) Check each item of an enumerated list, not the list itself — R3.3 + _Reason the first list parameter with choices refused every value_ +- [x] 8.3 (Unit) Guard the job id a response chose, not only a typed one — R3.6 + _Reason it reached the poll path unchecked_ + diff --git a/src/pixellab_cli/catalog.py b/src/pixellab_cli/catalog.py index 1441865..d73c668 100644 --- a/src/pixellab_cli/catalog.py +++ b/src/pixellab_cli/catalog.py @@ -1166,6 +1166,35 @@ def _image( ) +ANIMATE_OBJECT = Route( + name="object-animations", + method="POST", + path="/objects/{object_id}/animations", + kind=RouteKind.BACKGROUND_JOB, + summary="Animate an object, or add directions to an animation it already has.", + estimated_generations=1.0, + result_id_field="submissions", + poll_path=BACKGROUND_JOBS_PATH, + asset_id_field="animation_group_id", + params=( + Param("object_id", ParamKind.STRING, required=True), + Param("animation_description", ParamKind.STRING, help="The motion, described."), + Param( + "animation_group_id", + ParamKind.STRING, + help="An animation of this object to add directions to.", + ), + Param("directions", ParamKind.STRING_LIST, choices=DIRECTION), + Param("display_name", ParamKind.STRING), + Param("frame_count", ParamKind.INTEGER, minimum=4, maximum=16, default=8), + Param("mode", ParamKind.STRING, choices=("v3", "pro"), default="v3"), + Param("enhance_prompt", ParamKind.BOOLEAN, default=False), + Param("keep_first_frame", ParamKind.BOOLEAN, default=True), + Param("replace_existing", ParamKind.BOOLEAN, default=False), + ), + path_params=("object_id",), +) + LIST_UI_ASSETS = Route( name="ui-assets", method="GET", @@ -1230,6 +1259,7 @@ def _image( CHARACTER_SPRITESHEET, LIST_OBJECTS, GET_OBJECT, + ANIMATE_OBJECT, LIST_UI_ASSETS, GET_UI_ASSET, ) @@ -1237,6 +1267,19 @@ def _image( BY_NAME: dict[str, Route] = {route.name: route for route in ROUTES} +def frame_default(route: Route) -> int: + """How many frames a route draws when nobody says. + + Read off the route's own `frame_count` rather than repeated per command: every + animating command multiplies this by the number of directions to estimate what a + call will cost, and two copies of it drift the day one route's default changes. + """ + for param in route.params: + if param.name == "frame_count": + return int(param.default or 8) + return 8 + + def route(name: str) -> Route: """Look a route up by name, or say what the names are.""" try: diff --git a/src/pixellab_cli/commands/character.py b/src/pixellab_cli/commands/character.py index 8fe4e2a..bce2218 100644 --- a/src/pixellab_cli/commands/character.py +++ b/src/pixellab_cli/commands/character.py @@ -319,14 +319,6 @@ def check_pose_belongs( ) -def _frame_default(route) -> int: - """How many frames the route draws when nobody says. The estimate depends on it.""" - for param in route.params: - if param.name == "frame_count": - return int(param.default or 8) - return 8 - - def known_templates() -> set[str]: catalogue = load_templates() return {name for ids in catalogue.get("families", {}).values() for name in ids} @@ -1046,7 +1038,7 @@ def _animate( # template drives the skeleton instead, and stays on the route's tier. per_direction = route.estimated_generations if not template: - per_direction = float(frames or _frame_default(route)) + per_direction = float(frames or catalog.frame_default(route)) estimate = Cost(generations=per_direction * len(wanted)) output.stderr( f"{len(wanted)} direction(s), one job each — about " @@ -1058,7 +1050,7 @@ def _animate( # n-0023). Said before the call because the difference is otherwise discovered in # the file names, and a frame count is usually chosen to fit a loop. if not template: - generated = int(frames or _frame_default(route)) + generated = int(frames or catalog.frame_default(route)) stored = generated if drop_first_frame else generated + 1 output.stderr( f"{generated} frame(s) generated per direction, {stored} held" diff --git a/src/pixellab_cli/commands/prop.py b/src/pixellab_cli/commands/prop.py index 67e6e20..6b90749 100644 --- a/src/pixellab_cli/commands/prop.py +++ b/src/pixellab_cli/commands/prop.py @@ -135,6 +135,274 @@ def _list(context) -> None: output.emit(payload, lines, as_json=app_context.as_json) +def _read_object(app_context, object_id: str) -> dict[str, Any]: + """The object as the account holds it. Free, and every refusal below reads it.""" + payload = app_context.pixellab().call("object", object_id=object_id).raw + if not payload: + raise ValidationError(f"no object {object_id!r} on this account") + return payload + + +def _group(payload: dict[str, Any], group_id: str) -> dict[str, Any]: + """The animation `--into` names, or a refusal listing the ones that exist.""" + for animation in animations_of(payload): + if animation["animation_group_id"] == group_id: + return animation + held = [ + f"{animation['animation_group_id']} ({animation['name']})" + for animation in animations_of(payload) + ] + raise ValidationError( + f"this object has no animation {group_id!r}. " + + (f"It has: {', '.join(held)}." if held else "It has none yet."), + context={"animation_group_id": group_id}, + ) + + +def _directions_to_animate( + payload: dict[str, Any], held: dict[str, Any] | None, wanted: list[str], again: bool +) -> list[str]: + """Which directions this call will actually generate, named before it is made. + + The route would work the missing set out on its own — with a group and no + directions it fills in the cardinals that are not there yet — but then the number + of directions, and so the bill, would only be knowable afterwards. Reading the + object is free, so it is known first. + """ + rotations = [ + name for name in EIGHT_DIRECTION_ORDER if (payload.get("rotation_urls") or {}).get(name) + ] + covered = held["directions"] if held else [] + + if not wanted: + chosen = [name for name in rotations if name not in covered] if held else ["south"] + if not chosen: + raise ValidationError( + f"{held['name']!r} already covers every direction this object has: " + f"{', '.join(covered)}. Name one with --direction and --again to " + "generate it a second time.", + context={"directions": covered}, + ) + return chosen + + unknown = sorted(set(wanted) - set(EIGHT_DIRECTION_ORDER)) + if unknown: + raise ValidationError( + f"not a direction: {', '.join(unknown)}. The directions are: " + f"{', '.join(EIGHT_DIRECTION_ORDER)}" + ) + repeated = [name for name in wanted if name in covered] + if repeated and not again: + raise ValidationError( + f"{', '.join(repeated)} is already animated in {held['name']!r}, which holds " + f"{', '.join(covered)}. The route refuses it too, one round trip later. " + "Pass --again to generate it a second time.", + context={"repeated": repeated, "held": covered}, + ) + return list(wanted) + + +@app.command("animate") +def animate( + context: typer.Context, + object_id: str = typer.Argument(..., help="The object to animate."), + action: str = typer.Option(None, "--action", "-a", help="The motion, described."), + directions: list[str] = typer.Option( + None, + "--direction", + "-d", + help="Repeatable. Defaults to south, or to what the animation lacks.", + ), + into: str = typer.Option( + None, "--into", help="An animation of this object to add directions to. `object show`." + ), + frames: int = typer.Option(None, "--frames", help="Four to sixteen, and even."), + name: str = typer.Option(None, "--name", help="What to call the animation."), + enhance: bool = typer.Option( + False, "--enhance", help="Let the provider expand the action inside the call." + ), + drop_first_frame: bool = typer.Option( + False, "--drop-first-frame", help="Store only the frames generated." + ), + again: bool = typer.Option( + False, "--again", help="Animate a direction this animation already has." + ), +) -> None: + """Animate an object. Every direction is a separate job and a separate charge.""" + try: + _animate( + context, + object_id, + action, + directions, + into, + frames, + name, + enhance, + drop_first_frame, + again, + ) + except PixellabCliError as failure: + output.handle(failure) + + +def _animate( + context, object_id, action, directions, into, frames, name, enhance, drop_first_frame, again +) -> None: + app_context: AppContext = context.obj + route = catalog.route("object-animations") + + if not action and not into: + raise ValidationError( + "give --action, or --into to add directions to an animation that exists" + ) + + payload = _read_object(app_context, object_id) + held = _group(payload, into) if into else None + + # A one-direction object animates the only direction it has, and the route answers + # 400 to a `directions` it was given anyway. + single = int(payload.get("directions") or 0) == 1 + if single and directions: + raise ValidationError( + "a one-direction object animates the only direction it has: drop --direction", + context={"directions": list(directions)}, + ) + if single and held and held["directions"] and not again: + # The one direction it has is the one that animation covers, so this call adds + # nothing. The route refuses it too, one round trip later. + raise ValidationError( + f"{held['name']!r} already covers the only direction this object has. " + "Pass --again to generate it a second time.", + context={"held": held["directions"]}, + ) + wanted = [] if single else _directions_to_animate(payload, held, list(directions or []), again) + + arguments: dict[str, Any] = { + "object_id": object_id, + "animation_description": action, + "animation_group_id": into, + "directions": None if single else wanted, + "display_name": name, + "frame_count": frames, + "enhance_prompt": True if enhance else None, + "keep_first_frame": False if drop_first_frame else None, + "replace_existing": True if again else None, + } + body = build_request(route, arguments) + + covered = 1 if single else len(wanted) + generated = int(frames or catalog.frame_default(route)) + estimate = Cost(generations=float(generated) * covered) + output.stderr( + ("the single direction it has" if single else ", ".join(wanted)) + + f" — {covered} job(s), about {estimate.generations:g} generations in total." + ) + if held: + output.stderr( + f"adding to {held['name']!r}, which holds {', '.join(held['directions']) or 'nothing'}." + ) + output.stderr( + f"{generated} frame(s) generated per direction, " + f"{generated if drop_first_frame else generated + 1} held" + + ("" if drop_first_frame else " — the frame it starts on is kept as frame 0") + + "." + ) + + if app_context.dry_run: + output.emit( + output.dry_run_payload("pixellab", route.name, body, estimate), + output.describe_dry_run("pixellab", route.name, estimate), + as_json=app_context.as_json, + ) + return + + client = app_context.pixellab() + outcome = app_context.runner.run( + subject=app_context.subject, + kind="animations", + description=f"{object_id} {action or (held['name'] if held else '')}".strip(), + provider="pixellab", + route=route.name, + arguments=body, + call=lambda: client.call(route.name, **arguments), + translate=from_pixellab, + estimate=estimate, + name=name or action or (held["name"] if held else object_id), + roles=None if single else list(wanted), + links={"object_id": object_id, "animation_group_id": into, "directions": wanted}, + ) + output.emit( + output.run_payload(outcome), + [f"route: {route.name}", *output.describe_run(outcome)], + as_json=app_context.as_json, + ) + + +def animations_of(payload: dict[str, Any]) -> list[dict[str, Any]]: + """Every animation an object holds, with the directions each already covers. + + `directions` holds an object per direction — its name, when it was made and where + its frames are — rather than a list of names, the same shape a character's + animations come back in. + """ + gathered = [] + for animation in payload.get("animations") or []: + if not isinstance(animation, dict): + continue + covered = [ + entry.get("direction", "?") + for entry in animation.get("directions") or [] + if isinstance(entry, dict) + ] + gathered.append( + { + "animation_group_id": animation.get("animation_group_id"), + "name": animation.get("display_name") or animation.get("description") or "unnamed", + "directions": covered, + "frame_count": animation.get("frame_count"), + } + ) + return gathered + + +@app.command("show") +def show( + context: typer.Context, + object_id: str = typer.Argument(..., help="The object to describe."), +) -> None: + """One object: its rotations, its animations, and what extends each. Free.""" + try: + _show(context, object_id) + except PixellabCliError as failure: + output.handle(failure) + + +def _show(context, object_id) -> None: + app_context: AppContext = context.obj + payload = app_context.pixellab().call("object", object_id=object_id).raw + if not payload: + raise ValidationError(f"no object {object_id!r} on this account") + + rotations = [ + name for name in EIGHT_DIRECTION_ORDER if (payload.get("rotation_urls") or {}).get(name) + ] + animations = animations_of(payload) + lines = [ + f"{payload.get('name') or payload.get('prompt', object_id)} ({object_id})", + f"rotations: {', '.join(rotations) or 'none'}", + f"animations: {len(animations)}", + ] + for animation in animations: + # The group id first: it is what `object animate --into` takes, and the only + # place it is reported at all. + lines.append( + f" {animation['animation_group_id'] or '?'} {animation['name']} " + f"{', '.join(animation['directions']) or 'none'}" + ) + output.emit(payload, lines, as_json=app_context.as_json) + + def _load(path: Path): if not path.is_file(): raise ValidationError(f"{path} is not a file", context={"path": str(path)}) diff --git a/src/pixellab_cli/pixellab.py b/src/pixellab_cli/pixellab.py index 1edf33c..708749d 100644 --- a/src/pixellab_cli/pixellab.py +++ b/src/pixellab_cli/pixellab.py @@ -340,6 +340,7 @@ def _poll(self, job_id: str, poll_path: str, subject: str) -> dict[str, Any]: that collected a job by id dropped the identifiers the other kept, and said nothing about the call having been charged. """ + _refuse_a_path_of_its_own("job", job_id, subject) path = poll_path.replace("{id}", job_id) waited = 0.0 while True: @@ -450,7 +451,14 @@ def _poll_id(route: Route, payload: dict[str, Any]) -> str | None: # `characters/animations` returns one job per direction; the first is the one to # follow, and the rest are recorded in the raw payload. if isinstance(value, list) and value: - return str(value[0]) + first = value[0] + # `objects/{id}/animations` reports the same thing one layer in: `submissions` + # holds an object per direction, each with its own job. Which identifier is + # followed is this module's rule, not the route's shape, so it is one rule. + if isinstance(first, dict): + nested = first.get("background_job_id") + return str(nested) if isinstance(nested, str) else None + return str(first) return None @@ -462,6 +470,22 @@ def _poll_id(route: Route, payload: dict[str, Any]) -> str | None: PATH_SEPARATORS = ("/", "\\") +def _refuse_a_path_of_its_own(name: str, value: str, route: str) -> None: + """Refuse a value that would take the URL somewhere the route did not name. + + Used for both halves of the same problem: a path parameter the caller supplied, + and the job identifier the *provider* supplied, which is interpolated into the + poll path by the very next request — and that request carries the bearer token. + A response is not more trustworthy than an argument here; it is the same URL. + """ + if any(mark in value for mark in PATH_SEPARATORS) or ".." in value: + raise ValidationError( + f"{name} is part of the address this is sent to, so it carries no path " + f"of its own: {value!r}", + context={"route": route, name: value}, + ) + + def _split_path(route: Route, body: dict[str, Any]) -> tuple[str, dict[str, Any]]: """Move the path parameters out of the body and into the URL.""" if not route.path_params: @@ -469,13 +493,7 @@ def _split_path(route: Route, body: dict[str, Any]) -> tuple[str, dict[str, Any] remaining = dict(body) values = {name: remaining.pop(name, "") for name in route.path_params} for name, value in values.items(): - text = str(value) - if any(mark in text for mark in PATH_SEPARATORS) or ".." in text: - raise ValidationError( - f"{name} is part of the address this is sent to, so it carries no " - f"path of its own: {value!r}", - context={"route": route.name, name: value}, - ) + _refuse_a_path_of_its_own(name, str(value), route.name) return route.path.format(**values), remaining diff --git a/src/pixellab_cli/skill/pixellab-cli-scenes/SKILL.md b/src/pixellab_cli/skill/pixellab-cli-scenes/SKILL.md index cf1c1fa..fc9184d 100644 --- a/src/pixellab_cli/skill/pixellab-cli-scenes/SKILL.md +++ b/src/pixellab_cli/skill/pixellab-cli-scenes/SKILL.md @@ -86,12 +86,49 @@ so it does not read as pasted from another game. ``` pixellab-cli object new --directions --size --view --style --reference --name pixellab-cli object list +pixellab-cli object show ``` **Pro pricing.** A managed prop with its own id, in one direction or eight. Reach for it when the prop has to be seen from several angles or referred to later; a prop that only has to look right from one angle is `pixellab-cli sprite` for about one generation. +`object show` is free and prints, for each animation the prop has, the identifier that +adds directions to it. + +## A prop that moves, one direction at a time + +``` +pixellab-cli object animate --action/-a --direction/-d --into --frames + --name --enhance --drop-first-frame --again +``` + +**A generation per frame per direction.** The default is eight frames, so one direction +is about eight generations and all eight are about sixty-four. The command names the +directions and the total before it calls; read that line rather than the flags. + +**This is the one animation you can come back to.** A character's animation cannot take +another direction — the provider refuses the field — but an object's can: + +```bash +pixellab-cli --yes object animate -a "the barrel rocks on its base and settles back" +pixellab-cli object show # the animation_group_id is on the line +pixellab-cli --yes object animate --into +``` + +The second call adds **only the directions that animation does not have yet**, inherits +its description, and needs no `--action`. Naming none is the usual way to run it: the +missing set is worked out from the object, and the cost of exactly those directions is +said before anything is sent. `--again` regenerates one it already holds, which is the +only way past the refusal. + +Animate one direction first and look at it. A rocking barrel that reads wrong in south +reads wrong in eight directions, and finding that out costs eight generations instead of +sixty-four. + +A one-direction object animates the direction it has; `--direction` on one is refused, +because the route answers `400` to it. + ## Assembling a map is not a generation problem This tool makes tiles and props. Painting them into a level, auto-tiling while you paint, diff --git a/src/pixellab_cli/validate.py b/src/pixellab_cli/validate.py index 568f231..5b11f29 100644 --- a/src/pixellab_cli/validate.py +++ b/src/pixellab_cli/validate.py @@ -112,6 +112,12 @@ def _require(condition: bool, route: Route, param: Param, value: Any, expected: def _check_choice(route: Route, param: Param, value: Any) -> None: + # A list parameter enumerates what each of its items may be, not what the list as + # a whole may be: `directions=["north"]` is eight allowed values, one at a time. + if isinstance(value, (list, tuple)): + for item in value: + _check_choice(route, param, item) + return if value in (param.choices or ()): return allowed = ", ".join(repr(choice) for choice in param.choices or ()) diff --git a/tests/test_catalog.py b/tests/test_catalog.py index 1d434fb..6779bad 100644 --- a/tests/test_catalog.py +++ b/tests/test_catalog.py @@ -7,6 +7,7 @@ """ import json +import re from pathlib import Path import pytest @@ -64,9 +65,16 @@ def test_every_parameter_exists_on_the_endpoint(self, subject): properties, _ = request_properties(subject.path, subject.method) if not properties: pytest.skip("no request body") - unknown = sorted(subject.param_names - set(properties)) + # A path parameter is declared like any other and then taken back out of the + # body to build the URL, so it is not a property of the request and must not + # be looked for among them. + unknown = sorted(subject.param_names - set(properties) - set(subject.path_params)) assert not unknown, f"{subject.name}: not on the endpoint: {unknown}" + def test_every_path_parameter_is_one_the_path_names(self, subject): + named = set(re.findall(r"{([^}]+)}", subject.path)) + assert set(subject.path_params) <= named, f"{subject.name}: not in the path" + def test_every_required_parameter_of_the_endpoint_is_required_here(self, subject): properties, required = request_properties(subject.path, subject.method) if not properties: diff --git a/tests/test_commands_character.py b/tests/test_commands_character.py index 2965e38..7ef1f29 100644 --- a/tests/test_commands_character.py +++ b/tests/test_commands_character.py @@ -3495,3 +3495,315 @@ def test_a_direction_carrying_one_is_taken_out_too(self): } assert gather_animations(payload)[0]["directions"] == ["south[A"] + + +def an_object(object_id="obj-1", animations=None, directions=DIRECTIONS): + """The shape `GET /objects/{id}` returns, per `ObjectDetail` in the schema.""" + return { + "id": object_id, + "name": "a wooden barrel", + "prompt": "a wooden barrel", + "size": {"width": 64, "height": 64}, + "directions": len(directions), + "created_at": "2026-09-20T20:40:08Z", + "rotation_urls": rotation_urls(directions), + "animations": animations or [], + } + + +def an_object_animation(group="group-1", name="it rocks", directions=("south",), frames=9): + return { + "animation_group_id": group, + "description": name, + "display_name": None, + "frame_count": frames, + "directions": [ + {"direction": d, "created_at": "2026-09-20T20:40:08Z", "storage_urls": []} + for d in directions + ], + } + + +class TestShowingAnObject: + """R4.6. The group identifier is issued by the provider and reported nowhere else, + so without this there is no way to name the animation that gets more directions.""" + + @respx.mock + def test_the_rotations_and_the_animations_are_named(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond( + json=an_object(animations=[an_object_animation(directions=("south", "east"))]) + ) + + result = invoke(["object", "show", "obj-1"], tmp_path, monkeypatch) + + assert result.exit_code == 0 + assert "south, east" in result.output + assert "it rocks" in result.output + + @respx.mock + def test_the_group_identifier_is_what_gets_printed(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond( + json=an_object(animations=[an_object_animation(group="a0d7cc33")]) + ) + + result = invoke(["object", "show", "obj-1"], tmp_path, monkeypatch) + + assert "a0d7cc33" in result.output + + @respx.mock + def test_an_object_with_no_animations_says_none(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond(json=an_object()) + + result = invoke(["object", "show", "obj-1"], tmp_path, monkeypatch) + + assert "animations: 0" in result.output + + @respx.mock + def test_an_object_that_is_not_there_is_said_so(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond(json={}) + + result = invoke(["object", "show", "obj-1"], tmp_path, monkeypatch) + + assert result.exit_code != 0 + assert "no object" in result.output + + +ROCKING = "the barrel rocks on its base, tipping a little and settling back" + + +def object_animation_response(group="group-1", directions=("south",)): + """`AnimateObjectResponse`: one submission per direction, each with its own job.""" + return { + "animation_group_id": group, + "object_id": "obj-1", + "description": ROCKING, + "mode": "v3", + "frame_count": 8, + "submissions": [ + {"direction": name, "status": "processing", "background_job_id": f"job-{name}"} + for name in directions + ], + } + + +def mock_object_animation(directions=("south",), group="group-1"): + route = respx.post(f"{PIXELLAB_BASE_URL}/objects/obj-1/animations").respond( + json=object_animation_response(group=group, directions=directions) + ) + for name in directions: + respx.get(f"{PIXELLAB_BASE_URL}/background-jobs/job-{name}").respond( + json={"status": "completed", "last_response": {"images": [image_payload()]}} + ) + return route + + +class TestAnimatingAnObject: + """R2.41 and R2.43. Eight frames over eight directions is sixty-four generations, + so which directions and what they cost are said before the call, not after.""" + + @respx.mock + def test_the_action_and_the_direction_reach_the_route(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond(json=an_object()) + route = mock_object_animation() + + result = invoke(["object", "animate", "obj-1", "-a", ROCKING], tmp_path, monkeypatch) + + assert result.exit_code == 0 + sent = json.loads(route.calls.last.request.content) + assert sent["animation_description"] == ROCKING + assert sent["directions"] == ["south"] + + @respx.mock + def test_the_directions_and_the_cost_are_named_before_the_call(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond(json=an_object()) + mock_object_animation(directions=("south", "east")) + + result = invoke( + ["object", "animate", "obj-1", "-a", ROCKING, "-d", "south", "-d", "east"], + tmp_path, + monkeypatch, + ) + + assert "south, east" in result.output + assert "16 generations" in result.output + + @respx.mock + def test_neither_an_action_nor_an_animation_is_refused(self, tmp_path, monkeypatch): + result = invoke(["object", "animate", "obj-1"], tmp_path, monkeypatch) + + assert result.exit_code == 2 + assert "--action" in result.output + + @respx.mock + def test_a_one_direction_object_sends_no_directions(self, tmp_path, monkeypatch): + """The route answers 400 to a `directions` on a one-direction object.""" + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond( + json=an_object(directions=("south",)) | {"directions": 1} + ) + route = mock_object_animation() + + result = invoke(["object", "animate", "obj-1", "-a", ROCKING], tmp_path, monkeypatch) + + assert result.exit_code == 0 + assert "directions" not in json.loads(route.calls.last.request.content) + + @respx.mock + def test_a_direction_on_a_one_direction_object_is_refused(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond( + json=an_object(directions=("south",)) | {"directions": 1} + ) + route = respx.post(f"{PIXELLAB_BASE_URL}/objects/obj-1/animations") + + result = invoke( + ["object", "animate", "obj-1", "-a", ROCKING, "-d", "east"], tmp_path, monkeypatch + ) + + assert result.exit_code == 2 + assert not route.called + + @respx.mock + def test_a_direction_that_is_not_one_is_refused(self, tmp_path, monkeypatch): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond(json=an_object()) + + result = invoke( + ["object", "animate", "obj-1", "-a", ROCKING, "-d", "sideways"], tmp_path, monkeypatch + ) + + assert result.exit_code == 2 + assert "north-east" in result.output + + +class TestAddingDirectionsToAnObjectAnimation: + """R2.42 and R2.44. This is the route that lets an animation take more directions, + which is what the character route refuses outright.""" + + def barrel(self, covered=("south", "east")): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond( + json=an_object(animations=[an_object_animation(directions=covered)]) + ) + + @respx.mock + def test_the_group_is_sent_and_the_description_is_not_repeated(self, tmp_path, monkeypatch): + """The route inherits the description of the animation being extended.""" + self.barrel() + route = mock_object_animation(directions=("north",)) + + result = invoke( + ["object", "animate", "obj-1", "--into", "group-1", "-d", "north"], + tmp_path, + monkeypatch, + ) + + assert result.exit_code == 0 + sent = json.loads(route.calls.last.request.content) + assert sent["animation_group_id"] == "group-1" + assert "animation_description" not in sent + + @respx.mock + def test_without_directions_it_animates_what_the_animation_lacks(self, tmp_path, monkeypatch): + self.barrel(covered=("south", "east")) + route = mock_object_animation( + directions=("south-east", "north-east", "north", "north-west", "west", "south-west") + ) + + result = invoke(["object", "animate", "obj-1", "--into", "group-1"], tmp_path, monkeypatch) + + assert result.exit_code == 0 + sent = json.loads(route.calls.last.request.content) + assert "south" not in sent["directions"] + assert "east" not in sent["directions"] + assert set(sent["directions"]) == { + "south-east", + "north-east", + "north", + "north-west", + "west", + "south-west", + } + + @respx.mock + def test_a_direction_it_already_holds_is_refused(self, tmp_path, monkeypatch): + self.barrel() + route = respx.post(f"{PIXELLAB_BASE_URL}/objects/obj-1/animations") + + result = invoke( + ["object", "animate", "obj-1", "--into", "group-1", "-d", "south"], + tmp_path, + monkeypatch, + ) + + assert result.exit_code == 2 + assert "already animated" in result.output + assert not route.called + + @respx.mock + def test_asked_again_it_is_sent_with_replace_existing(self, tmp_path, monkeypatch): + self.barrel() + route = mock_object_animation() + + result = invoke( + ["object", "animate", "obj-1", "--into", "group-1", "-d", "south", "--again"], + tmp_path, + monkeypatch, + ) + + assert result.exit_code == 0 + assert json.loads(route.calls.last.request.content)["replace_existing"] is True + + @respx.mock + def test_an_animation_covering_everything_is_refused_rather_than_sent( + self, tmp_path, monkeypatch + ): + self.barrel(covered=DIRECTIONS) + route = respx.post(f"{PIXELLAB_BASE_URL}/objects/obj-1/animations") + + result = invoke(["object", "animate", "obj-1", "--into", "group-1"], tmp_path, monkeypatch) + + assert result.exit_code == 2 + assert "every direction" in result.output + assert not route.called + + @respx.mock + def test_an_animation_this_object_does_not_have_names_the_ones_it_does( + self, tmp_path, monkeypatch + ): + self.barrel() + + result = invoke(["object", "animate", "obj-1", "--into", "nope"], tmp_path, monkeypatch) + + assert result.exit_code == 2 + assert "group-1" in result.output + + +class TestAOneDirectionObjectThatIsAlreadyAnimated: + """A prop with one direction has nothing left to add once an animation covers it, + so the refusal the eight-direction path gives applies here too — the route says + the same thing, one round trip later.""" + + def barrel(self, animations): + respx.get(f"{PIXELLAB_BASE_URL}/objects/obj-1").respond( + json=an_object(animations=animations, directions=("south",)) | {"directions": 1} + ) + + @respx.mock + def test_adding_to_an_animation_that_covers_it_is_refused(self, tmp_path, monkeypatch): + self.barrel([an_object_animation(directions=("south",))]) + route = respx.post(f"{PIXELLAB_BASE_URL}/objects/obj-1/animations") + + result = invoke(["object", "animate", "obj-1", "--into", "group-1"], tmp_path, monkeypatch) + + assert result.exit_code == 2 + assert "only direction" in result.output + assert not route.called + + @respx.mock + def test_asked_again_it_is_sent(self, tmp_path, monkeypatch): + self.barrel([an_object_animation(directions=("south",))]) + route = mock_object_animation() + + result = invoke( + ["object", "animate", "obj-1", "--into", "group-1", "--again"], tmp_path, monkeypatch + ) + + assert result.exit_code == 0 + assert json.loads(route.calls.last.request.content)["replace_existing"] is True diff --git a/tests/test_pixellab.py b/tests/test_pixellab.py index b1817ab..68c4f00 100644 --- a/tests/test_pixellab.py +++ b/tests/test_pixellab.py @@ -550,3 +550,96 @@ def test_an_ordinary_asset_comes_through(self, client): respx.get(self.ADDRESS).respond(content=png_bytes()) assert client.download(self.ADDRESS) == png_bytes() + + +class TestAJobIdInsideAListOfObjects: + """R4.10. `objects/{id}/animations` answers with `submissions`: one object per + direction, each carrying its own job. The first is followed, exactly as the flat + list of a character animation is.""" + + def a_submission(self, direction, job): + return {"direction": direction, "status": "processing", "background_job_id": job} + + @respx.mock + def test_the_first_submission_is_the_job_that_is_polled(self, client): + respx.post(url("/objects/obj-1/animations")).respond( + json={ + "animation_group_id": "group-1", + "object_id": "obj-1", + "description": "it turns on the spot", + "mode": "v3", + "frame_count": 8, + "submissions": [ + self.a_submission("south", "job-south"), + self.a_submission("east", "job-east"), + ], + } + ) + polled = respx.get(url("/background-jobs/job-south")).respond( + json={"status": "completed", "last_response": {"images": [image_payload()]}} + ) + + result = client.call( + "object-animations", object_id="obj-1", animation_description="it turns" + ) + + assert polled.called + assert result.images == [png_bytes()] + + @respx.mock + def test_a_submission_with_no_job_is_not_polled_as_one(self, client): + """`background_job_id` is nullable, and a dictionary is not a job id.""" + respx.post(url("/objects/obj-1/animations")).respond( + json={ + "animation_group_id": "group-1", + "object_id": "obj-1", + "description": "it turns", + "mode": "v3", + "frame_count": 8, + "submissions": [{"direction": "south", "status": "failed"}], + } + ) + + with pytest.raises(ProviderError): + client.call("object-animations", object_id="obj-1", animation_description="it turns") + + +class TestAJobIdTheProviderChose: + """R3.6 on the other half of the same problem. The identifier a response hands + back is interpolated into the poll path, and the request that follows carries the + bearer token — a response is not more trustworthy than an argument here.""" + + @respx.mock + def test_a_job_id_carrying_a_path_is_refused_before_it_is_polled(self, client): + respx.post(url("/objects/obj-1/animations")).respond( + json={ + "animation_group_id": "group-1", + "object_id": "obj-1", + "description": "it turns", + "mode": "v3", + "frame_count": 8, + "submissions": [ + { + "direction": "south", + "status": "processing", + "background_job_id": "../../v2/characters", + } + ], + } + ) + elsewhere = respx.get(url("/characters")) + + with pytest.raises(ValidationError): + client.call("object-animations", object_id="obj-1", animation_description="it turns") + + assert not elsewhere.called + + @respx.mock + def test_the_flat_list_of_a_character_animation_is_guarded_too(self, client): + """`characters/animations` returns its ids one layer shallower; one rule.""" + respx.post(url("/characters/animations")).respond( + json={"background_job_ids": ["../../v2/balance"], "status": "processing"} + ) + + with pytest.raises(ValidationError): + client.call("characters-animations", character_id="char-9", action_description="walk") diff --git a/tests/test_validate.py b/tests/test_validate.py index c740bc0..c1dd18a 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -435,3 +435,23 @@ def test_a_style_image_of_unknown_size_is_left_to_the_route(self): body = self.request(EncodedImage(base64="ZmFrZQ==")) assert body["style_image"]["base64"] == "ZmFrZQ==" + + +class TestAnEnumeratedListChecksEachItem: + """R3.3. A list parameter enumerates what each item may be, not what the whole + list may be — `directions=["north"]` is one allowed value, not an unknown one.""" + + def test_a_list_of_allowed_values_passes(self): + route = catalog.route("object-animations") + + body = build_request(route, {"object_id": "obj-1", "directions": ["north", "south"]}) + + assert body["directions"] == ["north", "south"] + + def test_one_item_outside_the_set_is_refused_and_named(self): + route = catalog.route("object-animations") + + with pytest.raises(ValidationError) as refused: + build_request(route, {"object_id": "obj-1", "directions": ["north", "sideways"]}) + + assert "sideways" in str(refused.value)