Fix bulk card image downloader to use Scryfall CDN#10928
Open
phughk wants to merge 4 commits into
Open
Conversation
Contributor
Author
|
Uuid lookup is now code committed and can be run whenever. |
Contributor
Author
This was referenced Jun 9, 2026
c1f6b16 to
e4dd5ad
Compare
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>
e4dd5ad to
6430183
Compare
- 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


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:Scryfall's own recommendation:
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 --> HCDN URL formula (from Scryfall staff):
On-demand caching
The first lookup for any set (e.g.
neo) triggers a single HTTP GET to forge-extras forcdn_uuid/neo.json. That file is written to{cacheDir}/cdn_uuid/neo.jsonand 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 —
getCdnUrlreturnsnulland the caller falls back to the API as before.Graceful fallback — CDN is always optional
CdnUuidCache.getCdnUrlreturnsnullin every failure case:In any of these cases the caller falls back to
api.scryfall.comexactly 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:
CardEdition.getCardsLangCode()) — e.g."ja"for Japanese reprints"en") fallback — used when the preferred language has no UUID on recordnull— when neither is present; caller falls back to the Scryfall API, which resolves language in the URL itselfChanges
forge-gui/…/download/CdnUuidCache.javacdn_uuid/{set}.jsonfrom forge-extras; caches locally in{cacheDir}/cdn_uuid/; maps (set, cn, lang) → UUIDforge-gui/…/download/ScryfallBulkData.javaforge-gui/…/util/ImageFetcher.javaaddScryfallUrl; API URL is fallbackforge-gui/…/download/GuiDownloadFilteredCardImages.javaforge-gui-desktop/…/util/SwingImageFetcher.java.fullborder.jpgpath transformforge-gui-mobile/…/util/LibGDXImageFetcher.javaforge-gui/…/properties/ForgeConstants.javaCACHE_CDN_UUID_DIRandFORGE_EXTRAS_CDN_UUID_URL; removesCDN_UUID_DIRforge-gui/pom.xmlUnit tests
CdnUuidCacheTest(17 tests, no GUI required, no network):ja→en)Depends on
forge-extras PR — the
cdn_uuid/*.jsonfiles must be present in the forge-extras repository. Without them every CDN lookup returnsnulland the API fallback is used; this PR is safe to merge independently.