From 1f4c55cf82dff3162e103ce80d57a991d7b3a2a2 Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Wed, 19 Aug 2026 18:50:47 +0200 Subject: [PATCH 1/2] fix(client): let a face-down marker request pass the empty-name guard (#7549) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #7535 marker request deliberately carries no card name and no oracle id — only `tokenImageRef` names the printing. `useCardImage` short- circuited on exactly that shape (effect guard AND first-render snapshot), so `fetchTokenImageByRef` was unreachable and every face-down permanent fell back to the generic card back: the merged marker feature never worked in the live client. Both guards now let a present `tokenImageRef` through. The regression exercises the REAL hook (service layer stubbed at fetch, hook logic live) — the shipped component tests stubbed the hook itself, which is how a dead feature stayed green. Red-first verified: the row fails on the pre-fix guards, passes with them widened. Found by live playtest (the marker art data was present and correct all along — `scryfall-token-images.json` carries all three oracle keys with usable URLs). Co-Authored-By: Claude Fable 5 --- .../src/hooks/__tests__/useCardImage.test.tsx | 39 +++++++++++++++++++ client/src/hooks/useCardImage.ts | 8 +++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/client/src/hooks/__tests__/useCardImage.test.tsx b/client/src/hooks/__tests__/useCardImage.test.tsx index c2d35fcc4c..c862aeb819 100644 --- a/client/src/hooks/__tests__/useCardImage.test.tsx +++ b/client/src/hooks/__tests__/useCardImage.test.tsx @@ -201,4 +201,43 @@ describe("useCardImage", () => { }); expect(result.current.src).toBe("second.png"); }); + it("resolves a face-down marker from tokenImageRef alone — no name, no oracle id (#7549)", async () => { + // The #7535 marker request shape: cardName "" and only the ref naming the + // printing. The hook must NOT short-circuit on the empty name — that + // short-circuit is exactly what kept the merged marker feature from ever + // loading in the live client (the component tests stubbed this hook, so + // only a REAL-hook regression can hold the line). + vi.stubGlobal("fetch", vi.fn((url: string) => { + if (url === "/scryfall-token-images.json") { + return Promise.resolve(jsonResponse({ + "oracle:8f92f8d7-ec89-426f-86dc-fbc259eb5559:morph": { + scryfall_id: "morph-token-dtk", + oracle_id: "8f92f8d7-ec89-426f-86dc-fbc259eb5559", + face_names: ["morph"], + faces: [{ normal: "https://img.example/morph.jpg", art_crop: "https://img.example/morph-art.jpg" }], + name: "Morph", + layout: "token", + }, + })); + } + return Promise.resolve(jsonResponse({})); + })); + + const { useCardImage } = await import("../useCardImage"); + const { result } = renderHook(() => + useCardImage("", { + size: "normal", + isToken: true, + tokenImageRef: { + scryfall_id: "", + scryfall_oracle_id: "8f92f8d7-ec89-426f-86dc-fbc259eb5559", + face_name: "morph", + preset_id: "face-down-morph", + }, + }), + ); + + await waitFor(() => expect(result.current.src).toBe("https://img.example/morph.jpg")); + }); + }); diff --git a/client/src/hooks/useCardImage.ts b/client/src/hooks/useCardImage.ts index 33b816b153..4434ec1e3b 100644 --- a/client/src/hooks/useCardImage.ts +++ b/client/src/hooks/useCardImage.ts @@ -572,7 +572,11 @@ export function useCardImage( return; } - if (!cardName && !oracleId) { + // A face-down marker request carries NO name and NO oracle id — only the + // `tokenImageRef` names the printing. Bailing on the empty name here was + // what kept the #7535 markers from ever loading at runtime (#7549): the + // ref-driven fetch below never ran. + if (!cardName && !oracleId && !stableTokenImageRef) { setStateRequestKey(requestKey); setSrc(null); setIsRotated(false); @@ -666,7 +670,7 @@ export function useCardImage( isFlip: isCardImageFlipLayoutSync(resolvedOracleId, cardName), }; } - if (!cardName && !oracleId) { + if (!cardName && !oracleId && !stableTokenImageRef) { return { src: null, isLoading: false, isRotated: false, isFlip: false }; } const cachedEntry = imageRequestCache.get(requestKey); From b7f8612cb052309adb4c07fd1db1f7c729667484 Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:38:52 +0200 Subject: [PATCH 2/2] fix(client): treat a token ref with no ids as no ref (#7549) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review on #7550: a TokenImageRef whose scryfall_id AND scryfall_oracle_id are both empty held the widened empty-name guards open, so a request with no name, no oracle id and an unresolvable ref fell through to fetchTokenImageUrl("") — a `t:token !""` junk search. The guards now key on resolvableTokenImageRef (either id non-empty); our face-down markers carry only an oracle id (empty scryfall_id) and keep passing. Red-first: the new real-hook row (empty-ids ref -> no fetch at all, src null, not loading) fails on the old guards and passes now; the #7549 marker row and the unusable-ref name-fallback row are unchanged. Co-Authored-By: Claude Fable 5 --- .../src/hooks/__tests__/useCardImage.test.tsx | 43 +++++++++++++++++++ client/src/hooks/useCardImage.ts | 15 ++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/client/src/hooks/__tests__/useCardImage.test.tsx b/client/src/hooks/__tests__/useCardImage.test.tsx index c862aeb819..07a5245e36 100644 --- a/client/src/hooks/__tests__/useCardImage.test.tsx +++ b/client/src/hooks/__tests__/useCardImage.test.tsx @@ -240,4 +240,47 @@ describe("useCardImage", () => { await waitFor(() => expect(result.current.src).toBe("https://img.example/morph.jpg")); }); + it("fetches nothing for a token ref that names no printing — both ids empty (#7550 review)", async () => { + // A `TokenImageRef` is only a pointer when it carries at least one id. + // With BOTH `scryfall_id` and `scryfall_oracle_id` empty there is nothing + // to resolve — the request must short-circuit exactly like the empty-name + // case, not fall through to a `fetchTokenImageUrl("")` junk search. + const fetchTokenImageByRef = vi.fn().mockResolvedValue(null); + const fetchTokenImageUrl = vi.fn().mockResolvedValue(null); + vi.doMock("../../services/scryfall.ts", () => ({ + fetchCardImageAsset: vi.fn(), + fetchCardImageAssetByOracleId: vi.fn(), + fetchCardImageByOracleId: vi.fn(), + fetchCardImageUrl: vi.fn(), + fetchTokenImageByRef, + fetchTokenImageUrl, + findPrintingById: vi.fn(), + getCardPrintings: vi.fn().mockResolvedValue([]), + isCardImageRotatedSync: vi.fn().mockReturnValue(false), + isLocaleArtReady: vi.fn().mockReturnValue(true), + loadLocaleArt: vi.fn().mockResolvedValue(new Map()), + resolveFaceIndexSync: vi.fn().mockReturnValue(null), + resolveOracleIdSync: vi.fn().mockReturnValue(null), + resolvePrintingImageUrl: vi.fn(), + })); + + const { useCardImage } = await import("../useCardImage"); + const { result } = renderHook(() => + useCardImage("", { + size: "normal", + isToken: true, + tokenImageRef: { + scryfall_id: "", + scryfall_oracle_id: "", + preset_id: "face-down-morph", + }, + }), + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.src).toBeNull(); + expect(fetchTokenImageByRef).not.toHaveBeenCalled(); + expect(fetchTokenImageUrl).not.toHaveBeenCalled(); + }); + }); diff --git a/client/src/hooks/useCardImage.ts b/client/src/hooks/useCardImage.ts index 4434ec1e3b..8e12265ad3 100644 --- a/client/src/hooks/useCardImage.ts +++ b/client/src/hooks/useCardImage.ts @@ -442,6 +442,16 @@ export function useCardImage( // blinding the dependency check on the large effect below. // eslint-disable-next-line react-hooks/exhaustive-deps const stableTokenImageRef = useMemo(() => tokenImageRef, [tokenImageRefKey]); + // A token ref is only a pointer when it names a printing: with BOTH ids + // empty there is nothing to resolve, so such a ref must not hold the + // empty-name guards open — the request would fall through to a + // `fetchTokenImageUrl("")` junk search. Our face-down markers carry only an + // oracle id (empty `scryfall_id`), so either id keeps the guard open. + const resolvableTokenImageRef = + stableTokenImageRef && + (stableTokenImageRef.scryfall_id || stableTokenImageRef.scryfall_oracle_id) + ? stableTokenImageRef + : null; const oracleId = options?.oracleId ?? ""; const faceName = options?.faceName ?? ""; const scryfallId = options?.scryfallId ?? ""; @@ -576,7 +586,7 @@ export function useCardImage( // `tokenImageRef` names the printing. Bailing on the empty name here was // what kept the #7535 markers from ever loading at runtime (#7549): the // ref-driven fetch below never ran. - if (!cardName && !oracleId && !stableTokenImageRef) { + if (!cardName && !oracleId && !resolvableTokenImageRef) { setStateRequestKey(requestKey); setSrc(null); setIsRotated(false); @@ -646,6 +656,7 @@ export function useCardImage( filterPower, filterSubtypes, filterToughness, + resolvableTokenImageRef, stableTokenImageRef, tokenImageRefKey, isToken, @@ -670,7 +681,7 @@ export function useCardImage( isFlip: isCardImageFlipLayoutSync(resolvedOracleId, cardName), }; } - if (!cardName && !oracleId && !stableTokenImageRef) { + if (!cardName && !oracleId && !resolvableTokenImageRef) { return { src: null, isLoading: false, isRotated: false, isFlip: false }; } const cachedEntry = imageRequestCache.get(requestKey);