Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions specs/characters-and-animation/requirements.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions specs/characters-and-animation/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 36 additions & 0 deletions src/pixellab_cli/commands/character.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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."""
Expand All @@ -893,6 +923,7 @@ def animate(
terse,
any_pose,
again,
into,
seed,
)
except PixellabCliError as failure:
Expand All @@ -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")

Expand Down
50 changes: 50 additions & 0 deletions tests/test_commands_character.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading