From a4a8f22a5bd045602a16c75b8ebd397ccc60302c Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:01:19 +0200 Subject: [PATCH 1/2] fix(scripts): index token printings Scryfall does not label by layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Role tokens (Wilds of Eldraine: Wicked, Cursed, Royal, Young Hero, Monster, Virtuous, Sorcerer) rendered with no art. Scryfall prints the two Roles on one card with the second upside down, so it classifies all 6 printings as `layout: "flip"` — not "token" or "double_faced_token". The generator selected on `layout` alone, so `scryfall-token-images.json` carried zero Role entries and all 12 Role presets in known-tokens.toml had a `token_image_ref` that resolved to nothing in `fetchTokenImageByRef`. `is_token_printing` keeps the two layout values as a fast path and adds a layout-agnostic clause: a printing whose type line marks it as a token is a token, whatever Scryfall calls its layout. Roles are not alone out there — the Secret Lair `Mechtitan // Mechtitan` is `layout: "reversible_card"` with a null top-level type line and token-typed faces, and the two presets pointing at it were silently degrading to the `tneo` printing's art through the `oracle:` fallback. Gating the clause on "flip" would have fixed the Role instance and left the class. `type_line` is the discriminator rather than `set_type` because two of the twelve Role presets point at the Wicked // Cursed printing in The List (`set_type: "masters"`), and rather than `layout` because that would pull the genuine flip cards (21 cards, 39 printings — Budoka Gardener, Erayo, ...) into a table of tokens. Both the top-level and per-face type lines are tested: 81 of 120 `double_faced_token` entries have no "Token "-prefixed top-level `type_line` (62 read "Card // Card") and the Mechtitan printing has none at all. `validate_token_images` is the single authority for "is this table healthy", and all three gates now use it: * **The skip-guard.** Skipping on existence alone is what let this table stay wrong — any checkout, cache or dev machine already holding a Role-less file would never regenerate it, so the fix would reach nobody who had run the script before. It now skips only a table that still passes the check; a stale or truncated one costs 4s to rebuild. * **A pre-install gate**, in the shape gen-scryfall-locale-images.sh uses for its own empty classes, with a message separating the benign cause from the real one. Nothing ever noticed the class was empty; the table stayed plausible and the art simply never appeared. * **The recovery branch** inside `scryfall_finalize_download`, where the generic "is this JSON" would accept an empty table that beat us into place. The check is arithmetic, not a string compare against "0": jq exits 0 with EMPTY stdout on a 0-byte or whitespace-only input, so `|| echo 0` never fires and `[ "" = 0 ]` reads as healthy. Both gates in this file got that wrong once during review. Generation also builds into a `.tmp` staging file (the suffix `.gitignore` already covers here) under a trap and installs it with the shared `scryfall_finalize_download`, instead of truncating the output before jq runs. Also gitignores the finalizer's rename diagnostic: this script is its first caller to stage into a tracked directory. Measured against the shipped table: 17 keys added, 0 removed, 0 changed. Of the 2869 presets carrying a `token_image_ref`, all 2869 now resolve on their exact `scryfall:` key — previously 12 missed entirely and 2 fell back to another printing's art. Both faces of a flip pair share one `image_uris` — the printed card is a single image — so both Roles resolve to it, unrotated, exactly as this codebase already renders every other flip card. Co-Authored-By: Claude Opus 5 --- .gitignore | 3 + scripts/gen-scryfall-token-images.sh | 127 ++++++++++++++++++++++++++- 2 files changed, 126 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 8188a1e52e..913984c62f 100644 --- a/.gitignore +++ b/.gitignore @@ -56,6 +56,9 @@ client/public/parser-warning-patterns.json # Atomic-write staging files from gen-card-data.sh (cleaned on normal exit, # but may linger after SIGKILL / power loss). client/public/*.tmp +# scryfall_finalize_download stages its rename diagnostic beside the target; +# gen-scryfall-token-images.sh is the first caller to aim it at a tracked dir. +client/public/*.mv-error.* # Coverage client/coverage/ diff --git a/scripts/gen-scryfall-token-images.sh b/scripts/gen-scryfall-token-images.sh index 4b7fbd104f..2107ac819b 100755 --- a/scripts/gen-scryfall-token-images.sh +++ b/scripts/gen-scryfall-token-images.sh @@ -8,6 +8,30 @@ DATA_DIR="data/scryfall" CARDS_FILE="$DATA_DIR/default-cards.json" OUTPUT="client/public/scryfall-token-images.json" +# Printings the token `layout` values do not carry — everything the type-line +# clause of `is_token_printing` below contributes. Zero of them means the table +# is empty or has lost the class; see the block comment on the pre-install gate. +count_off_layout_tokens() { + jq '[.[] + | select((.layout == "token" or .layout == "double_faced_token") | not) + | .scryfall_id] | unique | length' "$1" +} + +# The single authority for "is this table healthy", used by all three gates in +# this script: the skip-guard, the pre-install check, and the recovery branch +# inside scryfall_finalize_download (where the generic "is this JSON" would +# otherwise accept an empty table that beat us into place). +# +# `|| echo 0` would NOT normalise the failure, and neither does a string compare +# against "0": jq exits 0 with EMPTY stdout on a 0-byte or whitespace-only +# input, so the fallback never fires and `[ "" = 0 ]` reads as healthy. Both +# checks in this file got that wrong once. Keep the arithmetic compare. +validate_token_images() { + local count + count=$(count_off_layout_tokens "$1" 2>/dev/null) || count=0 + [ "${count:-0}" -gt 0 ] +} + echo "=== Scryfall Token Image Generation ===" if [ ! -f "$CARDS_FILE" ]; then @@ -17,7 +41,13 @@ if [ ! -f "$CARDS_FILE" ]; then echo "Downloaded $CARDS_FILE." fi -if [ -f "$OUTPUT" ]; then +# The sibling generators skip on existence alone. That is what let this table +# stay wrong: a checkout, cache or dev machine already holding a Role-less file +# never regenerates it, and this fix would reach nobody who had run the script +# before. Skip only a table that still passes the health check — a stale or +# truncated one costs 4s to rebuild, and a genuinely empty class fails loudly +# at the gate below instead of being certified here. +if [ -f "$OUTPUT" ] && validate_token_images "$OUTPUT"; then echo "Skipping generation — $OUTPUT already exists (delete to regenerate)." exit 0 fi @@ -25,9 +55,68 @@ fi echo "Generating $OUTPUT..." mkdir -p "$(dirname "$OUTPUT")" +# `layout` alone does not enumerate the token printings. +# +# Role tokens (Wilds of Eldraine) are printed two-to-a-card with the second +# Role upside down, so Scryfall classifies all 6 printings as "flip" rather +# than "token". They fell through the layout filter, so every Role preset in +# known-tokens.toml carried a `token_image_ref` that resolved to nothing and +# the token rendered artless in play. +# +# `is_token_printing` therefore keeps the two layout values as a fast path and +# adds a layout-agnostic third clause: a printing whose type line marks it as a +# token is a token, whatever Scryfall calls its layout. Three details are +# load-bearing: +# +# * The clause is deliberately NOT conjoined with `layout == "flip"`. Roles +# are not alone out here — the Secret Lair Mechtitan is +# `layout: "reversible_card"` with a null top-level type_line and +# token-typed faces, and two presets point at it. Gating on "flip" would +# fix the Role instance and leave the class, which is the shape of bug +# this branch exists to end. Today it admits exactly 7 printings that the +# two layout values do not (the type-line test itself is true for 2797). +# * The type line is what keeps the genuine flip *cards* (21 of them, +# 39 printings — Budoka Gardener, Erayo, Akki Lavarunner, ...) out of a +# table of tokens. `set_type` cannot replace it: two of the twelve Role +# presets point at the Wicked // Cursed printing in The List, whose +# `set_type` is "masters", not "token". +# * Both the top-level and the per-face type lines are tested, because +# Scryfall does not reliably put the token marker on the top-level one: +# 81 of 120 `double_faced_token` entries lack a "Token "-prefixed +# `type_line` (62 read "Card // Card"), and the Mechtitan printing above +# has none at all. Those survive on `layout` today, but a top-level-only +# test would silently drop the next multi-face token that has to come in +# through this clause. +# +# Build into a temp and install it atomically, the same temp+rename the bulk +# downloads in lib/scryfall-fetch.sh use. Writing straight to $OUTPUT truncates +# it before jq runs, so an interrupt or an OOM on the 600 MB+ input leaves a +# 0-byte file that the skip-guard at the top of this script then certifies as +# done on every later run — the silent-empty-table failure this script has +# already had once, reached through the other door. +# +# The `.tmp` suffix is what `.gitignore` covers for staging files in this +# directory (`client/public/*.tmp`, added for gen-card-data.sh); without it an +# interrupted run leaves an untracked 2 MB file in a tracked tree. `mktemp` +# creates at 0600; the mode is pinned to 644 because this file is served as a +# static asset alongside the other sidecars in client/public, and that must not +# depend on the umask of whoever ran the build. +TMP=$(mktemp "$OUTPUT.XXXXXX.tmp") +trap 'rm -f "$TMP"' EXIT INT TERM +chmod 644 "$TMP" + +# NOTE: the jq program below is a single-quoted shell string. An apostrophe +# anywhere inside it, comments included, terminates the string. + jq -c "$SCRYFALL_JQ_PRELUDE"' + def is_token_printing: + .layout == "token" + or .layout == "double_faced_token" + or ([.type_line?, (.card_faces[]?.type_line?)] + | any(.[] | strings; startswith("Token "))); + [.[] | - select(.layout == "token" or .layout == "double_faced_token") | + select(is_token_printing) | select(.id != null) | . as $card | { @@ -59,8 +148,38 @@ jq -c "$SCRYFALL_JQ_PRELUDE"' else [] end )[] ] | from_entries -' "$CARDS_FILE" > "$OUTPUT" +' "$CARDS_FILE" > "$TMP" + +# Everything the type-line clause contributes over the two layout values — the +# entries that carry no token `layout` at all. The Roles went unindexed for the +# life of this file because nothing ever noticed the class was empty: the table +# stayed plausible and the art simply never appeared. Fail generation instead, +# in the shape gen-scryfall-locale-images.sh uses for its own empty classes — +# an empty class means an upstream shape change to look at, not that the class +# stopped existing. +# +# This is a presence check, not a completeness one. Asserting that every +# `token_image_ref` in known-tokens.toml resolves is the stronger invariant and +# does hold as of this commit — but it does not belong in a deploy-blocking +# build step. known-tokens.toml is generated by tokens-gen from MTGJSON, while +# this table comes from a day-cached Scryfall snapshot; a new set whose tokens +# reach MTGJSON first would fail the assertion on the new refs with nothing +# actually wrong. That check belongs in a reporting job. A partial upstream loss +# therefore still gets through here. +if ! validate_token_images "$TMP"; then + echo "ERROR: the type-line clause of is_token_printing selected nothing." >&2 + echo " Either Scryfall reclassified these printings into a token 'layout', in" >&2 + echo " which case confirm they still arrive and then drop the clause; or the" >&2 + echo " 'Token ' type-line prefix changed, in which case the Role tokens (Wilds" >&2 + echo " of Eldraine) and the Secret Lair Mechtitan are about to render artless" >&2 + echo " again. Do not just delete this check." >&2 + exit 1 +fi +TYPE_LINE_COUNT=$(count_off_layout_tokens "$TMP") + +scryfall_finalize_download "$TMP" "$OUTPUT" validate_token_images +trap - EXIT INT TERM ENTRY_COUNT=$(jq 'length' "$OUTPUT") FILE_SIZE=$(du -h "$OUTPUT" | cut -f1) -echo "Generated $OUTPUT ($FILE_SIZE, $ENTRY_COUNT entries)" +echo "Generated $OUTPUT ($FILE_SIZE, $ENTRY_COUNT entries, $TYPE_LINE_COUNT via type line)" From 7ab197b156e218a78b7792433060ceda35048345 Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Thu, 13 Aug 2026 15:15:34 +0200 Subject: [PATCH 2/2] fix(scripts): stage through a portable mktemp template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mkstemp(3) is EINVAL unless the template ends in XXXXXX; accepting a suffix after the X run is a GNU extension, documented as such under GNU mktemp(1) --suffix. `$OUTPUT.XXXXXX.tmp` therefore aborts this generator under `set -e` on BSD and macOS, before it can index anything at all — including the Role printings this branch exists to restore. Use `$OUTPUT.tmp.XXXXXX`, the form gen-card-data.sh:248 and lib/mtgjson-fetch.sh:70 already stage with, and add the `client/public/*.tmp.*` ignore rule that name needs: the existing `client/public/*.tmp` covers fixed-name staging files and does not match a randomised tail, so without it an interrupted run would leave an untracked 2 MB file in a tracked tree. No tracked path in client/public matches the new rule. Reported by @matthewevans in review of #7335. Co-Authored-By: Claude Opus 5 --- .gitignore | 4 ++++ scripts/gen-scryfall-token-images.sh | 26 +++++++++++++++++++------- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 913984c62f..11218c5004 100644 --- a/.gitignore +++ b/.gitignore @@ -56,6 +56,10 @@ client/public/parser-warning-patterns.json # Atomic-write staging files from gen-card-data.sh (cleaned on normal exit, # but may linger after SIGKILL / power loss). client/public/*.tmp +# The same, for callers that stage through mktemp and so carry a random tail +# after `.tmp` (gen-scryfall-token-images.sh). The template cannot be +# `.XXXXXX.tmp` instead: BSD/macOS mktemp rejects anything after the X run. +client/public/*.tmp.* # scryfall_finalize_download stages its rename diagnostic beside the target; # gen-scryfall-token-images.sh is the first caller to aim it at a tracked dir. client/public/*.mv-error.* diff --git a/scripts/gen-scryfall-token-images.sh b/scripts/gen-scryfall-token-images.sh index 2107ac819b..e0d2a2c48d 100755 --- a/scripts/gen-scryfall-token-images.sh +++ b/scripts/gen-scryfall-token-images.sh @@ -95,13 +95,25 @@ mkdir -p "$(dirname "$OUTPUT")" # done on every later run — the silent-empty-table failure this script has # already had once, reached through the other door. # -# The `.tmp` suffix is what `.gitignore` covers for staging files in this -# directory (`client/public/*.tmp`, added for gen-card-data.sh); without it an -# interrupted run leaves an untracked 2 MB file in a tracked tree. `mktemp` -# creates at 0600; the mode is pinned to 644 because this file is served as a -# static asset alongside the other sidecars in client/public, and that must not -# depend on the umask of whoever ran the build. -TMP=$(mktemp "$OUTPUT.XXXXXX.tmp") +# The random run has to be the LAST component of the template. Accepting a +# suffix after it is a GNU extension, documented as such in GNU mktemp(1) under +# `--suffix` ("implied if TEMPLATE does not end in X"); the underlying +# mkstemp(3) contract is EINVAL unless the final six characters are XXXXXX +# (checked directly against libc: `$OUTPUT.XXXXXX.tmp` gives errno 22, +# `$OUTPUT.tmp.XXXXXX` succeeds). `$OUTPUT.XXXXXX.tmp` was reported failing on +# macOS in review of #7335, a platform this repo ships desktop builds for +# (shell-release.yml:164). Trailing X is portable everywhere and is the house +# form here: gen-card-data.sh:248 and lib/mtgjson-fetch.sh:70 both stage so. +# +# `.gitignore` carries `client/public/*.tmp.*` for the name that produces. The +# existing `client/public/*.tmp` covers the fixed-name staging files in +# gen-card-data.sh and does not match a randomised tail; with no rule at all, an +# interrupted run leaves an untracked 2 MB file in a tracked tree. +# +# `mktemp` creates at 0600. The mode is pinned to 644 because this file is +# served as a static asset alongside the other sidecars in client/public, and +# that must not depend on the umask of whoever ran the build. +TMP=$(mktemp "$OUTPUT.tmp.XXXXXX") trap 'rm -f "$TMP"' EXIT INT TERM chmod 644 "$TMP"