[PictureLoader] Pace requests and run the throttle timers on the worker thread

Previously the whole backed-up queue was drained in a burst as soon as a
request was enqueued, sending up to 10 requests back-to-back and then
immediately re-filling the quota one second later. That hard-bursts a
rate-limited API like Scryfall's (10 requests/second) into a 30 second
lockout.

Introduce a pacing timer that dispatches a single queue entry every
100 ms, so the per-second allowance is used smoothly instead of in spikes,
and keep the quota timer at 1 second. Also fix both timers' thread
affinity: they are QTimer value members and so are not QObject children,
meaning moveToThread() on the worker left them on the main thread while
the slot code started them from the picture thread, which was a no-op that
also warned. They are moved to the worker thread explicitly and started
lazily from there.

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp

@@ -17,8 +17,10 @@
 #include <version_string.h>
 
 static constexpr int MAX_REQUESTS_PER_SEC = 10;
-static constexpr int MIN_HOST_QUOTA = 1;          ///< Floor for the per-host request allowance
-static constexpr qint64 QUOTA_RECOVER_MS = 60000; ///< Idle time before a reduced quota starts recovering
+static constexpr int MIN_HOST_QUOTA = 1;                ///< Floor for the per-host request allowance
+static constexpr qint64 QUOTA_RECOVER_MS = 60000;       ///< Idle time before a reduced quota starts recovering
+static constexpr int DISPATCH_INTERVAL_MS = 100;        ///< Pacing between individual network requests
+static constexpr qint64 QUOTA_RESET_INTERVAL_MS = 1000; ///< Interval at which the request quota resets
 
 CardPictureLoaderWorker::CardPictureLoaderWorker()
     : QObject(nullptr), picDownload(SettingsCache::instance().downloads().getPicDownload()),
@@ -60,11 +62,18 @@ CardPictureLoaderWorker::CardPictureLoaderWorker()
     pictureLoaderThread->start(QThread::LowPriority);
     moveToThread(pictureLoaderThread);
 
+    // QTimer value members are not QObject children, so moveToThread on the worker doesn't move
+    // them. They must live in the worker's thread to be started from the slot code that runs there.
+    requestTimer.moveToThread(pictureLoaderThread);
+    dispatchTimer.moveToThread(pictureLoaderThread);
+
     connect(this, &CardPictureLoaderWorker::imageLoadEnqueued, this, &CardPictureLoaderWorker::handleImageLoadEnqueued);
 
     connect(&requestTimer, &QTimer::timeout, this, &CardPictureLoaderWorker::resetRequestQuota);
-    requestTimer.setInterval(1000);
-    requestTimer.start();
+    requestTimer.setInterval(static_cast<int>(QUOTA_RESET_INTERVAL_MS));
+
+    connect(&dispatchTimer, &QTimer::timeout, this, &CardPictureLoaderWorker::dispatchQueuedRequest);
+    dispatchTimer.setInterval(DISPATCH_INTERVAL_MS);
 }
 
 CardPictureLoaderWorker::~CardPictureLoaderWorker()
@@ -84,8 +93,8 @@ void CardPictureLoaderWorker::queueRequest(const QUrl &url, CardPictureLoaderWor
             SettingsCache::instance().cacheStorage().getCardPictureLoaderCacheMethod()) ==
             CardPictureLoaderCacheMethod::CacheMethod::NETWORK_CACHE &&
         cache->metaData(url).isValid()) {
-        // If we hit a cached url, we get to make the request for free, since it won't contribute towards the
-        // rate-limit
+        // A request that will be served from the disk cache never touches the network and therefore
+        // doesn't use up any of the rate limit, so it gets to skip the queue.
         makeRequest(url, worker);
         return;
     }
@@ -107,10 +116,13 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture
     req.setHeader(QNetworkRequest::UserAgentHeader, QString("Cockatrice %1").arg(VERSION_STRING));
     req.setRawHeader("Accept", "image/avif,image/webp,image/apng,image/,/*;q=0.8");
 
-    bool useNetworkCache =
-        !picDownload && static_cast<CardPictureLoaderCacheMethod::CacheMethod>(
-                            SettingsCache::instance().cacheStorage().getCardPictureLoaderCacheMethod()) ==
-                            CardPictureLoaderCacheMethod::CacheMethod::NETWORK_CACHE;
+    // Cached entries are served straight from the disk cache even when picture downloads are
+    // enabled: re-fetching an already-cached image would burn the rate limit for nothing. Only a
+    // genuine cache miss goes to the network, and only when downloads are enabled.
+    bool useNetworkCache = static_cast<CardPictureLoaderCacheMethod::CacheMethod>(
+                               SettingsCache::instance().cacheStorage().getCardPictureLoaderCacheMethod()) ==
+                               CardPictureLoaderCacheMethod::CacheMethod::NETWORK_CACHE &&
+                           (cache->metaData(url).isValid() || !picDownload);
 
     req.setAttribute(QNetworkRequest::CacheLoadControlAttribute,
                      useNetworkCache ? QNetworkRequest::AlwaysCache : QNetworkRequest::AlwaysNetwork);
@@ -144,8 +156,29 @@ void CardPictureLoaderWorker::resetRequestQuota()
 
 void CardPictureLoaderWorker::processQueuedRequests()
 {
-    while (requestQuota > 0 && processSingleRequest()) {
+    if (requestLoadQueue.isEmpty()) {
+        dispatchTimer.stop();
+        return;
+    }
+    // Start lazily from the worker's own thread: QTimer must be started in the thread it lives in.
+    if (!requestTimer.isActive()) {
+        requestTimer.start();
+    }
+    dispatchTimer.start();
+}
+
+void CardPictureLoaderWorker::dispatchQueuedRequest()
+{
+    if (requestLoadQueue.isEmpty() || requestQuota <= 0) {
+        dispatchTimer.stop();
+        return;
+    }
+
+    if (processSingleRequest()) {
         --requestQuota;
+    } else {
+        // No queued host currently has allowance left in this second; wait for the quota reset.
+        dispatchTimer.stop();
     }
 }
 

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h

@@ -89,6 +89,9 @@ public slots:
     /** @brief Processes all queued requests respecting the request quota. */
     void processQueuedRequests();
 
+    /** @brief Chooses a request from the queue and starts it, respecting the quota and pacing. */
+    void dispatchQueuedRequest();
+
     /**
      * @brief Processes a single queued request.
      * @return true if a request was processed, false if queue is empty.
@@ -120,6 +123,7 @@ private:
 
     int requestQuota;                       ///< Remaining requests allowed per second
     QTimer requestTimer;                    ///< Timer to reset the request quota
+    QTimer dispatchTimer;                   ///< Timer pacing individual network requests
     QHash<QString, int> hostRequestQuota;   ///< Sustained per-host request allowance
     QHash<QString, int> hostQuotaRemaining; ///< Per-host allowance left in the current second
     QHash<QString, QDateTime> hostLast429;  ///< When each host was last rate limited