feat: APNG animated card effects for T3/T4 refractor tiers (#186) #188
No reviewers
Labels
No Label
ai-changes-requested
ai-failed
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
bug
enhancement
evolution
performance
phase-0
phase-1a
phase-1b
phase-1c
phase-1d
security
tech-debt
todo
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cal/paper-dynasty-database#188
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "issue/186-feat-apng-animated-card-effects-for-t3-t4-refracto"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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.htmlare defined withanimation-play-state: pausedfor static PNG capture. To generate animation frames, we inject an override<style>tag via Playwright'sadd_style_tagthat:animation-play-state: running !importantanimation-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 servicegenerate_animated_card(page, html_content, output_path, tier)— captures N frames, assembles withapnglibraryapng_cache_path(cardset_id, card_type, player_id, d, variant)— canonical.apngcache pathapp/routers_v2/players.py: NewGET /{player_id}/{card_type}card/{d}/{variant}/animatedendpoint.apngexists + no?tier=override) → serves immediatelyrequirements.txt: Addedapng==3.1.1Cache / S3 path convention
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.1should be verified/pinned after install testing in the Docker buildimage/pngMIME type and are backward-compatible with PNG (non-supporting clients show the first frame)AI Code Review
Files Reviewed
app/services/apng_generator.py(added)app/routers_v2/players.py(modified — new/animatedendpoint)requirements.txt(modified)Findings
Correctness
No issues found.
from ..services.apng_generator import apng_cache_path, generate_animated_cardis added at the top ofplayers.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 existingget_batter_cardendpoint (line 887).TemplateResponserenders synchronously in__init__, so.bodyis available immediately. ✅progress = i / num_frames(0.0 to (N−1)/N) withdelay_s = -progress * durationproduces 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_denAPNG framing:delay=200, delay_den=1000= 200ms;delay=250, delay_den=1000= 250ms. Correct per the APNGfcTLspec. ✅#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. ✅NamedTemporaryFile(delete=False)+ explicitos.unlinkinfinallyblock is the correct pattern for passing a file path to Playwright. ✅tier is Nonecache bypass: Cache is skipped when?tier=override is set, allowing dev re-renders — consistent with the existing PNG endpoint's behavior. ✅resolve_refractor_tieris called before card/ratings fetches; T0–T2 returns 404 immediately. Efficient. ✅Security
No issues found.
player_idvalidated viaPlayer.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.
apng_generator.pyare module-level. ✅apng==3.1.1is pinned inrequirements.txt. ✅apng_cache_pathreturns a relative path using the samestorage/cards/cardset-{id}/...convention as the existing PNG cache. ✅Suggestions
generate_animated_cardand the/animatedendpoint 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.get_animated_cardis essentially a copy of the same logic inget_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.apng==3.1.1is 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
1cf0e7eac3to30b5eefa29