fix(pet): key sprite keyframes on the resolved track; cap render durations (#9140)

Follow-up polish to #8730: key @keyframes on the resolved row/frames (so a same-row fallback like hover on a jumping-less pet doesn't restart idle), mirror the importer's 60s per-frame cap at render, and trim comments to the 1-2 line convention.
This commit is contained in:
Neil 2026-07-16 22:21:58 -07:00 committed by GitHub
parent 13c690b05a
commit 0cd50eecdb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 49 additions and 30 deletions

View File

@ -75,8 +75,8 @@ function spriteDiv(container: HTMLElement): HTMLDivElement {
return div
}
// The @keyframes name is `pet-<useId>-<animationName>-<dragGeneration>`: the
// animation name keeps a switched-to row from reusing the prior timeline, and
// The @keyframes name is `pet-<useId>-<row>-<frames>-<dragGeneration>`: the
// resolved track keeps a switched-to row from reusing the prior timeline, and
// the generation suffix restarts a same-row grab from frame 0.
function animationName(container: HTMLElement): string {
return spriteDiv(container).style.animation.split(' ')[0]
@ -118,7 +118,6 @@ describe('PetOverlay grab-and-hold pointer interaction', () => {
// it renders as step-end rather than steps()).
expect(spriteDiv(container).style.animationPlayState).toBe('running')
expect(spriteDiv(container).style.animation).toContain('step-end')
expect(animationName(container)).toContain('idle')
const idleName = animationName(container)
// Grab and hold still: mint a fresh restart (frame 0) and freeze there while
@ -139,18 +138,35 @@ describe('PetOverlay grab-and-hold pointer interaction', () => {
// Drag right past the 4px deadzone: switch to the running-right row (row 1,
// 8 frames, no durations → steps(8)) and resume animating. The keyframes
// name changes with the row, so it starts from frame 0 rather than reusing
// the idle timeline.
// name changes with the resolved row, so it starts from frame 0 rather than
// reusing the idle timeline.
firePointer(wrapper, 'pointermove', 70, 80)
expect(spriteDiv(container).style.animationPlayState).toBe('running')
expect(spriteDiv(container).style.animation).toContain('steps(8)')
expect(animationName(container)).toContain('running-right')
expect(animationName(container)).not.toBe(heldName)
// Release restores the live agent state (idle) and resumes animating.
// Release returns to the idle row (step-end) and resumes animating. Same row
// and generation as the grab-hold, so it's the held-idle identity again.
firePointer(wrapper, 'pointerup', 70, 80)
expect(spriteDiv(container).style.animationPlayState).toBe('running')
expect(spriteDiv(container).style.animation).toContain('step-end')
expect(animationName(container)).toContain('idle')
expect(animationName(container)).toBe(heldName)
})
it('does not restart the idle track when hover falls back to the same row', () => {
;({ container, root } = renderPetOverlay())
const wrapper = container.querySelector('.pointer-events-auto')
if (!wrapper) {
throw new Error('draggable wrapper not found')
}
// The mocked sprite has no jumping row, so hover (→ jumping) resolves back to
// idle. Keying the keyframes on the resolved row means the name must not
// change — otherwise the unchanged idle animation restarts on hover. React
// synthesizes onPointerEnter/Leave from pointerover/pointerout.
const idleName = animationName(container)
firePointer(wrapper, 'pointerover', 0, 0)
expect(animationName(container)).toBe(idleName)
firePointer(wrapper, 'pointerout', 0, 0)
expect(animationName(container)).toBe(idleName)
})
})

View File

@ -61,9 +61,7 @@ function SpriteFrame({
// that restarts from frame 0 even when the state row is unchanged.
restartKey: number
}): React.JSX.Element {
// Why: name the @keyframes per animation (+restartKey for same-row grabs) so a
// switched-to row starts at frame 0 instead of inheriting the prior timeline.
const animKeyframesId = `${useId().replace(/[^a-zA-Z0-9_-]/g, '')}-${animationName}-${restartKey}`
const baseId = useId().replace(/[^a-zA-Z0-9_-]/g, '')
const anim =
sprite.animations?.[animationName] ||
(sprite.defaultAnimation && sprite.animations?.[sprite.defaultAnimation]) ||
@ -72,6 +70,11 @@ function SpriteFrame({
// Why: clamp to >=1 so an empty/invalid manifest can't produce steps(0),
// which is rejected as invalid CSS and freezes the animation.
const frames = Math.max(1, anim?.frames ?? sprite.columns ?? 1)
// Why: name the @keyframes by the RESOLVED track (+restartKey for same-row
// grabs), so a genuine row change starts at frame 0 while a state that falls
// back to the same row (e.g. hover on a pet without a jumping row) doesn't
// needlessly restart.
const animKeyframesId = `${baseId}-${row}-${frames}-${restartKey}`
// Why: allow fractional downscaling so frames larger than maxSize shrink to
// fit instead of overflowing the overlay; mirrors DetectedSpriteFrame's math.
const scale = Math.min(maxSize / sprite.frameWidth, maxSize / sprite.frameHeight)
@ -406,9 +409,8 @@ export function PetOverlay(): React.JSX.Element {
>
<style>{PET_BOB_KEYFRAMES_CSS}</style>
{sprite ? (
// Why: remount per pet so switching cached sprites can't inherit the
// previous pet's animation timeline (same @keyframes name → carried
// currentTime); each pet starts clean.
// Why: remount per pet so a switched-to sprite starts a fresh
// animation instead of inheriting the prior pet's currentTime.
<SpriteFrame
key={url}
url={url}

View File

@ -12,10 +12,8 @@ export type PetAnimationName =
export type PetDragAnimation = 'running-right' | 'running-left' | null
// Why: direction tracks horizontal travel only. `accepted` (advance the
// baseline) fires solely on a >=4px horizontal move, so sub-threshold jitter
// and vertical drags keep the last direction without resetting the baseline —
// otherwise a slow diagonal drag would reset before ever crossing 4px.
// Why: direction tracks horizontal travel only; `accepted` (advance the baseline)
// fires only on a >=4px horizontal move so slow diagonal drags still accumulate.
export function nextPetDragAnimation(
current: PetDragAnimation,
deltaX: number

View File

@ -31,6 +31,9 @@ describe('buildSpriteAnimationCss', () => {
[100, 100],
[100, -1, 100],
[100, Number.NaN, 100],
// Above the importer's 60s cap: corrupt/hand-edited persisted holds must
// not freeze the overlay.
[100, 70_000, 100],
// Untrusted persisted data: non-arrays with a matching `length` must not
// reach .every and throw.
{ length: 3 } as unknown as number[],

View File

@ -1,6 +1,10 @@
// Sprite-sheet keyframe CSS for the pet overlay. Kept as a pure module so the
// pacing math is unit-testable without mounting the overlay or a DOM.
// Mirror of the importer's per-frame cap (pet.ts zod schema): a hold longer than
// this at render would freeze the overlay, so reject it on the render side too.
const MAX_FRAME_DURATION_MS = 60_000
export type SpriteAnimationCss = {
keyframesCss: string
animationCss: string
@ -57,13 +61,13 @@ function validFrameDurations(
frameDurationsMs: number[] | undefined,
frames: number
): number[] | null {
// Why: Array.isArray, not a truthiness check — persisted/RPC-synced sprites
// are untrusted, so a corrupt non-array value (e.g. { length: 6 }) must fail
// here rather than throw on .length/.every during render.
// Why: Array.isArray + bounds, not a truthiness check — persisted/RPC-synced
// sprites are untrusted, so a corrupt non-array or out-of-range hold falls back
// to uniform pacing instead of throwing or freezing the overlay.
if (
Array.isArray(frameDurationsMs) &&
frameDurationsMs.length === frames &&
frameDurationsMs.every((ms) => Number.isFinite(ms) && ms > 0)
frameDurationsMs.every((ms) => Number.isFinite(ms) && ms > 0 && ms <= MAX_FRAME_DURATION_MS)
) {
return frameDurationsMs
}
@ -71,10 +75,8 @@ function validFrameDurations(
}
// Cumulative step-end stops, one per frame. Returns null (→ uniform fallback)
// when a frame is too short to survive our 4-decimal precision: either two
// stops collapse to the same percentage, or the final stop rounds to 100% and
// so has no interval before the loop. Either way the frame would vanish, so we
// degrade to uniform pacing rather than silently drop it.
// when a frame is too short to survive 4-decimal precision (two stops collapse,
// or the final stop rounds to 100%) so no frame is silently dropped.
function stepEndStops(
durations: number[],
totalMs: number,

View File

@ -35,10 +35,8 @@ export function usePetPointerInteraction(
// Why: horizontal baseline for the drag-direction hysteresis, advanced only on
// an accepted direction. Kept separate from dragOffsetRef (position math).
const dragBaselineXRef = useRef(0)
// Why: direction and the owning pointer are read+written inside pointer
// handlers, so keep them in refs immune to React's render batching. Reading
// the direction from state would let two coalesced moves in one commit
// resurrect a stale direction; `dragAnimation` state exists only to render.
// Why: read+written inside handlers, so keep in refs immune to render batching
// (state would let two coalesced moves resurrect a stale direction).
const dragDirectionRef = useRef<PetDragAnimation>(null)
const activePointerRef = useRef<number | null>(null)