Cleanup codebase

for FileSystem.cpp:
Instead of checking if Linux or FreeBSD, check if its not Windows and not OSX. Chances are other operating systems run a DE that adheres to the XDG Desktop standard (.desktop). The check isn't good enough anyways since alternative shells for Windows exist, it will never be an accurate check. In any case this function is unused.

WorldListPage.cpp:
Redo confusing switch statement plagued with fall throughs, now well defined.

LaunchController.cpp:
Remove cringe. Also fix warning and make the unimplemented case(s) more explicit.

VersionProxyModel.cpp:
Add fallthrough for warning suppression.

WorldListPage.cpp:
redo `mceditState`

TranslationsModel.cpp:
Move up definition of `column` variable to when it is needed, clear up switch cases

FlameInstanceCreationTask.cpp:
Fallthrough intentionally

SkinUpload.cpp:
Make `getVariant`

ResourcePack.cpp:
Add new values for 1.19.3+

meta/Index.cpp:
Make clear switch statement behavior

JavaWizardPage.cpp:
Fix case fallthrough

Yggdrasil.cpp:
Fix case fallthrough

AccountList.cpp:
Fix case fallthrough,

WinDarkmode.cpp:
Add an explanation and fix warnings due to FARPROC casts.

Signed-off-by: jdp_ <42700985+jdpatdiscord@users.noreply.github.com>

launcher/FileSystem.cpp

