fix(desktop): Bot Mode pet picker selects locally hatched pets via pet.thumb
Selecting a self-generated pet in the Bot avatar picker always failed with "Could not load that pet — try another", and its tile showed only a name. Locally hatched pets have no petdex manifest entry, so pet.gallery reports an empty spritesheetUrl; the picker cropped frame 0 client-side from that URL and bailed on the empty string. The same raw CDN fetch also lacked a User-Agent, which the petdex CDN rejects with 403 (#90465), so manifest pets could fail on the same path. Route tile rendering and selection through the gateway's pet.thumb RPC, which already backs the Settings pet picker: it crops frame 0 server-side from the installed sheet on disk (or the host-validated CDN URL for uninstalled pets) and returns a same-origin PNG data URI. The client cache is keyed by slug, evicts failures so a blip never poisons a tile, and races a 15s deadline so a hung RPC cannot park a pending promise forever. Based on analysis from PR #90931, whose target (plugin.js) has since been decomposed into pet.tsx. Co-authored-by: m1k3s0 <44042865+m1k3s0@users.noreply.github.com>
This commit is contained in:
@@ -1,15 +1,16 @@
|
||||
/**
|
||||
* Pet tiles: frame 0 of a petdex spritesheet, extracted once and cached.
|
||||
* Pet tiles: frame 0 of a petdex spritesheet, cropped server-side by the
|
||||
* gateway's `pet.thumb` RPC and cached per slug.
|
||||
*
|
||||
* A "spritesheet" is the FULL animation sheet (1536×1872 webp, ~2MB, an 8×9
|
||||
* grid) — using it directly as an <img> downloads megabytes per tile and shows
|
||||
* the sheet squashed. So each sheet is fetched once, cropped to frame 0,
|
||||
* downscaled, and the resulting data URL is cached per URL.
|
||||
* Why the tile goes through the gateway rather than fetching the CDN sheet
|
||||
* itself: a locally hatched pet has no manifest entry, so `pet.gallery` reports
|
||||
* an EMPTY spritesheetUrl — a client-side fetch could never render or select
|
||||
* it ("Could not load that pet"). `pet.thumb` reads the installed sheet off
|
||||
* disk, so the slug alone is enough.
|
||||
*
|
||||
* The regression the cache created: a FAILED fetch was left parked in the
|
||||
* cache as a resolved-null promise, so one network blip poisoned that pet for
|
||||
* the rest of the session — the tile never recovered, not even on reopen. A
|
||||
* failure must be evicted; a success must not be refetched.
|
||||
* The regression the cache created: a FAILED load was left parked in the
|
||||
* cache as a resolved-null promise, so one blip poisoned that pet for the rest
|
||||
* of the session. A failure must be evicted; a success must not be re-requested.
|
||||
*/
|
||||
|
||||
import { fireEvent, render, waitFor } from '@testing-library/react'
|
||||
@@ -47,13 +48,18 @@ vi.mock('./i18n', () => ({
|
||||
vi.mock('./shared', () => ({ ID: 'hermes-bots' }))
|
||||
|
||||
const SHEET = 'https://pets.example/a.webp'
|
||||
const ICON = 'data:image/png;base64,ok'
|
||||
|
||||
/** Record every fetch so the cache behaviour is observable. */
|
||||
const fetches: Array<{ init?: RequestInit; url: string }> = []
|
||||
/** Every `pet.thumb` call, so the cache behaviour is observable. */
|
||||
const thumbs: Array<{ slug: string; url: string }> = []
|
||||
|
||||
function stubFetch(handler: () => Promise<unknown>) {
|
||||
vi.stubGlobal('fetch', async (url: string, init?: RequestInit) => {
|
||||
fetches.push({ init, url })
|
||||
function stubThumb(handler: () => Promise<{ dataUri?: string; ok: boolean }>) {
|
||||
hostMock.request.mockImplementation(async (method: string, params: { slug: string; url: string }) => {
|
||||
if (method !== 'pet.thumb') {
|
||||
throw new Error(`unexpected RPC ${method}`)
|
||||
}
|
||||
|
||||
thumbs.push(params)
|
||||
|
||||
return handler()
|
||||
})
|
||||
@@ -67,23 +73,17 @@ async function loadPetTab() {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
fetches.length = 0
|
||||
thumbs.length = 0
|
||||
useQueryMock.mockReturnValue({ data: { pets: [{ displayName: 'Axolotl', slug: 'axolotl', spritesheetUrl: SHEET }] } })
|
||||
vi.stubGlobal('createImageBitmap', async () => ({ close: () => undefined }))
|
||||
vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue({
|
||||
drawImage: () => undefined
|
||||
} as unknown as CanvasRenderingContext2D)
|
||||
vi.spyOn(HTMLCanvasElement.prototype, 'toDataURL').mockReturnValue('data:image/png;base64,ok')
|
||||
stubThumb(async () => ({ ok: true, dataUri: ICON }))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
describe('the pet gallery', () => {
|
||||
it('reserves paint space around boundary tiles inside the bounded scroller', async () => {
|
||||
stubFetch(async () => ({ blob: async () => new Blob() }))
|
||||
const PetTab = await loadPetTab()
|
||||
const view = render(<PetTab image={null} onImage={vi.fn()} />)
|
||||
await waitFor(() => expect(view.container.querySelector('img')).toBeTruthy())
|
||||
@@ -112,13 +112,12 @@ describe('the pet gallery', () => {
|
||||
}))
|
||||
}
|
||||
})
|
||||
stubFetch(async () => ({ blob: async () => new Blob() }))
|
||||
const PetTab = await loadPetTab()
|
||||
const onImage = vi.fn()
|
||||
const view = render(<PetTab image={null} onImage={onImage} />)
|
||||
const first = view.getByText('Pet 0').closest('button')!
|
||||
fireEvent.click(first)
|
||||
await waitFor(() => expect(onImage).toHaveBeenCalledWith('data:image/png;base64,ok'))
|
||||
await waitFor(() => expect(onImage).toHaveBeenCalledWith(ICON))
|
||||
const scroller = first.parentElement!.parentElement!
|
||||
Object.defineProperties(scroller, {
|
||||
clientHeight: { value: 220 },
|
||||
@@ -138,75 +137,56 @@ describe('the pet gallery', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('the sprite-frame cache', () => {
|
||||
it('never leaves a failed fetch parked in the cache', async () => {
|
||||
stubFetch(async () => {
|
||||
throw new Error('network')
|
||||
describe('the pet thumb cache', () => {
|
||||
it('selects a locally hatched pet that has no spritesheet URL', async () => {
|
||||
// Generator-hatched pets are absent from the petdex manifest, so the
|
||||
// gallery reports spritesheetUrl: "". The gateway crops the installed
|
||||
// sheet off disk — the slug is the identity, not the URL.
|
||||
useQueryMock.mockReturnValue({
|
||||
data: { pets: [{ displayName: 'Mine', installed: true, slug: 'mine', spritesheetUrl: '' }] }
|
||||
})
|
||||
|
||||
const PetTab = await loadPetTab()
|
||||
const onImage = vi.fn()
|
||||
const view = render(<PetTab image={null} onImage={onImage} />)
|
||||
|
||||
fireEvent.click(view.getByText('Mine').closest('button')!)
|
||||
await waitFor(() => expect(onImage).toHaveBeenCalledWith(ICON))
|
||||
expect(thumbs.length).toBeGreaterThan(0)
|
||||
expect(thumbs.every(call => call.slug === 'mine')).toBe(true)
|
||||
expect(hostMock.notify).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('never leaves a failed load parked in the cache', async () => {
|
||||
stubThumb(async () => {
|
||||
throw new Error('gateway')
|
||||
})
|
||||
|
||||
const PetTab = await loadPetTab()
|
||||
const first = render(<PetTab image={null} onImage={vi.fn()} />)
|
||||
|
||||
await waitFor(() => expect(fetches).toHaveLength(1))
|
||||
// The fetch is abortable: a hung sheet must not hold a slot forever.
|
||||
expect(fetches[0].init?.signal).toBeTruthy()
|
||||
|
||||
await waitFor(() => expect(thumbs).toHaveLength(1))
|
||||
first.unmount()
|
||||
|
||||
const second = render(<PetTab image={null} onImage={vi.fn()} />)
|
||||
|
||||
// Reopening retries rather than serving the poisoned null.
|
||||
await waitFor(() => expect(fetches).toHaveLength(2))
|
||||
await waitFor(() => expect(thumbs).toHaveLength(2))
|
||||
expect(second.container.querySelector('img')).toBeNull()
|
||||
})
|
||||
|
||||
it('fetches a successful sheet once and reuses the extracted frame', async () => {
|
||||
stubFetch(async () => ({ blob: async () => new Blob() }))
|
||||
|
||||
it('requests a successful thumb once and reuses it across mounts', async () => {
|
||||
const PetTab = await loadPetTab()
|
||||
const first = render(<PetTab image={null} onImage={vi.fn()} />)
|
||||
|
||||
await waitFor(() =>
|
||||
expect(first.container.querySelector('img')?.getAttribute('src')).toBe('data:image/png;base64,ok')
|
||||
)
|
||||
expect(fetches).toHaveLength(1)
|
||||
await waitFor(() => expect(first.container.querySelector('img')?.getAttribute('src')).toBe(ICON))
|
||||
expect(thumbs).toHaveLength(1)
|
||||
|
||||
first.unmount()
|
||||
|
||||
const second = render(<PetTab image={null} onImage={vi.fn()} />)
|
||||
|
||||
await waitFor(() =>
|
||||
expect(second.container.querySelector('img')?.getAttribute('src')).toBe('data:image/png;base64,ok')
|
||||
)
|
||||
expect(fetches).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('shares one fetch between tiles that point at the same sheet', async () => {
|
||||
useQueryMock.mockReturnValue({
|
||||
data: {
|
||||
pets: [
|
||||
{ displayName: 'Axolotl', slug: 'axolotl', spritesheetUrl: SHEET },
|
||||
{ displayName: 'Axolotl (shiny)', slug: 'axolotl-shiny', spritesheetUrl: SHEET }
|
||||
]
|
||||
}
|
||||
})
|
||||
stubFetch(async () => ({ blob: async () => new Blob() }))
|
||||
|
||||
const PetTab = await loadPetTab()
|
||||
const { container } = render(<PetTab image={null} onImage={vi.fn()} />)
|
||||
|
||||
await waitFor(() => expect(container.querySelectorAll('img')).toHaveLength(2))
|
||||
expect(fetches).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('does not fetch for a pet with no spritesheet', async () => {
|
||||
useQueryMock.mockReturnValue({ data: { pets: [{ displayName: 'Ghost', slug: 'ghost', spritesheetUrl: null }] } })
|
||||
stubFetch(async () => ({ blob: async () => new Blob() }))
|
||||
|
||||
const PetTab = await loadPetTab()
|
||||
const { findByText } = render(<PetTab image={null} onImage={vi.fn()} />)
|
||||
|
||||
await findByText('Ghost')
|
||||
expect(fetches).toHaveLength(0)
|
||||
await waitFor(() => expect(second.container.querySelector('img')?.getAttribute('src')).toBe(ICON))
|
||||
expect(thumbs).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -12,80 +12,71 @@ import { ID } from './shared'
|
||||
// ── pet tab: attach a petdex companion that lives beside the avatar ─────────
|
||||
|
||||
// A petdex "spritesheet" is the FULL animation sheet (1536×1872 webp, ~2MB;
|
||||
// 8×9 grid of 192×208 frames). Using it as an <img> both downloads megabytes
|
||||
// per tile and shows the whole sheet squashed. Extract frame 0 once per slug
|
||||
// via canvas, downscale to 96px, and cache the data URL. Concurrency-capped
|
||||
// so opening the tab doesn't fire dozens of 2MB fetches at once.
|
||||
const PET_FRAME_W = 192
|
||||
const PET_FRAME_H = 208
|
||||
// 8×9 grid of 192×208 frames). Cropping frame 0 client-side meant downloading
|
||||
// megabytes per tile, a UA-less CDN fetch the petdex CDN rejects with 403
|
||||
// (#90465), and it could never serve pets hatched locally — those are absent
|
||||
// from the petdex manifest, so `pet.gallery` reports an EMPTY spritesheetUrl.
|
||||
// The gateway's `pet.thumb` crops + downsamples frame 0 server-side (installed
|
||||
// sheet off disk, else the host-validated CDN URL) and returns a small PNG data
|
||||
// URI: one path for every pet, same-origin through the authenticated gateway.
|
||||
//
|
||||
// The gallery is 4500+ pets browsed 24 at a time, and each entry is a decoded
|
||||
// PNG data URL — an unbounded cache holds every pet the user ever scrolled
|
||||
// past for the life of the window. Five pages' worth keeps scrolling back up
|
||||
// instant; past that a revisit pays the fetch and crop again.
|
||||
const PET_FRAME_CACHE_MAX = 120
|
||||
const petFrameCache = new LruCache<string, Promise<null | string>>(PET_FRAME_CACHE_MAX)
|
||||
let petFetchActive = 0
|
||||
const petFetchQueue: Array<() => Promise<void>> = []
|
||||
// instant; past that a revisit pays the RPC again.
|
||||
const PET_THUMB_CACHE_MAX = 120
|
||||
// A hung gateway call must not park a pending promise in the cache forever.
|
||||
const PET_THUMB_TIMEOUT_MS = 15000
|
||||
const petThumbCache = new LruCache<string, Promise<null | string>>(PET_THUMB_CACHE_MAX)
|
||||
|
||||
function pumpPetQueue() {
|
||||
while (petFetchActive < 4 && petFetchQueue.length) {
|
||||
const job = petFetchQueue.shift()!
|
||||
petFetchActive++
|
||||
job().finally(() => {
|
||||
petFetchActive--
|
||||
pumpPetQueue()
|
||||
})
|
||||
}
|
||||
interface PetThumbResult {
|
||||
dataUri?: string
|
||||
ok: boolean
|
||||
}
|
||||
|
||||
function petFrameIcon(spriteUrl: null | string | undefined): Promise<null | string> {
|
||||
if (!spriteUrl) {
|
||||
function petThumbIcon(slug: string, spriteUrl: null | string | undefined): Promise<null | string> {
|
||||
if (!slug) {
|
||||
return Promise.resolve(null)
|
||||
}
|
||||
|
||||
if (!petFrameCache.has(spriteUrl)) {
|
||||
petFrameCache.set(
|
||||
spriteUrl,
|
||||
new Promise(resolve => {
|
||||
petFetchQueue.push(async () => {
|
||||
try {
|
||||
const resp = await fetch(spriteUrl, {
|
||||
signal: AbortSignal.timeout(15000)
|
||||
})
|
||||
if (!petThumbCache.has(slug)) {
|
||||
const deadline = new Promise<null>(resolve => setTimeout(() => resolve(null), PET_THUMB_TIMEOUT_MS))
|
||||
|
||||
const blob = await resp.blob()
|
||||
// Crop frame 0 during decode — never materialize the full sheet.
|
||||
const bitmap = await createImageBitmap(blob, 0, 0, PET_FRAME_W, PET_FRAME_H)
|
||||
const canvas = document.createElement('canvas')
|
||||
canvas.width = 96
|
||||
canvas.height = 104
|
||||
canvas.getContext('2d')!.drawImage(bitmap, 0, 0, 96, 104)
|
||||
bitmap.close()
|
||||
resolve(canvas.toDataURL('image/png'))
|
||||
} catch {
|
||||
petFrameCache.delete(spriteUrl)
|
||||
resolve(null)
|
||||
}
|
||||
})
|
||||
pumpPetQueue()
|
||||
const pending = Promise.race([
|
||||
host
|
||||
.request<PetThumbResult>('pet.thumb', { slug, url: spriteUrl || '' })
|
||||
.then(result => (result?.ok && result.dataUri ? result.dataUri : null))
|
||||
.catch(() => null),
|
||||
deadline
|
||||
])
|
||||
.then(icon => {
|
||||
// Never cache a failure: a transient backend error must not poison
|
||||
// the tile for the rest of the session.
|
||||
if (!icon) {
|
||||
petThumbCache.delete(slug)
|
||||
}
|
||||
|
||||
return icon
|
||||
})
|
||||
)
|
||||
|
||||
petThumbCache.set(slug, pending)
|
||||
}
|
||||
|
||||
return petFrameCache.get(spriteUrl)!
|
||||
return petThumbCache.get(slug)!
|
||||
}
|
||||
|
||||
interface PetThumbProps {
|
||||
size?: number
|
||||
slug: string
|
||||
spriteUrl?: null | string
|
||||
}
|
||||
|
||||
/** One pet tile image: frame 0 only, resolved lazily through the cache. */
|
||||
function PetThumb({ spriteUrl, size = 40 }: PetThumbProps) {
|
||||
/** One pet tile image: server-cropped frame 0, resolved lazily through the cache. */
|
||||
function PetThumb({ slug, spriteUrl, size = 40 }: PetThumbProps) {
|
||||
const [icon, setIcon] = useState<null | string>(null)
|
||||
useEffect(() => {
|
||||
let alive = true
|
||||
petFrameIcon(spriteUrl).then(url => {
|
||||
petThumbIcon(slug, spriteUrl).then(url => {
|
||||
if (alive) {
|
||||
setIcon(url)
|
||||
}
|
||||
@@ -94,7 +85,7 @@ function PetThumb({ spriteUrl, size = 40 }: PetThumbProps) {
|
||||
return () => {
|
||||
alive = false
|
||||
}
|
||||
}, [spriteUrl])
|
||||
}, [slug, spriteUrl])
|
||||
|
||||
if (!icon) {
|
||||
return (
|
||||
@@ -130,7 +121,7 @@ interface PetGalleryEntry {
|
||||
displayName?: string
|
||||
installed?: boolean
|
||||
slug: string
|
||||
/** Full animation sheet (1536×1872 webp); frame 0 is cropped out of it. */
|
||||
/** Full animation sheet (1536×1872 webp); empty for locally hatched pets. */
|
||||
spritesheetUrl?: null | string
|
||||
}
|
||||
|
||||
@@ -244,11 +235,12 @@ export function PetTab({ image, onImage }: PetTabProps) {
|
||||
)}
|
||||
key={pet.slug}
|
||||
onClick={() => {
|
||||
// The pet IS the profile picture: extract frame 0
|
||||
// and hand it to the dialog as the avatar image.
|
||||
// The pet IS the profile picture: the server-cropped frame 0
|
||||
// becomes the dialog's avatar image (works for locally
|
||||
// hatched pets too — they have no spritesheet URL at all).
|
||||
// Persisted when the user hits Save.
|
||||
setSelectedSlug(pet.slug)
|
||||
void petFrameIcon(pet.spritesheetUrl).then(icon => {
|
||||
void petThumbIcon(pet.slug, pet.spritesheetUrl).then(icon => {
|
||||
if (icon) {
|
||||
onImage(icon)
|
||||
} else {
|
||||
@@ -261,7 +253,7 @@ export function PetTab({ image, onImage }: PetTabProps) {
|
||||
})
|
||||
}}
|
||||
>
|
||||
<PetThumb size={40} spriteUrl={pet.spritesheetUrl} />
|
||||
<PetThumb size={40} slug={pet.slug} spriteUrl={pet.spritesheetUrl} />
|
||||
<span className="w-full truncate text-center text-[0.6rem] text-(--ui-text-tertiary)">
|
||||
{pet.displayName}
|
||||
</span>
|
||||
|
||||
Reference in New Issue
Block a user