Parallelize picture loader.

cockatrice/CMakeLists.txt

@@ -68,6 +68,7 @@ set(cockatrice_SOURCES
     src/client/ui/phases_toolbar.cpp
     src/client/ui/picture_loader/picture_loader.cpp
     src/client/ui/picture_loader/picture_loader_worker.cpp
+    src/client/ui/picture_loader/picture_loader_worker_work.cpp
     src/client/ui/picture_loader/picture_to_load.cpp
     src/client/ui/pixel_map_generator.cpp
     src/client/ui/theme_manager.cpp

cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp

@@ -2,13 +2,15 @@
 
 #include "../../../game/cards/card_database_manager.h"
 #include "../../../settings/cache_settings.h"
+#include "picture_loader_worker_work.h"
 
-#include <QBuffer>
 #include <QDirIterator>
+#include <QJsonDocument>
 #include <QMovie>
 #include <QNetworkDiskCache>
 #include <QNetworkReply>
 #include <QThread>
+#include <utility>
 
 // Card back returned by gatherer when card is not found
 QStringList PictureLoaderWorker::md5Blacklist = QStringList() << "db0c48db407a907c16ade38de048a441";
@@ -19,11 +21,8 @@ PictureLoaderWorker::PictureLoaderWorker()
       picDownload(SettingsCache::instance().getPicDownload()), downloadRunning(false), loadQueueRunning(false),
       overrideAllCardArtWithPersonalPreference(SettingsCache::instance().getOverrideAllCardArtWithPersonalPreference())
 {
-    connect(this, &PictureLoaderWorker::startLoadQueue, this, &PictureLoaderWorker::processLoadQueue,
+    connect(&SettingsCache::instance(), SIGNAL(picsPathChanged()), this, SLOT(picsPathChanged()));
-            Qt::QueuedConnection);
+    connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged()));
-    connect(&SettingsCache::instance(), &SettingsCache::picsPathChanged, this, &PictureLoaderWorker::picsPathChanged);
-    connect(&SettingsCache::instance(), &SettingsCache::picDownloadChanged, this,
-            &PictureLoaderWorker::picDownloadChanged);
     connect(&SettingsCache::instance(), &SettingsCache::overrideAllCardArtWithPersonalPreferenceChanged, this,
             &PictureLoaderWorker::setOverrideAllCardArtWithPersonalPreference);
 
@@ -45,7 +44,6 @@ PictureLoaderWorker::PictureLoaderWorker()
     // Use a ManualRedirectPolicy since we keep track of redirects in picDownloadFinished
     // We can't use NoLessSafeRedirectPolicy because it is not applied with AlwaysCache
     networkManager->setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy);
-    connect(networkManager, &QNetworkAccessManager::finished, this, &PictureLoaderWorker::picDownloadFinished);
 
     cacheFilePath = SettingsCache::instance().getRedirectCachePath() + REDIRECT_CACHE_FILENAME;
     loadRedirectCache();
@@ -57,6 +55,9 @@ PictureLoaderWorker::PictureLoaderWorker()
     pictureLoaderThread = new QThread;
     pictureLoaderThread->start(QThread::LowPriority);
     moveToThread(pictureLoaderThread);
+
+    connect(&requestTimer, &QTimer::timeout, this, &PictureLoaderWorker::processQueuedRequests);
+    requestTimer.setSingleShot(true);
 }
 
 PictureLoaderWorker::~PictureLoaderWorker()
@@ -64,237 +65,128 @@ PictureLoaderWorker::~PictureLoaderWorker()
     pictureLoaderThread->deleteLater();
 }
 
-void PictureLoaderWorker::processLoadQueue()
+QNetworkReply *PictureLoaderWorker::makeRequest(const QUrl &url, PictureLoaderWorkerWork *worker)
 {
-    if (loadQueueRunning) {
+    constexpr int urlRequestDelayMs = 100; // Minimum delay between requests to the same URL (in milliseconds)
-        return;
+
+    // Check if we're rate-limited globally
+    if (rateLimited) {
+        requestQueue.append(qMakePair(url, worker));
+        return nullptr;
     }
 
-    loadQueueRunning = true;
+    // Check if a request was recently made to this URL
-    while (true) {
+    QDateTime now = QDateTime::currentDateTimeUtc();
-        mutex.lock();
+    if (lastRequestTime.contains(url)) {
-        if (loadQueue.isEmpty()) {
+        int elapsedMs = lastRequestTime[url].msecsTo(now);
-            mutex.unlock();
+        if (elapsedMs < urlRequestDelayMs) {
-            loadQueueRunning = false;
+            // Queue the request if too soon to reissue
-            return;
+            requestQueue.append(qMakePair(url, worker));
-        }
+            if (!requestTimer.isActive()) {
-        cardBeingLoaded = loadQueue.takeFirst();
+                requestTimer.start(urlRequestDelayMs);
-        mutex.unlock();
-
-        QString setName = cardBeingLoaded.getSetName();
-        QString cardName = cardBeingLoaded.getCard()->getName();
-        QString correctedCardName = cardBeingLoaded.getCard()->getCorrectedName();
-
-        qCDebug(PictureLoaderWorkerLog).nospace()
-            << "[card: " << cardName << " set: " << setName << "]: Trying to load picture";
-
-        // FIXME: This is a hack so that to keep old Cockatrice behavior
-        // (ignoring provider ID) when the "override all card art with personal
-        // preference" is set.
-        //
-        // Figure out a proper way to integrate the two systems at some point.
-        //
-        // Note: need to go through a member for
-        // overrideAllCardArtWithPersonalPreference as reading from the
-        // SettingsCache instance from the PictureLoaderWorker thread could
-        // cause race conditions.
-        //
-        // XXX: Reading from the CardDatabaseManager instance from the
-        // PictureLoaderWorker thread might not be safe either
-        bool searchCustomPics = overrideAllCardArtWithPersonalPreference ||
-                                CardDatabaseManager::getInstance()->isProviderIdForPreferredPrinting(
-                                    cardName, cardBeingLoaded.getCard()->getPixmapCacheKey());
-        if (searchCustomPics && cardImageExistsOnDisk(setName, correctedCardName, searchCustomPics)) {
-            continue;
-        }
-
-        qCDebug(PictureLoaderWorkerLog).nospace()
-            << "[card: " << cardName << " set: " << setName << "]: No custom picture, trying to download";
-        cardsToDownload.append(cardBeingLoaded);
-        cardBeingLoaded.clear();
-        if (!downloadRunning) {
-            startNextPicDownload();
-        }
-    }
-}
-
-bool PictureLoaderWorker::cardImageExistsOnDisk(QString &setName, QString &correctedCardname, bool searchCustomPics)
-{
-    QImage image;
-    QImageReader imgReader;
-    imgReader.setDecideFormatFromContent(true);
-    QList<QString> picsPaths = QList<QString>();
-
-    if (searchCustomPics) {
-        QDirIterator it(customPicsPath, QDirIterator::Subdirectories | QDirIterator::FollowSymlinks);
-
-        // Recursively check all subdirectories of the CUSTOM folder
-        while (it.hasNext()) {
-            QString thisPath(it.next());
-            QFileInfo thisFileInfo(thisPath);
-
-            if (thisFileInfo.isFile() &&
-                (thisFileInfo.fileName() == correctedCardname || thisFileInfo.completeBaseName() == correctedCardname ||
-                 thisFileInfo.baseName() == correctedCardname)) {
-                picsPaths << thisPath; // Card found in the CUSTOM directory, somewhere
             }
+            return nullptr;
         }
     }
 
-    if (!setName.isEmpty()) {
+    // Update the last request time for this URL
-        picsPaths << picsPath + "/" + setName + "/" + correctedCardname
+    lastRequestTime[url] = now;
-                  // We no longer store downloaded images there, but don't just ignore
-                  // stuff that old versions have put there.
-                  << picsPath + "/downloadedPics/" + setName + "/" + correctedCardname;
-    }
 
-    // Iterates through the list of paths, searching for images with the desired
+    // Check for cached redirects
-    // name with any QImageReader-supported
-    // extension
-    for (const auto &_picsPath : picsPaths) {
-        imgReader.setFileName(_picsPath);
-        if (imgReader.read(&image)) {
-            qCDebug(PictureLoaderWorkerLog).nospace()
-                << "[card: " << correctedCardname << " set: " << setName << "]: Picture found on disk.";
-            imageLoaded(cardBeingLoaded.getCard(), image);
-            return true;
-        }
-        imgReader.setFileName(_picsPath + ".full");
-        if (imgReader.read(&image)) {
-            qCDebug(PictureLoaderWorkerLog).nospace()
-                << "[card: " << correctedCardname << " set: " << setName << "]: Picture.full found on disk.";
-            imageLoaded(cardBeingLoaded.getCard(), image);
-            return true;
-        }
-        imgReader.setFileName(_picsPath + ".xlhq");
-        if (imgReader.read(&image)) {
-            qCDebug(PictureLoaderWorkerLog).nospace()
-                << "[card: " << correctedCardname << " set: " << setName << "]: Picture.xlhq found on disk.";
-            imageLoaded(cardBeingLoaded.getCard(), image);
-            return true;
-        }
-    }
-
-    return false;
-}
-
-void PictureLoaderWorker::startNextPicDownload()
-{
-    if (cardsToDownload.isEmpty()) {
-        cardBeingDownloaded.clear();
-        downloadRunning = false;
-        return;
-    }
-
-    downloadRunning = true;
-
-    cardBeingDownloaded = cardsToDownload.takeFirst();
-
-    QString picUrl = cardBeingDownloaded.getCurrentUrl();
-
-    if (picUrl.isEmpty()) {
-        downloadRunning = false;
-        picDownloadFailed();
-    } else {
-        QUrl url(picUrl);
-        qCDebug(PictureLoaderWorkerLog).nospace() << "[card: " << cardBeingDownloaded.getCard()->getCorrectedName()
-                                                  << " set: " << cardBeingDownloaded.getSetName()
-                                                  << "]: Trying to fetch picture from url " << url.toDisplayString();
-        makeRequest(url);
-    }
-}
-
-void PictureLoaderWorker::picDownloadFailed()
-{
-    /* Take advantage of short circuiting here to call the nextUrl until one
-       is not available.  Only once nextUrl evaluates to false will this move
-       on to nextSet.  If the Urls for a particular card are empty, this will
-       effectively go through the sets for that card. */
-    if (cardBeingDownloaded.nextUrl() || cardBeingDownloaded.nextSet()) {
-        mutex.lock();
-        loadQueue.prepend(cardBeingDownloaded);
-        mutex.unlock();
-    } else {
-        qCWarning(PictureLoaderWorkerLog).nospace()
-            << "[card: " << cardBeingDownloaded.getCard()->getCorrectedName()
-            << " set: " << cardBeingDownloaded.getSetName() << "]: Picture NOT found, "
-            << (picDownload ? "download failed" : "downloads disabled")
-            << ", no more url combinations to try: BAILING OUT";
-        imageLoaded(cardBeingDownloaded.getCard(), QImage());
-        cardBeingDownloaded.clear();
-    }
-    emit startLoadQueue();
-}
-
-bool PictureLoaderWorker::imageIsBlackListed(const QByteArray &picData)
-{
-    QString md5sum = QCryptographicHash::hash(picData, QCryptographicHash::Md5).toHex();
-    return md5Blacklist.contains(md5sum);
-}
-
-QNetworkReply *PictureLoaderWorker::makeRequest(const QUrl &url)
-{
-    // Check if the redirect is cached
     QUrl cachedRedirect = getCachedRedirect(url);
     if (!cachedRedirect.isEmpty()) {
-        qCDebug(PictureLoaderWorkerLog).nospace()
+        return makeRequest(cachedRedirect, worker);
-            << "[card: " << cardBeingDownloaded.getCard()->getCorrectedName()
-            << " set: " << cardBeingDownloaded.getSetName() << "]: Using cached redirect for " << url.toDisplayString()
-            << " to " << cachedRedirect.toDisplayString();
-        return makeRequest(cachedRedirect); // Use the cached redirect
     }
 
     QNetworkRequest req(url);
-
-    // QNetworkDiskCache leaks file descriptors when downloading compressed
-    // files, see https://bugreports.qt.io/browse/QTBUG-135641
-    //
-    // We can set the Accept-Encoding header manually to disable Qt's automatic
-    // response decompression, but then we would have to deal with decompression
-    // ourselves.
-    //
-    // Since we are dowloading images that are usually stored in a
-    // compressed format (e.g. jpeg or webp), it's not clear compression at the
-    // HTTP level helps; in fact, images are typically returned without
-    // compression. Redirects, on the other hand, are compressed and cause file
-    // descriptor leaks -- but since redirects have no payload, we don't really
-    // care either.
-    //
-    // In the end, just do the simple thing and disable HTTP compression.
-    req.setRawHeader("accept-encoding", "identity");
-
     if (!picDownload) {
         req.setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::AlwaysCache);
     }
 
     QNetworkReply *reply = networkManager->get(req);
 
-    connect(reply, &QNetworkReply::finished, this, [this, reply, url]() {
+    // Connect reply handling
+    connect(reply, &QNetworkReply::finished, this, [this, reply, url, worker]() {
         QVariant redirectTarget = reply->attribute(QNetworkRequest::RedirectionTargetAttribute);
-
         if (redirectTarget.isValid()) {
             QUrl redirectUrl = redirectTarget.toUrl();
             if (redirectUrl.isRelative()) {
                 redirectUrl = url.resolved(redirectUrl);
             }
-
             cacheRedirect(url, redirectUrl);
-            qCDebug(PictureLoaderWorkerLog).nospace()
-                << "[card: " << cardBeingDownloaded.getCard()->getCorrectedName()
-                << " set: " << cardBeingDownloaded.getSetName() << "]: Caching redirect from " << url.toDisplayString()
-                << " to " << redirectUrl.toDisplayString();
         }
 
+        if (reply->error() == QNetworkReply::NoError) {
+            worker->picDownloadFinished(reply);
+        } else if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 429) {
+            handleRateLimit(url, worker);
+        } else {
+            worker->picDownloadFinished(reply);
+        }
         reply->deleteLater();
     });
 
     return reply;
 }
 
+void PictureLoaderWorker::handleRateLimit(const QUrl &url, PictureLoaderWorkerWork *worker)
+{
+    // Prevent multiple rate-limit handling
+    if (!rateLimited) {
+        int retryAfter = 70;
+        rateLimited = true;
+        qWarning() << "Rate limit exceeded! Queuing requests for" << retryAfter << "seconds.";
+
+        // Start a timer to reset the rate-limited state
+        rateLimitTimer.singleShot(retryAfter * 1000, this, [this]() {
+            qWarning() << "Rate limit expired. Resuming queued requests.";
+            processQueuedRequests();
+        });
+    }
+
+    // Always queue the request even if already rate-limited
+    requestQueue.append(qMakePair(url, worker));
+}
+
+void PictureLoaderWorker::processQueuedRequests()
+{
+    qWarning() << "Resuming queued requests";
+    rateLimited = false;
+
+    if (!requestQueue.isEmpty()) {
+        // Process requests one by one, respecting the URL delay
+        auto request = requestQueue.takeFirst();
+        QDateTime now = QDateTime::currentDateTimeUtc();
+
+        if (lastRequestTime.contains(request.first)) {
+            int elapsedMs = lastRequestTime[request.first].msecsTo(now);
+            if (elapsedMs < 100) {
+                // Delay request if needed
+                requestQueue.prepend(request);       // Put back in the queue
+                requestTimer.start(100 - elapsedMs); // Schedule for the remaining time
+                return;
+            }
+        }
+
+        makeRequest(request.first, request.second);
+    }
+}
+
+void PictureLoaderWorker::enqueueImageLoad(const CardInfoPtr &card)
+{
+    auto worker = new PictureLoaderWorkerWork(this, card);
+    Q_UNUSED(worker);
+}
+
+void PictureLoaderWorker::imageLoadedSuccessfully(CardInfoPtr card, const QImage &image)
+{
+    emit imageLoaded(std::move(card), image);
+}
+
 void PictureLoaderWorker::cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl)
 {
     redirectCache[originalUrl] = qMakePair(redirectUrl, QDateTime::currentDateTimeUtc());
-    saveRedirectCache();
+    // saveRedirectCache();
 }
 
 QUrl PictureLoaderWorker::getCachedRedirect(const QUrl &originalUrl) const
@@ -353,129 +245,6 @@ void PictureLoaderWorker::cleanStaleEntries()
     }
 }
 
-void PictureLoaderWorker::picDownloadFinished(QNetworkReply *reply)
-{
-    bool isFromCache = reply->attribute(QNetworkRequest::SourceIsFromCacheAttribute).toBool();
-
-    if (reply->error()) {
-        if (isFromCache) {
-            qCDebug(PictureLoaderWorkerLog).nospace()
-                << "[card: " << cardBeingDownloaded.getCard()->getName() << " set: " << cardBeingDownloaded.getSetName()
-                << "]: Removing corrupted cache file for url " << reply->url().toDisplayString() << " and retrying ("
-                << reply->errorString() << ")";
-
-            networkManager->cache()->remove(reply->url());
-
-            makeRequest(reply->url());
-        } else {
-            qCDebug(PictureLoaderWorkerLog).nospace()
-                << "[card: " << cardBeingDownloaded.getCard()->getName() << " set: " << cardBeingDownloaded.getSetName()
-                << "]: " << (picDownload ? "Download" : "Cache search") << " failed for url "
-                << reply->url().toDisplayString() << " (" << reply->errorString() << ")";
-
-            picDownloadFailed();
-            startNextPicDownload();
-        }
-
-        reply->deleteLater();
-        return;
-    }
-
-    // List of status codes from https://doc.qt.io/qt-6/qnetworkreply.html#redirected
-    int statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
-    if (statusCode == 301 || statusCode == 302 || statusCode == 303 || statusCode == 305 || statusCode == 307 ||
-        statusCode == 308) {
-        QUrl redirectUrl = reply->header(QNetworkRequest::LocationHeader).toUrl();
-        qCDebug(PictureLoaderWorkerLog).nospace()
-            << "[card: " << cardBeingDownloaded.getCard()->getName() << " set: " << cardBeingDownloaded.getSetName()
-            << "]: following " << (isFromCache ? "cached redirect" : "redirect") << " to "
-            << redirectUrl.toDisplayString();
-        makeRequest(redirectUrl);
-        reply->deleteLater();
-        return;
-    }
-
-    // peek is used to keep the data in the buffer for use by QImageReader
-    const QByteArray &picData = reply->peek(reply->size());
-
-    if (imageIsBlackListed(picData)) {
-        qCDebug(PictureLoaderWorkerLog).nospace()
-            << "[card: " << cardBeingDownloaded.getCard()->getName() << " set: " << cardBeingDownloaded.getSetName()
-            << "]: Picture found, but blacklisted, will consider it as not found";
-
-        picDownloadFailed();
-        reply->deleteLater();
-        startNextPicDownload();
-        return;
-    }
-
-    QImage testImage;
-
-    QImageReader imgReader;
-    imgReader.setDecideFormatFromContent(true);
-    imgReader.setDevice(reply);
-
-    bool logSuccessMessage = false;
-
-    static const int riffHeaderSize = 12; // RIFF_HEADER_SIZE from webp/format_constants.h
-    auto replyHeader = reply->peek(riffHeaderSize);
-
-    if (replyHeader.startsWith("RIFF") && replyHeader.endsWith("WEBP")) {
-        auto imgBuf = QBuffer(this);
-        imgBuf.setData(reply->readAll());
-
-        auto movie = QMovie(&imgBuf);
-        movie.start();
-        movie.stop();
-
-        imageLoaded(cardBeingDownloaded.getCard(), movie.currentImage());
-        logSuccessMessage = true;
-    } else if (imgReader.read(&testImage)) {
-        imageLoaded(cardBeingDownloaded.getCard(), testImage);
-        logSuccessMessage = true;
-    } else {
-        qCDebug(PictureLoaderWorkerLog).nospace()
-            << "[card: " << cardBeingDownloaded.getCard()->getName() << " set: " << cardBeingDownloaded.getSetName()
-            << "]: Possible " << (isFromCache ? "cached" : "downloaded") << " picture at "
-            << reply->url().toDisplayString() << " could not be loaded: " << reply->errorString();
-
-        picDownloadFailed();
-    }
-
-    if (logSuccessMessage) {
-        qCInfo(PictureLoaderWorkerLog).nospace()
-            << "[card: " << cardBeingDownloaded.getCard()->getName() << " set: " << cardBeingDownloaded.getSetName()
-            << "]: Image successfully " << (isFromCache ? "loaded from cached" : "downloaded from") << " url "
-            << reply->url().toDisplayString();
-    }
-
-    reply->deleteLater();
-    startNextPicDownload();
-}
-
-void PictureLoaderWorker::enqueueImageLoad(CardInfoPtr card)
-{
-    QMutexLocker locker(&mutex);
-
-    // avoid queueing the same card more than once
-    if (!card || card == cardBeingLoaded.getCard() || card == cardBeingDownloaded.getCard()) {
-        return;
-    }
-
-    for (const PictureToLoad &pic : loadQueue) {
-        if (pic.getCard() == card)
-            return;
-    }
-
-    for (const PictureToLoad &pic : cardsToDownload) {
-        if (pic.getCard() == card)
-            return;
-    }
-
-    loadQueue.append(PictureToLoad(card));
-    emit startLoadQueue();
-}
-
 void PictureLoaderWorker::picDownloadChanged()
 {
     QMutexLocker locker(&mutex);

cockatrice/src/client/ui/picture_loader/picture_loader_worker.h

@@ -1,6 +1,8 @@
 #ifndef PICTURE_LOADER_WORKER_H
 #define PICTURE_LOADER_WORKER_H
 
+#include "../../../game/cards/card_database.h"
+#include "picture_loader_worker_work.h"
 #include "../../../game/cards/card_info.h"
 #include "picture_to_load.h"
 
@@ -8,6 +10,7 @@
 #include <QMutex>
 #include <QNetworkAccessManager>
 #include <QObject>
+#include <QTimer>
 
 #define REDIRECT_HEADER_NAME "redirects"
 #define REDIRECT_ORIGINAL_URL "original"
@@ -17,6 +20,7 @@
 
 inline Q_LOGGING_CATEGORY(PictureLoaderWorkerLog, "picture_loader.worker");
 
+class PictureLoaderWorkerWork;
 class PictureLoaderWorker : public QObject
 {
     Q_OBJECT
@@ -24,9 +28,15 @@ public:
     explicit PictureLoaderWorker();
     ~PictureLoaderWorker() override;
 
-    void enqueueImageLoad(CardInfoPtr card);
+    void enqueueImageLoad(const CardInfoPtr &card);
     void clearNetworkCache();
 
+public slots:
+    QNetworkReply *makeRequest(const QUrl &url, PictureLoaderWorkerWork *workThread);
+    void handleRateLimit(const QUrl &url, PictureLoaderWorkerWork *worker);
+    void processQueuedRequests();
+    void imageLoadedSuccessfully(CardInfoPtr card, const QImage &image);
+
 private:
     static QStringList md5Blacklist;
 
@@ -42,18 +52,13 @@ private:
     PictureToLoad cardBeingLoaded;
     PictureToLoad cardBeingDownloaded;
     bool picDownload, downloadRunning, loadQueueRunning;
+    bool rateLimited = false;
+    QTimer rateLimitTimer;
+    QList<QPair<QUrl, PictureLoaderWorkerWork *>> requestQueue;
+    QHash<QUrl, QDateTime> lastRequestTime; // Tracks the last request time for each URL
+    QTimer requestTimer;                    // Timer for processing delayed requests
     bool overrideAllCardArtWithPersonalPreference;
-    void startNextPicDownload();
 
-    /** Emit the `imageLoaded` signal and return `true` if a picture is found on
-        disk, return `false` otherwise.
-
-        If `searchCustomPics` is `true`, the CUSTOM folder is searched for a
-        matching image first; otherwise, only the set-based folders are used. */
-    bool cardImageExistsOnDisk(QString &setName, QString &correctedCardName, bool searchCustomPics);
-
-    bool imageIsBlackListed(const QByteArray &);
-    QNetworkReply *makeRequest(const QUrl &url);
     void cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl);
     QUrl getCachedRedirect(const QUrl &originalUrl) const;
     void loadRedirectCache();
@@ -61,14 +66,9 @@ private:
     void cleanStaleEntries();
 
 private slots:
-    void picDownloadFinished(QNetworkReply *reply);
-    void picDownloadFailed();
-
     void picDownloadChanged();
     void picsPathChanged();
     void setOverrideAllCardArtWithPersonalPreference(bool _overrideAllCardArtWithPersonalPreference);
-public slots:
-    void processLoadQueue();
 
 signals:
     void startLoadQueue();

cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp

@@ -0,0 +1,240 @@
+#include "picture_loader_worker_work.h"
+
+#include "../../../game/cards/card_database_manager.h"
+#include "../../../settings/cache_settings.h"
+#include "picture_loader_worker.h"
+
+#include <QBuffer>
+#include <QDirIterator>
+#include <QLoggingCategory>
+#include <QMovie>
+#include <QNetworkDiskCache>
+#include <QNetworkReply>
+#include <QThread>
+
+// Card back returned by gatherer when card is not found
+QStringList PictureLoaderWorkerWork::md5Blacklist = QStringList() << "db0c48db407a907c16ade38de048a441";
+
+PictureLoaderWorkerWork::PictureLoaderWorkerWork(PictureLoaderWorker *_worker, const CardInfoPtr &toLoad)
+    : QThread(nullptr), worker(_worker), cardToDownload(toLoad)
+{
+    connect(this, &PictureLoaderWorkerWork::requestImageDownload, worker, &PictureLoaderWorker::makeRequest,
+            Qt::QueuedConnection);
+    connect(this, &PictureLoaderWorkerWork::imageLoaded, worker, &PictureLoaderWorker::imageLoadedSuccessfully,
+            Qt::QueuedConnection);
+    pictureLoaderThread = new QThread;
+    pictureLoaderThread->start(QThread::LowPriority);
+    moveToThread(pictureLoaderThread);
+    startNextPicDownload();
+}
+
+PictureLoaderWorkerWork::~PictureLoaderWorkerWork()
+{
+    pictureLoaderThread->deleteLater();
+}
+
+bool PictureLoaderWorkerWork::cardImageExistsOnDisk(QString &setName, QString &correctedCardname)
+{
+    QImage image;
+    QImageReader imgReader;
+    imgReader.setDecideFormatFromContent(true);
+    QList<QString> picsPaths = QList<QString>();
+    QDirIterator it(SettingsCache::instance().getCustomPicsPath(),
+                    QDirIterator::Subdirectories | QDirIterator::FollowSymlinks);
+
+    // Recursively check all subdirectories of the CUSTOM folder
+    while (it.hasNext()) {
+        QString thisPath(it.next());
+        QFileInfo thisFileInfo(thisPath);
+
+        if (thisFileInfo.isFile() &&
+            (thisFileInfo.fileName() == correctedCardname || thisFileInfo.completeBaseName() == correctedCardname ||
+             thisFileInfo.baseName() == correctedCardname)) {
+            picsPaths << thisPath; // Card found in the CUSTOM directory, somewhere
+        }
+    }
+
+    if (!setName.isEmpty()) {
+        picsPaths << SettingsCache::instance().getPicsPath() + "/" + setName + "/" + correctedCardname
+                  // We no longer store downloaded images there, but don't just ignore
+                  // stuff that old versions have put there.
+                  << SettingsCache::instance().getPicsPath() + "/downloadedPics/" + setName + "/" + correctedCardname;
+    }
+
+    // Iterates through the list of paths, searching for images with the desired
+    // name with any QImageReader-supported
+    // extension
+    for (const auto &_picsPath : picsPaths) {
+        imgReader.setFileName(_picsPath);
+        if (imgReader.read(&image)) {
+            qCDebug(PictureLoaderWorkerWorkLog).nospace()
+                << "PictureLoader: [card: " << correctedCardname << " set: " << setName << "]: Picture found on disk.";
+            imageLoaded(cardToDownload.getCard(), image);
+            return true;
+        }
+        imgReader.setFileName(_picsPath + ".full");
+        if (imgReader.read(&image)) {
+            qCDebug(PictureLoaderWorkerWorkLog).nospace() << "PictureLoader: [card: " << correctedCardname
+                                                          << " set: " << setName << "]: Picture.full found on disk.";
+            imageLoaded(cardToDownload.getCard(), image);
+            return true;
+        }
+        imgReader.setFileName(_picsPath + ".xlhq");
+        if (imgReader.read(&image)) {
+            qCDebug(PictureLoaderWorkerWorkLog).nospace() << "PictureLoader: [card: " << correctedCardname
+                                                          << " set: " << setName << "]: Picture.xlhq found on disk.";
+            imageLoaded(cardToDownload.getCard(), image);
+            return true;
+        }
+    }
+
+    return false;
+}
+
+void PictureLoaderWorkerWork::startNextPicDownload()
+{
+    QString picUrl = cardToDownload.getCurrentUrl();
+
+    if (picUrl.isEmpty()) {
+        downloadRunning = false;
+        picDownloadFailed();
+    } else {
+        QUrl url(picUrl);
+        qCDebug(PictureLoaderWorkerWorkLog).nospace()
+            << "PictureLoader: [card: " << cardToDownload.getCard()->getCorrectedName()
+            << " set: " << cardToDownload.getSetName() << "]: Trying to fetch picture from url "
+            << url.toDisplayString();
+        emit requestImageDownload(url, this);
+    }
+}
+
+void PictureLoaderWorkerWork::picDownloadFailed()
+{
+    /* Take advantage of short-circuiting here to call the nextUrl until one
+       is not available.  Only once nextUrl evaluates to false will this move
+       on to nextSet.  If the Urls for a particular card are empty, this will
+       effectively go through the sets for that card. */
+    if (cardToDownload.nextUrl() || cardToDownload.nextSet()) {
+        startNextPicDownload();
+    } else {
+        qCDebug(PictureLoaderWorkerWorkLog).nospace()
+            << "PictureLoader: [card: " << cardToDownload.getCard()->getCorrectedName()
+            << " set: " << cardToDownload.getSetName() << "]: Picture NOT found, "
+            << (picDownload ? "download failed" : "downloads disabled")
+            << ", no more url combinations to try: BAILING OUT";
+        imageLoaded(cardToDownload.getCard(), QImage());
+    }
+    emit startLoadQueue();
+}
+
+void PictureLoaderWorkerWork::picDownloadFinished(QNetworkReply *reply)
+{
+    bool isFromCache = reply->attribute(QNetworkRequest::SourceIsFromCacheAttribute).toBool();
+
+    if (reply->error()) {
+        if (isFromCache) {
+            qCDebug(PictureLoaderWorkerWorkLog).nospace()
+                << "PictureLoader: [card: " << cardToDownload.getCard()->getName()
+                << " set: " << cardToDownload.getSetName() << "]: Removing corrupted cache file for url "
+                << reply->url().toDisplayString() << " and retrying (" << reply->errorString() << ")";
+
+            networkManager->cache()->remove(reply->url());
+
+            requestImageDownload(reply->url(), this);
+        } else {
+            qCDebug(PictureLoaderWorkerWorkLog).nospace()
+                << "PictureLoader: [card: " << cardToDownload.getCard()->getName()
+                << " set: " << cardToDownload.getSetName() << "]: " << (picDownload ? "Download" : "Cache search")
+                << " failed for url " << reply->url().toDisplayString() << " (" << reply->errorString() << ")";
+
+            picDownloadFailed();
+            startNextPicDownload();
+        }
+
+        reply->deleteLater();
+        return;
+    }
+
+    // List of status codes from https://doc.qt.io/qt-6/qnetworkreply.html#redirected
+    int statusCode = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    if (statusCode == 301 || statusCode == 302 || statusCode == 303 || statusCode == 305 || statusCode == 307 ||
+        statusCode == 308) {
+        QUrl redirectUrl = reply->header(QNetworkRequest::LocationHeader).toUrl();
+        qCDebug(PictureLoaderWorkerWorkLog).nospace()
+            << "PictureLoader: [card: " << cardToDownload.getCard()->getName()
+            << " set: " << cardToDownload.getSetName() << "]: following "
+            << (isFromCache ? "cached redirect" : "redirect") << " to " << redirectUrl.toDisplayString();
+        requestImageDownload(redirectUrl, this);
+        reply->deleteLater();
+        return;
+    }
+
+    if (statusCode == 429) {
+        qWarning() << "Scryfall API limit reached!";
+    }
+
+    // peek is used to keep the data in the buffer for use by QImageReader
+    const QByteArray &picData = reply->peek(reply->size());
+
+    if (imageIsBlackListed(picData)) {
+        qCDebug(PictureLoaderWorkerWorkLog).nospace()
+            << "PictureLoader: [card: " << cardToDownload.getCard()->getName()
+            << " set: " << cardToDownload.getSetName()
+            << "]: Picture found, but blacklisted, will consider it as not found";
+
+        picDownloadFailed();
+        reply->deleteLater();
+        startNextPicDownload();
+        return;
+    }
+
+    QImage testImage;
+
+    QImageReader imgReader;
+    imgReader.setDecideFormatFromContent(true);
+    imgReader.setDevice(reply);
+
+    bool logSuccessMessage = false;
+
+    static const int riffHeaderSize = 12; // RIFF_HEADER_SIZE from webp/format_constants.h
+    auto replyHeader = reply->peek(riffHeaderSize);
+
+    if (replyHeader.startsWith("RIFF") && replyHeader.endsWith("WEBP")) {
+        auto imgBuf = QBuffer(this);
+        imgBuf.setData(reply->readAll());
+
+        auto movie = QMovie(&imgBuf);
+        movie.start();
+        movie.stop();
+
+        imageLoaded(cardToDownload.getCard(), movie.currentImage());
+        logSuccessMessage = true;
+    } else if (imgReader.read(&testImage)) {
+        imageLoaded(cardToDownload.getCard(), testImage);
+        logSuccessMessage = true;
+    } else {
+        qCDebug(PictureLoaderWorkerWorkLog).nospace()
+            << "PictureLoader: [card: " << cardToDownload.getCard()->getName()
+            << " set: " << cardToDownload.getSetName() << "]: Possible " << (isFromCache ? "cached" : "downloaded")
+            << " picture at " << reply->url().toDisplayString() << " could not be loaded: " << reply->errorString();
+
+        picDownloadFailed();
+    }
+
+    if (logSuccessMessage) {
+        qCDebug(PictureLoaderWorkerWorkLog).nospace()
+            << "PictureLoader: [card: " << cardToDownload.getCard()->getName()
+            << " set: " << cardToDownload.getSetName() << "]: Image successfully "
+            << (isFromCache ? "loaded from cached" : "downloaded from") << " url " << reply->url().toDisplayString();
+    } else {
+        startNextPicDownload();
+    }
+
+    reply->deleteLater();
+}
+
+bool PictureLoaderWorkerWork::imageIsBlackListed(const QByteArray &picData)
+{
+    QString md5sum = QCryptographicHash::hash(picData, QCryptographicHash::Md5).toHex();
+    return md5Blacklist.contains(md5sum);
+}

cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.h

@@ -0,0 +1,50 @@
+#ifndef PICTURE_LOADER_WORKER_WORK_H
+#define PICTURE_LOADER_WORKER_WORK_H
+
+#include "../../../game/cards/card_database.h"
+#include "picture_loader_worker.h"
+#include "picture_to_load.h"
+
+#include <QLoggingCategory>
+#include <QMutex>
+#include <QNetworkAccessManager>
+#include <QObject>
+#include <QThread>
+
+#define REDIRECT_HEADER_NAME "redirects"
+#define REDIRECT_ORIGINAL_URL "original"
+#define REDIRECT_URL "redirect"
+#define REDIRECT_TIMESTAMP "timestamp"
+#define REDIRECT_CACHE_FILENAME "cache.ini"
+
+inline Q_LOGGING_CATEGORY(PictureLoaderWorkerWorkLog, "picture_loader.worker");
+
+class PictureLoaderWorker;
+class PictureLoaderWorkerWork : public QThread
+{
+    Q_OBJECT
+public:
+    explicit PictureLoaderWorkerWork(PictureLoaderWorker *worker, const CardInfoPtr &toLoad);
+    ~PictureLoaderWorkerWork() override;
+    PictureLoaderWorker *worker;
+    PictureToLoad cardToDownload;
+public slots:
+    void picDownloadFinished(QNetworkReply *reply);
+    void picDownloadFailed();
+
+private:
+    static QStringList md5Blacklist;
+    QThread *pictureLoaderThread;
+    QNetworkAccessManager *networkManager;
+    bool picDownload, downloadRunning, loadQueueRunning;
+    void startNextPicDownload();
+    bool cardImageExistsOnDisk(QString &setName, QString &correctedCardName);
+    bool imageIsBlackListed(const QByteArray &);
+
+signals:
+    void startLoadQueue();
+    void imageLoaded(CardInfoPtr card, const QImage &image);
+    void requestImageDownload(const QUrl &url, PictureLoaderWorkerWork *instance);
+};
+
+#endif // PICTURE_LOADER_WORKER_WORK_H

oracle/CMakeLists.txt

@@ -22,6 +22,7 @@ set(oracle_SOURCES
     ../cockatrice/src/game/cards/card_info.cpp
     ../cockatrice/src/client/ui/picture_loader/picture_loader.cpp
     ../cockatrice/src/client/ui/picture_loader/picture_loader_worker.cpp
+    ../cockatrice/src/client/ui/picture_loader/picture_loader_worker_work.cpp
     ../cockatrice/src/client/ui/picture_loader/picture_to_load.cpp
     ../cockatrice/src/game/cards/card_database_parser/card_database_parser.cpp
     ../cockatrice/src/game/cards/card_database_parser/cockatrice_xml_3.cpp