From 881b2f2b385f19d9b64a149fca3c3741a803a172 Mon Sep 17 00:00:00 2001 From: flow Date: Wed, 2 Mar 2022 18:35:59 -0300 Subject: refactor: Use a single indexed pack for mods Since there's little difference between them, let's remove duplication and merge them. --- launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp') diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp index 35cd743a..4dfc8504 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp @@ -76,7 +76,7 @@ void ModrinthPage::onSelectionChanged(QModelIndex first, QModelIndex second) { return; } - current = listModel->data(first, Qt::UserRole).value(); + current = listModel->data(first, Qt::UserRole).value(); QString text = ""; QString name = current.name; @@ -84,8 +84,8 @@ void ModrinthPage::onSelectionChanged(QModelIndex first, QModelIndex second) { text = name; else text = "" + name + ""; - text += "
" + tr(" by ") + "" + - current.author.name + "

"; + text += "
" + tr(" by ") + "" + + current.authors[0].name + "

"; ui->packDescription->setHtml(text + current.description); if (!current.versionsLoaded) { @@ -98,7 +98,7 @@ void ModrinthPage::onSelectionChanged(QModelIndex first, QModelIndex second) { new NetJob(QString("Modrinth::ModVersions(%1)").arg(current.name), APPLICATION->network()); auto response = new QByteArray(); - QString addonId = current.addonId; + QString addonId = current.addonId.toString(); netJob->addNetAction(Net::Download::makeByteArray( QString("https://api.modrinth.com/v2/project/%1/version").arg(addonId), response)); -- cgit From 0dd1c26cf3cd68cd83f5d9da6cf34d4aa3f30db2 Mon Sep 17 00:00:00 2001 From: flow Date: Wed, 2 Mar 2022 21:17:10 -0300 Subject: refactor: extract common code in mod pages and model This creates a hierarchy in which ModPage and ModModel are the parents of every mod provider, providing the basic functionality common to all of them. It also imposes a unique .ui file (they were already equal before, just duplicated basically) on all mod providers. --- .../ui/pages/modplatform/modrinth/ModrinthPage.cpp | 217 ++++----------------- 1 file changed, 40 insertions(+), 177 deletions(-) (limited to 'launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp') diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp index 4dfc8504..aa3efe55 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp @@ -1,5 +1,5 @@ #include "ModrinthPage.h" -#include "ui_ModrinthPage.h" +#include "ui_ModPage.h" #include @@ -12,194 +12,57 @@ #include "minecraft/PackProfile.h" #include "ui/dialogs/ModDownloadDialog.h" -ModrinthPage::ModrinthPage(ModDownloadDialog *dialog, BaseInstance *instance) - : QWidget(dialog), m_instance(instance), ui(new Ui::ModrinthPage), - dialog(dialog) { - ui->setupUi(this); - connect(ui->searchButton, &QPushButton::clicked, this, - &ModrinthPage::triggerSearch); - ui->searchEdit->installEventFilter(this); - listModel = new Modrinth::ListModel(this); - ui->packView->setModel(listModel); - - ui->versionSelectionBox->view()->setVerticalScrollBarPolicy( - Qt::ScrollBarAsNeeded); - ui->versionSelectionBox->view()->parentWidget()->setMaximumHeight(300); - - // index is used to set the sorting with the modrinth api - ui->sortByBox->addItem(tr("Sort by Relevence")); - ui->sortByBox->addItem(tr("Sort by Downloads")); - ui->sortByBox->addItem(tr("Sort by Follows")); - ui->sortByBox->addItem(tr("Sort by last updated")); - ui->sortByBox->addItem(tr("Sort by newest")); - - connect(ui->sortByBox, SIGNAL(currentIndexChanged(int)), this, - SLOT(triggerSearch())); - connect(ui->packView->selectionModel(), &QItemSelectionModel::currentChanged, - this, &ModrinthPage::onSelectionChanged); - connect(ui->versionSelectionBox, &QComboBox::currentTextChanged, this, - &ModrinthPage::onVersionSelectionChanged); - connect(ui->modSelectionButton, &QPushButton::clicked, this, - &ModrinthPage::onModSelected); -} - -ModrinthPage::~ModrinthPage() { delete ui; } - -bool ModrinthPage::eventFilter(QObject *watched, QEvent *event) { - if (watched == ui->searchEdit && event->type() == QEvent::KeyPress) { - QKeyEvent *keyEvent = static_cast(event); - if (keyEvent->key() == Qt::Key_Return) { - triggerSearch(); - keyEvent->accept(); - return true; - } - } - return QWidget::eventFilter(watched, event); +ModrinthPage::ModrinthPage(ModDownloadDialog* dialog, BaseInstance* instance) + : ModPage(dialog, instance) +{ + listModel = new Modrinth::ListModel(this); + ui->packView->setModel(listModel); + + // index is used to set the sorting with the modrinth api + ui->sortByBox->addItem(tr("Sort by Relevence")); + ui->sortByBox->addItem(tr("Sort by Downloads")); + ui->sortByBox->addItem(tr("Sort by Follows")); + ui->sortByBox->addItem(tr("Sort by last updated")); + ui->sortByBox->addItem(tr("Sort by newest")); + + // sometimes Qt just ignores virtual slots and doesn't work as intended it seems, + // so it's best not to connect them in the parent's contructor... + connect(ui->sortByBox, SIGNAL(currentIndexChanged(int)), this, SLOT(triggerSearch())); + connect(ui->packView->selectionModel(), &QItemSelectionModel::currentChanged, this, &ModrinthPage::onSelectionChanged); + connect(ui->versionSelectionBox, &QComboBox::currentTextChanged, this, &ModrinthPage::onVersionSelectionChanged); + connect(ui->modSelectionButton, &QPushButton::clicked, this, &ModrinthPage::onModSelected); } bool ModrinthPage::shouldDisplay() const { return true; } -void ModrinthPage::openedImpl() { - updateSelectionButton(); - triggerSearch(); -} - -void ModrinthPage::triggerSearch() { - listModel->searchWithTerm(ui->searchEdit->text(), - ui->sortByBox->currentIndex()); -} - -void ModrinthPage::onSelectionChanged(QModelIndex first, QModelIndex second) { - ui->versionSelectionBox->clear(); - - if (!first.isValid()) { - return; - } - - current = listModel->data(first, Qt::UserRole).value(); - QString text = ""; - QString name = current.name; - - if (current.websiteUrl.isEmpty()) - text = name; - else - text = "" + name + ""; - text += "
" + tr(" by ") + "" + - current.authors[0].name + "

