[PictureLoader] Seed per-host allowances on demand and skip hosts in 429 backoff

The quota reset re-filled every host's remaining allowance to a full
MAX_REQUESTS_PER_SEC as soon as the queue had a request for it. A server
that was just rate limited could therefore be hammered again at full speed
immediately after (or even during) recovery.

Only seed a host's allowance the first time it is dispatched in the
current second, seeded from its reduced sustained quota, and skip hosts
still inside their 429 backoff window entirely. This makes the pacing
commit's burst-free behavior hold per host too, instead of just smoothing
the global aggregate.

cockatrice/src/client/sound_engine.cpp

@@ -94,7 +94,7 @@ QStringMap &SoundEngine::getAvailableThemes()
     QDir dir;
     availableThemes.clear();
 
-    // load themes from user profile dir
+    // Load themes from user profile dir
 
     dir.setPath(SettingsCache::instance().getDataPath() + "/sounds");
 
@@ -104,7 +104,7 @@ QStringMap &SoundEngine::getAvailableThemes()
         }
     }
 
-    // load themes from cockatrice system dir
+    // Load themes from Cockatrice system dir
     dir.setPath(qApp->applicationDirPath() +
 #ifdef Q_OS_MAC
                 "/../Resources/sounds"

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);
@@ -126,6 +138,9 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture
 void CardPictureLoaderWorker::resetRequestQuota()
 {
     requestQuota = MAX_REQUESTS_PER_SEC;
+    // Allowances are seeded per host on demand in processSingleRequest(), so a
+    // rate-limited host never gets a fresh full quota mid-second.
+    hostQuotaRemaining.clear();
 
     QDateTime now = QDateTime::currentDateTime();
     for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end(); ++it) {
@@ -134,27 +149,53 @@ void CardPictureLoaderWorker::resetRequestQuota()
         }
     }
 
