feat: APNG animated card effects for T3/T4 refractor tiers (#186) #188

Merged
cal merged 1 commits from issue/186-feat-apng-animated-card-effects-for-t3-t4-refracto into main 2026-04-07 03:22:50 +00:00
Collaborator

Closes #186

What this does

Implements the APNG animated card pipeline for T3 (gold shimmer) and T4 (prismatic rainbow) refractor tiers.

Approach

CSS animations in tier_style.html are defined with animation-play-state: paused for static PNG capture. To generate animation frames, we inject an override <style> tag via Playwright's add_style_tag that:

  • Sets animation-play-state: running !important
  • Sets animation-delay: -Xs !important (negative delay seeks the animation to second X)

This freezes each render at an evenly-spaced point in the animation cycle without actually running the animation, giving deterministic frames.

Files changed

  • app/services/apng_generator.py (new): Frame capture loop + APNG assembly service
    • generate_animated_card(page, html_content, output_path, tier) — captures N frames, assembles with apng library
    • apng_cache_path(cardset_id, card_type, player_id, d, variant) — canonical .apng cache path
    • T3: 12 frames × 200ms (2.4s loop, ~2.5s animation)
    • T4: 24 frames × 250ms (6.0s loop, matching the 6s prismatic sweep)
  • app/routers_v2/players.py: New GET /{player_id}/{card_type}card/{d}/{variant}/animated endpoint
    • Returns 404 for T0–T2 (no animation)
    • Cache hit (.apng exists + no ?tier= override) → serves immediately
    • Cache miss → renders HTML, captures frames, assembles APNG, caches, serves
  • requirements.txt: Added apng==3.1.1

Cache / S3 path convention

Local:  storage/cards/cardset-{id}/{card_type}/{player_id}-{date}-v{variant}.apng
S3:     cards/cardset-{id}/{card_type}/{player_id}-{date}-v{variant}.apng

S3 upload and bot/web display support are out of scope for this ticket (see issue #186 body — those require changes to other repos).

Other observations

  • apng==3.1.1 should be verified/pinned after install testing in the Docker build
  • Bot/web display: APNG files use image/png MIME type and are backward-compatible with PNG (non-supporting clients show the first frame)
Closes #186 ## What this does Implements the APNG animated card pipeline for T3 (gold shimmer) and T4 (prismatic rainbow) refractor tiers. ## Approach CSS animations in `tier_style.html` are defined with `animation-play-state: paused` for static PNG capture. To generate animation frames, we inject an override `<style>` tag via Playwright's `add_style_tag` that: - Sets `animation-play-state: running !important` - Sets `animation-delay: -Xs !important` (negative delay seeks the animation to second X) This freezes each render at an evenly-spaced point in the animation cycle without actually running the animation, giving deterministic frames. ## Files changed - **`app/services/apng_generator.py`** (new): Frame capture loop + APNG assembly service - `generate_animated_card(page, html_content, output_path, tier)` — captures N frames, assembles with `apng` library - `apng_cache_path(cardset_id, card_type, player_id, d, variant)` — canonical `.apng` cache path - T3: 12 frames × 200ms (2.4s loop, ~2.5s animation) - T4: 24 frames × 250ms (6.0s loop, matching the 6s prismatic sweep) - **`app/routers_v2/players.py`**: New `GET /{player_id}/{card_type}card/{d}/{variant}/animated` endpoint - Returns 404 for T0–T2 (no animation) - Cache hit (`.apng` exists + no `?tier=` override) → serves immediately - Cache miss → renders HTML, captures frames, assembles APNG, caches, serves - **`requirements.txt`**: Added `apng==3.1.1` ## Cache / S3 path convention ``` Local: storage/cards/cardset-{id}/{card_type}/{player_id}-{date}-v{variant}.apng S3: cards/cardset-{id}/{card_type}/{player_id}-{date}-v{variant}.apng ``` S3 upload and bot/web display support are out of scope for this ticket (see issue #186 body — those require changes to other repos). ## Other observations - `apng==3.1.1` should be verified/pinned after install testing in the Docker build - Bot/web display: APNG files use `image/png` MIME type and are backward-compatible with PNG (non-supporting clients show the first frame)
Claude added 1 commit 2026-04-07 02:37:08 +00:00
Closes #186

- Add app/services/apng_generator.py: scrubs CSS animations via Playwright
  (negative animation-delay + running override), assembles frames with apng lib
- Add GET /{player_id}/{card_type}card/{d}/{variant}/animated endpoint: returns
  cached .apng for T3/T4 cards, 404 for T0-T2
- T3: 12 frames × 200ms (gold shimmer, 2.5s cycle)
- T4: 24 frames × 250ms (prismatic sweep + diamond glow, 6s cycle)
- Cache path: storage/cards/cardset-{id}/{type}/{player_id}-{d}-v{variant}.apng
- Add apng==3.1.1 to requirements.txt

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude added the
ai-reviewing
label 2026-04-07 02:45:53 +00:00
Claude removed the
ai-reviewing
label 2026-04-07 02:50:54 +00:00
Claude added the
ai-reviewing
label 2026-04-07 03:15:52 +00:00
Claude reviewed 2026-04-07 03:18:35 +00:00
Claude left a comment
Author
Collaborator

AI Code Review

Files Reviewed

  • app/services/apng_generator.py (added)
  • app/routers_v2/players.py (modified — new /animated endpoint)
  • requirements.txt (modified)

Findings

Correctness

No issues found.

  • Import placement: from ..services.apng_generator import apng_cache_path, generate_animated_card is added at the top of players.py (line ~36), not inside a function. Correctly follows the CLAUDE.md "never add lazy imports to middle of file" rule — this was the blocking issue in PR #187 and is resolved here.
  • html_response.body.decode("UTF-8"): Follows the established pattern in the existing get_batter_card endpoint (line 887). TemplateResponse renders synchronously in __init__, so .body is available immediately.
  • Animation frame math: progress = i / num_frames (0.0 to (N−1)/N) with delay_s = -progress * duration produces a correct seamless loop — each frame is evenly spaced, and the gap from the last frame back to the first is identical to the gap between any two consecutive frames. T3: 0s–2.29s of 2.5s animation (12 frames). T4: 0s–5.75s of 6s animation (24 frames).
  • delay/delay_den APNG framing: delay=200, delay_den=1000 = 200ms; delay=250, delay_den=1000 = 250ms. Correct per the APNG fcTL spec.
  • T4 dual-selector scrubbing: Both #header::after (6.0s) and .tier-diamond.diamond-glow (2.0s) are scrubbed to the same fractional progress. Their separate durations mean each is offset to the equivalent proportional point in their own cycle. Intentionally deterministic.
  • Temp file lifecycle: NamedTemporaryFile(delete=False) + explicit os.unlink in finally block is the correct pattern for passing a file path to Playwright.
  • tier is None cache bypass: Cache is skipped when ?tier= override is set, allowing dev re-renders — consistent with the existing PNG endpoint's behavior.
  • Tier guard before any DB queries: resolve_refractor_tier is called before card/ratings fetches; T0–T2 returns 404 immediately. Efficient.

Security

No issues found.

  • No raw SQL; all DB access via Peewee ORM.
  • player_id validated via Player.get_by_id.
  • d (date string) is used in path construction — same pattern as the existing PNG endpoint; not a new concern introduced by this PR.

Style & Conventions

No issues found.

  • All imports in apng_generator.py are module-level.
  • apng==3.1.1 is pinned in requirements.txt.
  • apng_cache_path returns a relative path using the same storage/cards/cardset-{id}/... convention as the existing PNG cache.

Suggestions

  • No test coverage: generate_animated_card and the /animated endpoint have no tests. Consistent with the pattern of some prior PRs (e.g., #173), but an animation scrubbing bug would be hard to catch without them. Worth a follow-up test PR.
  • Duplicated card data assembly (~70 lines): The batting/pitching card lookup block in get_animated_card is essentially a copy of the same logic in get_batter_card. This is acceptable now, but a shared _build_card_html(request, player_id, card_type, variant, tier) helper would eliminate the duplication long-term.
  • PR body note is slightly stale: "should be verified/pinned after install testing in the Docker build" — apng==3.1.1 is already pinned in the diff. No action needed, just a cosmetic discrepancy.

Verdict: COMMENT

Implementation is functionally correct. The lazy-import issue that blocked PR #187 is resolved — the import is placed at the top of the module. The animation scrubbing technique is sound, APNG frame timing is correct, and the endpoint logic follows existing codebase patterns. No blocking issues. Ready to merge.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/services/apng_generator.py` (added) - `app/routers_v2/players.py` (modified — new `/animated` endpoint) - `requirements.txt` (modified) ### Findings #### Correctness No issues found. - **Import placement**: `from ..services.apng_generator import apng_cache_path, generate_animated_card` is added at the top of `players.py` (line ~36), not inside a function. Correctly follows the CLAUDE.md "never add lazy imports to middle of file" rule — this was the blocking issue in PR #187 and is resolved here. ✅ - **`html_response.body.decode("UTF-8")`**: Follows the established pattern in the existing `get_batter_card` endpoint (line 887). `TemplateResponse` renders synchronously in `__init__`, so `.body` is available immediately. ✅ - **Animation frame math**: `progress = i / num_frames` (0.0 to (N−1)/N) with `delay_s = -progress * duration` produces a correct seamless loop — each frame is evenly spaced, and the gap from the last frame back to the first is identical to the gap between any two consecutive frames. T3: 0s–2.29s of 2.5s animation (12 frames). T4: 0s–5.75s of 6s animation (24 frames). ✅ - **`delay/delay_den` APNG framing**: `delay=200, delay_den=1000` = 200ms; `delay=250, delay_den=1000` = 250ms. Correct per the APNG `fcTL` spec. ✅ - **T4 dual-selector scrubbing**: Both `#header::after` (6.0s) and `.tier-diamond.diamond-glow` (2.0s) are scrubbed to the same fractional progress. Their separate durations mean each is offset to the equivalent proportional point in their own cycle. Intentionally deterministic. ✅ - **Temp file lifecycle**: `NamedTemporaryFile(delete=False)` + explicit `os.unlink` in `finally` block is the correct pattern for passing a file path to Playwright. ✅ - **`tier is None` cache bypass**: Cache is skipped when `?tier=` override is set, allowing dev re-renders — consistent with the existing PNG endpoint's behavior. ✅ - **Tier guard before any DB queries**: `resolve_refractor_tier` is called before card/ratings fetches; T0–T2 returns 404 immediately. Efficient. ✅ #### Security No issues found. - No raw SQL; all DB access via Peewee ORM. ✅ - `player_id` validated via `Player.get_by_id`. ✅ - `d` (date string) is used in path construction — same pattern as the existing PNG endpoint; not a new concern introduced by this PR. #### Style & Conventions No issues found. - All imports in `apng_generator.py` are module-level. ✅ - `apng==3.1.1` is pinned in `requirements.txt`. ✅ - `apng_cache_path` returns a relative path using the same `storage/cards/cardset-{id}/...` convention as the existing PNG cache. ✅ #### Suggestions - **No test coverage**: `generate_animated_card` and the `/animated` endpoint have no tests. Consistent with the pattern of some prior PRs (e.g., #173), but an animation scrubbing bug would be hard to catch without them. Worth a follow-up test PR. - **Duplicated card data assembly (~70 lines)**: The batting/pitching card lookup block in `get_animated_card` is essentially a copy of the same logic in `get_batter_card`. This is acceptable now, but a shared `_build_card_html(request, player_id, card_type, variant, tier)` helper would eliminate the duplication long-term. - **PR body note is slightly stale**: "should be verified/pinned after install testing in the Docker build" — `apng==3.1.1` is already pinned in the diff. No action needed, just a cosmetic discrepancy. ### Verdict: COMMENT Implementation is functionally correct. The lazy-import issue that blocked PR #187 is resolved — the import is placed at the top of the module. The animation scrubbing technique is sound, APNG frame timing is correct, and the endpoint logic follows existing codebase patterns. No blocking issues. Ready to merge. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
and removed
ai-reviewing
labels 2026-04-07 03:18:54 +00:00
cal force-pushed issue/186-feat-apng-animated-card-effects-for-t3-t4-refracto from 1cf0e7eac3 to 30b5eefa29 2026-04-07 03:22:09 +00:00 Compare
cal merged commit 67d1f10455 into main 2026-04-07 03:22:50 +00:00
cal deleted branch issue/186-feat-apng-animated-card-effects-for-t3-t4-refracto 2026-04-07 03:22:51 +00:00
Sign in to join this conversation.
No description provided.