[PictureLoader] Let unlocked hosts skip dispatch pacing; adjust limits per URL

Two refinements to the per-host request caps:

- Unlocked hosts (UNLIMITED_HOST_QUOTA, e.g. cards.scryfall.io) no longer
  wait on the 100ms dispatch pacing or consume the global per-second quota.
  dispatchQueuedRequest fires their queued requests back-to-back, bounded
  only by their 429 backoff window and Qt's per-host connection pool, so
  an unthrottled CDN is not artificially slowed.
- The deck editor download settings page replaces the static grid of one
  spinbox per known host with an "Adjust Rate Limit" toolbar action on the
  URL list. It picks the host out of the selected URL and clamps the entry
  against the developer cap table (including for user-added URLs).

Also fixes a review finding: resetRequestQuota could write the
UNLIMITED_HOST_QUOTA sentinel (-1) into the sustained per-host quota when a
host became unlocked mid-run, permanently poisoning its allowance. Stale
entries for unlocked hosts are now dropped, and the per-second seed is
clamped against the effective ceiling so a lowered limit applies immediately.

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp

@@ -147,12 +147,19 @@ void CardPictureLoaderWorker::resetRequestQuota()
     hostQuotaRemaining.clear();
 
     QDateTime now = QDateTime::currentDateTime();
-    for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end(); ++it) {
+    for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end();) {
+        // An unlocked host has no per-host allowance; drop any stale entry instead of
+        // recovering it towards the UNLIMITED_HOST_QUOTA sentinel, which would poison it.
+        if (hostAllowanceCeiling(it.key()) == DownloadSettings::UNLIMITED_HOST_QUOTA) {
+            it = hostRequestQuota.erase(it);
+            continue;
+        }
         if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) {
             // Recover towards the host's effective allowance ceiling, which may be
             // lowered by the user's per-host request limits.
             it.value() = qMin(hostAllowanceCeiling(it.key()), it.value() + 1);
         }
+        ++it;
     }
 
     processQueuedRequests();
@@ -173,6 +180,27 @@ void CardPictureLoaderWorker::processQueuedRequests()
 
 void CardPictureLoaderWorker::dispatchQueuedRequest()
 {
+    if (requestLoadQueue.isEmpty()) {
+        dispatchTimer.stop();
+        return;
+    }
+
+    // Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the dispatch pacing and the
+    // global per-second quota: dispatch every queued request for them back-to-back, bounded
+    // only by their 429 backoff window and Qt's per-host connection pool.
+    QDateTime now = QDateTime::currentDateTime();
+    for (int i = 0; i < requestLoadQueue.size();) {
+        const auto &request = requestLoadQueue.at(i);
+        const QString host = request.first.host();
+        if (hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA &&
+            !CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) {
+            makeRequest(request.first, request.second);
+            requestLoadQueue.removeAt(i);
+        } else {
+            ++i;
+        }
+    }
+
     if (requestLoadQueue.isEmpty() || requestQuota <= 0) {
         dispatchTimer.stop();
         return;
@@ -197,17 +225,11 @@ bool CardPictureLoaderWorker::processSingleRequest()
             continue;
         }
         const int ceiling = hostAllowanceCeiling(host);
-        // Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the per-host allowance
-        // entirely; only the global quota and request pacing still apply.
-        if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) {
-            makeRequest(request.first, request.second);
-            requestLoadQueue.removeAt(i);
-            return true;
-        }
         // 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.
+        // doesn't get a fresh full quota the moment it is queried mid-second. Clamp
+        // against the ceiling so a lowered user cap applies from this second onward.
         if (!hostQuotaRemaining.contains(host)) {
-            hostQuotaRemaining.insert(host, hostRequestQuota.value(host, ceiling));
+            hostQuotaRemaining.insert(host, qMin(ceiling, hostRequestQuota.value(host, ceiling)));
         }
         int allowance = hostQuotaRemaining.value(host);
         if (allowance > 0) {
@@ -232,12 +254,13 @@ int CardPictureLoaderWorker::hostAllowanceCeiling(const QString &host) const
 
 void CardPictureLoaderWorker::onHostRateLimited(const QString &host)
 {
-    if (hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA) {
+    const int ceiling = hostAllowanceCeiling(host);
+    if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) {
         // Unlocked hosts have no per-host allowance to halve; the shared backoff
         // window tracked by the rate limiter still paces them.
         return;
     }
-    hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, hostAllowanceCeiling(host)) / 2));
+    hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, ceiling) / 2));
     hostLast429.insert(host, QDateTime::currentDateTime());
 }
 

cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp

@@ -10,16 +10,14 @@
 #include <QInputDialog>
 #include <QLineEdit>
 #include <QMessageBox>
-#include <QSet>
 #include <QToolBar>
 #include <QUrl>
-#include <algorithm>
 #include <libcockatrice/settings/download_settings.h>
 #include <libcockatrice/settings/paths_settings.h>
 #include <libcockatrice/settings/personal_settings.h>
 #include <libcockatrice/utility/macros.h>
 
-static constexpr int UNLOCKED_HOST_LIMIT_MAX = 50; ///< Upper bound for hosts unlocked by the developer
+static constexpr int UNLOCKED_HOST_LIMIT_MAX = 50; ///< Upper bound for rate limits on hosts unlocked by the developer
 
 DeckEditorSettingsPage::DeckEditorSettingsPage()
 {
@@ -70,11 +68,16 @@ DeckEditorSettingsPage::DeckEditorSettingsPage()
     aRemove->setIcon(themePixmap(QStringLiteral("icons/decrement")));
     connect(aRemove, &QAction::triggered, this, &DeckEditorSettingsPage::actRemoveURL);
 
+    aRateLimit = new QAction(this);
+    aRateLimit->setIcon(themePixmap(QStringLiteral("icons/cogwheel")));
+    connect(aRateLimit, &QAction::triggered, this, &DeckEditorSettingsPage::actAdjustRateLimit);
+
     auto *urlToolBar = new QToolBar;
     urlToolBar->setOrientation(Qt::Vertical);
     urlToolBar->addAction(aAdd);
     urlToolBar->addAction(aRemove);
     urlToolBar->addAction(aEdit);
+    urlToolBar->addAction(aRateLimit);
     urlToolBar->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::MinimumExpanding);
 
     auto *urlListLayout = new QHBoxLayout;
@@ -101,53 +104,6 @@ DeckEditorSettingsPage::DeckEditorSettingsPage()
             &DownloadSettings::setDownloadSpoilerStatus);
     connect(&mcDownloadSpoilersCheckBox, &QCheckBox::toggled, this, &DeckEditorSettingsPage::setSpoilersEnabled);
 
-    // Per-host request limit group: one spinbox per known picture host. A spinbox at its
-    // lower bound (0 for unlocked hosts, the developer cap for capped hosts) means "follow
-    // the developer default"; the worker clamps any explicit value against the developer cap.
-    mpRequestLimitGroupBox = new QGroupBox;
-    auto *requestLimitLayout = new QGridLayout;
-
-    const QHash<QString, int> &devCaps = DownloadSettings::getDeveloperHostCaps();
-    const QHash<QString, int> userLimits = SettingsCache::instance().downloads().getHostRequestLimits();
-
-    QSet<QString> hosts;
-    for (const QString &urlTemplate : SettingsCache::instance().downloads().getAllURLs()) {
-        hosts.insert(QUrl(urlTemplate).host());
-    }
-    const QList<QString> devHosts = devCaps.keys();
-    for (const QString &devHost : devHosts) {
-        hosts.insert(devHost);
-    }
-
-    QList<QString> sortedHosts(hosts.cbegin(), hosts.cend());
-    std::sort(sortedHosts.begin(), sortedHosts.end(),
-              [](const QString &a, const QString &b) { return a.localeAwareCompare(b) < 0; });
-
-    int hostRow = 1;
-    for (const QString &host : sortedHosts) {
-        const int devCap = devCaps.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT);
-        const bool unlocked = devCap == DownloadSettings::UNLIMITED_HOST_QUOTA;
-
-        auto *hostLabel = new QLabel(host);
-        auto *spinBox = new QSpinBox;
-        if (unlocked) {
-            spinBox->setRange(0, UNLOCKED_HOST_LIMIT_MAX); // 0 means "unlimited"
-            spinBox->setValue(userLimits.value(host, 0));
-        } else {
-            spinBox->setRange(DownloadSettings::MIN_HOST_REQUEST_LIMIT, devCap);
-            spinBox->setValue(userLimits.value(host, devCap));
-        }
-        connect(spinBox, &QSpinBox::valueChanged, this, &DeckEditorSettingsPage::storeRequestLimits);
-
-        requestLimitLayout->addWidget(hostLabel, hostRow, 0);
-        requestLimitLayout->addWidget(spinBox, hostRow, 1);
-        requestLimitSpinBoxes.insert(host, spinBox);
-        ++hostRow;
-    }
-
-    requestLimitLayout->addWidget(&requestLimitHelpLabel, hostRow, 0, 1, 2);
-    mpRequestLimitGroupBox->setLayout(requestLimitLayout);
-
     mpGeneralGroupBox = new QGroupBox;
     mpGeneralGroupBox->setLayout(lpGeneralGrid);
 
@@ -156,7 +112,6 @@ DeckEditorSettingsPage::DeckEditorSettingsPage()
 
     auto *lpMainLayout = new QVBoxLayout;
     lpMainLayout->addWidget(mpGeneralGroupBox);
-    lpMainLayout->addWidget(mpRequestLimitGroupBox);
     lpMainLayout->addWidget(mpSpoilerGroupBox);
 
     setLayout(lpMainLayout);
@@ -217,22 +172,52 @@ void DeckEditorSettingsPage::storeSettings()
     SettingsCache::instance().downloads().setDownloadUrls(downloadUrls);
 }
 
-void DeckEditorSettingsPage::storeRequestLimits()
+void DeckEditorSettingsPage::actAdjustRateLimit()
 {
-    QHash<QString, int> stored;
-    const QHash<QString, int> &devCaps = DownloadSettings::getDeveloperHostCaps();
-    for (auto it = requestLimitSpinBoxes.cbegin(); it != requestLimitSpinBoxes.cend(); ++it) {
-        const QString host = it.key();
-        const int value = it.value()->value();
-        const int devCap = devCaps.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT);
-        // Only values that differ from the developer default are persisted; the worker
-        // treats a missing entry as "follow the developer default".
-        const int developerDefault = devCap == DownloadSettings::UNLIMITED_HOST_QUOTA ? 0 : devCap;
-        if (value != developerDefault) {
-            stored.insert(host, value);
-        }
+    if (urlList->currentItem() == nullptr) {
+        QMessageBox::information(this, tr("Adjust Rate Limit"), tr("Select a URL in the list first."));
+        return;
     }
-    SettingsCache::instance().downloads().setHostRequestLimits(stored);
+
+    const QString host = QUrl(urlList->currentItem()->text()).host();
+    if (host.isEmpty()) {
+        QMessageBox::information(this, tr("Adjust Rate Limit"), tr("The selected URL does not have a valid host."));
+        return;
+    }
+
+    const QHash<QString, int> &devCaps = DownloadSettings::getDeveloperHostCaps();
+    const QHash<QString, int> currentLimits = SettingsCache::instance().downloads().getHostRequestLimits();
+    const int devCap = devCaps.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT);
+    const bool unlocked = devCap == DownloadSettings::UNLIMITED_HOST_QUOTA;
+
+    bool ok = false;
+    int minimum;
+    int maximum;
+    int defaultValue;
+    if (unlocked) {
+        minimum = 0; // 0 means "unlimited"
+        maximum = UNLOCKED_HOST_LIMIT_MAX;
+        defaultValue = currentLimits.value(host, 0);
+    } else {
+        minimum = DownloadSettings::MIN_HOST_REQUEST_LIMIT;
+        maximum = devCap;
+        defaultValue = currentLimits.value(host, devCap);
+    }
+
+    const int value = QInputDialog::getInt(this, tr("Adjust Rate Limit for %1").arg(host),
+                                           tr("Requests per second (developer maximum is %1):").arg(maximum),
+                                           defaultValue, minimum, maximum, 1, &ok);
+    if (!ok) {
+        return;
+    }
+
+    QHash<QString, int> limits = currentLimits;
+    if (unlocked ? value == 0 : value == devCap) {
+        limits.remove(host);
+    } else {
+        limits.insert(host, value);
+    }
+    SettingsCache::instance().downloads().setHostRequestLimits(limits);
 }
 
 void DeckEditorSettingsPage::urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int)
@@ -301,9 +286,6 @@ void DeckEditorSettingsPage::setSpoilersEnabled(bool anInput)
 void DeckEditorSettingsPage::retranslateUi()
 {
     mpGeneralGroupBox->setTitle(tr("URL Download Priority"));
-    mpRequestLimitGroupBox->setTitle(tr("Per-Host Request Limit"));
-    requestLimitHelpLabel.setText(tr("Pictures per second per host. Hosts can be lowered below their developer "
-                                     "limit but never raised above it; 0 means the host is not throttled per host."));
     mpSpoilerGroupBox->setTitle(tr("Spoilers"));
     mcDownloadSpoilersCheckBox.setText(tr("Download Spoilers Automatically"));
     mcSpoilerSaveLabel.setText(tr("Spoiler Location:"));
@@ -318,4 +300,5 @@ void DeckEditorSettingsPage::retranslateUi()
     aAdd->setText(tr("Add New URL"));
     aEdit->setText(tr("Edit URL"));
     aRemove->setText(tr("Remove URL"));
-}
+    aRateLimit->setText(tr("Adjust Rate Limit"));
+}

cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h

@@ -5,11 +5,9 @@
 
 #include <QCheckBox>
 #include <QGroupBox>
-#include <QHash>
 #include <QLabel>
 #include <QListWidget>
 #include <QPushButton>
-#include <QSpinBox>
 
 class DeckEditorSettingsPage : public AbstractSettingsPage
 {
@@ -21,7 +19,6 @@ public:
 
 private slots:
     void storeSettings();
-    void storeRequestLimits();
     void urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int);
     void setSpoilersEnabled(bool);
     void spoilerPathButtonClicked();
@@ -30,6 +27,7 @@ private slots:
     void actAddURL();
     void actRemoveURL();
     void actEditURL();
+    void actAdjustRateLimit();
     void resetDownloadedURLsButtonClicked();
 
 private:
@@ -37,16 +35,12 @@ private:
     QLabel urlLinkLabel;
     QCheckBox picDownloadCheckBox;
     QListWidget *urlList;
-    QAction *aAdd, *aEdit, *aRemove;
+    QAction *aAdd, *aEdit, *aRemove, *aRateLimit;
     QCheckBox mcDownloadSpoilersCheckBox;
     QLabel msDownloadSpoilersLabel;
     QGroupBox *mpGeneralGroupBox;
     QGroupBox *mpSpoilerGroupBox;
 
-    QGroupBox *mpRequestLimitGroupBox;
-    QLabel requestLimitHelpLabel;
-    QHash<QString, QSpinBox *> requestLimitSpinBoxes;
-
     QLineEdit *mpSpoilerSavePathLineEdit;
     QLabel mcSpoilerSaveLabel;
     QLabel lastUpdatedLabel;

libcockatrice_settings/libcockatrice/settings/download_settings.cpp

@@ -12,7 +12,7 @@ const QStringList DownloadSettings::DEFAULT_DOWNLOAD_URLS = {
 // Developer-set ceilings for the per-host request allowance. Users may lower a host's
 // allowance via the download settings, but can never raise it above these values. Hosts
 // not listed default to DEFAULT_HOST_REQUEST_LIMIT. A cap of UNLIMITED_HOST_QUOTA marks a
-// host that is never throttled per host (request pacing and 429 backoff still apply).
+// host that is never throttled per host and skips the dispatch pacing (429 backoff still applies).
 const QHash<QString, int> DownloadSettings::DEVELOPER_HOST_CAPS = {
     // The Scryfall API enforces 10 requests/second; stay one under so a burst can't trip 429s.
     {"api.scryfall.com", 9},

libcockatrice_settings/libcockatrice/settings/download_settings.h

@@ -24,7 +24,7 @@ public:
     static constexpr int DEFAULT_HOST_REQUEST_LIMIT = 10;
     /** @brief Floor for any per-host request allowance. */
     static constexpr int MIN_HOST_REQUEST_LIMIT = 1;
-    /** @brief Developer cap marking a host as never throttled per host (pacing still applies). */
+    /** @brief Developer cap marking a host as never throttled per host or by the dispatch pacing. */
     static constexpr int UNLIMITED_HOST_QUOTA = -1;
 
     /**