@@ -363,7 +363,7 @@ QString getDesktopDir()
 // Cross-platform Shortcut creation
 bool createShortCut(QString location, QString dest, QStringList args, QString name, QString icon)
 {
-#if defined(Q_OS_LINUX) || defined(Q_OS_FREEBSD)
+#if !defined(Q_OS_WIN) && !defined(Q_OS_OSX)
     location = PathCombine(location, name + ".desktop");
 
     QFile f(location);
@@ -389,25 +389,6 @@ bool createShortCut(QString location, QString dest, QStringList args, QString na
     f.setPermissions(f.permissions() | QFileDevice::ExeOwner | QFileDevice::ExeGroup | QFileDevice::ExeOther);
 
     return true;
-#elif defined Q_OS_WIN
-    // TODO: Fix
-    //    QFile file(PathCombine(location, name + ".lnk"));
-    //    WCHAR *file_w;
-    //    WCHAR *dest_w;
-    //    WCHAR *args_w;
-    //    file.fileName().toWCharArray(file_w);
-    //    dest.toWCharArray(dest_w);
-
-    //    QString argStr;
-    //    for (int i = 0; i < args.count(); i++)
-    //    {
-    //        argStr.append(args[i]);
-    //        argStr.append(" ");
-    //    }
-    //    argStr.toWCharArray(args_w);
-
-    //    return SUCCEEDED(CreateLink(file_w, dest_w, args_w));
-    return false;
 #else
     qWarning("Desktop Shortcuts not supported on your platform!");
     return false;

launcher/LaunchController.cpp

@@ -190,7 +190,7 @@ void LaunchController::login() {
         switch(m_accountToUse->accountState()) {
             case AccountState::Offline: {
                 m_session->wants_online = false;
-                // NOTE: fallthrough is intentional
+                [[fallthrough]];
             }
             case AccountState::Online: {
                 if(!m_session->wants_online) {
@@ -223,7 +223,6 @@ void LaunchController::login() {
                         APPLICATION->settings()->set("LastOfflinePlayerName", usedname);
                     }
                     m_session->MakeOffline(usedname);
-                    // offline flavored game from here :3
                 }
                 if(m_accountToUse->ownsMinecraft()) {
                     if(!m_accountToUse->hasProfile()) {
@@ -270,7 +269,7 @@ void LaunchController::login() {
                 // This means some sort of soft error that we can fix with a refresh ... so let's refresh.
             case AccountState::Unchecked: {
                 m_accountToUse->refresh();
-                // NOTE: fallthrough intentional
+                [[fallthrough]];
             }
             case AccountState::Working: {
                 // refresh is in progress, we need to wait for it to finish to proceed.
@@ -283,12 +282,11 @@ void LaunchController::login() {
                 progDialog.execWithTask(task.get());
                 continue;
             }
-            // FIXME: this is missing - the meaning is that the account is queued for refresh and we should wait for that
-            /*
             case AccountState::Queued: {
+                // FIXME: this is missing - the meaning is that the account is queued for refresh and we should wait for that
+                qWarning() << "AccountState::Queued is not implemented";
                 return;
             }
-            */
             case AccountState::Expired: {
                 auto errorString = tr("The account has expired and needs to be logged into manually again.");
                 QMessageBox::warning(
@@ -325,6 +323,9 @@ void LaunchController::login() {
                 emitFailed(errorString);
                 return;
             }
+            default: {
+                qWarning() << "Invalid AccountState enum";
+            }
         }
     }
     emitFailed(tr("Failed to launch."));

launcher/VersionProxyModel.cpp

@@ -211,6 +211,7 @@ QVariant VersionProxyModel::data(const QModelIndex &index, int role) const
                             return tr("Latest");
                         }
                     }
+                    [[fallthrough]];
                 }
                 default:
                 {
@@ -254,6 +255,7 @@ QVariant VersionProxyModel::data(const QModelIndex &index, int role) const
                         }
                         return pixmap;
                     }
+                    [[fallthrough]];
                 }
                 default:
                 {

launcher/meta/Index.cpp

@@ -45,11 +45,9 @@ QVariant Index::data(const QModelIndex &index, int role) const
     switch (role)
     {
     case Qt::DisplayRole:
-        switch (index.column())
+        if (index.column() == 0)
-        {
+            return list->humanReadable();
-        case 0: return list->humanReadable();
+        break;
-        default: break;
-        }
     case UidRole: return list->uid();
     case NameRole: return list->name();
     case ListPtrRole: return QVariant::fromValue(list);
@@ -70,10 +68,7 @@ QVariant Index::headerData(int section, Qt::Orientation orientation, int role) c
     {
         return tr("Name");
     }
-    else
-    {
     return QVariant();
-    }
 }
 
 bool Index::hasUid(const QString &uid) const

launcher/minecraft/auth/AccountData.h

@@ -85,6 +85,7 @@ enum class AccountState {
     Disabled,
     Errored,
     Expired,
+    Queued,
     Gone
 };
 

launcher/minecraft/auth/AccountList.cpp

@@ -328,6 +328,12 @@ QVariant AccountList::data(const QModelIndex &index, int role) const
                     case AccountState::Gone: {
                         return tr("Gone", "Account status");
                     }
+                    case AccountState::Queued: {
+                        qWarning() << "Unhandled account state Queued";
+                        [[fallthrough]];
+                    }
+                    default:
+                        return tr("Unknown", "Account status");
                 }
             }
 
@@ -341,8 +347,9 @@ QVariant AccountList::data(const QModelIndex &index, int role) const
                 else {
                     return tr("No", "Can Migrate?");
                 }
+                qWarning() << "Unhandled case in MigrationColumn";
+                [[fallthrough]];
             }
-
             default:
                 return QVariant();
             }
@@ -359,7 +366,7 @@ QVariant AccountList::data(const QModelIndex &index, int role) const
                 case ProfileNameColumn:
                     return account == m_defaultAccount ? Qt::Checked : Qt::Unchecked;
             }
-
+            [[fallthrough]];
         default:
             return QVariant();
     }

launcher/minecraft/auth/Yggdrasil.cpp

@@ -273,6 +273,7 @@ void Yggdrasil::processReply() {
             AccountTaskState::STATE_FAILED_GONE,
             tr("The Mojang account no longer exists. It may have been migrated to a Microsoft account.")
         );
+        break;
     }
     default:
         changeState(

launcher/minecraft/mod/Mod.cpp

@@ -82,6 +82,8 @@ std::pair<int, bool> Mod::compare(const Resource& other, SortType type) const
             auto res = Resource::compare(other, type);
             if (res.first != 0)
                 return res;
+            // FIXME: Determine if this is a legitimate fallthrough
+            [[fallthrough]];
         }
         case SortType::VERSION: {
             auto this_ver = Version(version());

launcher/minecraft/mod/Resource.cpp

@@ -66,6 +66,7 @@ std::pair<int, bool> Resource::compare(const Resource& other, SortType type) con
                 return { 1, type == SortType::ENABLED };
             if (!enabled() && other.enabled())
                 return { -1, type == SortType::ENABLED };
+            [[fallthrough]];
         case SortType::NAME: {
             QString this_name{ name() };
             QString other_name{ other.name() };
@@ -76,6 +77,7 @@ std::pair<int, bool> Resource::compare(const Resource& other, SortType type) con
             auto compare_result = QString::compare(this_name, other_name, Qt::CaseInsensitive);
             if (compare_result != 0)
                 return { compare_result, type == SortType::NAME };
+            [[fallthrough]];
         }
         case SortType::DATE:
             if (dateTimeChanged() > other.dateTimeChanged())

launcher/minecraft/mod/ResourcePack.cpp

@@ -11,11 +11,18 @@
 // Values taken from:
 // https://minecraft.fandom.com/wiki/Tutorials/Creating_a_resource_pack#Formatting_pack.mcmeta
 static const QMap<int, std::pair<Version, Version>> s_pack_format_versions = {
-    { 1, { Version("1.6.1"), Version("1.8.9") } }, { 2, { Version("1.9"), Version("1.10.2") } },
+    { 1, { Version("1.6.1"), Version("1.8.9") } }, 
-    { 3, { Version("1.11"), Version("1.12.2") } }, { 4, { Version("1.13"), Version("1.14.4") } },
+    { 2, { Version("1.9"), Version("1.10.2") } },
-    { 5, { Version("1.15"), Version("1.16.1") } }, { 6, { Version("1.16.2"), Version("1.16.5") } },
+    { 3, { Version("1.11"), Version("1.12.2") } }, 
-    { 7, { Version("1.17"), Version("1.17.1") } }, { 8, { Version("1.18"), Version("1.18.2") } },
+    { 4, { Version("1.13"), Version("1.14.4") } },
+    { 5, { Version("1.15"), Version("1.16.1") } }, 
+    { 6, { Version("1.16.2"), Version("1.16.5") } },
+    { 7, { Version("1.17"), Version("1.17.1") } }, 
+    { 8, { Version("1.18"), Version("1.18.2") } },
     { 9, { Version("1.19"), Version("1.19.2") } },
+    { 12, { Version("1.19.3"), Version("1.19.3") } },
+    { 13, { Version("1.19.4"), Version("1.19.4") } },
+    { 14, { Version("1.20"), Version("1.20") } }
 };
 
 void ResourcePack::setPackFormat(int new_format_id)
@@ -85,6 +92,7 @@ std::pair<int, bool> ResourcePack::compare(const Resource& other, SortType type)
             auto res = Resource::compare(other, type);
             if (res.first != 0)
                 return res;
+            [[fallthrough]];
         }
         case SortType::PACK_FORMAT: {
             auto this_ver = packFormat();

launcher/minecraft/services/SkinUpload.cpp

@@ -42,12 +42,13 @@
 
 QByteArray getVariant(SkinUpload::Model model) {
     switch (model) {
-        default:
-            qDebug() << "Unknown skin type!";
         case SkinUpload::STEVE:
             return "CLASSIC";
         case SkinUpload::ALEX:
             return "SLIM";
+        default:
+            qDebug() << "Unknown skin type!";
+            return "CLASSIC";
     }
 }
 

launcher/modplatform/EnsureMetadataTask.cpp

@@ -405,7 +405,8 @@ NetJob::Ptr EnsureMetadataTask::flameProjectsTask()
     QHash<QString, QString> addonIds;
     for (auto const& hash : m_mods.keys()) {
         if (m_temp_versions.contains(hash)) {
-            auto const& data = m_temp_versions.find(hash).value();
+            auto const& dataObj = m_temp_versions.find(hash);
+            auto const& data = dataObj.value();
 
             auto id_str = data.addonId.toString();
             if (!id_str.isEmpty())

launcher/modplatform/flame/FlameInstanceCreationTask.cpp

@@ -420,7 +420,7 @@ void FlameCreationTask::setupDownloadJob(QEventLoop& loop)
         switch (result.type) {
             case Flame::File::Type::Folder: {
                 logWarning(tr("This 'Folder' may need extracting: %1").arg(relpath));
-                // fall-through intentional, we treat these as plain old mods and dump them wherever.
+                [[fallthrough]];
             }
             case Flame::File::Type::SingleFile:
             case Flame::File::Type::Mod: {

launcher/translations/TranslationsModel.cpp

@@ -418,8 +418,6 @@ QVariant TranslationsModel::data(const QModelIndex& index, int role) const
         return QVariant();
 
     int row = index.row();
-    auto column = static_cast<Column>(index.column());
-
     if (row < 0 || row >= d->m_languages.size())
         return QVariant();
 
@@ -428,22 +426,19 @@ QVariant TranslationsModel::data(const QModelIndex& index, int role) const
     {
     case Qt::DisplayRole:
     {
+        auto column = static_cast<Column>(index.column());
         switch(column)
         {
             case Column::Language: 
-            {
                 return lang.languageName();
-            }
             case Column::Completeness: 
-            {
                 return QString("%1%").arg(lang.percentTranslated(), 3, 'f', 1);
-            }
+            default: 
+                return QVariant();
         }
     }
     case Qt::ToolTipRole:
-    {
         return tr("%1:\n%2 translated\n%3 fuzzy\n%4 total").arg(lang.key, QString::number(lang.translated), QString::number(lang.fuzzy), QString::number(lang.total));
-    }
     case Qt::UserRole:
         return lang.key;
     default:

launcher/ui/WinDarkmode.cpp

@@ -8,16 +8,15 @@ namespace WinDarkmode {
 void setDarkWinTitlebar(WId winid, bool darkmode)
 {
     HWND hwnd = reinterpret_cast<HWND>(winid);
-    BOOL dark = (BOOL) darkmode;
+    BOOL dark = (BOOL)darkmode;
 
     HMODULE hUxtheme = LoadLibraryExW(L"uxtheme.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
     HMODULE hUser32 = GetModuleHandleW(L"user32.dll");
-    fnAllowDarkModeForWindow AllowDarkModeForWindow
+
-        = reinterpret_cast<fnAllowDarkModeForWindow>(GetProcAddress(hUxtheme, MAKEINTRESOURCEA(133)));
+    // For those confused how this works, dlls have export numbers but some can have no name and these have no name. So an address is gotten by export number. If this ever changes, it will break.
-    fnSetPreferredAppMode SetPreferredAppMode
+    fnAllowDarkModeForWindow AllowDarkModeForWindow = (fnAllowDarkModeForWindow)(PVOID)GetProcAddress(hUxtheme, MAKEINTRESOURCEA(133));
-        = reinterpret_cast<fnSetPreferredAppMode>(GetProcAddress(hUxtheme, MAKEINTRESOURCEA(135)));
+    fnSetPreferredAppMode SetPreferredAppMode = (fnSetPreferredAppMode)(PVOID)GetProcAddress(hUxtheme, MAKEINTRESOURCEA(135));
-    fnSetWindowCompositionAttribute SetWindowCompositionAttribute
+    fnSetWindowCompositionAttribute SetWindowCompositionAttribute = (fnSetWindowCompositionAttribute)(PVOID)GetProcAddress(hUser32, "SetWindowCompositionAttribute");
-        = reinterpret_cast<fnSetWindowCompositionAttribute>(GetProcAddress(hUser32, "SetWindowCompositionAttribute"));
 
     SetPreferredAppMode(AllowDark);
     AllowDarkModeForWindow(hwnd, dark);

launcher/ui/pages/instance/WorldListPage.cpp

@@ -312,28 +312,22 @@ void WorldListPage::mceditError()
 
 void WorldListPage::mceditState(LoggedProcess::State state)
 {
-    bool failed = false;
     switch(state)
     {
         case LoggedProcess::NotRunning:
         case LoggedProcess::Starting:
             return;
+        case LoggedProcess::Running:
+        case LoggedProcess::Finished:
+            m_mceditStarting = false;
+            return;
         case LoggedProcess::FailedToStart:
         case LoggedProcess::Crashed:
         case LoggedProcess::Aborted:
-        {
-            failed = true;
-        }
-        case LoggedProcess::Running:
-        case LoggedProcess::Finished:
-        {
-            m_mceditStarting = false;
-            break;
-        }
-    }
-    if(failed)
-    {
             mceditError();
+            return;
+        default:
+            qWarning() << "Invalid MCEdit state";
     }
 }
 

launcher/ui/setupwizard/JavaWizardPage.cpp

@@ -69,6 +69,7 @@ bool JavaWizardPage::validatePage()
         case JavaSettingsWidget::ValidationStatus::AllOK:
         {
             settings->set("JavaPath", m_java_widget->javaPath());
+            [[fallthrough]];
         }
         case JavaSettingsWidget::ValidationStatus::JavaBad:
         {