New Orchestrator and Worker

cockatrice/CMakeLists.txt

@@ -95,9 +95,12 @@ set(cockatrice_SOURCES
     src/game/phase.cpp
     src/client/ui/phases_toolbar.cpp
     src/client/ui/picture_loader/picture_loader.cpp
+    src/client/ui/picture_loader/picture_loader_orchestrator.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/picture_loader/rate_limited_network_manager.cpp
+    src/client/ui/picture_loader/new_picture_loader_orchestrator.cpp
+    src/client/ui/picture_loader/new_picture_loader_worker.cpp
     src/game/zones/pile_zone.cpp
     src/client/ui/pixel_map_generator.cpp
     src/game/player/player.cpp

cockatrice/resources/config/qtlogging.ini

@@ -32,7 +32,7 @@
 # user_info_connection = false
 
 # picture_loader = false
-# picture_loader.worker = false
+# picture_loader.orchestrator = false
 # picture_loader.card_back_cache_fail = false
 # picture_loader.picture_to_load = false
 # deck_loader = false

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

@@ -0,0 +1,65 @@
+#include "new_picture_loader_orchestrator.h"
+
+#include "new_picture_loader_worker.h"
+#include "picture_to_load.h"
+
+#include <QImage>
+#include <QNetworkRequest>
+#include <QThread>
+#include <QTimer>
+
+NewPictureLoaderOrchestrator::NewPictureLoaderOrchestrator(const unsigned int _maxRequestsPerSecond, QObject *parent)
+    : QObject(parent), maxRequestsPerSecond(_maxRequestsPerSecond)
+{
+    dispatchTimer = new QTimer(this);
+    connect(dispatchTimer, &QTimer::timeout, this, &NewPictureLoaderOrchestrator::dequeueRequests);
+    dispatchTimer->start(1000);
+}
+
+void NewPictureLoaderOrchestrator::enqueueImageLoad(CardInfoPtr card)
+{
+    const PictureToLoad cardToDownload(card);
+    const QUrl cardImageUrl(cardToDownload.getCurrentUrl());
+    auto *networkRequest = new QNetworkRequest(cardImageUrl);
+
+    qDebug() << "Enqueued" << card->getName() << "for" << card->getPixmapCacheKey();
+    requestsQueue.enqueue(std::make_pair<>(card, networkRequest));
+}
+
+void NewPictureLoaderOrchestrator::dequeueRequests()
+{
+    dispatchTimer->stop();
+
+    for (unsigned int i = 0; i < maxRequestsPerSecond && !requestsQueue.isEmpty(); ++i) {
+        const auto &cardInfoAndRequest = requestsQueue.dequeue();
+
+        auto *worker = new NewPictureLoaderWorker(nullptr, cardInfoAndRequest.first, cardInfoAndRequest.second);
+
+        auto *workerThread = new QThread;
+
+        // Handle startup and cleanup for the worker thread
+        connect(workerThread, &QThread::started, worker, &NewPictureLoaderWorker::doWork);
+        connect(workerThread, &QThread::finished, workerThread, &QObject::deleteLater);
+
+        // Kill the thread when it is done working (whether successful or not)
+        connect(worker, &NewPictureLoaderWorker::workFinished, workerThread, &QThread::quit);
+
+        // If the picture downloading was successful, cleanup the assets & load the image
+        connect(worker, &NewPictureLoaderWorker::workFinishedSuccessfully, this, [=]() {
+            delete cardInfoAndRequest.second;
+        });
+        connect(worker, &NewPictureLoaderWorker::workFinishedSuccessfully, this,
+                &NewPictureLoaderOrchestrator::loadImage);
+
+        // If the picture downloading was unsuccessful, re-enqueue the contents for later
+        connect(worker, &NewPictureLoaderWorker::workFinishedUnsuccessfully, this, [=, this]() {
+            qDebug() << "There was an error downloading" << cardInfoAndRequest.first->getName();
+            requestsQueue.enqueue(cardInfoAndRequest);
+        });
+
+        worker->moveToThread(workerThread);
+        workerThread->start(QThread::LowPriority);
+    }
+
+    dispatchTimer->start();
+}

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

@@ -0,0 +1,33 @@
+#ifndef COCKATRICE_NEW_PICTURE_LOADER_ORCHESTRATOR_H
+#define COCKATRICE_NEW_PICTURE_LOADER_ORCHESTRATOR_H
+
+#include "../../../game/cards/card_database.h"
+
+#include <QObject>
+#include <QQueue>
+
+class QNetworkRequest;
+class QTimer;
+
+class QNetworkRequest;
+
+class NewPictureLoaderOrchestrator : public QObject
+{
+    Q_OBJECT
+
+public:
+    explicit NewPictureLoaderOrchestrator(unsigned int _maxRequestsPerSecond, QObject *parent = nullptr);
+    void enqueueImageLoad(CardInfoPtr card);
+
+signals:
+    void loadImage(CardInfoPtr cardInfoPtr, QImage *image);
+
+private:
+    unsigned int maxRequestsPerSecond;
+    QQueue<QPair<CardInfoPtr, QNetworkRequest *>> requestsQueue;
+    QTimer *dispatchTimer;
+
+    void dequeueRequests();
+};
+
+#endif // COCKATRICE_NEW_PICTURE_LOADER_ORCHESTRATOR_H

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

@@ -0,0 +1,67 @@
+#include "new_picture_loader_worker.h"
+
+#include <QBuffer>
+#include <QImageReader>
+#include <QMovie>
+#include <QNetworkAccessManager>
+#include <QNetworkReply>
+
+NewPictureLoaderWorker::NewPictureLoaderWorker(QObject *parent,
+                                               CardInfoPtr _cardInfoPtr,
+                                               QNetworkRequest *_networkRequest)
+    : QObject(parent), networkManager(new QNetworkAccessManager(this)), networkRequest(_networkRequest),
+      cardInfoPtr(_cardInfoPtr)
+{
+    networkManager->setTransferTimeout();
+}
+
+void NewPictureLoaderWorker::doWork()
+{
+    qDebug() << "Starting Download for" << cardInfoPtr->getName() << networkRequest->url();
+    auto *reply = networkManager->get(*networkRequest);
+    connect(reply, &QNetworkReply::finished, this, [this, reply]() {
+        std::optional<QImage*> image;
+
+        if (reply->error() == QNetworkReply::NoError) {
+            image = getImageFromReply(reply);
+        }
+
+        reply->deleteLater();
+        if (image.has_value()) {
+            qDebug() << "Download successful for" << cardInfoPtr->getName() << networkRequest->url();
+            emit workFinishedSuccessfully(cardInfoPtr, image.value());
+        } else {
+            qDebug() << "Download failed for" << cardInfoPtr->getName() << networkRequest->url();
+            emit workFinishedUnsuccessfully();
+        }
+        emit workFinished();
+    });
+}
+
+std::optional<QImage*> NewPictureLoaderWorker::getImageFromReply(QNetworkReply *networkReply)
+{
+    QImageReader imgReader;
+    imgReader.setDecideFormatFromContent(true);
+    imgReader.setDevice(networkReply);
+
+    static const int riffHeaderSize = 12; // RIFF_HEADER_SIZE from webp/format_constants.h
+    const auto &replyHeader = networkReply->peek(riffHeaderSize);
+
+    if (replyHeader.startsWith("RIFF") && replyHeader.endsWith("WEBP")) {
+        auto imgBuf = QBuffer(this);
+        imgBuf.setData(networkReply->readAll());
+
+        auto movie = QMovie(&imgBuf);
+        movie.start();
+        movie.stop();
+
+        return new QImage(movie.currentImage());
+    }
+
+    auto *testImage = new QImage();
+    if (imgReader.read(testImage)) {
+        return testImage;
+    }
+
+    return std::nullopt;
+}

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

@@ -0,0 +1,31 @@
+#ifndef COCKATRICE_NEW_PICTURE_LOADER_WORKER_H
+#define COCKATRICE_NEW_PICTURE_LOADER_WORKER_H
+
+#include "../../../game/cards/card_database.h"
+
+#include <QNetworkAccessManager>
+#include <QNetworkRequest>
+#include <QObject>
+
+class NewPictureLoaderWorker : public QObject
+{
+    Q_OBJECT
+
+public:
+    explicit NewPictureLoaderWorker(QObject *parent, CardInfoPtr _cardInfoPtr, QNetworkRequest *_networkRequest);
+    void doWork();
+
+signals:
+    void workFinishedSuccessfully(CardInfoPtr, QImage *);
+    void workFinishedUnsuccessfully();
+    void workFinished();
+
+private:
+    QNetworkAccessManager *networkManager;
+    QNetworkRequest *networkRequest;
+    CardInfoPtr cardInfoPtr;
+
+    std::optional<QImage*> getImageFromReply(QNetworkReply *networkReply);
+};
+
+#endif // COCKATRICE_NEW_PICTURE_LOADER_WORKER_H

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

@@ -1,6 +1,7 @@
 #include "picture_loader.h"
 
 #include "../../../settings/cache_settings.h"
+#include "new_picture_loader_orchestrator.h"
 
 #include <QApplication>
 #include <QBuffer>
@@ -22,17 +23,16 @@
 
 PictureLoader::PictureLoader() : QObject(nullptr)
 {
-    worker = new PictureLoaderWorker;
     connect(&SettingsCache::instance(), SIGNAL(picsPathChanged()), this, SLOT(picsPathChanged()));
     connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged()));
 
-    connect(worker, SIGNAL(imageLoaded(CardInfoPtr, const QImage &)), this,
-            SLOT(imageLoaded(CardInfoPtr, const QImage &)));
+    orchestrator = new NewPictureLoaderOrchestrator(10, this);
+    connect(orchestrator, &NewPictureLoaderOrchestrator::loadImage, this, &PictureLoader::imageLoaded);
 }
 
 PictureLoader::~PictureLoader()
 {
-    worker->deleteLater();
+    orchestrator->deleteLater();
 }
 
 void PictureLoader::getCardBackPixmap(QPixmap &pixmap, QSize size)
@@ -89,23 +89,24 @@ void PictureLoader::getPixmap(QPixmap &pixmap, CardInfoPtr card, QSize size)
     }
 
     // add the card to the load queue
-    qCDebug(PictureLoaderLog) << "Enqueuing " << card->getName() << " for " << card->getPixmapCacheKey();
-    getInstance().worker->enqueueImageLoad(card);
+    qCDebug(PictureLoaderLog) << "Enqueuing" << card->getName() << "for" << card->getPixmapCacheKey();
+    getInstance().orchestrator->enqueueImageLoad(card);
 }
 
-void PictureLoader::imageLoaded(CardInfoPtr card, const QImage &image)
+void PictureLoader::imageLoaded(CardInfoPtr card, QImage *image)
 {
-    if (image.isNull()) {
+    if (image->isNull()) {
         QPixmapCache::insert(card->getPixmapCacheKey(), QPixmap());
     } else {
         if (card->getUpsideDownArt()) {
-            QImage mirrorImage = image.mirrored(true, true);
+            QImage mirrorImage = image->mirrored(true, true);
             QPixmapCache::insert(card->getPixmapCacheKey(), QPixmap::fromImage(mirrorImage));
         } else {
-            QPixmapCache::insert(card->getPixmapCacheKey(), QPixmap::fromImage(image));
+            QPixmapCache::insert(card->getPixmapCacheKey(), QPixmap::fromImage(*image));
         }
     }
 
+    delete image;
     card->emitPixmapUpdated();
 }
 
@@ -123,7 +124,7 @@ void PictureLoader::clearPixmapCache()
 
 void PictureLoader::clearNetworkCache()
 {
-    getInstance().worker->clearNetworkCache();
+//    getInstance().orchestrator->clearNetworkCache();
 }
 
 void PictureLoader::cacheCardPixmaps(QList<CardInfoPtr> cards)
@@ -141,7 +142,7 @@ void PictureLoader::cacheCardPixmaps(QList<CardInfoPtr> cards)
             continue;
         }
 
-        getInstance().worker->enqueueImageLoad(card);
+        getInstance().orchestrator->enqueueImageLoad(card);
     }
 }
 

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

@@ -2,7 +2,7 @@
 #define PICTURELOADER_H
 
 #include "../../../game/cards/card_database.h"
-#include "picture_loader_worker.h"
+#include "new_picture_loader_orchestrator.h"
 
 #include <QLoggingCategory>
 
@@ -26,7 +26,7 @@ private:
     PictureLoader(PictureLoader const &);
     void operator=(PictureLoader const &);
 
-    PictureLoaderWorker *worker;
+    NewPictureLoaderOrchestrator *orchestrator;
 
 public:
     static void getPixmap(QPixmap &pixmap, CardInfoPtr card, QSize size);
@@ -45,6 +45,6 @@ private slots:
     void picsPathChanged();
 
 public slots:
-    void imageLoaded(CardInfoPtr card, const QImage &image);
+    void imageLoaded(CardInfoPtr card, QImage *image);
 };
 #endif

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

@@ -0,0 +1,242 @@
+#include "picture_loader_orchestrator.h"
+
+#include "../../../game/cards/card_database_manager.h"
+#include "../../../settings/cache_settings.h"
+#include "picture_loader_worker.h"
+#include "rate_limited_network_manager.h"
+
+#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 PictureLoaderOrchestrator::md5Blacklist = QStringList() << "db0c48db407a907c16ade38de048a441";
+
+/*
+ * Generic idea:
+ * - Orchestrator can fire off up to X threads per second, each which will run right away
+ * - Orchestrator will keep a backlog of requests
+ * -
+ *
+ */
+
+PictureLoaderOrchestrator::PictureLoaderOrchestrator()
+    : QObject(nullptr), picsPath(SettingsCache::instance().getPicsPath()),
+      customPicsPath(SettingsCache::instance().getCustomPicsPath()),
+      picDownload(SettingsCache::instance().getPicDownload()), downloadRunning(false), loadQueueRunning(false)
+{
+    connect(&SettingsCache::instance(), SIGNAL(picsPathChanged()), this, SLOT(picsPathChanged()));
+    connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged()));
+
+    networkManager = new QNetworkAccessManager(this);
+    // We need a timeout to ensure requests don't hang indefinitely in case of
+    // cache corruption, see related Qt bug: https://bugreports.qt.io/browse/QTBUG-111397
+    // Use Qt's default timeout (30s, as of 2023-02-22)
+#if (QT_VERSION >= QT_VERSION_CHECK(5, 15, 0))
+    networkManager->setTransferTimeout();
+#endif
+    auto cache = new QNetworkDiskCache(this);
+    cache->setCacheDirectory(SettingsCache::instance().getNetworkCachePath());
+    cache->setMaximumCacheSize(1024L * 1024L *
+                               static_cast<qint64>(SettingsCache::instance().getNetworkCacheSizeInMB()));
+    // Note: the settings is in MB, but QNetworkDiskCache uses bytes
+    connect(&SettingsCache::instance(), &SettingsCache::networkCacheSizeChanged, cache,
+            [cache](int newSizeInMB) { cache->setMaximumCacheSize(1024L * 1024L * static_cast<qint64>(newSizeInMB)); });
+    networkManager->setCache(cache);
+    // 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);
+
+    cacheFilePath = SettingsCache::instance().getRedirectCachePath() + REDIRECT_CACHE_FILENAME;
+    loadRedirectCache();
+    cleanStaleEntries();
+
+    connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this,
+            &PictureLoaderOrchestrator::saveRedirectCache);
+
+    pictureLoaderThread = new QThread;
+    pictureLoaderThread->start(QThread::LowPriority);
+    moveToThread(pictureLoaderThread);
+}
+
+PictureLoaderOrchestrator::~PictureLoaderOrchestrator()
+{
+    pictureLoaderThread->deleteLater();
+}
+
+QNetworkReply *PictureLoaderOrchestrator::makeRequest(const QUrl &url, PictureLoaderWorker *worker)
+{
+    if (rateLimited) {
+        // Queue the request if currently rate-limited
+        requestQueue.append(qMakePair(url, worker));
+        return nullptr; // No immediate request
+    }
+
+    QUrl cachedRedirect = getCachedRedirect(url);
+    if (!cachedRedirect.isEmpty()) {
+        return makeRequest(cachedRedirect, worker);
+    }
+
+    QNetworkRequest req(url);
+    if (!picDownload) {
+        req.setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::AlwaysCache);
+    }
+
+    QNetworkReply *reply = networkManager->get(req);
+
+    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);
+        }
+
+        if (reply->error() == QNetworkReply::NoError) {
+            worker->picDownloadFinished(reply);
+        } else if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 429) {
+            handleRateLimit(reply, url, worker);
+        } else {
+            worker->picDownloadFinished(reply);
+        }
+        reply->deleteLater();
+    });
+
+    return reply;
+}
+
+void PictureLoaderOrchestrator::handleRateLimit(QNetworkReply *reply, const QUrl &url, PictureLoaderWorker *worker)
+{
+    QByteArray responseData = reply->readAll();
+    QJsonDocument jsonDoc = QJsonDocument::fromJson(responseData);
+
+    if (jsonDoc.isObject()) {
+        QJsonObject jsonObj = jsonDoc.object();
+        if (jsonObj.value("object").toString() == "error" && jsonObj.value("code").toString() == "rate_limited") {
+            int retryAfter = 70; // Default retry delay
+
+            // Prevent multiple rate-limit handling
+            if (!rateLimited) {
+                rateLimited = true;
+                qWarning() << "Scryfall rate limit hit! 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 PictureLoaderOrchestrator::processQueuedRequests()
+{
+    qWarning() << "Resuming queued requests";
+    rateLimited = false;
+
+    while (!requestQueue.isEmpty()) {
+        QPair<QUrl, PictureLoaderWorker *> request = requestQueue.takeFirst();
+        makeRequest(request.first, request.second);
+    }
+}
+
+void PictureLoaderOrchestrator::enqueueImageLoad(const CardInfoPtr &card)
+{
+    auto worker = new PictureLoaderWorker(this, card);
+    Q_UNUSED(worker);
+}
+
+void PictureLoaderOrchestrator::imageLoadedSuccessfully(CardInfoPtr card, const QImage &image)
+{
+    emit imageLoaded(std::move(card), image);
+}
+
+void PictureLoaderOrchestrator::cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl)
+{
+    redirectCache[originalUrl] = qMakePair(redirectUrl, QDateTime::currentDateTimeUtc());
+    // saveRedirectCache();
+}
+
+QUrl PictureLoaderOrchestrator::getCachedRedirect(const QUrl &originalUrl) const
+{
+    if (redirectCache.contains(originalUrl)) {
+        return redirectCache[originalUrl].first;
+    }
+    return {};
+}
+
+void PictureLoaderOrchestrator::loadRedirectCache()
+{
+    QSettings settings(cacheFilePath, QSettings::IniFormat);
+
+    redirectCache.clear();
+    int size = settings.beginReadArray(REDIRECT_HEADER_NAME);
+    for (int i = 0; i < size; ++i) {
+        settings.setArrayIndex(i);
+        QUrl originalUrl = settings.value(REDIRECT_ORIGINAL_URL).toUrl();
+        QUrl redirectUrl = settings.value(REDIRECT_URL).toUrl();
+        QDateTime timestamp = settings.value(REDIRECT_TIMESTAMP).toDateTime();
+
+        if (originalUrl.isValid() && redirectUrl.isValid()) {
+            redirectCache[originalUrl] = qMakePair(redirectUrl, timestamp);
+        }
+    }
+    settings.endArray();
+}
+
+void PictureLoaderOrchestrator::saveRedirectCache() const
+{
+    QSettings settings(cacheFilePath, QSettings::IniFormat);
+
+    settings.beginWriteArray(REDIRECT_HEADER_NAME, static_cast<int>(redirectCache.size()));
+    int index = 0;
+    for (auto it = redirectCache.cbegin(); it != redirectCache.cend(); ++it) {
+        settings.setArrayIndex(index++);
+        settings.setValue(REDIRECT_ORIGINAL_URL, it.key());
+        settings.setValue(REDIRECT_URL, it.value().first);
+        settings.setValue(REDIRECT_TIMESTAMP, it.value().second);
+    }
+    settings.endArray();
+}
+
+void PictureLoaderOrchestrator::cleanStaleEntries()
+{
+    QDateTime now = QDateTime::currentDateTimeUtc();
+
+    auto it = redirectCache.begin();
+    while (it != redirectCache.end()) {
+        if (it.value().second.addDays(SettingsCache::instance().getRedirectCacheTtl()) < now) {
+            it = redirectCache.erase(it); // Remove stale entry
+        } else {
+            ++it;
+        }
+    }
+}
+
+void PictureLoaderOrchestrator::picDownloadChanged()
+{
+    QMutexLocker locker(&mutex);
+    picDownload = SettingsCache::instance().getPicDownload();
+}
+
+void PictureLoaderOrchestrator::picsPathChanged()
+{
+    QMutexLocker locker(&mutex);
+    picsPath = SettingsCache::instance().getPicsPath();
+    customPicsPath = SettingsCache::instance().getCustomPicsPath();
+}
+
+void PictureLoaderOrchestrator::clearNetworkCache()
+{
+    networkManager->cache()->clear();
+}

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

@@ -0,0 +1,74 @@
+#ifndef PICTURE_LOADER_WORKER_H
+#define PICTURE_LOADER_WORKER_H
+
+#include "../../../game/cards/card_database.h"
+#include "picture_loader_worker.h"
+#include "picture_to_load.h"
+#include "rate_limited_network_manager.h"
+
+#include <QLoggingCategory>
+#include <QMutex>
+#include <QNetworkAccessManager>
+#include <QObject>
+#include <QTimer>
+
+#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(PictureLoaderWorkerLog, "picture_loader.orchestrator");
+
+class PictureLoaderWorker;
+class PictureLoaderOrchestrator : public QObject
+{
+    Q_OBJECT
+public:
+    explicit PictureLoaderOrchestrator();
+    ~PictureLoaderOrchestrator() override;
+
+    void enqueueImageLoad(const CardInfoPtr &card);
+    void clearNetworkCache();
+
+public slots:
+    QNetworkReply *makeRequest(const QUrl &url, PictureLoaderWorker *workThread);
+    void handleRateLimit(QNetworkReply *reply, const QUrl &url, PictureLoaderWorker *worker);
+    void processQueuedRequests();
+    void imageLoadedSuccessfully(CardInfoPtr card, const QImage &image);
+
+private:
+    static QStringList md5Blacklist;
+
+    QThread *pictureLoaderThread;
+    QString picsPath, customPicsPath;
+    QList<PictureToLoad> loadQueue;
+    QMutex mutex;
+    QNetworkAccessManager *networkManager;
+    QHash<QUrl, QPair<QUrl, QDateTime>> redirectCache; // Stores redirect and timestamp
+    QString cacheFilePath;                             // Path to persistent storage
+    static constexpr int CacheTTLInDays = 30;          // TODO: Make user configurable
+    QList<PictureToLoad> cardsToDownload;
+    PictureToLoad cardBeingLoaded;
+    PictureToLoad cardBeingDownloaded;
+    bool picDownload, downloadRunning, loadQueueRunning;
+    bool rateLimited = false;
+    QTimer rateLimitTimer;
+    QList<QPair<QUrl, PictureLoaderWorker *>> requestQueue;
+
+    void cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl);
+    QUrl getCachedRedirect(const QUrl &originalUrl) const;
+    void loadRedirectCache();
+    void saveRedirectCache() const;
+    void cleanStaleEntries();
+
+private slots:
+    void picDownloadChanged();
+    void picsPathChanged();
+
+signals:
+    void startLoadQueue();
+    void imageLoaded(CardInfoPtr card, const QImage &image);
+};
+
+#endif // PICTURE_LOADER_WORKER_H

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

@@ -2,56 +2,30 @@
 
 #include "../../../game/cards/card_database_manager.h"
 #include "../../../settings/cache_settings.h"
-#include "picture_loader_worker_work.h"
+#include "picture_loader_orchestrator.h"
 
+#include <QBuffer>
 #include <QDirIterator>
-#include <QJsonDocument>
+#include <QLoggingCategory>
 #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";
 
-PictureLoaderWorker::PictureLoaderWorker()
-    : QObject(nullptr), picsPath(SettingsCache::instance().getPicsPath()),
-      customPicsPath(SettingsCache::instance().getCustomPicsPath()),
-      picDownload(SettingsCache::instance().getPicDownload()), downloadRunning(false), loadQueueRunning(false)
+PictureLoaderWorker::PictureLoaderWorker(PictureLoaderOrchestrator *_orchestrator, const CardInfoPtr &toLoad)
+    : QThread(nullptr), orchestrator(_orchestrator), cardToDownload(toLoad)
 {
-    connect(&SettingsCache::instance(), SIGNAL(picsPathChanged()), this, SLOT(picsPathChanged()));
-    connect(&SettingsCache::instance(), SIGNAL(picDownloadChanged()), this, SLOT(picDownloadChanged()));
-
-    networkManager = new QNetworkAccessManager(this);
-    // We need a timeout to ensure requests don't hang indefinitely in case of
-    // cache corruption, see related Qt bug: https://bugreports.qt.io/browse/QTBUG-111397
-    // Use Qt's default timeout (30s, as of 2023-02-22)
-#if (QT_VERSION >= QT_VERSION_CHECK(5, 15, 0))
-    networkManager->setTransferTimeout();
-#endif
-    auto cache = new QNetworkDiskCache(this);
-    cache->setCacheDirectory(SettingsCache::instance().getNetworkCachePath());
-    cache->setMaximumCacheSize(1024L * 1024L *
-                               static_cast<qint64>(SettingsCache::instance().getNetworkCacheSizeInMB()));
-    // Note: the settings is in MB, but QNetworkDiskCache uses bytes
-    connect(&SettingsCache::instance(), &SettingsCache::networkCacheSizeChanged, cache,
-            [cache](int newSizeInMB) { cache->setMaximumCacheSize(1024L * 1024L * static_cast<qint64>(newSizeInMB)); });
-    networkManager->setCache(cache);
-    // 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);
-
-    cacheFilePath = SettingsCache::instance().getRedirectCachePath() + REDIRECT_CACHE_FILENAME;
-    loadRedirectCache();
-    cleanStaleEntries();
-
-    connect(QCoreApplication::instance(), &QCoreApplication::aboutToQuit, this,
-            &PictureLoaderWorker::saveRedirectCache);
-
+    connect(this, &PictureLoaderWorker::requestImageDownload, orchestrator, &PictureLoaderOrchestrator::makeRequest,
+            Qt::QueuedConnection);
+    connect(this, &PictureLoaderWorker::imageLoaded, orchestrator, &PictureLoaderOrchestrator::imageLoadedSuccessfully,
+            Qt::QueuedConnection);
     pictureLoaderThread = new QThread;
     pictureLoaderThread->start(QThread::LowPriority);
     moveToThread(pictureLoaderThread);
+    startNextPicDownload();
 }
 
 PictureLoaderWorker::~PictureLoaderWorker()
@@ -59,175 +33,208 @@ PictureLoaderWorker::~PictureLoaderWorker()
     pictureLoaderThread->deleteLater();
 }
 
-QNetworkReply *PictureLoaderWorker::makeRequest(const QUrl &url, PictureLoaderWorkerWork *worker)
+bool PictureLoaderWorker::cardImageExistsOnDisk(QString &setName, QString &correctedCardname)
 {
-    if (rateLimited) {
-        // Queue the request if currently rate-limited
-        requestQueue.append(qMakePair(url, worker));
-        return nullptr; // No immediate request
-    }
+    QImage image;
+    QImageReader imgReader;
+    imgReader.setDecideFormatFromContent(true);
+    QList<QString> picsPaths = QList<QString>();
+    QDirIterator it(SettingsCache::instance().getCustomPicsPath(),
+                    QDirIterator::Subdirectories | QDirIterator::FollowSymlinks);
 
-    QUrl cachedRedirect = getCachedRedirect(url);
-    if (!cachedRedirect.isEmpty()) {
-        return makeRequest(cachedRedirect, worker);
-    }
+    // Recursively check all subdirectories of the CUSTOM folder
+    while (it.hasNext()) {
+        QString thisPath(it.next());
+        QFileInfo thisFileInfo(thisPath);
 
-    QNetworkRequest req(url);
-    if (!picDownload) {
-        req.setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::AlwaysCache);
-    }
-
-    QNetworkReply *reply = networkManager->get(req);
-
-    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);
+        if (thisFileInfo.isFile() &&
+            (thisFileInfo.fileName() == correctedCardname || thisFileInfo.completeBaseName() == correctedCardname ||
+             thisFileInfo.baseName() == correctedCardname)) {
+            picsPaths << thisPath; // Card found in the CUSTOM directory, somewhere
         }
+    }
 
