clean up DownloadSettings (#5194)

* refactor DownloadSettings

* only reset to default on first run

* use c++ foreach

* use addItems

* move default urls to static const

cockatrice/src/client/ui/window_main.cpp

@@ -883,6 +883,7 @@ void MainWindow::startupConfigCheck()
         // no config found, 99% new clean install
         qDebug() << "Startup: old client version empty, assuming first start after clean install";
         alertForcedOracleRun(VERSION_STRING, false);
+        SettingsCache::instance().downloads().resetToDefaultURLs(); // populate the download urls
         SettingsCache::instance().setClientVersion(VERSION_STRING);
     } else if (SettingsCache::instance().getClientVersion() != VERSION_STRING) {
         // config found, from another (presumably older) version

cockatrice/src/dialogs/dlg_settings.cpp

@@ -588,8 +588,7 @@ DeckEditorSettingsPage::DeckEditorSettingsPage()
     connect(urlList->model(), SIGNAL(rowsMoved(const QModelIndex, int, int, const QModelIndex, int)), this,
             SLOT(urlListChanged(const QModelIndex, int, int, const QModelIndex, int)));
 
-    for (int i = 0; i < SettingsCache::instance().downloads().getCount(); i++)
+    urlList->addItems(SettingsCache::instance().downloads().getAllURLs());
-        urlList->addItem(SettingsCache::instance().downloads().getDownloadUrlAt(i));
 
     auto aAdd = new QAction(this);
     aAdd->setIcon(QPixmap("theme:icons/increment"));
@@ -694,7 +693,7 @@ DeckEditorSettingsPage::DeckEditorSettingsPage()
 
 void DeckEditorSettingsPage::resetDownloadedURLsButtonClicked()
 {
-    SettingsCache::instance().downloads().clear();
+    SettingsCache::instance().downloads().resetToDefaultURLs();
     urlList->clear();
     urlList->addItems(SettingsCache::instance().downloads().getAllURLs());
     QMessageBox::information(this, tr("Success"), tr("Download URLs have been reset."));
@@ -774,11 +773,13 @@ void DeckEditorSettingsPage::actEditURL()
 void DeckEditorSettingsPage::storeSettings()
 {
     qInfo() << "URL Priority Reset";
-    SettingsCache::instance().downloads().clear();
+
+    QStringList downloadUrls;
     for (int i = 0; i < urlList->count(); i++) {
         qInfo() << "Priority" << i << ":" << urlList->item(i)->text();
-        SettingsCache::instance().downloads().setDownloadUrlAt(i, urlList->item(i)->text());
+        downloadUrls << urlList->item(i)->text();
     }
+    SettingsCache::instance().downloads().setDownloadUrls(downloadUrls);
 }
 
 void DeckEditorSettingsPage::urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int)

cockatrice/src/settings/download_settings.cpp

@@ -2,56 +2,28 @@
 
 #include "settings_manager.h"
 
+const QStringList DownloadSettings::DEFAULT_DOWNLOAD_URLS = {
+    "https://api.scryfall.com/cards/!set:uuid!?format=image&face=!prop:side!",
+    "https://api.scryfall.com/cards/multiverse/!set:muid!?format=image",
+    "https://gatherer.wizards.com/Handlers/Image.ashx?multiverseid=!set:muid!&type=card",
+    "https://gatherer.wizards.com/Handlers/Image.ashx?name=!name!&type=card"};
+
 DownloadSettings::DownloadSettings(const QString &settingPath, QObject *parent = nullptr)
     : SettingsManager(settingPath + "downloads.ini", parent)
 {
-    downloadURLs = getValue("urls", "downloads").value<QStringList>();
 }
 
-void DownloadSettings::setDownloadUrlAt(int index, const QString &url)
+void DownloadSettings::setDownloadUrls(const QStringList &downloadURLs)
 {
-    downloadURLs.insert(index, url);
     setValue(QVariant::fromValue(downloadURLs), "urls", "downloads");
 }
 
-/**
- * If reset or first run, this method contains the default URLs we will populate
- */
 QStringList DownloadSettings::getAllURLs()
 {
-    // First run, these will be empty
+    return getValue("urls", "downloads").toStringList();
-    if (downloadURLs.count() == 0) {
-        populateDefaultURLs();
-    }
-
-    return downloadURLs;
 }
 
-void DownloadSettings::populateDefaultURLs()
+void DownloadSettings::resetToDefaultURLs()
 {
-    downloadURLs.clear();
+    setValue(QVariant::fromValue(DEFAULT_DOWNLOAD_URLS), "urls", "downloads");
-    downloadURLs.append("https://api.scryfall.com/cards/!set:uuid!?format=image&face=!prop:side!");
-    downloadURLs.append("https://api.scryfall.com/cards/multiverse/!set:muid!?format=image");
-    downloadURLs.append("https://gatherer.wizards.com/Handlers/Image.ashx?multiverseid=!set:muid!&type=card");
-    downloadURLs.append("https://gatherer.wizards.com/Handlers/Image.ashx?name=!name!&type=card");
-    setValue(QVariant::fromValue(downloadURLs), "urls", "downloads");
-}
-
-QString DownloadSettings::getDownloadUrlAt(int index)
-{
-    if (0 <= index && index < downloadURLs.size()) {
-        return downloadURLs[index];
-    }
-
-    return "";
-}
-
-int DownloadSettings::getCount()
-{
-    return downloadURLs.size();
-}
-
-void DownloadSettings::clear()
-{
-    downloadURLs.clear();
 }

cockatrice/src/settings/download_settings.h

@@ -10,20 +10,14 @@ class DownloadSettings : public SettingsManager
     Q_OBJECT
     friend class SettingsCache;
 
+    static const QStringList DEFAULT_DOWNLOAD_URLS;
+
 public:
     explicit DownloadSettings(const QString &, QObject *);
 
     QStringList getAllURLs();
-    QString getDownloadUrlAt(int);
+    void setDownloadUrls(const QStringList &downloadURLs);
-    void setDownloadUrlAt(int, const QString &);
+    void resetToDefaultURLs();
-    int getCount();
-    void clear();
-
-private:
-    QStringList downloadURLs;
-
-private:
-    void populateDefaultURLs();
 };
 
 #endif // COCKATRICE_DOWNLOADSETTINGS_H