Apply suggestion from code review

Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>

launcher/java/JavaMetadata.cpp

@@ -31,8 +31,10 @@ DownloadType parseDownloadType(QString javaDownload)
 {
     if (javaDownload == "manifest")
         return DownloadType::Manifest;
-    // if (javaDownload == "archive")
+    else if (javaDownload == "archive")
         return DownloadType::Archive;
+    else
+        return DownloadType::Unknown;
 }
 QString downloadTypeToString(DownloadType javaDownload)
 {
@@ -41,8 +43,10 @@ QString downloadTypeToString(DownloadType javaDownload)
             return "manifest";
         case DownloadType::Archive:
             return "archive";
+        case DownloadType::Unknown:
+            break;
     }
-    return "";
+    return "unknown";
 }
 MetadataPtr parseJavaMeta(const QJsonObject& in)
 {

launcher/java/JavaMetadata.h

@@ -28,7 +28,7 @@
 
 namespace Java {
 
-enum class DownloadType { Manifest, Archive };
+enum class DownloadType { Manifest, Archive, Unknown };
 
 class Metadata : public BaseVersion {
    public:

launcher/java/JavaUtils.cpp

@@ -463,12 +463,10 @@ QString JavaUtils::getJavaCheckPath()
 
 QStringList getMinecraftJavaBundle()
 {
-    QString executable = "java";
     QStringList processpaths;
 #if defined(Q_OS_OSX)
     processpaths << FS::PathCombine(QDir::homePath(), FS::PathCombine("Library", "Application Support", "minecraft", "runtime"));
 #elif defined(Q_OS_WIN32)
-    executable += "w.exe";
 
     auto appDataPath = QProcessEnvironment::systemEnvironment().value("APPDATA", "");
     processpaths << FS::PathCombine(QFileInfo(appDataPath).absoluteFilePath(), ".minecraft", "runtime");
@@ -493,7 +491,7 @@ QStringList getMinecraftJavaBundle()
         auto binFound = false;
         for (auto& entry : entries) {
             if (entry.baseName() == "bin") {
-                javas.append(FS::PathCombine(entry.canonicalFilePath(), executable));
+                javas.append(FS::PathCombine(entry.canonicalFilePath(), JavaUtils::javaExecutable));
                 binFound = true;
                 break;
             }
@@ -507,19 +505,20 @@ QStringList getMinecraftJavaBundle()
     return javas;
 }
 
+#if defined(Q_OS_WIN32)
+const QString JavaUtils::javaExecutable = "javaw.exe";
+#else
+const QString JavaUtils::javaExecutable = "java";
+#endif
+
 QStringList getPrismJavaBundle()
 {
     QList<QString> javas;
 
-    QString executable = "java";
-#if defined(Q_OS_WIN32)
-    executable += "w.exe";
-#endif
-
     auto scanDir = [&](QString prefix) {
-        javas.append(FS::PathCombine(prefix, "jre", "bin", executable));
+        javas.append(FS::PathCombine(prefix, "jre", "bin", JavaUtils::javaExecutable));
-        javas.append(FS::PathCombine(prefix, "bin", executable));
+        javas.append(FS::PathCombine(prefix, "bin", JavaUtils::javaExecutable));
-        javas.append(FS::PathCombine(prefix, executable));
+        javas.append(FS::PathCombine(prefix, JavaUtils::javaExecutable));
     };
     auto scanJavaDir = [&](const QString& dirPath) {
         QDir dir(dirPath);

launcher/java/JavaUtils.h

@@ -42,4 +42,5 @@ class JavaUtils : public QObject {
 #endif
 
     static QString getJavaCheckPath();
+    static const QString javaExecutable;
 };

launcher/java/download/ArchiveDownloadTask.cpp

@@ -87,7 +87,7 @@ void ArchiveDownloadTask::extractJava(QString input)
         }
         emitSucceeded();
         return;
-    }
+    } else if (input.endsWith("zip")) {
         auto zip = std::make_shared<QuaZip>(input);
         if (!zip->open(QuaZip::mdUnzip)) {
             emitFailed(tr("Unable to open supplied zip file."));
@@ -95,7 +95,7 @@ void ArchiveDownloadTask::extractJava(QString input)
         }
         auto files = zip->getFileNameList();
         if (files.isEmpty()) {
-        emitFailed(tr("Empty archive"));
+            emitFailed(tr("No files were found in the supplied zip file,"));
             return;
         }
         m_task = makeShared<MMCZip::ExtractZipTask>(zip, m_final_path, files[0]);
@@ -126,6 +126,9 @@ void ArchiveDownloadTask::extractJava(QString input)
         m_task->start();
     }
 
+    emitFailed(tr("Could not determine archive type!"));
+}
+
 bool ArchiveDownloadTask::abort()
 {
     auto aborted = canAbort();

launcher/minecraft/launch/AutoInstallJava.cpp

@@ -44,6 +44,7 @@
 #include "SysInfo.h"
 #include "java/JavaInstall.h"
 #include "java/JavaInstallList.h"
+#include "java/JavaUtils.h"
 #include "java/JavaVersion.h"
 #include "java/download/ArchiveDownloadTask.h"
 #include "java/download/ManifestDownloadTask.h"
@@ -91,7 +92,7 @@ void AutoInstallJava::executeTask()
         return;
     }
     if (m_supported_arch.isEmpty()) {
-        emit logLine(tr("Your system(%1 %2) is not compatible with automatic Java installation. Using the default Java path.")
+        emit logLine(tr("Your system (%1-%2) is not compatible with automatic Java installation. Using the default Java path.")
                          .arg(SysInfo::currentSystem(), SysInfo::useQTForArch()),
                      MessageLevel::Warning);
         emitSucceeded();
@@ -99,7 +100,8 @@ void AutoInstallJava::executeTask()
     }
     auto wantedJavaName = packProfile->getProfile()->getCompatibleJavaName();
     if (wantedJavaName.isEmpty()) {
-        emit logLine(tr("Your meta information is out of date or doesn't have the information necessary to determine what installation of Java should be used. "
+        emit logLine(tr("Your meta information is out of date or doesn't have the information necessary to determine what installation of "
+                        "Java should be used. "
                         "Using the default Java path."),
                      MessageLevel::Warning);
         emitSucceeded();
@@ -133,17 +135,13 @@ void AutoInstallJava::setJavaPath(QString path)
 
 void AutoInstallJava::setJavaPathFromPartial()
 {
-    QString executable = "java";
-#if defined(Q_OS_WIN32)
-    executable += "w.exe";
-#endif
     auto packProfile = m_instance->getPackProfile();
     auto javaName = packProfile->getProfile()->getCompatibleJavaName();
     QDir javaDir(APPLICATION->javaPath());
     // just checking if the executable is there should suffice
     // but if needed this can be achieved through refreshing the javalist
     // and retrieving the path that contains the java name
-    auto relativeBinary = FS::PathCombine(javaName, "bin", executable);
+    auto relativeBinary = FS::PathCombine(javaName, "bin", JavaUtils::javaExecutable);
     auto finalPath = javaDir.absoluteFilePath(relativeBinary);
     if (QFileInfo::exists(finalPath)) {
         setJavaPath(finalPath);
@@ -169,6 +167,9 @@ void AutoInstallJava::downloadJava(Meta::Version::Ptr version, QString javaName)
                 case Java::DownloadType::Archive:
                     m_current_task = makeShared<Java::ArchiveDownloadTask>(java->url, final_path, java->checksumType, java->checksumHash);
                     break;
+                case Java::DownloadType::Unknown:
+                    emitFailed(tr("Could not determine Java download type!"));
+                    return;
             }
             auto deletePath = [final_path] { FS::deletePath(final_path); };
             connect(m_current_task.get(), &Task::failed, this, [this, deletePath](QString reason) {
@@ -201,7 +202,8 @@ void AutoInstallJava::tryNextMajorJava()
     auto wantedJavaName = packProfile->getProfile()->getCompatibleJavaName();
     auto majorJavaVersions = packProfile->getProfile()->getCompatibleJavaMajors();
     if (m_majorJavaVersionIndex >= majorJavaVersions.length()) {
-        emit logLine(tr("No versions of Java were found for your operating system: %1-%2").arg(SysInfo::currentSystem(), SysInfo::useQTForArch()),
+        emit logLine(
+            tr("No versions of Java were found for your operating system: %1-%2").arg(SysInfo::currentSystem(), SysInfo::useQTForArch()),
             MessageLevel::Warning);
         emit logLine(tr("No compatible version of Java was found. Using the default one."), MessageLevel::Warning);
         emitSucceeded();

launcher/ui/java/InstallJavaDialog.cpp

@@ -211,6 +211,7 @@ void InstallDialog::done(int result)
             if (meta) {
                 Task::Ptr task;
                 auto final_path = FS::PathCombine(APPLICATION->javaPath(), meta->m_name);
+                auto deletePath = [final_path] { FS::deletePath(final_path); };
                 switch (meta->downloadType) {
                     case Java::DownloadType::Manifest:
                         task = makeShared<ManifestDownloadTask>(meta->url, final_path, meta->checksumType, meta->checksumHash);
@@ -218,8 +219,11 @@ void InstallDialog::done(int result)
                     case Java::DownloadType::Archive:
                         task = makeShared<ArchiveDownloadTask>(meta->url, final_path, meta->checksumType, meta->checksumHash);
                         break;
+                    case Java::DownloadType::Unknown:
+                        QString error = QString(tr("Could not determine Java download type!"));
+                        CustomMessageBox::selectable(this, tr("Error"), error, QMessageBox::Warning)->show();
+                        deletePath();
                 }
-                auto deletePath = [final_path] { FS::deletePath(final_path); };
                 connect(task.get(), &Task::failed, this, [this, &deletePath](QString reason) {
                     QString error = QString("Java download failed: %1").arg(reason);
                     CustomMessageBox::selectable(this, tr("Error"), error, QMessageBox::Warning)->show();

launcher/ui/widgets/JavaSettingsWidget.cpp

@@ -339,17 +339,12 @@ void JavaSettingsWidget::javaVersionSelected(BaseVersion::Ptr version)
 
 void JavaSettingsWidget::on_javaBrowseBtn_clicked()
 {
-    QString filter;
+    auto filter = QString("Java (%1)").arg(JavaUtils::javaExecutable);
-#if defined Q_OS_WIN32
+    auto raw_path = QFileDialog::getOpenFileName(this, tr("Find Java executable"), QString(), filter);
-    filter = "Java (javaw.exe)";
-#else
-    filter = "Java (java)";
-#endif
-    QString raw_path = QFileDialog::getOpenFileName(this, tr("Find Java executable"), QString(), filter);
     if (raw_path.isEmpty()) {
         return;
     }
-    QString cooked_path = FS::NormalizePath(raw_path);
+    auto cooked_path = FS::NormalizePath(raw_path);
     m_javaPathTextBox->setText(cooked_path);
     checkJavaPath(cooked_path);
 }