-    for (const auto &request : requestLoadQueue) {
-        const QString host = request.first.host();
-        hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC));
-    }
-
     processQueuedRequests();
 }
 
 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();
     }
 }
 
 bool CardPictureLoaderWorker::processSingleRequest()
 {
+    QDateTime now = QDateTime::currentDateTime();
     for (int i = 0; i < requestLoadQueue.size(); ++i) {
         const auto &request = requestLoadQueue.at(i);
-        QString host = request.first.host();
-        int allowance = hostQuotaRemaining.value(host, MAX_REQUESTS_PER_SEC);
+        const QString host = request.first.host();
+        // Don't dispatch requests to a host that is currently in its 429 backoff.
+        if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) {
+            continue;
+        }
+        // Seed the allowance only now, so a host that was rate limited last second
+        // doesn't get a fresh full quota the moment it is queried mid-second.
+        if (!hostQuotaRemaining.contains(host)) {
+            hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC));
+        }
+        int allowance = hostQuotaRemaining.value(host);
         if (allowance > 0) {
             hostQuotaRemaining.insert(host, allowance - 1);
             makeRequest(request.first, request.second);

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

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp

@@ -22,6 +22,11 @@ static const QStringList MD5_BLACKLIST = {
     "fbc7d763c08771c260b39e2115414eeb"  // Current card back hash
 };
 
+ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter()
+{
+    return s_rateLimiter;
+}
+
 CardPictureLoaderWorkerWork::CardPictureLoaderWorkerWork(const CardPictureLoaderWorker *worker, const ExactCard &toLoad)
     : QObject(nullptr), cardToDownload(CardPictureToLoad(toLoad)),
       picDownload(SettingsCache::instance().downloads().getPicDownload())

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h

@@ -43,6 +43,9 @@ public:
 
     CardPictureToLoad cardToDownload; ///< The card and associated URLs to try downloading
 
+    /** @brief Shared per-server 429 backoff state. */
+    static ServerRateLimiter &rateLimiter();
+
 public slots:
     /**
      * @brief Handles a finished network reply for the card image.

cockatrice/src/interface/deck_loader/deck_file_format.h

@@ -17,7 +17,7 @@ enum Format
     PlainText,
 
     /**
-     * This is cockatrice's native deck file format, and supports deck metadata such as banner cards and tags.
+     * This is Cockatrice's native deck file format, and supports deck metadata such as banner cards and tags.
      * Stored as .cod files.
      */
     Cockatrice

cockatrice/src/interface/deck_loader/deck_loader.cpp

@@ -50,7 +50,7 @@ DeckLoader::loadFromFile(const QString &fileName, DeckFileFormat::Format fmt, bo
             result = deckList.loadFromFile_Native(&file);
             if (!result) {
                 qCInfo(DeckLoaderLog) << "Failed to load " << fileName
-                                      << "as cockatrice format; retrying as plain format";
+                                      << "as Cockatrice format; retrying as plain format";
                 file.seek(0);
                 result = deckList.loadFromFile_Plain(&file, CardNameNormalizer());
                 fmt = DeckFileFormat::PlainText;

cockatrice/src/interface/deck_loader/deck_loader.h

@@ -131,7 +131,7 @@ public:
     static void printDeckList(QPrinter *printer, const DeckList &deckList);
 
     /**
-     * Converts the given deck's file to the cockatrice file format.
+     * Converts the given deck's file to the Cockatrice file format.
      * Uses the lastLoadInfo in the LoadedDeck to determine the current name of the file and where to save to.
      * @param deck The deck to convert. Should have valid lastLoadInfo. Will update the lastLoadInfo.
      * @return Whether the conversion succeeded.

cockatrice/src/interface/theme_manager.cpp

@@ -224,7 +224,7 @@ QStringMap &ThemeManager::getAvailableThemes()
         }
     }
 
-    // load themes from cockatrice system dir
+    // Load themes from Cockatrice system dir
     dir.setPath(systemThemesBasePath());
 
     for (QString themeName : dir.entryList(QDir::AllDirs | QDir::NoDotAndDotDot, QDir::Name)) {

cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp

@@ -478,13 +478,13 @@ void DlgSettings::closeEvent(QCloseEvent *event)
         case Invalid:
             loadErrorMessage = tr("Your card database is invalid.\n\n"
                                   "Cockatrice may not function correctly with an invalid database\n\n"
-                                  "You may need to rerun oracle to update your card database.\n\n"
+                                  "You may need to rerun Oracle to update your card database.\n\n"
                                   "Would you like to change your database location setting?");
             break;
         case VersionTooOld:
             loadErrorMessage = tr("Your card database version is too old.\n\n"
                                   "This can cause problems loading card information or images\n\n"
-                                  "Usually this can be fixed by rerunning oracle to to update your card database.\n\n"
+                                  "Usually this can be fixed by rerunning Oracle to to update your card database.\n\n"
                                   "Would you like to change your database location setting?");
             break;
         case NotLoaded:

cockatrice/src/interface/window_main.cpp

@@ -92,8 +92,8 @@
 #include <libcockatrice/settings/updates_settings.h>
 
 #define GITHUB_PAGES_URL "https://cockatrice.github.io"
-#define GITHUB_CONTRIBUTORS_URL "https://github.com/Cockatrice/Cockatrice/graphs/contributors?type=c"
-#define GITHUB_CONTRIBUTE_URL "https://github.com/Cockatrice/Cockatrice#cockatrice"
+#define GITHUB_CONTRIBUTORS_URL "https://github.com/Cockatrice/Cockatrice/graphs/contributors"
+#define GITHUB_CONTRIBUTE_URL "https://github.com/Cockatrice/Cockatrice#"
 #define GITHUB_TRANSIFEX_TRANSLATORS_URL "https://github.com/Cockatrice/Cockatrice/wiki/Translator-Hall-of-Fame"
 #define GITHUB_TRANSLATOR_FAQ_URL "https://github.com/Cockatrice/Cockatrice/wiki/Translation-FAQ"
 #define GITHUB_ISSUES_URL "https://github.com/Cockatrice/Cockatrice/issues"
@@ -1050,7 +1050,7 @@ void MainWindow::createCardUpdateProcess(bool background)
 
     if (dir.exists(binaryName)) {
         updaterCmd = dir.absoluteFilePath(binaryName);
-    } else { // try and find the directory oracle is stored in the build directory
+    } else { // try and find the directory Oracle is stored in the build directory
         QDir findLocalDir(dir);
         findLocalDir.cdUp();
         findLocalDir.cd(getCardUpdaterBinaryName());

cockatrice/src/main.cpp

@@ -231,7 +231,7 @@ int main(int argc, char *argv[])
     // These values are only used by the settings loader/saver
     // Wrong or outdated values are kept to not break things
     QCoreApplication::setOrganizationName("Cockatrice");
-    QCoreApplication::setOrganizationDomain("cockatrice.de");
+    QCoreApplication::setOrganizationDomain("cockatrice.github.io");
     QCoreApplication::setApplicationName("Cockatrice");
     QCoreApplication::setApplicationVersion(VERSION_STRING);
 
@@ -250,7 +250,7 @@ int main(int argc, char *argv[])
 
     // Command-line parser
     QCommandLineParser parser;
-    parser.setApplicationDescription("Cockatrice");
+    parser.setApplicationDescription("Cockatrice Client");
     parser.addHelpOption();
     parser.addVersionOption();
 
@@ -350,8 +350,8 @@ int main(int argc, char *argv[])
     qCInfo(MainLog) << "MainWindow constructor finished";
 
     ui.setWindowIcon(themePixmap(QStringLiteral("cockatrice")));
-    // set name of the app desktop file; used by wayland to load the window icon
-    QGuiApplication::setDesktopFileName("cockatrice");
+    // Set name of the app desktop file; used by wayland to load the window icon
+    QGuiApplication::setDesktopFileName("Cockatrice");
 
     SettingsCache::instance().network().setClientID(generateClientID());
 

oracle/src/main.cpp

@@ -50,8 +50,8 @@ int main(int argc, char *argv[])
     QApplication app(argc, argv);
 
     QCoreApplication::setOrganizationName("Cockatrice");
-    QCoreApplication::setOrganizationDomain("cockatrice");
-    // this can't be changed, as it influences the default save path for cards.xml
+    QCoreApplication::setOrganizationDomain("Cockatrice");
+    // This can't be changed, as it influences the default save path for cards.xml
     QCoreApplication::setApplicationName("Cockatrice");
 
     // If the program is opened with the -s flag, it will only do spoilers. Otherwise it will do MTGJSON/Tokens
@@ -83,7 +83,7 @@ int main(int argc, char *argv[])
     QIcon icon("theme:appicon.svg");
     wizard.setWindowIcon(icon);
     // set name of the app desktop file; used by wayland to load the window icon
-    QGuiApplication::setDesktopFileName("oracle");
+    QGuiApplication::setDesktopFileName("Oracle");
 
     wizard.show();
 

oracle/src/pages.cpp

@@ -796,7 +796,7 @@ void SaveSetsPage::retranslateUi()
 {
     setTitle(tr("Sets imported"));
     if (wizard()->downloadedPlainXml) {
-        setSubTitle(tr("A cockatrice database file of %1 MB has been downloaded.")
+        setSubTitle(tr("A Cockatrice card database file of %1 MB has been downloaded.")
                         .arg(qRound(wizard()->xmlData.size() / 1000000.0)));
     } else {
         setSubTitle(tr("The following sets have been found:"));

oracle/src/parsehelpers.cpp

@@ -20,7 +20,7 @@
  * Note that "...enters tapped unless..." returns false.
  *
  * @param name The name of the card
- * @param text The oracle text of the card
+ * @param text The Oracle text of the card
  */
 bool parseCipt(const QString &name, const QString &text)
 {

oracle/src/raw_json_scanner.h

@@ -54,7 +54,7 @@ using ScanProgressCallback = std::function<void(qsizetype bytesRead, qsizetype t
  * @brief Scans a full MTGJSON document without materializing the JSON tree.
  *
  * Splits the top-level "data" object into per-set byte ranges and reads each
- * set's metadata directly from the raw bytes. The oracle importer can then
+ * set's metadata directly from the raw bytes. The Oracle importer can then
  * parse one set at a time during import, keeping peak memory far below a single
  * QJsonDocument::fromJson() over the whole file.
  *

servatrice/src/servatrice_database_interface.cpp

@@ -86,9 +86,10 @@ bool Servatrice_DatabaseInterface::openDatabase()
                                              << dbversion << "to version" << expectedversion;
             return false;
         } else if (dbversion > expectedversion) {
-            qCCritical(DatabaseInterfaceLog) << poolStr << "Error opening database: the database schema version"
-                                             << dbversion << "is too new, you need to update servatrice"
-                                             << "(this servatrice actually uses version" << expectedversion << ")";
+            qCCritical(DatabaseInterfaceLog)
+                << poolStr << "Error opening database: the database schema version" << dbversion
+                << "is too new, you need to update Servatrice" << "(Currently running Servatrice actually uses version"
+                << expectedversion << ")";
             return false;
         }
     } else {