"; - ui->packDescription->setHtml(text + current.description); - - if (!current.versionsLoaded) { - qDebug() << "Loading Modrinth mod versions"; - - ui->modSelectionButton->setText(tr("Loading versions...")); - ui->modSelectionButton->setEnabled(false); - - auto netJob = - new NetJob(QString("Modrinth::ModVersions(%1)").arg(current.name), - APPLICATION->network()); - auto response = new QByteArray(); - QString addonId = current.addonId.toString(); - netJob->addNetAction(Net::Download::makeByteArray( - QString("https://api.modrinth.com/v2/project/%1/version").arg(addonId), - response)); - - QObject::connect(netJob, &NetJob::succeeded, this, [this, response, addonId] { - if(addonId != current.addonId){ - return; - } - QJsonParseError parse_error; - QJsonDocument doc = QJsonDocument::fromJson(*response, &parse_error); - if (parse_error.error != QJsonParseError::NoError) { - qWarning() << "Error while parsing JSON response from Modrinth at " - << parse_error.offset +void ModrinthPage::onModVersionSucceed(ModPage* instance, QByteArray* response, QString addonId) +{ + if (addonId != current.addonId) { return; } + QJsonParseError parse_error; + QJsonDocument doc = QJsonDocument::fromJson(*response, &parse_error); + if (parse_error.error != QJsonParseError::NoError) { + qWarning() << "Error while parsing JSON response from Modrinth at " << parse_error.offset << " reason: " << parse_error.errorString(); qWarning() << *response; return; - } - QJsonArray arr = doc.array(); - try { - Modrinth::loadIndexedPackVersions(current, arr, APPLICATION->network(), - m_instance); - } catch (const JSONValidationError &e) { + } + QJsonArray arr = doc.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 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; - } + 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(); - }); - - QObject::connect(netJob, &NetJob::finished, this, [response, netJob] { - netJob->deleteLater(); - delete response; - }); - - netJob->start(); - } else { - for (int i = 0; i < current.versions.size(); i++) { - ui->versionSelectionBox->addItem(current.versions[i].version, - QVariant(i)); - } - if (ui->versionSelectionBox->count() == 0) { - ui->versionSelectionBox->addItem(tr("No Valid Version found !"), - QVariant(-1)); } + 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 ModrinthPage::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")); - } -} - -void ModrinthPage::onVersionSelectionChanged(QString data) { - if (data.isNull() || data.isEmpty()) { - selectedVersion = -1; - return; - } - selectedVersion = ui->versionSelectionBox->currentData().toInt(); - updateSelectionButton(); -} - -void ModrinthPage::onModSelected() { - auto &version = current.versions[selectedVersion]; - if (dialog->isModSelected(current.name, version.fileName)) { - dialog->removeSelectedMod(current.name); - } else { - dialog->addSelectedMod(current.name, - new ModDownloadTask(version.downloadUrl, - version.fileName, dialog->mods)); - } - - updateSelectionButton(); } -- cgit From 5e9d49a91082c53907db84df809d21c3bdc8bcac Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 6 Mar 2022 13:54:05 -0300 Subject: refactor: use only a single unique_ptr for the api --- launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp') diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp index aa3efe55..4e4a9db4 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp @@ -13,7 +13,7 @@ #include "ui/dialogs/ModDownloadDialog.h" ModrinthPage::ModrinthPage(ModDownloadDialog* dialog, BaseInstance* instance) - : ModPage(dialog, instance) + : ModPage(dialog, instance, new ModrinthAPI()) { listModel = new Modrinth::ListModel(this); ui->packView->setModel(listModel); -- cgit From 5a638fa97711231638615f920462ed5f5f4507e0 Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 6 Mar 2022 15:23:00 -0300 Subject: refactor: move "get versions" task from page to model This seems more reasonable --- launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp') diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp index 4e4a9db4..7ac9b406 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp @@ -35,7 +35,7 @@ ModrinthPage::ModrinthPage(ModDownloadDialog* dialog, BaseInstance* instance) bool ModrinthPage::shouldDisplay() const { return true; } -void ModrinthPage::onModVersionSucceed(ModPage* instance, QByteArray* response, QString addonId) +void ModrinthPage::onGetVersionsSucceeded(ModPage* instance, QByteArray* response, QString addonId) { if (addonId != current.addonId) { return; } QJsonParseError parse_error; -- cgit From d755174bee1b1c5ba507d1d407236190501c7940 Mon Sep 17 00:00:00 2001 From: flow Date: Sun, 6 Mar 2022 16:04:24 -0300 Subject: clarify some method names and comments --- launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp') diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp index 7ac9b406..944f8afb 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp @@ -35,7 +35,7 @@ ModrinthPage::ModrinthPage(ModDownloadDialog* dialog, BaseInstance* instance) bool ModrinthPage::shouldDisplay() const { return true; } -void ModrinthPage::onGetVersionsSucceeded(ModPage* instance, QByteArray* response, QString addonId) +void ModrinthPage::onRequestVersionsSucceeded(ModPage* instance, QByteArray* response, QString addonId) { if (addonId != current.addonId) { return; } QJsonParseError parse_error; -- cgit From f714adf6d2cc94f20ba37f2776d0d61e22267f0e Mon Sep 17 00:00:00 2001 From: flow Date: Mon, 7 Mar 2022 16:22:57 -0300 Subject: refactor: move NetJob away from ModModel to ModAPI This is done so that 1. ModAPI behaves more like an actual API instead of just a helper, and 2. Allows for more easily creating other mod providers that may or may not use network tasks (foreshadowing lol) --- launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) (limited to 'launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp') diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp index 944f8afb..fa703e38 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp @@ -35,22 +35,15 @@ ModrinthPage::ModrinthPage(ModDownloadDialog* dialog, BaseInstance* instance) bool ModrinthPage::shouldDisplay() const { return true; } -void ModrinthPage::onRequestVersionsSucceeded(ModPage* instance, QByteArray* response, QString addonId) +void ModrinthPage::onRequestVersionsSucceeded(QJsonDocument& response, QString addonId) { if (addonId != current.addonId) { return; } - QJsonParseError parse_error; - QJsonDocument doc = QJsonDocument::fromJson(*response, &parse_error); - if (parse_error.error != QJsonParseError::NoError) { - qWarning() << "Error while parsing JSON response from Modrinth at " << parse_error.offset - << " reason: " << parse_error.errorString(); - qWarning() << *response; - return; - } - QJsonArray arr = doc.array(); + + QJsonArray arr = response.array(); try { Modrinth::loadIndexedPackVersions(current, arr, APPLICATION->network(), m_instance); } catch (const JSONValidationError& e) { - qDebug() << *response; + qDebug() << response; qWarning() << "Error while reading Modrinth mod version: " << e.cause(); } auto packProfile = ((MinecraftInstance*)m_instance)->getPackProfile(); -- cgit From 9c57b54a81a9026aa86a983a203df17c023eaa8d Mon Sep 17 00:00:00 2001 From: flow Date: Mon, 7 Mar 2022 19:29:59 -0300 Subject: 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. --- .../ui/pages/modplatform/modrinth/ModrinthPage.cpp | 39 ++++------------------ 1 file changed, 7 insertions(+), 32 deletions(-) (limited to 'launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp') diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp index fa703e38..cf01506e 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp @@ -1,15 +1,7 @@ #include "ModrinthPage.h" #include "ui_ModPage.h" -#include - -#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; } - -void ModrinthPage::onRequestVersionsSucceeded(QJsonDocument& response, QString addonId) +bool ModrinthPage::validateVersion(ModPlatform::IndexedVersion& ver, QString mineVer, QString loaderVer) const { - if (addonId != current.addonId) { return; } - - 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(); + return ver.mcVersion.contains(mineVer) && ver.loaders.contains(loaderVer); } + +// 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; } -- cgit From 8409aa2571d57f015a634a220107d199e88ba2fd Mon Sep 17 00:00:00 2001 From: flow Date: Tue, 8 Mar 2022 11:12:35 -0300 Subject: tidy: Fix clang-tidy issues on files changed in this PR The checks used are roughly the same as the ones proposed in the clang-tidy PR (except perhaps that I used modernize-* instead of listing them individually,though I don't think this caused any readability detriments). In ModrinthModel.cpp and FlameModModel.cpp I ignored the modernize-avoid-c-arrays one, mostly because making the sorts array an std::array would most likely increase the code complexity because of the virtual function. Aside from that, the static_cast warning from Application.h was not dealt with, since it's not in this PR's scope. --- launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp') diff --git a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp index cf01506e..aebfee74 100644 --- a/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp +++ b/launcher/ui/pages/modplatform/modrinth/ModrinthPage.cpp @@ -25,7 +25,7 @@ ModrinthPage::ModrinthPage(ModDownloadDialog* dialog, BaseInstance* instance) connect(ui->modSelectionButton, &QPushButton::clicked, this, &ModrinthPage::onModSelected); } -bool ModrinthPage::validateVersion(ModPlatform::IndexedVersion& ver, QString mineVer, QString loaderVer) const +auto ModrinthPage::validateVersion(ModPlatform::IndexedVersion& ver, QString mineVer, QString loaderVer) const -> bool { return ver.mcVersion.contains(mineVer) && ver.loaders.contains(loaderVer); } @@ -33,4 +33,4 @@ bool ModrinthPage::validateVersion(ModPlatform::IndexedVersion& ver, QString min // 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; } +auto ModrinthPage::shouldDisplay() const -> bool { return true; } -- cgit