Skip to content

Support official card image languages - #11507

Open
churrufli wants to merge 4 commits into
Card-Forge:masterfrom
churrufli:support-image-languages-clean
Open

Support official card image languages#11507
churrufli wants to merge 4 commits into
Card-Forge:masterfrom
churrufli:support-image-languages-clean

Conversation

@churrufli

Copy link
Copy Markdown
Contributor

No description provided.

@tool4ever

Copy link
Copy Markdown
Contributor

Seems partially redundant to #10928 - if your goal is to avoid 404 responses maybe other languages shouldn't be attempted to fetch unless files generated from that PR are present...?

@churrufli

Copy link
Copy Markdown
Contributor Author

Thanks for flagging this — there's definitely some shared ground, but I don't think the two are actually redundant.

#10928 solves where to fetch from (CDN vs. rate-limited API), and per its description its own language handling is just "edition's default language → English → null" — it doesn't expose a user-facing choice between languages, it just automates what Forge already does per edition today.

This PR solves a different problem: a Preferences-level "download card art in language X" toggle across the languages Scryfall indexes, only attempting a non-default language when the bulk-data-derived index confirms that exact printing exists in it — so the 404-avoidance here doesn't depend on which endpoint (API or CDN) ends up serving the request.

To generate that index I wrote a small Python script (update_languages.py) that pulls Scryfall's all_cards bulk export directly, filters out digital-only prints, and builds a compact per-set/collector-number bitmask of which of the 10 supported languages each printing exists in (skipping English-only entries, since absence from the file already implies "English only"). It's meant to be re-run periodically to refresh card_languages.txt as new sets/languages are released.

I'd rather not gate this on #10928's cdn_uuid files being present, since that PR is still open and its data is explicitly optional/fallback — tying language selection to it would silently disable the language preference for anyone without that cache populated yet, even though our index (built straight from Scryfall's all_cards bulk export) already knows with certainty whether the printing exists in that language.

Once #10928 lands the two could compose rather than merge: resolve the language here first, then use (set, collector_number, resolvedLang) to decide CDN vs. API via CdnUuidCache. I'm spanish sorry for the translation BTW. Cheers
update_languages.py

Comment thread forge-core/src/main/java/forge/util/ImageUtil.java Outdated
@tool4ever

Copy link
Copy Markdown
Contributor

I still suspect you're trying to solve a problem that doesn't exist in this form:
we already have languages/cardnames-* files for card translation (only russian hasn't been generated yet) so they can easily be reused to lookup existence

I would assume they contain none/minimal entries without hosted images existing...?
Unless we want to start using them for adding inofficial translations but I'm not sure that'd be a good idea either 🤔

@churrufli

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback.
The existing languages/cardnames-* files are translations of card names + oracle text. They are keyed by card name, not by the actual Scryfall printing key (set code + collector number + face).
That’s a different problem from “does this exact printing have official art in language X?”.
Using the name files as a proxy would be imprecise:
A translated name doesn’t guarantee that a specific printing has a hosted image in that language.
Multiple printings of the same card can differ.
Special cases (meld, funny cards, digital-only, etc.) wouldn’t be handled cleanly.
The index added in this PR comes directly from Scryfall’s bulk data, filtered to physical printings, and uses the exact key Scryfall uses for images. That gives a reliable answer without 404s and without depending on the translation files or on #10928’s optional CDN data.
Once #10928 lands the two can compose nicely: resolve the preferred language first, then decide CDN vs API. I think that it's a good start point.

@tool4ever

Copy link
Copy Markdown
Contributor

Hmn, I wonder if user is playing in a different language he'd rather have the one translated image available or stick with the the untranslated variant of another set? (especially if it's the same art) 🤔

@churrufli

Copy link
Copy Markdown
Contributor Author

@tool4ever Fair point, but this choice doesn't really exist today at all — right now there's no way to get card art in another language. Let's get this in first, and once people can actually use it we'll see what users say and figure out from there whether cross-printing substitution is worth adding.

@churrufli
churrufli requested a review from Hanmac August 4, 2026 23:52
@tool4ever

Copy link
Copy Markdown
Contributor

Hmn I see, I really thought autodownloader would try user language first already...

But since CardLang field in editions already exists could also just extend that to contain more than one language instead?
Though most sets contain a few english only ones, so maybe some modifier for exclusion would be the less verbose option 🤔
And even if not all are correctly excluded at first a few 404 for exotic promo variants don't seem a real problem 🤷‍♂️

@churrufli

churrufli commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Extending CardLang to a list is a valid but bigger, different approach — today only 23 of ~680 edition files even set it, and it's only ever consumed by this PR's code. My current approach already avoids the 404s, changes nothing for English users, and is strictly 1:1: it only fetches that exact printing's art when my generated language index confirms it exists in the selected language — never a substitute from another printing.

I'd rather get this merged first and iterate on the edition-level approach afterward. Hoping this can get merged soon.

@churrufli
churrufli force-pushed the support-image-languages-clean branch from 39749e3 to 649fecf Compare August 6, 2026 06:31
@tool4ever

Copy link
Copy Markdown
Contributor

Right, I understand your goal and it will certainly be good to have support for this but please be aware that when using AI it loves to complicate things so that you're stuck with it - my concerns are:
a) this creates a parallel system and decouples the data from our general practice of managing it via edition files
b) it creates extra dependency to use Python if you want to update + the bit encoding makes manual editing rather unrealistic
c)

and it's only ever consumed by this PR's code

the field is already connected in code today, just not filled and used fully

@churrufli

churrufli commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback. I understand the concerns you raised, but I believe my current approach is the most viable and efficient solution to improve the project at this stage.

  • File efficiency: The Python script generates a bitmask to keep the final text file as lightweight as possible. Allowing manual editing would require a much heavier format, which I don’t think is optimal for the application’s performance.
  • Python usage: I noticed that the project already uses Python scripts for other maintenance tasks, so adding update_languages.py to periodically refresh the data should not be an issue, but rather a standard maintenance tool.
  • Avoiding 404 errors: Unlike the suggested approach, I don’t think 404 errors should be considered acceptable, even if they are rare. The index provides a reliable check by confirming whether a specific printing exists in the selected language before attempting the download, avoiding unnecessary requests.

I would also like to clarify that the overall logic and structure of this solution are my own. While I used AI assistance for parts of the technical implementation, the design and approach were developed by me to specifically address this improvement.

I’ll wait for the feedback from the other reviewers and see what they think. Thank you all for your time and for following up on my PR.

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.

3 participants