From 62e4845dcab8f7a2b457aa1edd540a12d061c067 Mon Sep 17 00:00:00 2001 From: Naomi <103967@gmail.com> Date: Sun, 22 Jun 2025 15:17:28 +0200 Subject: [PATCH] Review feedback Signed-off-by: Naomi <103967@gmail.com> --- launcher/FileSystem.cpp | 5 --- launcher/FileSystem.h | 7 ---- launcher/minecraft/MinecraftInstance.cpp | 2 +- .../minecraft/UpdateSharedDirectoriesTask.cpp | 35 +++++++++---------- .../minecraft/UpdateSharedDirectoriesTask.h | 5 ++- launcher/ui/dialogs/FileConflictDialog.ui | 16 ++++----- .../pages/instance/ExternalResourcesPage.ui | 12 +++---- launcher/ui/pages/instance/ScreenshotsPage.ui | 6 ++-- launcher/ui/pages/instance/WorldListPage.ui | 6 ++-- .../ui/widgets/MinecraftSettingsWidget.ui | 4 +-- 10 files changed, 42 insertions(+), 56 deletions(-) diff --git a/launcher/FileSystem.cpp b/launcher/FileSystem.cpp index 9182282d9..97969e0db 100644 --- a/launcher/FileSystem.cpp +++ b/launcher/FileSystem.cpp @@ -277,11 +277,6 @@ bool ensureFolderPathExists(const QString folderPathName) return ensureFolderPathExists(QFileInfo(folderPathName)); } -bool checkFolderPathExists(const QString& folderPathName) -{ - return QDir(folderPathName).exists(); -} - bool checkFolderPathEmpty(const QString& folderPathName) { return QDir(folderPathName).isEmpty(QDir::Filters(QDir::AllEntries | QDir::NoDotAndDotDot | QDir::Hidden | QDir::System)); diff --git a/launcher/FileSystem.h b/launcher/FileSystem.h index 7771da8bb..455d81457 100644 --- a/launcher/FileSystem.h +++ b/launcher/FileSystem.h @@ -99,13 +99,6 @@ bool ensureFolderPathExists(const QFileInfo folderPath); */ bool ensureFolderPathExists(const QString folderPathName); -/** - * @brief Check if the given folder exists - * @param folderPathName The path to a folder to check - * @return True if the given folder exists - */ -bool checkFolderPathExists(const QString& folderPathName); - /** * @brief Check if the given folder is empty or doesn't exist * @param folderPathName The path to a folder to check diff --git a/launcher/minecraft/MinecraftInstance.cpp b/launcher/minecraft/MinecraftInstance.cpp index 9c9f66241..506ae0e36 100644 --- a/launcher/minecraft/MinecraftInstance.cpp +++ b/launcher/minecraft/MinecraftInstance.cpp @@ -1317,7 +1317,7 @@ QList MinecraftInstance::getJarMods() const void MinecraftInstance::applySettings() { // Shared directories - m_update_shared_directories_task = std::make_shared(this, nullptr); + m_update_shared_directories_task = std::make_shared(this); m_update_shared_directories_task->start(); } diff --git a/launcher/minecraft/UpdateSharedDirectoriesTask.cpp b/launcher/minecraft/UpdateSharedDirectoriesTask.cpp index 0b8e469a1..a72953b52 100644 --- a/launcher/minecraft/UpdateSharedDirectoriesTask.cpp +++ b/launcher/minecraft/UpdateSharedDirectoriesTask.cpp @@ -42,12 +42,15 @@ bool interactiveMove(const QString& source, const QString& destination, bool rec FileConflictDialog dialog(source, destination, true, parent); FileConflictDialog::Result result = dialog.execWithResult(); - if (result == FileConflictDialog::Cancel) + switch(result) { + case FileConflictDialog::Cancel: return false; - else if (result == FileConflictDialog::ChooseDestination) + case FileConflictDialog::ChooseDestination: return FS::deletePath(source); - else if (result == FileConflictDialog::ChooseSource) + case FileConflictDialog::ChooseSource: FS::deletePath(destination); + break; + } } return FS::move(source, destination); @@ -58,9 +61,8 @@ class TryCreateSymlinkTask : public Task { explicit TryCreateSymlinkTask(const QString& source, const QString& destination, MinecraftInstance* instance, - const QString& setting, - QWidget* parent) - : m_source(source), m_destination(destination), m_inst(instance), m_setting(setting), m_parent(parent) + const QString& setting) + : m_source(source), m_destination(destination), m_inst(instance), m_setting(setting) { setObjectName("TryCreateSymlinkTask"); } @@ -98,9 +100,9 @@ class TryCreateSymlinkTask : public Task { } FS::deletePath(m_destination); - } else if (FS::checkFolderPathExists(m_destination)) { + } else if (QFileInfo::exists(m_destination)) { if (!FS::checkFolderPathEmpty(m_destination)) { - if (!interactiveMove(m_destination, m_source, true, m_parent)) { + if (!interactiveMove(m_destination, m_source, true)) { fail(tr("Failed to create shared folder.\nEnsure that \"%1\" is empty.").arg(m_destination)); return; } @@ -140,36 +142,33 @@ class TryCreateSymlinkTask : public Task { QString m_destination; MinecraftInstance* m_inst; QString m_setting; - QWidget* m_parent; }; -UpdateSharedDirectoriesTask::UpdateSharedDirectoriesTask(MinecraftInstance* inst, QWidget* parent) - : Task(parent), m_inst(inst), m_parent(parent) +UpdateSharedDirectoriesTask::UpdateSharedDirectoriesTask(MinecraftInstance* inst) + : Task(), m_inst(inst) {} -UpdateSharedDirectoriesTask::~UpdateSharedDirectoriesTask() {} - void UpdateSharedDirectoriesTask::executeTask() { auto tasks = makeShared("UpdateSharedDirectoriesTask"); auto screenshotsTask = makeShared(m_inst->settings()->get("SharedScreenshotsPath").toString(), - m_inst->screenshotsDir(), m_inst, "UseSharedScreenshotsFolder", m_parent); + m_inst->screenshotsDir(), m_inst, "UseSharedScreenshotsFolder"); connect(screenshotsTask.get(), &Task::failed, this, &UpdateSharedDirectoriesTask::notifyFailed); tasks->addTask(screenshotsTask); auto savesTask = makeShared(m_inst->settings()->get("SharedSavesPath").toString(), m_inst->worldDir(), m_inst, - "UseSharedSavesFolder", m_parent); + "UseSharedSavesFolder"); connect(savesTask.get(), &Task::failed, this, &UpdateSharedDirectoriesTask::notifyFailed); tasks->addTask(savesTask); auto resoucePacksTask = makeShared(m_inst->settings()->get("SharedResourcePacksPath").toString(), - m_inst->resourcePacksDir(), m_inst, "UseSharedResourcePacksFolder", m_parent); + m_inst->resourcePacksDir(), m_inst, "UseSharedResourcePacksFolder"); connect(resoucePacksTask.get(), &Task::failed, this, &UpdateSharedDirectoriesTask::notifyFailed); tasks->addTask(resoucePacksTask); auto texturePacksTask = makeShared(m_inst->settings()->get("SharedResourcePacksPath").toString(), - m_inst->texturePacksDir(), m_inst, "UseSharedResourcePacksFolder", m_parent); + m_inst->texturePacksDir(), m_inst, "UseSharedResourcePacksFolder"); connect(texturePacksTask.get(), &Task::failed, this, &UpdateSharedDirectoriesTask::notifyFailed); tasks->addTask(texturePacksTask); @@ -182,6 +181,6 @@ void UpdateSharedDirectoriesTask::executeTask() void UpdateSharedDirectoriesTask::notifyFailed(QString reason) { - CustomMessageBox::selectable(m_parent, tr("Failed"), reason, QMessageBox::Warning, QMessageBox::Ok)->exec(); + CustomMessageBox::selectable(nullptr, tr("Failed"), reason, QMessageBox::Warning, QMessageBox::Ok)->exec(); emit failed(reason); } diff --git a/launcher/minecraft/UpdateSharedDirectoriesTask.h b/launcher/minecraft/UpdateSharedDirectoriesTask.h index dadab0d71..c1c1f852b 100644 --- a/launcher/minecraft/UpdateSharedDirectoriesTask.h +++ b/launcher/minecraft/UpdateSharedDirectoriesTask.h @@ -6,8 +6,8 @@ class MinecraftInstance; class UpdateSharedDirectoriesTask : public Task { public: - explicit UpdateSharedDirectoriesTask(MinecraftInstance* inst, QWidget* parent = 0); - virtual ~UpdateSharedDirectoriesTask(); + explicit UpdateSharedDirectoriesTask(MinecraftInstance* inst); + virtual ~UpdateSharedDirectoriesTask = default; protected: virtual void executeTask() override; @@ -17,6 +17,5 @@ class UpdateSharedDirectoriesTask : public Task { private: MinecraftInstance* m_inst; - QWidget* m_parent; Task::Ptr m_tasks; }; diff --git a/launcher/ui/dialogs/FileConflictDialog.ui b/launcher/ui/dialogs/FileConflictDialog.ui index c71afd59a..207c16852 100644 --- a/launcher/ui/dialogs/FileConflictDialog.ui +++ b/launcher/ui/dialogs/FileConflictDialog.ui @@ -26,14 +26,14 @@ Would you like to overwrite the destination? - Qt::AlignmentFlag::AlignCenter + Qt::AlignCenter - QLayout::SizeConstraint::SetDefaultConstraint + QLayout::SetDefaultConstraint @@ -49,7 +49,7 @@ <html><head/><body><p><span style=" font-weight:700;">Source</span></p></body></html> - Qt::AlignmentFlag::AlignHCenter|Qt::AlignmentFlag::AlignTop + Qt::AlignHCenter|Qt::AlignTop @@ -65,7 +65,7 @@ <html><head/><body><p><b>Name:</b></p><p>Size:</p><p>Date:</p></body></html> - Qt::AlignmentFlag::AlignLeading|Qt::AlignmentFlag::AlignLeft|Qt::AlignmentFlag::AlignTop + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop @@ -79,7 +79,7 @@ <html><head/><body><p><span style=" font-weight:700;">Destination</span></p></body></html> - Qt::AlignmentFlag::AlignHCenter|Qt::AlignmentFlag::AlignTop + Qt::AlignHCenter|Qt::AlignTop @@ -89,7 +89,7 @@ <html><head/><body><p>Name:</p><p>Size:</p><p>Date:</p></body></html> - Qt::AlignmentFlag::AlignLeading|Qt::AlignmentFlag::AlignLeft|Qt::AlignmentFlag::AlignTop + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop @@ -100,10 +100,10 @@ - Qt::Orientation::Horizontal + Qt::Horizontal - QDialogButtonBox::StandardButton::Cancel + QDialogButtonBox::Cancel diff --git a/launcher/ui/pages/instance/ExternalResourcesPage.ui b/launcher/ui/pages/instance/ExternalResourcesPage.ui index 934f00255..47b41320b 100644 --- a/launcher/ui/pages/instance/ExternalResourcesPage.ui +++ b/launcher/ui/pages/instance/ExternalResourcesPage.ui @@ -60,7 +60,7 @@ true - QAbstractItemView::DragDropMode::DropOnly + QAbstractItemView::DropOnly true @@ -235,11 +235,6 @@ - - WideBar - QToolBar -
ui/widgets/WideBar.h
- ModListView QTreeView @@ -251,6 +246,11 @@
ui/widgets/InfoFrame.h
1 + + WideBar + QToolBar +
ui/widgets/WideBar.h
+ treeView diff --git a/launcher/ui/pages/instance/ScreenshotsPage.ui b/launcher/ui/pages/instance/ScreenshotsPage.ui index beb2c6427..225ab379f 100644 --- a/launcher/ui/pages/instance/ScreenshotsPage.ui +++ b/launcher/ui/pages/instance/ScreenshotsPage.ui @@ -40,10 +40,10 @@ - QAbstractItemView::SelectionMode::ExtendedSelection + QAbstractItemView::ExtendedSelection - QAbstractItemView::SelectionBehavior::SelectRows + QAbstractItemView::SelectRows @@ -54,7 +54,7 @@ Actions - Qt::ToolButtonStyle::ToolButtonTextOnly + Qt::ToolButtonTextOnly RightToolBarArea diff --git a/launcher/ui/pages/instance/WorldListPage.ui b/launcher/ui/pages/instance/WorldListPage.ui index b89240bcb..f8ebae21b 100644 --- a/launcher/ui/pages/instance/WorldListPage.ui +++ b/launcher/ui/pages/instance/WorldListPage.ui @@ -43,7 +43,7 @@ true - QAbstractItemView::DragDropMode::DragDrop + QAbstractItemView::DragDrop true @@ -72,10 +72,10 @@ Actions - Qt::ToolBarArea::LeftToolBarArea|Qt::ToolBarArea::RightToolBarArea + Qt::LeftToolBarArea|Qt::RightToolBarArea - Qt::ToolButtonStyle::ToolButtonTextOnly + Qt::ToolButtonTextOnly false diff --git a/launcher/ui/widgets/MinecraftSettingsWidget.ui b/launcher/ui/widgets/MinecraftSettingsWidget.ui index 8358595e7..4fdb99f04 100644 --- a/launcher/ui/widgets/MinecraftSettingsWidget.ui +++ b/launcher/ui/widgets/MinecraftSettingsWidget.ui @@ -824,7 +824,7 @@ It is most likely you will need to change the path - please refer to the mod's w Settings - Qt::AlignmentFlag::AlignLeading|Qt::AlignmentFlag::AlignLeft|Qt::AlignmentFlag::AlignTop + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop @@ -861,7 +861,7 @@ It is most likely you will need to change the path - please refer to the mod's w false - Qt::AlignmentFlag::AlignLeading|Qt::AlignmentFlag::AlignLeft|Qt::AlignmentFlag::AlignTop + Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop true