refactor: move things around so that related things are close together

This also adds some comments around ModModel.cpp and ModPage.cpp to add
some ease of reading the code.

Also move some things from headers to cpp files.

launcher/ui/pages/modplatform/ModModel.cpp

@@ -11,6 +11,24 @@ namespace ModPlatform {
 
 ListModel::ListModel(ModPage* parent) : QAbstractListModel(parent), m_parent(parent) {}
 
+QString ListModel::debugName() const
+{
+    return m_parent->debugName();
+}
+
+
+/******** Make data requests ********/
+
+void ListModel::fetchMore(const QModelIndex& parent)
+{
+    if (parent.isValid()) return;
+    if (nextSearchOffset == 0) {
+        qWarning() << "fetchMore with 0 offset is wrong...";
+        return;
+    }
+    performPaginatedSearch();
+}
+
 QVariant ListModel::data(const QModelIndex& index, int role) const
 {
     int pos = index.row();
@@ -41,47 +59,6 @@ QVariant ListModel::data(const QModelIndex& index, int role) const
     return QVariant();
 }
 
-QString ListModel::debugName() const
-{
-    return m_parent->debugName();
-}
-
-void ListModel::logoLoaded(QString logo, QIcon out)
-{
-    m_loadingLogos.removeAll(logo);
-    m_logoMap.insert(logo, out);
-    for (int i = 0; i < modpacks.size(); i++) {
-        if (modpacks[i].logoName == logo) { emit dataChanged(createIndex(i, 0), createIndex(i, 0), { Qt::DecorationRole }); }
-    }
-}
-
-void ListModel::logoFailed(QString logo)
-{
-    m_failedLogos.append(logo);
-    m_loadingLogos.removeAll(logo);
-}
-
-void ListModel::fetchMore(const QModelIndex& parent)
-{
-    if (parent.isValid()) return;
-    if (nextSearchOffset == 0) {
-        qWarning() << "fetchMore with 0 offset is wrong...";
-        return;
-    }
-    performPaginatedSearch();
-}
-
-void ListModel::getLogo(const QString& logo, const QString& logoUrl, LogoCallback callback)
-{
-    if (m_logoMap.contains(logo)) {
-        callback(APPLICATION->metacache()
-                     ->resolveEntry(m_parent->metaEntryBase(), QString("logos/%1").arg(logo.section(".", 0, 0)))
-                     ->getFullPath());
-    } else {
-        requestLogo(logo, logoUrl);
-    }
-}
-
 void ListModel::requestModVersions(ModPlatform::IndexedPack const& current)
 {
     m_parent->apiProvider()->getVersions(this, current.addonId.toString());
@@ -118,6 +95,60 @@ void ListModel::searchWithTerm(const QString& term, const int sort)
     performPaginatedSearch();
 }
 
+void ListModel::getLogo(const QString& logo, const QString& logoUrl, LogoCallback callback)
+{
+    if (m_logoMap.contains(logo)) {
+        callback(APPLICATION->metacache()
+                     ->resolveEntry(m_parent->metaEntryBase(), QString("logos/%1").arg(logo.section(".", 0, 0)))
+                     ->getFullPath());
+    } else {
+        requestLogo(logo, logoUrl);
+    }
+}
+
+void ListModel::requestLogo(QString logo, QString url)
+{
+    if (m_loadingLogos.contains(logo) || m_failedLogos.contains(logo)) { return; }
+
+    MetaEntryPtr entry =
+        APPLICATION->metacache()->resolveEntry(m_parent->metaEntryBase(), QString("logos/%1").arg(logo.section(".", 0, 0)));
+    auto job = new NetJob(QString("%1 Icon Download %2").arg(m_parent->debugName()).arg(logo), APPLICATION->network());
+    job->addNetAction(Net::Download::makeCached(QUrl(url), entry));
+
+    auto fullPath = entry->getFullPath();
+    QObject::connect(job, &NetJob::succeeded, this, [this, logo, fullPath, job] {
+        job->deleteLater();
+        emit logoLoaded(logo, QIcon(fullPath));
+        if (waitingCallbacks.contains(logo)) { waitingCallbacks.value(logo)(fullPath); }
+    });
+
+    QObject::connect(job, &NetJob::failed, this, [this, logo, job] {
+        job->deleteLater();
+        emit logoFailed(logo);
+    });
+
+    job->start();
+    m_loadingLogos.append(logo);
+}
+
+
+/******** Request callbacks ********/
+
+void ListModel::logoLoaded(QString logo, QIcon out)
+{
+    m_loadingLogos.removeAll(logo);
+    m_logoMap.insert(logo, out);
+    for (int i = 0; i < modpacks.size(); i++) {
+        if (modpacks[i].logoName == logo) { emit dataChanged(createIndex(i, 0), createIndex(i, 0), { Qt::DecorationRole }); }
+    }
+}
+
+void ListModel::logoFailed(QString logo)
+{
+    m_failedLogos.append(logo);
+    m_loadingLogos.removeAll(logo);
+}
+
 void ListModel::searchRequestFinished(QJsonDocument& doc)
 {
     jobPtr.reset();
@@ -175,32 +206,18 @@ void ListModel::searchRequestFailed(QString reason)
 
 void ListModel::versionRequestSucceeded(QJsonDocument doc, QString addonId)
 {
-    m_parent->onRequestVersionsSucceeded(doc, addonId);
+    auto& current = m_parent->getCurrent();
-}
+    if (addonId != current.addonId) { return; }
+    
+    QJsonArray arr = doc.array();
+    try {
+        loadIndexedPackVersions(current, arr);
+    } catch (const JSONValidationError& e) {
+        qDebug() << doc;
+        qWarning() << "Error while reading " << debugName() << " mod version: " << e.cause();
+    }
 
-void ListModel::requestLogo(QString logo, QString url)
+    m_parent->updateModVersions();
-{
-    if (m_loadingLogos.contains(logo) || m_failedLogos.contains(logo)) { return; }
-
-    MetaEntryPtr entry =
-        APPLICATION->metacache()->resolveEntry(m_parent->metaEntryBase(), QString("logos/%1").arg(logo.section(".", 0, 0)));
-    auto job = new NetJob(QString("%1 Icon Download %2").arg(m_parent->debugName()).arg(logo), APPLICATION->network());
-    job->addNetAction(Net::Download::makeCached(QUrl(url), entry));
-
-    auto fullPath = entry->getFullPath();
-    QObject::connect(job, &NetJob::succeeded, this, [this, logo, fullPath, job] {
-        job->deleteLater();
-        emit logoLoaded(logo, QIcon(fullPath));
-        if (waitingCallbacks.contains(logo)) { waitingCallbacks.value(logo)(fullPath); }
-    });
-
-    QObject::connect(job, &NetJob::failed, this, [this, logo, job] {
-        job->deleteLater();
-        emit logoFailed(logo);
-    });
-
-    job->start();
-    m_loadingLogos.append(logo);
 }
 
 }  // namespace ModPlatform

launcher/ui/pages/modplatform/ModModel.h

@@ -36,6 +36,9 @@ class ListModel : public QAbstractListModel {
     void searchWithTerm(const QString& term, const int sort);
     void requestModVersions(const ModPlatform::IndexedPack& current);
 
+    virtual void loadIndexedPack(ModPlatform::IndexedPack& m, QJsonObject& obj) = 0;
+    virtual void loadIndexedPackVersions(ModPlatform::IndexedPack& m, QJsonArray& arr) = 0;
+
     void getLogo(const QString& logo, const QString& logoUrl, LogoCallback callback);
 
     inline bool canFetchMore(const QModelIndex& parent) const override { return searchState == CanPossiblyFetchMore; };
@@ -54,7 +57,6 @@ class ListModel : public QAbstractListModel {
     void performPaginatedSearch();
 
    protected:
-    virtual void loadIndexedPack(ModPlatform::IndexedPack& m, QJsonObject& obj) = 0;
     virtual QJsonArray documentToArray(QJsonDocument& obj) const = 0;
     virtual const char** getSorts() const = 0;
 

launcher/ui/pages/modplatform/ModPage.cpp

@@ -3,6 +3,8 @@
 
 #include <QKeyEvent>
 
+#include "minecraft/MinecraftInstance.h"
+#include "minecraft/PackProfile.h"
 #include "ui/dialogs/ModDownloadDialog.h"
 
 ModPage::ModPage(ModDownloadDialog* dialog, BaseInstance* instance, ModAPI* api)
@@ -22,6 +24,9 @@ ModPage::~ModPage()
     delete ui;
 }
 
+
+/******** Qt things ********/
+
 void ModPage::openedImpl()
 {
     updateSelectionButton();
@@ -41,21 +46,8 @@ bool ModPage::eventFilter(QObject* watched, QEvent* event)
     return QWidget::eventFilter(watched, event);
 }
 
-void ModPage::updateSelectionButton()
-{
-    if (!isOpened || selectedVersion < 0) {
-        ui->modSelectionButton->setEnabled(false);
-        return;
-    }
 
-    ui->modSelectionButton->setEnabled(true);
+/******** Callbacks to events in the UI (set up in the derived classes) ********/
-    auto& version = current.versions[selectedVersion];
-    if (!dialog->isModSelected(current.name, version.fileName)) {
-        ui->modSelectionButton->setText(tr("Select mod for download"));
-    } else {
-        ui->modSelectionButton->setText(tr("Deselect mod for download"));
-    }
-}
 
 void ModPage::triggerSearch()
 {
@@ -130,3 +122,44 @@ void ModPage::onModSelected()
 
     updateSelectionButton();
 }
+
+
+/******** Make changes to the UI ********/
+
+void ModPage::updateModVersions()
+{
+    auto packProfile = (static_cast<MinecraftInstance*>(m_instance))->getPackProfile();
+
+    QString mcVersion = packProfile->getComponentVersion("net.minecraft");
+    QString loaderString = (packProfile->getComponentVersion("net.minecraftforge").isEmpty()) ? "fabric" : "forge";
+
+    for (int i = 0; i < current.versions.size(); i++) {
+        auto version = current.versions[i];
+        //NOTE: Flame doesn't care about loaderString, so passing it changes nothing.
+        if (!validateVersion(version, mcVersion, loaderString)) { 
+            continue; 
+        }
+        ui->versionSelectionBox->addItem(version.version, QVariant(i));
+    }
+    if (ui->versionSelectionBox->count() == 0) { ui->versionSelectionBox->addItem(tr("No valid version found!"), QVariant(-1)); }
+
+    ui->modSelectionButton->setText(tr("Cannot select invalid version :("));
+    updateSelectionButton();
+}
+
+
+void ModPage::updateSelectionButton()
+{
+    if (!isOpened || selectedVersion < 0) {
+        ui->modSelectionButton->setEnabled(false);
+        return;
+    }
+
+    ui->modSelectionButton->setEnabled(true);
+    auto& version = current.versions[selectedVersion];
+    if (!dialog->isModSelected(current.name, version.fileName)) {
+        ui->modSelectionButton->setText(tr("Select mod for download"));
+    } else {
+        ui->modSelectionButton->setText(tr("Deselect mod for download"));
+    }
+}

launcher/ui/pages/modplatform/ModPage.h

@@ -35,9 +35,12 @@ class ModPage : public QWidget, public BasePage {
 
 
     virtual bool shouldDisplay() const override = 0;
+    virtual bool validateVersion(ModPlatform::IndexedVersion& ver, QString mineVer, QString loaderVer = "") const = 0;
+
     const ModAPI* apiProvider() const { return api.get(); };
 
-    virtual void onRequestVersionsSucceeded(QJsonDocument&, QString) = 0;
+    ModPlatform::IndexedPack& getCurrent() { return current; }
+    void updateModVersions();
 
     void openedImpl() override;
     bool eventFilter(QObject* watched, QEvent* event) override;
@@ -45,7 +48,6 @@ class ModPage : public QWidget, public BasePage {
     BaseInstance* m_instance;
 
    protected:
-
     void updateSelectionButton();
 
    protected slots:

launcher/ui/pages/modplatform/flame/FlameModModel.cpp

@@ -1,7 +1,24 @@
 #include "FlameModModel.h"
 
+#include "modplatform/flame/FlameModIndex.h"
+
 namespace FlameMod {
 
 const char* ListModel::sorts[6]{ "Featured", "Popularity", "LastUpdated", "Name", "Author", "TotalDownloads" };
 
+void ListModel::loadIndexedPack(ModPlatform::IndexedPack& m, QJsonObject& obj)
+{
+    FlameMod::loadIndexedPack(m, obj);
+};
+
+void ListModel::loadIndexedPackVersions(ModPlatform::IndexedPack& m, QJsonArray& arr)
+{
+    FlameMod::loadIndexedPackVersions(m, arr, APPLICATION->network(), m_parent->m_instance);
+};
+
+QJsonArray ListModel::documentToArray(QJsonDocument& obj) const
+{
+    return obj.array();
+}
+
 }  // namespace FlameMod

launcher/ui/pages/modplatform/flame/FlameModModel.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "FlameModPage.h"
-#include "modplatform/flame/FlameModIndex.h"
 
 namespace FlameMod {
 
@@ -14,12 +13,13 @@ class ListModel : public ModPlatform::ListModel {
     virtual ~ListModel() = default;
 
    private:
-    void loadIndexedPack(ModPlatform::IndexedPack& m, QJsonObject& obj) override { FlameMod::loadIndexedPack(m, obj); };
+    void loadIndexedPack(ModPlatform::IndexedPack& m, QJsonObject& obj) override;
+    void loadIndexedPackVersions(ModPlatform::IndexedPack& m, QJsonArray& arr) override;
 
-    QJsonArray documentToArray(QJsonDocument& obj) const override { return obj.array(); };
+    QJsonArray documentToArray(QJsonDocument& obj) const override;
 
     static const char* sorts[6]; 
-    const char** getSorts() const override { return sorts; };
+    inline const char** getSorts() const override { return sorts; };
 };
 
 }  // namespace FlameMod

launcher/ui/pages/modplatform/flame/FlameModPage.cpp

@@ -1,15 +1,7 @@
 #include "FlameModPage.h"
 #include "ui_ModPage.h"
 
-#include <QKeyEvent>
-
-#include "Application.h"
 #include "FlameModModel.h"
-#include "InstanceImportTask.h"
-#include "Json.h"
-#include "ModDownloadTask.h"
-#include "minecraft/MinecraftInstance.h"
-#include "minecraft/PackProfile.h"
 #include "ui/dialogs/ModDownloadDialog.h"
 
 FlameModPage::FlameModPage(ModDownloadDialog* dialog, BaseInstance* instance) 
@@ -34,31 +26,13 @@ FlameModPage::FlameModPage(ModDownloadDialog* dialog, BaseInstance* instance)
     connect(ui->modSelectionButton, &QPushButton::clicked, this, &FlameModPage::onModSelected);
 }
 
-bool FlameModPage::shouldDisplay() const { return true; }
+bool FlameModPage::validateVersion(ModPlatform::IndexedVersion& ver, QString mineVer, QString loaderVer) const
-
-void FlameModPage::onRequestVersionsSucceeded(QJsonDocument& doc, QString addonId)
 {
-    if (addonId != current.addonId) {
+    (void) loaderVer;
-        return;  // wrong request
+    return ver.mcVersion.contains(mineVer);
-    }
-
-    QJsonArray arr = doc.array();
-    try {
-        FlameMod::loadIndexedPackVersions(current, arr, APPLICATION->network(), m_instance);
-    } catch (const JSONValidationError& e) {
-        qDebug() << doc;
-        qWarning() << "Error while reading Flame mod version: " << e.cause();
-    }
-    auto packProfile = ((MinecraftInstance*)m_instance)->getPackProfile();
-    QString mcVersion = packProfile->getComponentVersion("net.minecraft");
-    QString loaderString = (packProfile->getComponentVersion("net.minecraftforge").isEmpty()) ? "fabric" : "forge";
-    for (int i = 0; i < current.versions.size(); i++) {
-        auto version = current.versions[i];
-        if (!version.mcVersion.contains(mcVersion)) { continue; }
-        ui->versionSelectionBox->addItem(version.version, QVariant(i));
-    }
-    if (ui->versionSelectionBox->count() == 0) { ui->versionSelectionBox->addItem(tr("No Valid Version found!"), QVariant(-1)); }
-
-    ui->modSelectionButton->setText(tr("Cannot select invalid version :("));
-    updateSelectionButton();
 }
+
+// I don't know why, but doing this on the parent class makes it so that
+// other mod providers start loading before being selected, at least with
+// my Qt, so we need to implement this in every derived class...
+bool FlameModPage::shouldDisplay() const { return true; }

launcher/ui/pages/modplatform/flame/FlameModPage.h

@@ -19,8 +19,7 @@ class FlameModPage : public ModPage {
     inline QString debugName() const override { return tr("Flame"); }
     inline QString metaEntryBase() const override { return "FlameMods"; };
 
-    bool shouldDisplay() const override;
+    bool validateVersion(ModPlatform::IndexedVersion& ver, QString mineVer, QString loaderVer = "") const override;
 
-   private:
+    bool shouldDisplay() const override;
-    void onRequestVersionsSucceeded(QJsonDocument&, QString) override;
 };

launcher/ui/pages/modplatform/modrinth/ModrinthModel.cpp

@@ -1,7 +1,24 @@
 #include "ModrinthModel.h"
 
+#include "modplatform/modrinth/ModrinthPackIndex.h"
+
 namespace Modrinth {
 
-const char* ListModel::sorts[5] { "relevance", "downloads", "follows", "updated", "newest" };
+const char* ListModel::sorts[5]{ "relevance", "downloads", "follows", "updated", "newest" };
+
+void ListModel::loadIndexedPack(ModPlatform::IndexedPack& m, QJsonObject& obj)
+{
+    Modrinth::loadIndexedPack(m, obj);
+}
+
+void ListModel::loadIndexedPackVersions(ModPlatform::IndexedPack& m, QJsonArray& arr)
+{
+    Modrinth::loadIndexedPackVersions(m, arr, APPLICATION->network(), m_parent->m_instance);
+};
+
+QJsonArray ListModel::documentToArray(QJsonDocument& obj) const
+{
+    return obj.object().value("hits").toArray();
+}
 
 }  // namespace Modrinth

launcher/ui/pages/modplatform/modrinth/ModrinthModel.h

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "ModrinthPage.h"
-#include "modplatform/modrinth/ModrinthPackIndex.h"
 
 namespace Modrinth {
 
@@ -13,12 +12,13 @@ class ListModel : public ModPlatform::ListModel {
     virtual ~ListModel() = default;
 
    private:
-    void loadIndexedPack(ModPlatform::IndexedPack& m, QJsonObject& obj) override { Modrinth::loadIndexedPack(m, obj); };
+    void loadIndexedPack(ModPlatform::IndexedPack& m, QJsonObject& obj) override;
-
+    void loadIndexedPackVersions(ModPlatform::IndexedPack& m, QJsonArray& arr) override;
-    QJsonArray documentToArray(QJsonDocument& obj) const override { return obj.object().value("hits").toArray(); };
+    
+    QJsonArray documentToArray(QJsonDocument& obj) const override;
 
     static const char* sorts[5];
-    const char** getSorts() const override { return sorts; };
+    inline const char** getSorts() const override { return sorts; };
 };
 
 }  // namespace Modrinth

launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp

@@ -1,15 +1,7 @@
 #include "ModrinthPage.h"
 #include "ui_ModPage.h"
 
-#include <QKeyEvent>
-
-#include "Application.h"
-#include "InstanceImportTask.h"
-#include "Json.h"
-#include "ModDownloadTask.h"
 #include "ModrinthModel.h"
-#include "minecraft/MinecraftInstance.h"
-#include "minecraft/PackProfile.h"
 #include "ui/dialogs/ModDownloadDialog.h"
 
 ModrinthPage::ModrinthPage(ModDownloadDialog* dialog, BaseInstance* instance)
@@ -33,29 +25,12 @@ ModrinthPage::ModrinthPage(ModDownloadDialog* dialog, BaseInstance* instance)
     connect(ui->modSelectionButton, &QPushButton::clicked, this, &ModrinthPage::onModSelected);
 }
 
-bool ModrinthPage::shouldDisplay() const { return true; }
+bool ModrinthPage::validateVersion(ModPlatform::IndexedVersion& ver, QString mineVer, QString loaderVer) const
-
-void ModrinthPage::onRequestVersionsSucceeded(QJsonDocument& response, QString addonId)
 {
-    if (addonId != current.addonId) { return; }
+    return ver.mcVersion.contains(mineVer) && ver.loaders.contains(loaderVer);
-    
-    QJsonArray arr = response.array();
-    try {
-        Modrinth::loadIndexedPackVersions(current, arr, APPLICATION->network(), m_instance);
-    } catch (const JSONValidationError& e) {
-        qDebug() << response;
-        qWarning() << "Error while reading Modrinth mod version: " << e.cause();
-    }
-    auto packProfile = ((MinecraftInstance*)m_instance)->getPackProfile();
-    QString mcVersion = packProfile->getComponentVersion("net.minecraft");
-    QString loaderString = (packProfile->getComponentVersion("net.minecraftforge").isEmpty()) ? "fabric" : "forge";
-    for (int i = 0; i < current.versions.size(); i++) {
-        auto version = current.versions[i];
-        if (!version.mcVersion.contains(mcVersion) || !version.loaders.contains(loaderString)) { continue; }
-        ui->versionSelectionBox->addItem(version.version, QVariant(i));
-    }
-    if (ui->versionSelectionBox->count() == 0) { ui->versionSelectionBox->addItem(tr("No Valid Version found !"), QVariant(-1)); }
-
-    ui->modSelectionButton->setText(tr("Cannot select invalid version :("));
-    updateSelectionButton();
 }
+
+// I don't know why, but doing this on the parent class makes it so that
+// other mod providers start loading before being selected, at least with
+// my Qt, so we need to implement this in every derived class...
+bool ModrinthPage::shouldDisplay() const { return true; }

launcher/ui/pages/modplatform/modrinth/ModrinthPage.h

@@ -19,8 +19,7 @@ class ModrinthPage : public ModPage {
     inline QString debugName() const override { return tr("Modrinth"); }
     inline QString metaEntryBase() const override { return "ModrinthPacks"; };
 
-    bool shouldDisplay() const override;
+    bool validateVersion(ModPlatform::IndexedVersion& ver, QString mineVer, QString loaderVer = "") const override;
 
-   private:
+    bool shouldDisplay() const override;
-    void onRequestVersionsSucceeded(QJsonDocument&, QString) override;
 };