GH-4699 Clean some things up

Add a menu to select between MMC/Modrinth format packs instead of the custom dialog
Treat 404s on requests to the Modrinth API as success, as the API returns a 404 if a hash was not found, and we don't want to retry the download in this case
Improve logging

launcher/CMakeLists.txt

@@ -786,8 +786,6 @@ SET(LAUNCHER_SOURCES
     ui/dialogs/SkinUploadDialog.h
     ui/dialogs/CreateShortcutDialog.cpp
     ui/dialogs/CreateShortcutDialog.h
-    ui/dialogs/SelectInstanceExportFormatDialog.cpp
-    ui/dialogs/SelectInstanceExportFormatDialog.h
         ui/dialogs/ModrinthExportDialog.cpp
     ui/dialogs/ModrinthExportDialog.h
 
@@ -888,7 +886,6 @@ qt5_wrap_ui(LAUNCHER_UI
     ui/dialogs/LoginDialog.ui
     ui/dialogs/EditAccountDialog.ui
     ui/dialogs/CreateShortcutDialog.ui
-    ui/dialogs/SelectInstanceExportFormatDialog.ui
     ui/dialogs/ModrinthExportDialog.ui
 )
 

launcher/ModrinthInstanceExportTask.cpp

@@ -58,7 +58,8 @@ void ModrinthInstanceExportTask::executeTask()
 
     m_netJob = new NetJob(tr("Modrinth pack export"), APPLICATION->network());
 
-    for (QString filePath: filesToResolve) {
+    for (const QString &filePath: filesToResolve) {
+        qDebug() << "Attempting to resolve file hash from Modrinth API: " << filePath;
         QFile file(filePath);
 
         if (file.open(QFile::ReadOnly)) {
@@ -74,7 +75,8 @@ void ModrinthInstanceExportTask::executeTask()
 
             m_netJob->addNetAction(Net::Download::makeByteArray(
                     QString("https://api.modrinth.com/v2/version_file/%1?algorithm=sha512").arg(hash),
-                    &m_responses.last().response
+                    &m_responses.last().response,
+                    Net::Download::Options(Net::Download::Option::AllowNotFound)
             ));
         }
     }
@@ -114,7 +116,7 @@ void ModrinthInstanceExportTask::lookupSucceeded()
 
             resolvedFiles << fileData;
         } catch (const Json::JsonException &e) {
-            qDebug() << "File " << data.fileInfo.path() << " failed to process for reason " << e.cause() << ", adding to overrides";
+            qDebug() << "File " << data.fileInfo.absoluteFilePath() << " failed to process for reason " << e.cause() << ", adding to overrides";
             failedFiles << data.fileInfo;
         }
     }
@@ -178,7 +180,6 @@ void ModrinthInstanceExportTask::lookupSucceeded()
                 QString src = file.absoluteFilePath();
                 tmpDir.mkpath("overrides/" + gameDir.relativeFilePath(file.absolutePath()));
                 QString dest = tmpDir.path() + "/overrides/" + gameDir.relativeFilePath(src);
-                qDebug() << dest;
                 if (!QFile::copy(file.absoluteFilePath(), dest)) {
                     emitFailed(tr("Failed to copy file %1 to overrides").arg(src));
                     return;
@@ -206,13 +207,13 @@ void ModrinthInstanceExportTask::lookupSucceeded()
         return;
     }
 
+    qDebug() << "Successfully exported Modrinth pack to " << m_settings.exportPath;
     emitSucceeded();
 }
 
-void ModrinthInstanceExportTask::lookupFailed(const QString &)
+void ModrinthInstanceExportTask::lookupFailed(const QString &reason)
 {
-    lookupSucceeded(); // the NetJob will fail if some files were not found on Modrinth, we still want to continue in that case
+    emitFailed(reason);
-    // FIXME: the NetJob will retry each download 3 times if it fails, we should probably stop it from doing that
 }
 
 void ModrinthInstanceExportTask::lookupProgress(qint64 current, qint64 total)

launcher/net/Download.cpp

@@ -122,6 +122,13 @@ void Download::downloadError(QNetworkReply::NetworkError error)
         qCritical() << "Aborted " << m_url.toString();
         m_status = Job_Aborted;
     }
