Skip to content

Fix bulk card image downloader to use Scryfall CDN#10928

Open
phughk wants to merge 4 commits into
Card-Forge:masterfrom
phughk:fix-10413-card-image-downloader
Open

Fix bulk card image downloader to use Scryfall CDN#10928
phughk wants to merge 4 commits into
Card-Forge:masterfrom
phughk:fix-10413-card-image-downloader

Conversation

@phughk

@phughk phughk commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Problem

Forge fetches card images by hitting api.scryfall.com/cards/{set}/{cn} once per card. That endpoint is rate-limited. When many users run the game simultaneously, the flood of per-card requests causes Scryfall to return HTTP 429 errors and eventually block the application entirely — a problem reported on the PR thread in June 2026 and confirmed by Scryfall staff:

"I remember we did however block some applications recently that were just hammering the API for card data without regard to rate limit … This would have led to the application eventually getting blocked from our API."
— Scryfall dev, Scarlett

Scryfall's own recommendation:

"api.scryfall.com is rate limited. *.scryfall.io is our CDN. All card images … You can access it without rate limit."

Solution

When a UUID is available for a card, skip the API call and fetch from the CDN directly. UUIDs are stored per-set in the forge-extras repository and fetched on demand at runtime.

flowchart LR
    A[Card needs image] --> B{Local set JSON\ncached?}
    B -- yes --> C{UUID found\nfor set/cn?}
    B -- no --> D[Fetch set JSON\nfrom forge-extras]
    D --> E[Write to local\ncache]
    E --> C
    C -- yes --> F[cards.scryfall.io CDN\nno rate limit]
    C -- no/stale --> G[api.scryfall.com\nrate-limited fallback]
    F --> H[Image saved]
    G --> H
Loading

CDN URL formula (from Scryfall staff):

https://cards.scryfall.io/{size}/{face}/{uuid[0]}/{uuid[1]}/{uuid}.jpg

On-demand caching

The first lookup for any set (e.g. neo) triggers a single HTTP GET to forge-extras for cdn_uuid/neo.json. That file is written to {cacheDir}/cdn_uuid/neo.json and used for all subsequent lookups in the same run and in future sessions — no re-download until the file is manually cleared.

Sets released after the last forge-extras update have no JSON file — getCdnUrl returns null and the caller falls back to the API as before.

Graceful fallback — CDN is always optional

CdnUuidCache.getCdnUrl returns null in every failure case:

  • Set JSON not present in forge-extras (new or unknown set)
  • Network unavailable at fetch time
  • Collector number absent in the set JSON (card added after last data update)
  • No matching language entry

In any of these cases the caller falls back to api.scryfall.com exactly as before. No card ever fails to download because of a missing UUID.

Language priority

The set JSON stores a UUID per language for each collector number. Selection order:

  1. Edition's preferred language (CardEdition.getCardsLangCode()) — e.g. "ja" for Japanese reprints
  2. English ("en") fallback — used when the preferred language has no UUID on record
  3. null — when neither is present; caller falls back to the Scryfall API, which resolves language in the URL itself

Changes

File What changed
forge-gui/…/download/CdnUuidCache.java New. Fetches cdn_uuid/{set}.json from forge-extras; caches locally in {cacheDir}/cdn_uuid/; maps (set, cn, lang) → UUID
forge-gui/…/download/ScryfallBulkData.java New. Constructs CDN URLs from UUID
forge-gui/…/util/ImageFetcher.java Tries CDN URL first in addScryfallUrl; API URL is fallback
forge-gui/…/download/GuiDownloadFilteredCardImages.java Same CDN-first priority in batch downloader
forge-gui-desktop/…/util/SwingImageFetcher.java Treats CDN URLs like API URLs for .fullborder.jpg path transform
forge-gui-mobile/…/util/LibGDXImageFetcher.java Same fix for mobile/adventure
forge-gui/…/properties/ForgeConstants.java Adds CACHE_CDN_UUID_DIR and FORGE_EXTRAS_CDN_UUID_URL; removes CDN_UUID_DIR
forge-gui/pom.xml Adds Gson dependency (for JSON parsing)

Unit tests

CdnUuidCacheTest (17 tests, no GUI required, no network):

  • Happy path, art_crop size
  • Language fallback (jaen)
  • DFC front/back UUIDs; same-UUID DFC
  • Empty face string treated as front
  • Set-code case normalisation
  • Remote fetch path writes local cache file
  • Local cache used when remote unavailable
  • Null inputs, absent set, absent collector number, MISSING_SET caching

Depends on

forge-extras PR — the cdn_uuid/*.json files must be present in the forge-extras repository. Without them every CDN lookup returns null and the API fallback is used; this PR is safe to merge independently.

@phughk

phughk commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Note to reviewer: Because the software relies on a parquet file existing in the assets.zip, the release should include building that file. The file can be skipped (for example for local testing), or it can re-use a pre-downloaded file (ex cached for github runners).

Having a centralised place where the aggregation of cdn urls is done once I think is beneficial, despite making builds/releases more cumbersome.

The important aspect is that the engine should work if the parquet file is never generated.

Uuid lookup is now code committed and can be run whenever.

@phughk

phughk commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author
Screenshot 2026-06-09 at 00 25 04 Screenshot 2026-06-09 at 00 43 51

Updating the ticket description as well to be more accurate of recently pushed changes.

The screenshots are no longer relevant since lookup is committed in code, and users will never receive these prompts. Ie the feature is transparent.

Tested locally (with and without parquet uuids generated).

Introduces CdnUuidCache which lazily loads res/cdn_uuid/{setCode}/{cn}.json
files and resolves CDN URLs (cards.scryfall.io, no rate limit) for card image
fetching. ImageFetcher and GuiDownloadFilteredCardImages prefer CDN URLs when
the asset files are present, falling back to the Scryfall API and then the
cardforge server as before.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@phughk phughk force-pushed the fix-10413-card-image-downloader branch from e4dd5ad to 6430183 Compare June 9, 2026 22:34
Hugh Kaznowski and others added 3 commits June 10, 2026 00:05
- Add package-private `cdnBaseDirOverride` and `clearCacheForTesting()` so
  tests can supply a temp directory without triggering ForgeConstants/GUI init
- Switch ensureSetLoaded to use the override when set (production path unchanged)
- Log DEBUG with absolute path when a set directory is not found, making
  path-resolution failures visible in runtime logs
- Add CdnUuidCacheTest covering happy path, language fallback, DFC front/back,
  same-UUID DFC, missing set (MISSING_SET sentinel caching), missing collector
  number, null inputs, and set-code case normalisation (16 tests total, all pass)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both SwingImageFetcher and LibGDXImageFetcher only applied the
.full → .fullborder path transform for api.scryfall.com URLs.
CDN URLs (cards.scryfall.io) were saved as .full.jpg, which the
game's image display code doesn't find — causing repeated
re-download attempts.

Add URL_SCRYFALL_CDN constant to ForgeConstants and use it in
both fetchers so CDN downloads are treated identically to API
downloads for file-path purposes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CdnUuidCache now loads per-set JSON from {cacheDir}/cdn_uuid/{set}.json.
On first lookup for a set the file is fetched from forge-extras (no rate
limit) and written to the local cache for all subsequent resolutions.
Returns null on any failure — callers fall back to the Scryfall API as before.

ForgeConstants: replace CDN_UUID_DIR (res/) with CACHE_CDN_UUID_DIR (cache/)
and FORGE_EXTRAS_CDN_UUID_URL.

Tests use localCacheDirOverride + remoteBaseUrlOverride (file:// in tests)
to verify the full local-hit and remote-fetch paths without network access.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant