From 236764adf6cb985dfc6d00b9cbcba8eb176510ed Mon Sep 17 00:00:00 2001 From: Rachel Powers <508861+Ryex@users.noreply.github.com> Date: Fri, 7 Apr 2023 19:44:57 -0700 Subject: [PATCH] refactor: Qt can handle const& in signals and slots While most Qt types cna use implicit data sharing pasing our own structs means copies. const& ensure it's only copied as needed by Qt. Signed-off-by: Rachel Powers <508861+Ryex@users.noreply.github.com> --- launcher/tasks/ConcurrentTask.cpp | 10 ++++++---- launcher/tasks/ConcurrentTask.h | 2 +- launcher/tasks/Task.cpp | 2 +- launcher/tasks/Task.h | 6 +++--- launcher/ui/dialogs/ProgressDialog.cpp | 8 ++++---- launcher/ui/dialogs/ProgressDialog.h | 4 ++-- 6 files changed, 17 insertions(+), 15 deletions(-) diff --git a/launcher/tasks/ConcurrentTask.cpp b/launcher/tasks/ConcurrentTask.cpp index 439879e6..37435739 100644 --- a/launcher/tasks/ConcurrentTask.cpp +++ b/launcher/tasks/ConcurrentTask.cpp @@ -133,7 +133,7 @@ void ConcurrentTask::startNext() connect(next.get(), &Task::status, this, [this, next](QString msg) { subTaskStatus(next, msg); }); connect(next.get(), &Task::details, this, [this, next](QString msg) { subTaskDetails(next, msg); }); - connect(next.get(), &Task::stepProgress, this, [this, next](TaskStepProgress tp) { subTaskStepProgress(next, tp); }); + connect(next.get(), &Task::stepProgress, this, [this, next](TaskStepProgress const& tp) { subTaskStepProgress(next, tp); }); connect(next.get(), &Task::progress, this, [this, next](qint64 current, qint64 total) { subTaskProgress(next, current, total); }); @@ -224,13 +224,15 @@ void ConcurrentTask::subTaskProgress(Task::Ptr task, qint64 current, qint64 tota updateState(); } -void ConcurrentTask::subTaskStepProgress(Task::Ptr task, TaskStepProgress task_progress) +void ConcurrentTask::subTaskStepProgress(Task::Ptr task, TaskStepProgress const& task_progress) { Operation op = Operation::ADDED; if (!m_task_progress.contains(task_progress.uid)) { m_task_progress.insert(task_progress.uid, std::make_shared(task_progress)); op = Operation::ADDED; + emit stepProgress(task_progress); + updateStepProgress(task_progress, op); } else { auto tp = m_task_progress.value(task_progress.uid); @@ -243,10 +245,10 @@ void ConcurrentTask::subTaskStepProgress(Task::Ptr task, TaskStepProgress task_p tp->details = task_progress.details; op = Operation::CHANGED; + emit stepProgress(*tp.get()); + updateStepProgress(*tp.get(), op); } - emit stepProgress(task_progress); - updateStepProgress(task_progress, op); } void ConcurrentTask::updateStepProgress(TaskStepProgress const& changed_progress, Operation op) diff --git a/launcher/tasks/ConcurrentTask.h b/launcher/tasks/ConcurrentTask.h index 284fa345..aec707bc 100644 --- a/launcher/tasks/ConcurrentTask.h +++ b/launcher/tasks/ConcurrentTask.h @@ -77,7 +77,7 @@ slots: void subTaskStatus(Task::Ptr task, const QString &msg); void subTaskDetails(Task::Ptr task, const QString &msg); void subTaskProgress(Task::Ptr task, qint64 current, qint64 total); - void subTaskStepProgress(Task::Ptr task, TaskStepProgress task_step_progress); + void subTaskStepProgress(Task::Ptr task, TaskStepProgress const& task_step_progress); protected: // NOTE: This is not thread-safe. diff --git a/launcher/tasks/Task.cpp b/launcher/tasks/Task.cpp index fd1b8898..b0addd46 100644 --- a/launcher/tasks/Task.cpp +++ b/launcher/tasks/Task.cpp @@ -161,7 +161,7 @@ void Task::emitSucceeded() emit finished(); } -void Task::propogateStepProgress(TaskStepProgress task_progress) +void Task::propogateStepProgress(TaskStepProgress const& task_progress) { emit stepProgress(task_progress); } diff --git a/launcher/tasks/Task.h b/launcher/tasks/Task.h index d71eaf6b..04a09187 100644 --- a/launcher/tasks/Task.h +++ b/launcher/tasks/Task.h @@ -64,7 +64,7 @@ struct TaskStepProgress { QString status = ""; QString details = ""; TaskStepState state = TaskStepState::Waiting; - bool isDone() { return (state == TaskStepState::Failed) || (state == TaskStepState::Succeeded); } + bool isDone() const { return (state == TaskStepState::Failed) || (state == TaskStepState::Succeeded); } }; Q_DECLARE_METATYPE(TaskStepProgress) @@ -130,7 +130,7 @@ class Task : public QObject, public QRunnable { void failed(QString reason); void status(QString status); void details(QString details); - void stepProgress(TaskStepProgress task_progress); // + void stepProgress(TaskStepProgress const& task_progress); // /** Emitted when the canAbort() status has changed. */ @@ -153,7 +153,7 @@ class Task : public QObject, public QRunnable { virtual void emitAborted(); virtual void emitFailed(QString reason = ""); - virtual void propogateStepProgress(TaskStepProgress task_progress); + virtual void propogateStepProgress(TaskStepProgress const& task_progress); public slots: void setStatus(const QString& status); diff --git a/launcher/ui/dialogs/ProgressDialog.cpp b/launcher/ui/dialogs/ProgressDialog.cpp index 7594484e..94feee44 100644 --- a/launcher/ui/dialogs/ProgressDialog.cpp +++ b/launcher/ui/dialogs/ProgressDialog.cpp @@ -206,14 +206,14 @@ void ProgressDialog::changeStatus(const QString& status) updateSize(); } -void ProgressDialog::addTaskProgress(TaskStepProgress* progress) +void ProgressDialog::addTaskProgress(TaskStepProgress const& progress) { SubTaskProgressBar* task_bar = new SubTaskProgressBar(this); - taskProgress.insert(progress->uid, task_bar); + taskProgress.insert(progress.uid, task_bar); ui->taskProgressLayout->addWidget(task_bar); } -void ProgressDialog::changeStepProgress(TaskStepProgress task_progress) +void ProgressDialog::changeStepProgress(TaskStepProgress const& task_progress) { m_is_multi_step = true; if(ui->taskProgressScrollArea->isHidden()) { @@ -222,7 +222,7 @@ void ProgressDialog::changeStepProgress(TaskStepProgress task_progress) } if (!taskProgress.contains(task_progress.uid)) - addTaskProgress(&task_progress); + addTaskProgress(task_progress); auto task_bar = taskProgress.value(task_progress.uid); diff --git a/launcher/ui/dialogs/ProgressDialog.h b/launcher/ui/dialogs/ProgressDialog.h index 6779b949..fc9a0fbc 100644 --- a/launcher/ui/dialogs/ProgressDialog.h +++ b/launcher/ui/dialogs/ProgressDialog.h @@ -80,7 +80,7 @@ slots: void changeStatus(const QString &status); void changeProgress(qint64 current, qint64 total); - void changeStepProgress(TaskStepProgress task_progress); + void changeStepProgress(TaskStepProgress const& task_progress); private @@ -93,7 +93,7 @@ protected: private: bool handleImmediateResult(QDialog::DialogCode &result); - void addTaskProgress(TaskStepProgress* progress); + void addTaskProgress(TaskStepProgress const& progress); private: Ui::ProgressDialog *ui;