+    else if(error == QNetworkReply::ContentNotFoundError && (m_options & Option::AllowNotFound))
+    {
+        // The Modrinth API returns a 404 when a hash was not found when performing reverse hash lookup, we don't want to treat this as a failure
+        qDebug() << "Received 404 from " << m_url.toString() << ", continuing...";
+        m_status = Job_Finished;
+        return;
+    }
     else
     {
         if(m_options & Option::AcceptLocalFiles)

launcher/net/Download.h

@@ -32,7 +32,8 @@ public: /* types */
     enum class Option
     {
         NoOptions = 0,
-        AcceptLocalFiles = 1
+        AcceptLocalFiles = 1,
+        AllowNotFound =2
     };
     Q_DECLARE_FLAGS(Options, Option)
 

launcher/ui/MainWindow.cpp

@@ -84,7 +84,8 @@
 #include "ui/dialogs/EditAccountDialog.h"
 #include "ui/dialogs/NotificationDialog.h"
 #include "ui/dialogs/CreateShortcutDialog.h"
-#include "ui/dialogs/SelectInstanceExportFormatDialog.h"
+#include "ui/dialogs/ExportInstanceDialog.h"
+#include "ui/dialogs/ModrinthExportDialog.h"
 
 #include "UpdateController.h"
 #include "KonamiCode.h"
@@ -974,6 +975,31 @@ void MainWindow::showInstanceContextMenu(const QPoint &pos)
 
 void MainWindow::updateToolsMenu()
 {
+    QToolButton *exportButton = dynamic_cast<QToolButton*>(ui->instanceToolBar->widgetForAction(ui->actionExportInstance));
+    exportButton->setPopupMode(QToolButton::MenuButtonPopup);
+
+    QMenu *exportMenu = ui->actionExportInstance->menu();
+
+    if (exportMenu) {
+        exportMenu->clear();
+    } else {
+        exportMenu = new QMenu();
+    }
+
+    QAction *mmcExport = exportMenu->addAction(BuildConfig.LAUNCHER_NAME);
+    QAction *modrinthExport = exportMenu->addAction(tr("Modrinth"));
+
+    connect(mmcExport, &QAction::triggered, this, &MainWindow::on_actionExportInstance_triggered);
+    connect(modrinthExport, &QAction::triggered, [this]()
+    {
+        if (m_selectedInstance) {
+            ModrinthExportDialog dlg(m_selectedInstance, this);
+            dlg.exec();
+        }
+    });
+
+    ui->actionExportInstance->setMenu(exportMenu);
+
     QToolButton *launchButton = dynamic_cast<QToolButton*>(ui->instanceToolBar->widgetForAction(ui->actionLaunchInstance));
     QToolButton *launchOfflineButton = dynamic_cast<QToolButton*>(ui->instanceToolBar->widgetForAction(ui->actionLaunchInstanceOffline));
 
@@ -1756,7 +1782,7 @@ void MainWindow::on_actionExportInstance_triggered()
 {
     if (m_selectedInstance)
     {
-        SelectInstanceExportFormatDialog dlg(m_selectedInstance, this);
+        ExportInstanceDialog dlg(m_selectedInstance, this);
         dlg.exec();
     }
 }

launcher/ui/dialogs/SelectInstanceExportFormatDialog.cpp

@@ -1,37 +0,0 @@
-/*
- * Copyright 2023 arthomnix
- *
- * This source is subject to the Microsoft Public License (MS-PL).
- * Please see the COPYING.md file for more information.
- */
-
-#include "SelectInstanceExportFormatDialog.h"
-#include "ui_SelectInstanceExportFormatDialog.h"
-#include "BuildConfig.h"
-#include "ModrinthExportDialog.h"
-
-
-SelectInstanceExportFormatDialog::SelectInstanceExportFormatDialog(InstancePtr instance, QWidget *parent) :
-        QDialog(parent), ui(new Ui::SelectInstanceExportFormatDialog), m_instance(instance)
-{
-    ui->setupUi(this);
-    ui->mmcFormat->setText(BuildConfig.LAUNCHER_NAME);
-}
-
-void SelectInstanceExportFormatDialog::accept()
-{
-    if (ui->mmcFormat->isChecked()) {
-        ExportInstanceDialog dlg(m_instance, parentWidget());
-        QDialog::accept();
-        dlg.exec();
-    } else if (ui->modrinthFormat->isChecked()) {
-        ModrinthExportDialog dlg(m_instance, parentWidget());
-        QDialog::accept();
-        dlg.exec();
-    }
-}
-
-SelectInstanceExportFormatDialog::~SelectInstanceExportFormatDialog()
-{
-    delete ui;
-}

launcher/ui/dialogs/SelectInstanceExportFormatDialog.h

@@ -1,36 +0,0 @@
-/*
- * Copyright 2023 arthomnix
- *
- * This source is subject to the Microsoft Public License (MS-PL).
- * Please see the COPYING.md file for more information.
- */
-
-#pragma once
-
-#include <QDialog>
-#include "ExportInstanceDialog.h"
-
-
-QT_BEGIN_NAMESPACE
-namespace Ui
-{
-    class SelectInstanceExportFormatDialog;
-}
-QT_END_NAMESPACE
-
-class SelectInstanceExportFormatDialog : public QDialog
-{
-Q_OBJECT
-
-public:
-    explicit SelectInstanceExportFormatDialog(InstancePtr instance, QWidget *parent = nullptr);
-
-    ~SelectInstanceExportFormatDialog() override;
-
-private slots:
-    void accept() override;
-
-private:
-    Ui::SelectInstanceExportFormatDialog *ui;
-    InstancePtr m_instance;
-};

launcher/ui/dialogs/SelectInstanceExportFormatDialog.ui

@@ -1,95 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<ui version="4.0">
- <class>SelectInstanceExportFormatDialog</class>
- <widget class="QDialog" name="SelectInstanceExportFormatDialog">
-  <property name="geometry">
-   <rect>
-    <x>0</x>
-    <y>0</y>
-    <width>446</width>
-    <height>181</height>
-   </rect>
-  </property>
-  <property name="windowTitle">
-   <string>Select Instance Export Format</string>
-  </property>
-  <widget class="QWidget" name="verticalLayoutWidget">
-   <property name="geometry">
-    <rect>
-     <x>10</x>
-     <y>10</y>
-     <width>421</width>
-     <height>161</height>
-    </rect>
-   </property>
-   <layout class="QVBoxLayout" name="verticalLayout">
-    <item>
-     <widget class="QLabel" name="label">
-      <property name="text">
-       <string>Select export format</string>
-      </property>
-     </widget>
-    </item>
-    <item>
-     <widget class="QRadioButton" name="mmcFormat">
-      <property name="text">
-       <string>Launcher</string>
-      </property>
-      <property name="checked">
-       <bool>true</bool>
-      </property>
-     </widget>
-    </item>
-    <item>
-     <widget class="QRadioButton" name="modrinthFormat">
-      <property name="text">
-       <string>Modrinth (WIP)</string>
-      </property>
-     </widget>
-    </item>
-    <item>
-     <widget class="QDialogButtonBox" name="buttonBox">
-      <property name="standardButtons">
-       <set>QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set>
-      </property>
-     </widget>
-    </item>
-   </layout>
-  </widget>
- </widget>
- <resources/>
- <connections>
-  <connection>
-   <sender>buttonBox</sender>
-   <signal>accepted()</signal>
-   <receiver>SelectInstanceExportFormatDialog</receiver>
-   <slot>accept()</slot>
-   <hints>
-    <hint type="sourcelabel">
-     <x>220</x>
-     <y>152</y>
-    </hint>
-    <hint type="destinationlabel">
-     <x>222</x>
-     <y>90</y>
-    </hint>
-   </hints>
-  </connection>
-  <connection>
-   <sender>buttonBox</sender>
-   <signal>rejected()</signal>
-   <receiver>SelectInstanceExportFormatDialog</receiver>
-   <slot>reject()</slot>
-   <hints>
-    <hint type="sourcelabel">
-     <x>220</x>
-     <y>152</y>
-    </hint>
-    <hint type="destinationlabel">
-     <x>222</x>
-     <y>90</y>
-    </hint>
-   </hints>
-  </connection>
- </connections>
-</ui>