From 9cb62000816520a32d7010d08cfdf7e76b531694 Mon Sep 17 00:00:00 2001 From: jdp_ <42700985+jdpatdiscord@users.noreply.github.com> Date: Sun, 7 May 2023 03:55:56 -0400 Subject: [PATCH] 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 | 21 +----------------- launcher/LaunchController.cpp | 13 ++++++----- launcher/VersionProxyModel.cpp | 2 ++ launcher/meta/Index.cpp | 13 ++++------- launcher/minecraft/auth/AccountData.h | 1 + launcher/minecraft/auth/AccountList.cpp | 11 ++++++++-- launcher/minecraft/auth/Yggdrasil.cpp | 1 + launcher/minecraft/mod/Mod.cpp | 2 ++ launcher/minecraft/mod/Resource.cpp | 2 ++ launcher/minecraft/mod/ResourcePack.cpp | 16 ++++++++++---- launcher/minecraft/services/SkinUpload.cpp | 5 +++-- launcher/modplatform/EnsureMetadataTask.cpp | 3 ++- .../flame/FlameInstanceCreationTask.cpp | 2 +- launcher/translations/TranslationsModel.cpp | 15 +++++-------- launcher/ui/WinDarkmode.cpp | 13 +++++------ launcher/ui/pages/instance/WorldListPage.cpp | 22 +++++++------------ launcher/ui/setupwizard/JavaWizardPage.cpp | 1 + 17 files changed, 67 insertions(+), 76 deletions(-) diff --git a/launcher/FileSystem.cpp b/launcher/FileSystem.cpp index 689d1ff6..4f787710 100644 --- a/launcher/FileSystem.cpp +++ b/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; diff --git a/launcher/LaunchController.cpp b/launcher/LaunchController.cpp index ba63160e..2c5d8567 100644 --- a/launcher/LaunchController.cpp +++ b/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.")); diff --git a/launcher/VersionProxyModel.cpp b/launcher/VersionProxyModel.cpp index 032f21f9..fa4ab5a4 100644 --- a/launcher/VersionProxyModel.cpp +++ b/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: { diff --git a/launcher/meta/Index.cpp b/launcher/meta/Index.cpp index 6802470d..c831a486 100644 --- a/launcher/meta/Index.cpp +++ b/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()) - { - case 0: return list->humanReadable(); - default: break; - } + if (index.column() == 0) + return list->humanReadable(); + 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(); - } + return QVariant(); } bool Index::hasUid(const QString &uid) const diff --git a/launcher/minecraft/auth/AccountData.h b/launcher/minecraft/auth/AccountData.h index 092e1691..366d7731 100644 --- a/launcher/minecraft/auth/AccountData.h +++ b/launcher/minecraft/auth/AccountData.h @@ -85,6 +85,7 @@ enum class AccountState { Disabled, Errored, Expired, + Queued, Gone }; diff --git a/launcher/minecraft/auth/AccountList.cpp b/launcher/minecraft/auth/AccountList.cpp index b3b57c74..a3036a0a 100644 --- a/launcher/minecraft/auth/AccountList.cpp +++ b/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(); } diff --git a/launcher/minecraft/auth/Yggdrasil.cpp b/launcher/minecraft/auth/Yggdrasil.cpp index 29978411..490de5d7 100644 --- a/launcher/minecraft/auth/Yggdrasil.cpp +++ b/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( diff --git a/launcher/minecraft/mod/Mod.cpp b/launcher/minecraft/mod/Mod.cpp index 39023f69..5d5f6696 100644 --- a/launcher/minecraft/mod/Mod.cpp +++ b/launcher/minecraft/mod/Mod.cpp @@ -82,6 +82,8 @@ std::pair 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()); diff --git a/launcher/minecraft/mod/Resource.cpp b/launcher/minecraft/mod/Resource.cpp index 0fbcfd7c..824f65a1 100644 --- a/launcher/minecraft/mod/Resource.cpp +++ b/launcher/minecraft/mod/Resource.cpp @@ -66,6 +66,7 @@ std::pair 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 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()) diff --git a/launcher/minecraft/mod/ResourcePack.cpp b/launcher/minecraft/mod/ResourcePack.cpp index 3fc10a2f..4ca99344 100644 --- a/launcher/minecraft/mod/ResourcePack.cpp +++ b/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> s_pack_format_versions = { - { 1, { Version("1.6.1"), Version("1.8.9") } }, { 2, { Version("1.9"), Version("1.10.2") } }, - { 3, { Version("1.11"), Version("1.12.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") } }, + { 1, { Version("1.6.1"), Version("1.8.9") } }, + { 2, { Version("1.9"), Version("1.10.2") } }, + { 3, { Version("1.11"), Version("1.12.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 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(); diff --git a/launcher/minecraft/services/SkinUpload.cpp b/launcher/minecraft/services/SkinUpload.cpp index c7987875..68cf2ba9 100644 --- a/launcher/minecraft/services/SkinUpload.cpp +++ b/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"; } } diff --git a/launcher/modplatform/EnsureMetadataTask.cpp b/launcher/modplatform/EnsureMetadataTask.cpp index 234330a7..d292067e 100644 --- a/launcher/modplatform/EnsureMetadataTask.cpp +++ b/launcher/modplatform/EnsureMetadataTask.cpp @@ -405,7 +405,8 @@ NetJob::Ptr EnsureMetadataTask::flameProjectsTask() QHash 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()) diff --git a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp b/launcher/modplatform/flame/FlameInstanceCreationTask.cpp index 98e3a42c..859acc40 100644 --- a/launcher/modplatform/flame/FlameInstanceCreationTask.cpp +++ b/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: { diff --git a/launcher/translations/TranslationsModel.cpp b/launcher/translations/TranslationsModel.cpp index 2f57de3a..b7f9defd 100644 --- a/launcher/translations/TranslationsModel.cpp +++ b/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(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(index.column()); switch(column) { - case Column::Language: - { + case Column::Language: return lang.languageName(); - } - case Column::Completeness: - { + 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: diff --git a/launcher/ui/WinDarkmode.cpp b/launcher/ui/WinDarkmode.cpp index eac68e4f..8f5553d9 100644 --- a/launcher/ui/WinDarkmode.cpp +++ b/launcher/ui/WinDarkmode.cpp @@ -8,16 +8,15 @@ namespace WinDarkmode { void setDarkWinTitlebar(WId winid, bool darkmode) { HWND hwnd = reinterpret_cast(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(GetProcAddress(hUxtheme, MAKEINTRESOURCEA(133))); - fnSetPreferredAppMode SetPreferredAppMode - = reinterpret_cast(GetProcAddress(hUxtheme, MAKEINTRESOURCEA(135))); - fnSetWindowCompositionAttribute SetWindowCompositionAttribute - = reinterpret_cast(GetProcAddress(hUser32, "SetWindowCompositionAttribute")); + + // 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. + fnAllowDarkModeForWindow AllowDarkModeForWindow = (fnAllowDarkModeForWindow)(PVOID)GetProcAddress(hUxtheme, MAKEINTRESOURCEA(133)); + fnSetPreferredAppMode SetPreferredAppMode = (fnSetPreferredAppMode)(PVOID)GetProcAddress(hUxtheme, MAKEINTRESOURCEA(135)); + fnSetWindowCompositionAttribute SetWindowCompositionAttribute = (fnSetWindowCompositionAttribute)(PVOID)GetProcAddress(hUser32, "SetWindowCompositionAttribute"); SetPreferredAppMode(AllowDark); AllowDarkModeForWindow(hwnd, dark); diff --git a/launcher/ui/pages/instance/WorldListPage.cpp b/launcher/ui/pages/instance/WorldListPage.cpp index 85cc01ff..b50da970 100644 --- a/launcher/ui/pages/instance/WorldListPage.cpp +++ b/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(); + mceditError(); + return; + default: + qWarning() << "Invalid MCEdit state"; } } diff --git a/launcher/ui/setupwizard/JavaWizardPage.cpp b/launcher/ui/setupwizard/JavaWizardPage.cpp index 14683778..b1851878 100644 --- a/launcher/ui/setupwizard/JavaWizardPage.cpp +++ b/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: {