diff --git a/specs/characters-and-animation/requirements.md b/specs/characters-and-animation/requirements.md index 8003ecd..693398f 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/an-object-animation-takes-more-directions +branch: feat/character-animation-refuses-a-group-id delivery: in-review -pr: 82 +pr: 86 --- # Characters and animation — requirements @@ -89,6 +89,7 @@ different endpoint family and no skeleton. - **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. +- **R2.45** (ADDED) If an animation group is named when animating a character, then the character commands shall refuse the call before spending anything, shall say that a character animation does not take an animation group, and shall name what reaches the directions the motion lacks instead. ## R3 · Rotating diff --git a/specs/characters-and-animation/tasks.md b/specs/characters-and-animation/tasks.md index aea263f..7903b59 100644 --- a/specs/characters-and-animation/tasks.md +++ b/specs/characters-and-animation/tasks.md @@ -133,6 +133,8 @@ - [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_ +- [x] 7.5 (Unit) Refuse an animation group named on a character animation — R2.45 + _Reason the provider forbids animation_group_id on character animations, so the option has to be answered here rather than by a 422_ ## 8 · Animating an object - [x] 8.1 (Unit) Add the object animation route to the catalogue — R2.41 diff --git a/src/pixellab_cli/commands/character.py b/src/pixellab_cli/commands/character.py index bce2218..ea0585e 100644 --- a/src/pixellab_cli/commands/character.py +++ b/src/pixellab_cli/commands/character.py @@ -128,6 +128,28 @@ def motion_of(animation: dict[str, Any]) -> str: ) +def refuse_an_animation_group(into: str | None) -> None: + """Answer `--into` on a character rather than letting the provider answer it. + + `object animate --into` is real — `AnimateObjectRequest.animation_group_id` is + documented and supported — so the flag is the obvious thing to reach for here + too, and `/characters/animations` forbids the field outright: the body is + `extra="forbid"`, and it answers `{"type": "extra_forbidden", "loc": ["body", + "animation_group_id"]}` (see `n-0054`). Refused here, with the field named, so a + caller reads what to do instead of a 422 naming a field they never typed. + """ + if not into: + return + raise ValidationError( + "animation_group_id is not allowed for a character: PixelLab forbids the field " + "on /characters/animations and starts a new animation for every call, so there " + "is no group to add a direction to. Name every direction in one call with -d, " + "and `character show` reads the groups back as one animation. --into belongs to " + "`object animate`.", + context={"animation_group_id": into}, + ) + + def check_the_directions_are_new( payload: dict[str, Any], motion: str, wanted: list[str], again: bool ) -> dict[str, Any] | None: @@ -874,6 +896,14 @@ def animate( again: bool = typer.Option( False, "--again", help="Animate a direction this motion already has, a second time." ), + into: str = typer.Option( + None, + "--into", + "--group", + "--animation-group-id", + hidden=True, + help="Refused: `object animate` takes an animation group, a character does not.", + ), seed: int = typer.Option(None, "--seed", help="Repeat a previous generation."), ) -> None: """Animate a character. Every direction is a separate job and a separate charge.""" @@ -893,6 +923,7 @@ def animate( terse, any_pose, again, + into, seed, ) except PixellabCliError as failure: @@ -914,11 +945,16 @@ def _animate( terse, any_pose, again, + into, seed, ) -> None: app_context: AppContext = context.obj route = catalog.route("characters-animations") + # Before --action, so a call that only names a group is told about the group + # rather than about the action it is also missing. + refuse_an_animation_group(into) + if not action and not template: raise ValidationError("give either --action or --template") diff --git a/tests/test_commands_character.py b/tests/test_commands_character.py index 7ef1f29..ec3bb91 100644 --- a/tests/test_commands_character.py +++ b/tests/test_commands_character.py @@ -22,6 +22,7 @@ motion_asked_for, motion_of, ordered_rotations, + refuse_an_animation_group, ) from pixellab_cli.config import PIXELLAB_BASE_URL, PIXELLAB_SECRET_VAR from pixellab_cli.errors import ValidationError @@ -3261,6 +3262,55 @@ def test_a_character_with_no_animations_holds_nothing(self): assert gather_animations({"animations": None}) == [] +class TestAnAnimationGroupOnACharacter: + """R2.45: `object animate --into` is supported and `/characters/animations` forbids + the field, so the flag has to be answered here rather than by the provider.""" + + def test_nothing_is_refused_when_no_group_is_named(self): + assert refuse_an_animation_group(None) is None + assert refuse_an_animation_group("") is None + + def test_a_group_named_is_refused_and_the_field_is_named(self): + with pytest.raises(ValidationError) as refusal: + refuse_an_animation_group("group-1") + + assert "animation_group_id is not allowed for a character" in str(refusal.value) + assert refusal.value.context == {"animation_group_id": "group-1"} + + def test_the_refusal_says_what_to_do_instead(self): + with pytest.raises(ValidationError) as refusal: + refuse_an_animation_group("group-1") + + assert "-d" in str(refusal.value) + assert "object animate" in str(refusal.value) + + @respx.mock + def test_a_group_is_refused_before_the_character_is_read(self, tmp_path, monkeypatch): + """Ahead of --action, so the answer is about the group and not about the action.""" + character = respx.get(f"{PIXELLAB_BASE_URL}/characters/char-9") + route = respx.post(f"{PIXELLAB_BASE_URL}/characters/animations") + + result = invoke( + ["character", "animate", "char-9", "--into", "group-1"], tmp_path, monkeypatch + ) + + assert result.exit_code != 0 + assert "not allowed for a character" in result.output + assert not character.called + assert not route.called + + @respx.mock + def test_the_field_itself_is_refused_under_its_own_name(self, tmp_path, monkeypatch): + result = invoke( + ["character", "animate", "char-9", "--animation-group-id", "group-1", "-a", WALK_CYCLE], + tmp_path, + monkeypatch, + ) + + assert result.exit_code != 0 + assert "not allowed for a character" in result.output + + class TestADirectionThisMotionAlreadyHas: """R2.39: a second call reaches the directions the first did not cover, so one it did cover is a copy nobody asked for. R2.40: what it does reach joins the motion