CDN Multi-language card image downloads (continues #10928) - #11709
Open
churrufli wants to merge 23 commits into
Open
CDN Multi-language card image downloads (continues #10928)#11709churrufli wants to merge 23 commits into
churrufli wants to merge 23 commits into
Conversation
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>
- 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>
- Request Accept-Encoding: gzip when fetching a set's UUID JSON from forge-extras; raw.githubusercontent.com honors it (~48% smaller on the wire for this UUID-heavy JSON), decompressed transparently. - Store the local disk cache gzip-compressed too (.json.gz), saving space for whichever sets a user actually triggers a lookup for. The files committed to forge-extras itself stay plain, sorted text — compressing those would turn every regeneration into a full binary rewrite instead of the minimal diff the CLI now produces. Verified against the live forge-extras branch: server returns Content-Encoding: gzip (110,709 bytes) and decompresses back to the full 214,541-byte file. All 17 CdnUuidCacheTest cases still pass.
…est API Addresses the Discord discussion on forge-owned vs. user-owned CDN data: this adds a user-triggered path that doesn't depend on forge-extras at all, alongside (not instead of) the existing hosted data. Scryfall's /cards/manifest endpoint returns just id/set/collector_number /lang per card -- everything this lookup needs -- in ~15k-entry pages, without the ~100MB-2.5GB cost of a full bulk-data export. English coverage is currently ~8 pages (~25-30MB). It doesn't expose the actual CDN image URL, so a double-faced card's back face is assumed to share the front's id; verified against live Scryfall data that this holds for the overwhelming majority of DFCs (the CDN URL only differs by a front/back path segment), and the existing CDN-miss fallback already covers the rare exceptions. ScryfallManifestSync paginates the manifest (respecting its documented 10/minute rate limit), and supports incremental resync: entries are requested newest-image-update-first, with a persisted per-language watermark so a repeat sync typically stops after one page instead of re-walking the whole catalog. CdnUuidCache gains mergeSetEntries() (fills gaps without ever overwriting existing entries -- forge-extras/CLI data can carry a double-faced card's real distinct front/back UUIDs, which a manifest- derived guess must not clobber) and a public clearCache(), covering the simpler "let users purge it from settings" alternative to per-file expiry that came up in the same discussion. Both actions are wired into the mobile card-image-download screen, which is where this whole feature already lives (desktop never exposed "Download Card Images" -- only mobile/CardImageBrowserScreen does). 10 new tests in ScryfallManifestSyncTest (embedded HTTP server, no live network dependency) cover pagination, the incremental watermark, the never-overwrite merge policy, and cache clearing; all 27 CDN-related tests pass together.
Addresses tool4ever's review comment on PR Card-Forge#10928: a single-method class wrapping a URL format string didn't warrant its own file. CdnUuidCache.getCdnUrl() was the only production caller, so cdnUrl() moves there as a public static method; test call sites updated to match, and ScryfallBulkDataTest's formula assertion is folded into CdnUuidCacheTest instead of being dropped.
CdnUuidCache no longer fetches per-set JSON from the forge-extras repo. On a cache miss it now asks the new ScryfallSetSync to build that set's mapping on the spot from Scryfall's card search API (scoped to that one set, all languages), the client-side equivalent of what the old forge-extras/CLI generator did against a full bulk-data export. Unlike ScryfallManifestSync (which only ever sees a card's own id), the search API exposes each face's own image URL, so a double-faced card's rare genuinely-distinct back-face UUID is captured precisely via the new CdnUuidCache.mergeSetEntriesWithFaces, instead of assumed to match the front. This replaces the forge-scryfall-uuid-map CLI and forge-extras cdn_uuid data entirely -- both PRs are being closed in favor of this.
…age-downloader # Conflicts: # forge-gui-desktop/src/main/java/forge/util/SwingImageFetcher.java
…age-downloader # Conflicts: # forge-gui/res/languages/ko-KR.properties
No behavior change: condenses multi-paragraph javadoc down to one or two lines per method/class, drops inline comments that just restated an adjacent assert message or code branch, and removes a duplicated CDN/API/cardforge priority list that appeared in both a class and method javadoc.
Global ScryfallRateLimiter replaces scattered per-class throttling, paces per Scryfall's documented per-endpoint limits (500ms for search/named, 100ms otherwise), and backs off using the response's Retry-After header instead of a flat 5-minute cooldown. cards.scryfall.io CDN URLs continue to bypass the limiter entirely. CdnUuidCache's implicit auto-sync-on-miss no longer fires from gameplay's lazy image fetch or the bulk downloader's per-card lookup -- both are now read-only against the local cache (getCdnUrlIfCached), so a missing image during normal play never silently kicks off a full-set Scryfall search in the background. The bulk downloader's explicit warm-up loop is now the only thing that triggers a sync, waits out an active cooldown instead of skipping through it, and skips any set already cached locally. Add ScryfallBulkDataSync: resolves CDN links for every set in one pass from Scryfall's bulk data export instead of one paginated /cards/search call per set, wired up as a new button in both the desktop and mobile card image downloader screens. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
thanks, I suppose this iteration is the best of "both" worlds |
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
2 participants
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.
Summary
Continues the work from #10928 (CDN UUID cache / card image downloader). Adds language support for card image downloads: preferred-language selection, an optional multi-language index download, and text/i18n cleanup across the related UI.
What's included
Language preferences
Multi-language index download
all_cardsbulk file filtered to English + the chosen language (avoids caching the other ~16 languages that won't be used).default_cards, ~75 MB) is unchanged in behavior.lang:en OR lang:<language>instead of English only.image_statusfiltering (Scryfall): cards markedmissingorplaceholderare discarded instead of being cached as if they were real art (this mostly affects localized languages, where this status is more common).Bug fixes
CON) no longer break cache writes on that OS.FScrollPane: content was getting cut off at certain resolutions/orientations, leaving some controls unreachable.Text and i18n
No behavior change by default
Anyone who doesn't touch the new language options keeps exactly the same behavior as in #10928: English-only sync, same fast bulk-download button.
Assisted by Claude Opus 5