From fa89662172ba65fea0d439f6e9cad9aae60b61a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Fri, 18 Sep 2026 05:11:49 +0200 Subject: [PATCH] [PictureLoader] Add the localized picture URL explicitly, not implicitly Address review: silently prepending the Scryfall named-picture URL to the download list whenever a non-English card language was active was surprising, consumed quota per card when it failed, and could grab the wrong (canon) art on name collisions, with no way to turn it off. The insert is now opt-in and user-controlled: changing the card language adds the template to the top of the download URLs once (persisted, documented in the re-import prompt, and editable/removable in the deck editor settings), while the picture loader no longer injects it at request time. --- .../card_picture_to_load.cpp | 14 +-------- .../settings_page/general_settings_page.cpp | 30 ++++++++++++------- .../settings/download_settings.cpp | 15 ++++++++++ .../settings/download_settings.h | 2 ++ 4 files changed, 38 insertions(+), 23 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_to_load.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_to_load.cpp index 63d56b366..7cb502e92 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_to_load.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_to_load.cpp @@ -94,19 +94,7 @@ void CardPictureToLoad::populateSetUrls() } } - QStringList orderedTemplates = urlTemplates; - if (SettingsCache::instance().cardsDisplay().getCardLang() != "en") { - // Scryfall serves localized art from per-language printings, which have - // their own ids. The ids stored in the card database belong to the - // English prints, so id-based templates (cards.scryfall.io, - // api.scryfall.com/cards/!set:uuid!) can only ever resolve English - // scans; the `lang=` parameter is ignored on them. Resolve the localized - // printing by its translated name (and the language code) ahead of the - // id-based templates instead. - orderedTemplates.prepend("https://api.scryfall.com/cards/named?fuzzy=!localizedName!&lang=!sflang!" - "&format=image&face=!prop:side!"); - } - + const QStringList orderedTemplates = urlTemplates; for (const QString &urlTemplate : orderedTemplates) { QString transformedUrl = transformUrl(urlTemplate); diff --git a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp index 7dd539051..36436f8a3 100644 --- a/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/general_settings_page.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -453,16 +454,25 @@ void GeneralSettingsPage::cardLanguageBoxChanged(int index) CardPictureLoader::clearNetworkCache(); CardPictureLoader::clearPixmapCache(); - const QMessageBox::StandardButton answer = - QMessageBox::question(this, tr("Card text & images language changed"), - tr("

The card database only contains English card data. To see cards in %1, " - "Oracle must run once with this language selected and re-import the card " - "database.

" - "

The cached database and the downloaded card pictures have been cleared, so a " - "re-import is picked up without stale entries.

" - "

Run Oracle now?

") - .arg(cardLanguageBox.itemText(index)), - QMessageBox::Yes | QMessageBox::No, QMessageBox::Yes); + // Art is resolved by the translated card name for non-English languages, so the + // matching Scryfall URL is added to the top of the download list. It stays + // visible in the deck editor settings, where it can be removed or reordered. + const bool localizedUrlAdded = SettingsCache::instance().downloads().addLocalizedScryfallUrl(); + + QString message = tr("

The card database only contains English card data. To see cards in %1, " + "Oracle must run once with this language selected and re-import the card " + "database.

" + "

The cached database and the downloaded card pictures have been cleared, so a " + "re-import is picked up without stale entries.

") + .arg(cardLanguageBox.itemText(index)); + if (localizedUrlAdded) { + message += tr("

The Scryfall URL that resolves card art by translated name was added to the top of your " + "download list. You can remove or reorder it any time.

"); + } + message += tr("

Run Oracle now?

"); + + const QMessageBox::StandardButton answer = QMessageBox::question( + this, tr("Card text & images language changed"), message, QMessageBox::Yes | QMessageBox::No, QMessageBox::Yes); // The answer only controls whether Oracle starts right away; the caches stay // cleared so the next import or launch rebuilds them in the new language. diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp index 88e3f0cec..eb73e58ee 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp @@ -9,6 +9,9 @@ const QStringList DownloadSettings::DEFAULT_DOWNLOAD_URLS = { "https://gatherer.wizards.com/Handlers/Image.ashx?multiverseid=!set:muid!&type=card", "https://gatherer.wizards.com/Handlers/Image.ashx?name=!name!&type=card"}; +const QString DownloadSettings::SCRYFALL_NAMED_LOCALIZED_URL = + "https://api.scryfall.com/cards/named?fuzzy=!localizedName!&lang=!sflang!&format=image&face=!prop:side!"; + DownloadSettings::DownloadSettings(const QString &settingPath, QObject *parent = nullptr) : SettingsManager(settingPath + "downloads.ini", "downloads", QString(), parent) { @@ -29,6 +32,18 @@ void DownloadSettings::resetToDefaultURLs() setValue(QVariant::fromValue(DEFAULT_DOWNLOAD_URLS), "urls"); } +bool DownloadSettings::addLocalizedScryfallUrl() +{ + const QStringList urls = getAllURLs(); + if (urls.contains(SCRYFALL_NAMED_LOCALIZED_URL)) { + return false; + } + QStringList updated = urls; + updated.prepend(SCRYFALL_NAMED_LOCALIZED_URL); + setDownloadUrls(updated); + return true; +} + bool DownloadSettings::getPicDownload() const { return getValue("pictureDownload", QString(), QString(), true).toBool(); diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.h b/libcockatrice_settings/libcockatrice/settings/download_settings.h index a3a6f4ca9..ae49884f0 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/download_settings.h @@ -15,6 +15,7 @@ class DownloadSettings : public SettingsManager friend class SettingsCache; static const QStringList DEFAULT_DOWNLOAD_URLS; + static const QString SCRYFALL_NAMED_LOCALIZED_URL; public: explicit DownloadSettings(const QString &, QObject *); @@ -22,6 +23,7 @@ public: QStringList getAllURLs() const; void setDownloadUrls(const QStringList &downloadURLs); void resetToDefaultURLs(); + [[nodiscard]] bool addLocalizedScryfallUrl(); [[nodiscard]] bool getPicDownload() const; void setPicDownload(bool _picDownload); [[nodiscard]] bool getDownloadSpoilersStatus() const;