-        if (reply->error() == QNetworkReply::NoError) {
-            worker->picDownloadFinished(reply);
-        } else if (reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() == 429) {
-            handleRateLimit(reply, url, worker);
+    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 PictureLoaderWorker::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 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 (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 PictureLoaderWorker::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 {
-            worker->picDownloadFinished(reply);
+            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 reply;
-}
-
-void PictureLoaderWorker::handleRateLimit(QNetworkReply *reply, const QUrl &url, PictureLoaderWorkerWork *worker)
-{
-    QByteArray responseData = reply->readAll();
-    QJsonDocument jsonDoc = QJsonDocument::fromJson(responseData);
-
-    if (jsonDoc.isObject()) {
-        QJsonObject jsonObj = jsonDoc.object();
-        if (jsonObj.value("object").toString() == "error" && jsonObj.value("code").toString() == "rate_limited") {
-            int retryAfter = 70; // Default retry delay
-
-            // Prevent multiple rate-limit handling
-            if (!rateLimited) {
-                rateLimited = true;
-                qWarning() << "Scryfall rate limit hit! 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));
-        }
+        return;
     }
-}
 
-void PictureLoaderWorker::processQueuedRequests()
-{
-    qWarning() << "Resuming queued requests";
-    rateLimited = false;
-
-    while (!requestQueue.isEmpty()) {
-        QPair<QUrl, PictureLoaderWorkerWork *> request = requestQueue.takeFirst();
-        makeRequest(request.first, request.second);
+    // 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;
     }
-}
 
-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();
-}
-
-QUrl PictureLoaderWorker::getCachedRedirect(const QUrl &originalUrl) const
-{
-    if (redirectCache.contains(originalUrl)) {
-        return redirectCache[originalUrl].first;
+    if (statusCode == 429) {
+        qWarning() << "Scryfall API limit reached!";
     }
-    return {};
-}
 
-void PictureLoaderWorker::loadRedirectCache()
-{
-    QSettings settings(cacheFilePath, QSettings::IniFormat);
+    // peek is used to keep the data in the buffer for use by QImageReader
+    const QByteArray &picData = reply->peek(reply->size());
 
-    redirectCache.clear();
-    int size = settings.beginReadArray(REDIRECT_HEADER_NAME);
-    for (int i = 0; i < size; ++i) {
-        settings.setArrayIndex(i);
-        QUrl originalUrl = settings.value(REDIRECT_ORIGINAL_URL).toUrl();
-        QUrl redirectUrl = settings.value(REDIRECT_URL).toUrl();
-        QDateTime timestamp = settings.value(REDIRECT_TIMESTAMP).toDateTime();
+    if (imageIsBlackListed(picData)) {
+        qCDebug(PictureLoaderWorkerWorkLog).nospace()
+            << "PictureLoader: [card: " << cardToDownload.getCard()->getName()
+            << " set: " << cardToDownload.getSetName()
+            << "]: Picture found, but blacklisted, will consider it as not found";
 
-        if (originalUrl.isValid() && redirectUrl.isValid()) {
-            redirectCache[originalUrl] = qMakePair(redirectUrl, timestamp);
-        }
+        picDownloadFailed();
+        reply->deleteLater();
+        startNextPicDownload();
+        return;
     }
-    settings.endArray();
-}
 
-void PictureLoaderWorker::saveRedirectCache() const
-{
-    QSettings settings(cacheFilePath, QSettings::IniFormat);
+    QImage testImage;
 
-    settings.beginWriteArray(REDIRECT_HEADER_NAME, static_cast<int>(redirectCache.size()));
-    int index = 0;
-    for (auto it = redirectCache.cbegin(); it != redirectCache.cend(); ++it) {
-        settings.setArrayIndex(index++);
-        settings.setValue(REDIRECT_ORIGINAL_URL, it.key());
-        settings.setValue(REDIRECT_URL, it.value().first);
-        settings.setValue(REDIRECT_TIMESTAMP, it.value().second);
+    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();
     }
-    settings.endArray();
-}
 
-void PictureLoaderWorker::cleanStaleEntries()
-{
-    QDateTime now = QDateTime::currentDateTimeUtc();
-
-    auto it = redirectCache.begin();
-    while (it != redirectCache.end()) {
-        if (it.value().second.addDays(SettingsCache::instance().getRedirectCacheTtl()) < now) {
-            it = redirectCache.erase(it); // Remove stale entry
-        } else {
-            ++it;
-        }
+    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();
 }
 
-void PictureLoaderWorker::picDownloadChanged()
+bool PictureLoaderWorker::imageIsBlackListed(const QByteArray &picData)
 {
-    QMutexLocker locker(&mutex);
-    picDownload = SettingsCache::instance().getPicDownload();
-}
-
-void PictureLoaderWorker::picsPathChanged()
-{
-    QMutexLocker locker(&mutex);
-    picsPath = SettingsCache::instance().getPicsPath();
-    customPicsPath = SettingsCache::instance().getCustomPicsPath();
-}
-
-void PictureLoaderWorker::clearNetworkCache()
-{
-    networkManager->cache()->clear();
+    QString md5sum = QCryptographicHash::hash(picData, QCryptographicHash::Md5).toHex();
+    return md5Blacklist.contains(md5sum);
 }

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

@@ -1,15 +1,15 @@
-#ifndef PICTURE_LOADER_WORKER_H
-#define PICTURE_LOADER_WORKER_H
+#ifndef PICTURE_LOADER_WORKER_WORK_H
+#define PICTURE_LOADER_WORKER_WORK_H
 
 #include "../../../game/cards/card_database.h"
-#include "picture_loader_worker_work.h"
+#include "picture_loader_orchestrator.h"
 #include "picture_to_load.h"
 
 #include <QLoggingCategory>
 #include <QMutex>
 #include <QNetworkAccessManager>
 #include <QObject>
-#include <QTimer>
+#include <QThread>
 
 #define REDIRECT_HEADER_NAME "redirects"
 #define REDIRECT_ORIGINAL_URL "original"
@@ -17,57 +17,34 @@
 #define REDIRECT_TIMESTAMP "timestamp"
 #define REDIRECT_CACHE_FILENAME "cache.ini"
 
-inline Q_LOGGING_CATEGORY(PictureLoaderWorkerLog, "picture_loader.worker");
+inline Q_LOGGING_CATEGORY(PictureLoaderWorkerWorkLog, "picture_loader.orchestrator");
 
-class PictureLoaderWorkerWork;
-class PictureLoaderWorker : public QObject
+class PictureLoaderOrchestrator;
+class PictureLoaderWorker : public QThread
 {
     Q_OBJECT
 public:
-    explicit PictureLoaderWorker();
+    explicit PictureLoaderWorker(PictureLoaderOrchestrator *_orchestrator, const CardInfoPtr &toLoad);
     ~PictureLoaderWorker() override;
-
-    void enqueueImageLoad(const CardInfoPtr &card);
-    void clearNetworkCache();
-
+    PictureLoaderOrchestrator *orchestrator;
+    PictureToLoad cardToDownload;
 public slots:
-    QNetworkReply *makeRequest(const QUrl &url, PictureLoaderWorkerWork *workThread);
-    void handleRateLimit(QNetworkReply *reply, const QUrl &url, PictureLoaderWorkerWork *worker);
-    void processQueuedRequests();
-    void imageLoadedSuccessfully(CardInfoPtr card, const QImage &image);
+    void picDownloadFinished(QNetworkReply *reply);
+    void picDownloadFailed();
 
 private:
     static QStringList md5Blacklist;
-
     QThread *pictureLoaderThread;
-    QString picsPath, customPicsPath;
-    QList<PictureToLoad> loadQueue;
-    QMutex mutex;
     QNetworkAccessManager *networkManager;
-    QHash<QUrl, QPair<QUrl, QDateTime>> redirectCache; // Stores redirect and timestamp
-    QString cacheFilePath;                             // Path to persistent storage
-    static constexpr int CacheTTLInDays = 30;          // TODO: Make user configurable
-    QList<PictureToLoad> cardsToDownload;
-    PictureToLoad cardBeingLoaded;
-    PictureToLoad cardBeingDownloaded;
     bool picDownload, downloadRunning, loadQueueRunning;
-    bool rateLimited = false;
-    QTimer rateLimitTimer;
-    QList<QPair<QUrl, PictureLoaderWorkerWork *>> requestQueue;
-
-    void cacheRedirect(const QUrl &originalUrl, const QUrl &redirectUrl);
-    QUrl getCachedRedirect(const QUrl &originalUrl) const;
-    void loadRedirectCache();
-    void saveRedirectCache() const;
-    void cleanStaleEntries();
-
-private slots:
-    void picDownloadChanged();
-    void picsPathChanged();
+    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, PictureLoaderWorker *instance);
 };
 
-#endif // PICTURE_LOADER_WORKER_H
+#endif // PICTURE_LOADER_WORKER_WORK_H

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

@@ -1,240 +0,0 @@
-#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

@@ -1,50 +0,0 @@
-#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

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

@@ -0,0 +1,34 @@
+#include "rate_limited_network_manager.h"
+
+#include <QNetworkAccessManager>
+#include <QNetworkReply>
+#include <QThread>
+#include <QTimer>
+
+RateLimitedNetworkManager::RateLimitedNetworkManager(const int _maxRequestsPerSecond, QObject *parent)
+    : QNetworkAccessManager(parent), maxRequestsPerSecond(_maxRequestsPerSecond), currentRequestsCount(0)
+{
+    intervalTimer = new QTimer(this);
+    connect(intervalTimer, &QTimer::timeout, this, &RateLimitedNetworkManager::onIntervalTimerTimeout);
+    intervalTimer->start(1000); // Resets once per second
+}
+
+QNetworkReply *RateLimitedNetworkManager::getRateLimited(const QNetworkRequest &request)
+{
+    if (currentRequestsCount < maxRequestsPerSecond) {
+        ++currentRequestsCount;
+        qDebug() << "SENDING REQUEST" << request.url();
+        return QNetworkAccessManager::get(request);
+    }
+
+    // Not on main thread, can sleep
+    qDebug() << "SLEEPING ON REQUEST" << request.url();
+    QThread::msleep(100);
+    return getRateLimited(request);
+}
+
+void RateLimitedNetworkManager::onIntervalTimerTimeout()
+{
+
+    currentRequestsCount = 0;
+}

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

@@ -0,0 +1,26 @@
+#ifndef COCKATRICE_RATE_LIMITED_NETWORK_MANAGER_H
+#define COCKATRICE_RATE_LIMITED_NETWORK_MANAGER_H
+
+#include <QNetworkAccessManager>
+#include <QObject>
+
+class QTimer;
+
+class RateLimitedNetworkManager : public QNetworkAccessManager
+{
+    Q_OBJECT
+
+public:
+    RateLimitedNetworkManager(const int _maxRequestsPerSecond, QObject *parent = nullptr);
+    QNetworkReply *getRateLimited(const QNetworkRequest &request);
+
+private slots:
+    void onIntervalTimerTimeout();
+
+private:
+    int maxRequestsPerSecond;
+    int currentRequestsCount;
+    QTimer *intervalTimer;
+};
+
+#endif // COCKATRICE_RATE_LIMITED_NETWORK_MANAGER_H

oracle/CMakeLists.txt

@@ -19,9 +19,10 @@ set(oracle_SOURCES
     ../cockatrice/src/game/cards/card_database.cpp
     ../cockatrice/src/game/cards/card_database_manager.cpp
     ../cockatrice/src/client/ui/picture_loader/picture_loader.cpp
+    ../cockatrice/src/client/ui/picture_loader/picture_loader_orchestrator.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/client/ui/picture_loader/rate_limited_network_manager.cpp
     ../cockatrice/src/game/cards/card_database_parser/card_database_parser.cpp
     ../cockatrice/src/game/cards/card_database_parser/cockatrice_xml_3.cpp
     ../cockatrice/src/game/cards/card_database_parser/cockatrice_xml_4.cpp