From 61035f8ae0f13bbaff6504c79bbbc835be033789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 6 Sep 2026 04:50:15 +0200 Subject: [PATCH 1/5] [PictureLoader] Serve cached pictures from the disk cache instead of re-fetching them With picture downloads enabled, requests were issued with AlwaysNetwork cache control, which per Qt never consults the disk cache. A picture that had already been downloaded was therefore fetched from the network again on every session start, with the queue bypass letting those re-fetches skip the rate limit entirely. Treat the network cache as the intent of the 'Network Cache' storage method suggests: if the URL is already cached, serve it with AlwaysCache (no network, no quota); only a genuine miss goes to the network, and only when downloads are enabled. Cache hits skip the queue for free since they never consume the per-second request allowance. --- .../card_picture_loader_worker.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp index d288236d2..34092f361 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp @@ -84,8 +84,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 +107,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( - 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( + SettingsCache::instance().cacheStorage().getCardPictureLoaderCacheMethod()) == + CardPictureLoaderCacheMethod::CacheMethod::NETWORK_CACHE && + (cache->metaData(url).isValid() || !picDownload); req.setAttribute(QNetworkRequest::CacheLoadControlAttribute, useNetworkCache ? QNetworkRequest::AlwaysCache : QNetworkRequest::AlwaysNetwork); From 64b3b7e0b470911f4450b7e8baa2b87be5a0f8db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 6 Sep 2026 04:51:07 +0200 Subject: [PATCH 2/5] [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. --- .../card_picture_loader_worker.cpp | 40 ++++++++++++++++--- .../card_picture_loader_worker.h | 4 ++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp index 34092f361..4a2caaab4 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp @@ -17,8 +17,10 @@ #include 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(QUOTA_RESET_INTERVAL_MS)); + + connect(&dispatchTimer, &QTimer::timeout, this, &CardPictureLoaderWorker::dispatchQueuedRequest); + dispatchTimer.setInterval(DISPATCH_INTERVAL_MS); } CardPictureLoaderWorker::~CardPictureLoaderWorker() @@ -147,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(); } } diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h index d1c519b7a..9f7fd9437 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h +++ b/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 hostRequestQuota; ///< Sustained per-host request allowance QHash hostQuotaRemaining; ///< Per-host allowance left in the current second QHash hostLast429; ///< When each host was last rate limited From 5a6db206cebd00447dc9c5b8b60a19e4b1d75b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sun, 6 Sep 2026 04:52:25 +0200 Subject: [PATCH 3/5] [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. --- .../card_picture_loader_worker.cpp | 22 +++++++++++++------ .../card_picture_loader_worker_work.cpp | 5 +++++ .../card_picture_loader_worker_work.h | 3 +++ 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp index 4a2caaab4..3add23bfb 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp @@ -138,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) { @@ -146,11 +149,6 @@ 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(); } @@ -184,10 +182,20 @@ void CardPictureLoaderWorker::dispatchQueuedRequest() 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); diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp index 66c56337c..072a919d7 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp +++ b/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()) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h index 1e56a4373..8490cb3ac 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h +++ b/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. From 41b49ed4bce98ac2f0ba12c627e9a944a7541b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sat, 12 Sep 2026 16:58:10 +0200 Subject: [PATCH 4/5] [PictureLoader] Add user-configurable per-host request caps Picture downloads were throttled to a uniform 10 requests/second per host with no way to tune a specific server. A rate-limited API host (Scryfall caps at 10 req/s) can trip 429s during bursts, and CDN hosts with no rate limit were throttled needlessly. Introduce developer-owned per-host caps that users can only ever lower, never raise, exposed in the download settings page: - DownloadSettings::DEVELOPER_HOST_CAPS sets the ceiling per host (api.scryfall.com 9, cards.scryfall.io unlimited, others 10). - A new hostRequestLimits setting stores user overrides in downloads.ini; clampHostRequestLimit() bounds them to [1, devCap] so a user can reduce api.scryfall.com to 5 but never raise it above 9. - The picture worker seeds, halves on 429, and recovers its sustained per-host allowance against the effective ceiling instead of the global maximum, and skips per-host accounting entirely for unlocked hosts (cards.scryfall.io) while global pacing and 429 backoff still apply. - The deck editor settings page gains one spinbox per known host, each clamped to its developer cap. --- .../card_picture_loader_worker.cpp | 41 ++++++++-- .../card_picture_loader_worker.h | 9 +++ .../deck_editor_settings_page.cpp | 74 +++++++++++++++++++ .../settings_page/deck_editor_settings_page.h | 7 ++ .../settings/download_settings.cpp | 45 +++++++++++ .../settings/download_settings.h | 34 +++++++++ tests/settings/settings_defaults_test.cpp | 32 ++++++++ 7 files changed, 236 insertions(+), 6 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp index 3add23bfb..84c5a02c5 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp @@ -16,15 +16,16 @@ #include #include -static constexpr int MAX_REQUESTS_PER_SEC = 10; -static constexpr int MIN_HOST_QUOTA = 1; ///< Floor for the per-host request allowance +static constexpr int MAX_REQUESTS_PER_SEC = DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT; +static constexpr int MIN_HOST_QUOTA = DownloadSettings::MIN_HOST_REQUEST_LIMIT; 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()), - requestQuota(MAX_REQUESTS_PER_SEC) + requestQuota(MAX_REQUESTS_PER_SEC), + hostRequestLimits(SettingsCache::instance().downloads().getHostRequestLimits()) { networkManager = new QNetworkAccessManager(this); // We need a timeout to ensure requests don't hang indefinitely in case of @@ -74,6 +75,9 @@ CardPictureLoaderWorker::CardPictureLoaderWorker() connect(&dispatchTimer, &QTimer::timeout, this, &CardPictureLoaderWorker::dispatchQueuedRequest); dispatchTimer.setInterval(DISPATCH_INTERVAL_MS); + + connect(&SettingsCache::instance().downloads(), &DownloadSettings::hostRequestLimitsChanged, this, + [this] { hostRequestLimits = SettingsCache::instance().downloads().getHostRequestLimits(); }); } CardPictureLoaderWorker::~CardPictureLoaderWorker() @@ -145,7 +149,9 @@ void CardPictureLoaderWorker::resetRequestQuota() QDateTime now = QDateTime::currentDateTime(); for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end(); ++it) { if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) { - it.value() = qMin(MAX_REQUESTS_PER_SEC, it.value() + 1); + // 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); } } @@ -190,10 +196,18 @@ bool CardPictureLoaderWorker::processSingleRequest() if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) { 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. if (!hostQuotaRemaining.contains(host)) { - hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC)); + hostQuotaRemaining.insert(host, hostRequestQuota.value(host, ceiling)); } int allowance = hostQuotaRemaining.value(host); if (allowance > 0) { @@ -206,9 +220,24 @@ bool CardPictureLoaderWorker::processSingleRequest() return false; } +int CardPictureLoaderWorker::hostAllowanceCeiling(const QString &host) const +{ + const int devCap = DownloadSettings::getDeveloperHostCaps().value(host, MAX_REQUESTS_PER_SEC); + if (devCap == DownloadSettings::UNLIMITED_HOST_QUOTA && !hostRequestLimits.contains(host)) { + return DownloadSettings::UNLIMITED_HOST_QUOTA; + } + const int requested = hostRequestLimits.value(host, devCap); + return SettingsCache::instance().downloads().clampHostRequestLimit(host, requested); +} + void CardPictureLoaderWorker::onHostRateLimited(const QString &host) { - hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC) / 2)); + if (hostAllowanceCeiling(host) == 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)); hostLast429.insert(host, QDateTime::currentDateTime()); } diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h index 9f7fd9437..70bc36419 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h @@ -125,12 +125,21 @@ private: QTimer requestTimer; ///< Timer to reset the request quota QTimer dispatchTimer; ///< Timer pacing individual network requests QHash hostRequestQuota; ///< Sustained per-host request allowance + QHash hostRequestLimits; ///< User-set per-host request allowances QHash hostQuotaRemaining; ///< Per-host allowance left in the current second QHash hostLast429; ///< When each host was last rate limited CardPictureLoaderLocal *localLoader; ///< Loader for local images QSet currentlyLoading; ///< Deduplication: contains pixmapCacheKey currently being loaded + /** + * @brief Effective per-host allowance ceiling for a host. + * @param host The host to look up + * @return The allowance ceiling in requests/second, or DownloadSettings::UNLIMITED_HOST_QUOTA + * when the developer unlocked the host and no user limit is set for it. + */ + [[nodiscard]] int hostAllowanceCeiling(const QString &host) const; + /** @brief Returns cached redirect URL for the given original URL, if available. */ [[nodiscard]] QUrl getCachedRedirect(const QUrl &originalUrl) const; diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp index f3eac05b8..4337bf0f3 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp @@ -10,12 +10,17 @@ #include #include #include +#include #include +#include +#include #include #include #include #include +static constexpr int UNLOCKED_HOST_LIMIT_MAX = 50; ///< Upper bound for hosts unlocked by the developer + DeckEditorSettingsPage::DeckEditorSettingsPage() { picDownloadCheckBox.setChecked(SettingsCache::instance().downloads().getPicDownload()); @@ -96,6 +101,53 @@ 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 &devCaps = DownloadSettings::getDeveloperHostCaps(); + const QHash userLimits = SettingsCache::instance().downloads().getHostRequestLimits(); + + QSet hosts; + for (const QString &urlTemplate : SettingsCache::instance().downloads().getAllURLs()) { + hosts.insert(QUrl(urlTemplate).host()); + } + const QList devHosts = devCaps.keys(); + for (const QString &devHost : devHosts) { + hosts.insert(devHost); + } + + QList 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); @@ -104,6 +156,7 @@ DeckEditorSettingsPage::DeckEditorSettingsPage() auto *lpMainLayout = new QVBoxLayout; lpMainLayout->addWidget(mpGeneralGroupBox); + lpMainLayout->addWidget(mpRequestLimitGroupBox); lpMainLayout->addWidget(mpSpoilerGroupBox); setLayout(lpMainLayout); @@ -164,6 +217,24 @@ void DeckEditorSettingsPage::storeSettings() SettingsCache::instance().downloads().setDownloadUrls(downloadUrls); } +void DeckEditorSettingsPage::storeRequestLimits() +{ + QHash stored; + const QHash &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); + } + } + SettingsCache::instance().downloads().setHostRequestLimits(stored); +} + void DeckEditorSettingsPage::urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int) { storeSettings(); @@ -230,6 +301,9 @@ 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:")); diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h index 5db009c8a..b3745def4 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h @@ -5,9 +5,11 @@ #include #include +#include #include #include #include +#include class DeckEditorSettingsPage : public AbstractSettingsPage { @@ -19,6 +21,7 @@ public: private slots: void storeSettings(); + void storeRequestLimits(); void urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int); void setSpoilersEnabled(bool); void spoilerPathButtonClicked(); @@ -40,6 +43,10 @@ private: QGroupBox *mpGeneralGroupBox; QGroupBox *mpSpoilerGroupBox; + QGroupBox *mpRequestLimitGroupBox; + QLabel requestLimitHelpLabel; + QHash requestLimitSpinBoxes; + QLineEdit *mpSpoilerSavePathLineEdit; QLabel mcSpoilerSaveLabel; QLabel lastUpdatedLabel; diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp index cfa1c054e..c510d8863 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp @@ -9,6 +9,22 @@ const QStringList DownloadSettings::DEFAULT_DOWNLOAD_URLS = { "https://gatherer.wizards.com/Handlers/Image.ashx?multiverseid=!set:muid!&type=card", "https://gatherer.wizards.com/Handlers/Image.ashx?name=!name!&type=card"}; +// 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). +const QHash 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}, + // The Scryfall image CDN has no documented per-client rate limit. + {"cards.scryfall.io", UNLIMITED_HOST_QUOTA}, +}; + +const QHash &DownloadSettings::getDeveloperHostCaps() +{ + return DEVELOPER_HOST_CAPS; +} + DownloadSettings::DownloadSettings(const QString &settingPath, QObject *parent = nullptr) : SettingsManager(settingPath + "downloads.ini", "downloads", QString(), parent) { @@ -50,3 +66,32 @@ void DownloadSettings::setDownloadSpoilerStatus(bool _spoilerStatus) setValue(_spoilerStatus, "downloadSpoilers"); emit downloadSpoilerStatusChanged(); } + +QHash DownloadSettings::getHostRequestLimits() const +{ + const QVariantMap stored = getValue("hostRequestLimits").toMap(); + QHash hostRequestLimits; + for (auto it = stored.cbegin(); it != stored.cend(); ++it) { + hostRequestLimits.insert(it.key(), it.value().toInt()); + } + return hostRequestLimits; +} + +void DownloadSettings::setHostRequestLimits(const QHash &hostRequestLimits) +{ + QVariantMap stored; + for (auto it = hostRequestLimits.cbegin(); it != hostRequestLimits.cend(); ++it) { + stored.insert(it.key(), it.value()); + } + setValue(stored, "hostRequestLimits"); + emit hostRequestLimitsChanged(); +} + +int DownloadSettings::clampHostRequestLimit(const QString &host, int requested) const +{ + const int devCap = DEVELOPER_HOST_CAPS.value(host, DEFAULT_HOST_REQUEST_LIMIT); + if (devCap == UNLIMITED_HOST_QUOTA) { + return qMax(MIN_HOST_REQUEST_LIMIT, requested); + } + return qBound(MIN_HOST_REQUEST_LIMIT, requested, devCap); +} diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.h b/libcockatrice_settings/libcockatrice/settings/download_settings.h index a3a6f4ca9..e075a3c07 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/download_settings.h @@ -9,14 +9,34 @@ #include "settings_manager.h" +#include + class DownloadSettings : public SettingsManager { Q_OBJECT friend class SettingsCache; static const QStringList DEFAULT_DOWNLOAD_URLS; + static const QHash DEVELOPER_HOST_CAPS; public: + /** @brief Per-host request allowance (requests/second) when no developer cap applies. */ + 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). */ + static constexpr int UNLIMITED_HOST_QUOTA = -1; + + /** + * @brief Developer-set per-host allowance ceilings (requests/second), keyed by host. + * + * Hosts not present default to DEFAULT_HOST_REQUEST_LIMIT. An entry of + * UNLIMITED_HOST_QUOTA marks a host that users may still lower, but that is never + * throttled per host by default. Users can never raise a host's allowance above its + * developer cap. + */ + static const QHash &getDeveloperHostCaps(); + explicit DownloadSettings(const QString &, QObject *); QStringList getAllURLs() const; @@ -27,9 +47,23 @@ public: [[nodiscard]] bool getDownloadSpoilersStatus() const; void setDownloadSpoilerStatus(bool _spoilerStatus); + /** @brief User-set per-host request allowances (requests/second). Missing hosts use the developer default. */ + QHash getHostRequestLimits() const; + void setHostRequestLimits(const QHash &hostRequestLimits); + + /** + * @brief Clamps the user's requested allowance for a host against its developer cap. + * @param host The host to clamp for + * @param requested The user-requested allowance in requests/second + * @return The effective allowance. Users may lower a host's allowance but never raise it + * above the developer cap; hosts with UNLIMITED_HOST_QUOTA have no upper bound. + */ + [[nodiscard]] int clampHostRequestLimit(const QString &host, int requested) const; + signals: void picDownloadChanged(); void downloadSpoilerStatusChanged(); + void hostRequestLimitsChanged(); }; #endif // COCKATRICE_DOWNLOADSETTINGS_H diff --git a/tests/settings/settings_defaults_test.cpp b/tests/settings/settings_defaults_test.cpp index eda778ae9..932ddb99b 100644 --- a/tests/settings/settings_defaults_test.cpp +++ b/tests/settings/settings_defaults_test.cpp @@ -334,6 +334,38 @@ TEST_F(SettingsDefaultsTest, Download_DownloadSpoilersStatus_Default) ASSERT_EQ(s.getDownloadSpoilersStatus(), false); } +TEST_F(SettingsDefaultsTest, Download_HostRequestLimits_Default) +{ + DownloadSettings s(settingsPath, nullptr); + ASSERT_TRUE(s.getHostRequestLimits().isEmpty()); +} + +TEST_F(SettingsDefaultsTest, Download_HostRequestLimits_SetAndGet) +{ + DownloadSettings s(settingsPath, nullptr); + s.setHostRequestLimits({{"api.scryfall.com", 5}}); + const QHash limits = s.getHostRequestLimits(); + ASSERT_EQ(limits.size(), 1); + ASSERT_EQ(limits.value("api.scryfall.com"), 5); +} + +TEST_F(SettingsDefaultsTest, Download_HostRequestLimits_StackedHostCaps) +{ + DownloadSettings s(settingsPath, nullptr); + // The developer cap for the Scryfall API lowers the ceiling to 9; a user can + // reduce it further but can never raise it above the cap. + ASSERT_EQ(s.clampHostRequestLimit("api.scryfall.com", 9), 9); + ASSERT_EQ(s.clampHostRequestLimit("api.scryfall.com", 20), 9); + ASSERT_EQ(s.clampHostRequestLimit("api.scryfall.com", 5), 5); + ASSERT_EQ(s.clampHostRequestLimit("api.scryfall.com", 0), 1); + // Hosts without a developer cap fall back to the global default ceiling. + ASSERT_EQ(s.clampHostRequestLimit("gatherer.wizards.com", 10), 10); + ASSERT_EQ(s.clampHostRequestLimit("gatherer.wizards.com", 20), 10); + // The Scryfall CDN is unlocked: no upper bound (values are only floored). + ASSERT_EQ(s.clampHostRequestLimit("cards.scryfall.io", 20), 20); + ASSERT_EQ(s.clampHostRequestLimit("cards.scryfall.io", 0), 1); +} + // --- AppearanceSettings --- TEST_F(SettingsDefaultsTest, Appearance_ThemeName_Default) From 30f5142b595f90e6672cfedd1704a82f6c1726c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Sat, 12 Sep 2026 17:32:30 +0200 Subject: [PATCH 5/5] [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. --- .../card_picture_loader_worker.cpp | 47 +++++-- .../deck_editor_settings_page.cpp | 121 ++++++++---------- .../settings_page/deck_editor_settings_page.h | 10 +- .../settings/download_settings.cpp | 2 +- .../settings/download_settings.h | 2 +- 5 files changed, 91 insertions(+), 91 deletions(-) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp index 84c5a02c5..fab651d72 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp +++ b/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()); } diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp index 4337bf0f3..6223cd480 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp @@ -10,16 +10,14 @@ #include #include #include -#include #include #include -#include #include #include #include #include -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 &devCaps = DownloadSettings::getDeveloperHostCaps(); - const QHash userLimits = SettingsCache::instance().downloads().getHostRequestLimits(); - - QSet hosts; - for (const QString &urlTemplate : SettingsCache::instance().downloads().getAllURLs()) { - hosts.insert(QUrl(urlTemplate).host()); - } - const QList devHosts = devCaps.keys(); - for (const QString &devHost : devHosts) { - hosts.insert(devHost); - } - - QList 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 stored; - const QHash &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 &devCaps = DownloadSettings::getDeveloperHostCaps(); + const QHash 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 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")); -} \ No newline at end of file + aRateLimit->setText(tr("Adjust Rate Limit")); +} diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h index b3745def4..57de5699e 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h @@ -5,11 +5,9 @@ #include #include -#include #include #include #include -#include 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 requestLimitSpinBoxes; - QLineEdit *mpSpoilerSavePathLineEdit; QLabel mcSpoilerSaveLabel; QLabel lastUpdatedLabel; diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp index c510d8863..5293e390b 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp +++ b/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 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}, diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.h b/libcockatrice_settings/libcockatrice/settings/download_settings.h index e075a3c07..9fcf9e61d 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.h +++ b/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; /**