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>
PrismLauncher · Apr 8, 2023 · 79afdcf · 79afdcf
1 parent 6308234
commit 79afdcf
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 15 deletions.
diff --git 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<TaskStepProgress>(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
@@ -77,7 +77,7 @@ protected
     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
@@ -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
@@ -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
@@ -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
@@ -80,7 +80,7 @@ public
 
     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 @@ private
 
 private:
     bool handleImmediateResult(QDialog::DialogCode &result);
-    void addTaskProgress(TaskStepProgress* progress);
+    void addTaskProgress(TaskStepProgress const& progress);
 
 private:
     Ui::ProgressDialog *ui;