From 0d406f2bdcf2c403460dd78d9d31164d838af843 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 31 Jul 2019 08:35:57 -0400 Subject: [PATCH 1/8] DolphinQt/Config/ARCodeWidget: Deduplicate ini path We can just store this to a const local and use it to avoid doing the same work twice. --- Source/Core/DolphinQt/Config/ARCodeWidget.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Source/Core/DolphinQt/Config/ARCodeWidget.cpp b/Source/Core/DolphinQt/Config/ARCodeWidget.cpp index eecefd0dee..2c045d4fa9 100644 --- a/Source/Core/DolphinQt/Config/ARCodeWidget.cpp +++ b/Source/Core/DolphinQt/Config/ARCodeWidget.cpp @@ -168,11 +168,13 @@ void ARCodeWidget::UpdateList() void ARCodeWidget::SaveCodes() { - IniFile game_ini_local; - game_ini_local.Load(File::GetUserPath(D_GAMESETTINGS_IDX) + m_game_id + ".ini"); - ActionReplay::SaveCodes(&game_ini_local, m_ar_codes); + const auto ini_path = + std::string(File::GetUserPath(D_GAMESETTINGS_IDX)).append(m_game_id).append(".ini"); - game_ini_local.Save(File::GetUserPath(D_GAMESETTINGS_IDX) + m_game_id + ".ini"); + IniFile game_ini_local; + game_ini_local.Load(ini_path); + ActionReplay::SaveCodes(&game_ini_local, m_ar_codes); + game_ini_local.Save(ini_path); } void ARCodeWidget::AddCode(ActionReplay::ARCode code) From 4d8d2acae70e58fe4b0b86aa59f2db7af0ef5f87 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 31 Jul 2019 09:01:35 -0400 Subject: [PATCH 2/8] DolphinQt/Config/ARCodeWidget: Avoid unnecessary disk operations If a user indicates that they want to clone and edit an AR code, then click cancel on the following dialog, we shouldn't actually clone the code. We also shouldn't resave the codes if the edit dialog is opened and then closed again via cancel, as there's nothing that actually changed. This way we don't perform disk accesses unless they're actually necessary. --- Source/Core/DolphinQt/Config/ARCodeWidget.cpp | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/Source/Core/DolphinQt/Config/ARCodeWidget.cpp b/Source/Core/DolphinQt/Config/ARCodeWidget.cpp index 2c045d4fa9..4609209bc5 100644 --- a/Source/Core/DolphinQt/Config/ARCodeWidget.cpp +++ b/Source/Core/DolphinQt/Config/ARCodeWidget.cpp @@ -191,40 +191,43 @@ void ARCodeWidget::OnCodeAddClicked() ar.active = true; CheatCodeEditor ed(this); - ed.SetARCode(&ar); + if (ed.exec() == QDialog::Rejected) + return; - if (ed.exec()) - { - m_ar_codes.push_back(std::move(ar)); + m_ar_codes.push_back(std::move(ar)); - UpdateList(); - SaveCodes(); - } + UpdateList(); + SaveCodes(); } void ARCodeWidget::OnCodeEditClicked() { - auto items = m_code_list->selectedItems(); - + const auto items = m_code_list->selectedItems(); if (items.empty()) return; - const auto* selected = items[0]; - + const auto* const selected = items[0]; auto& current_ar = m_ar_codes[m_code_list->row(selected)]; - bool user_defined = current_ar.user_defined; - - ActionReplay::ARCode ar = current_ar; - CheatCodeEditor ed(this); + if (current_ar.user_defined) + { + ed.SetARCode(¤t_ar); - ed.SetARCode(user_defined ? ¤t_ar : &ar); - ed.exec(); + if (ed.exec() == QDialog::Rejected) + return; + } + else + { + ActionReplay::ARCode ar = current_ar; + ed.SetARCode(&ar); - if (!user_defined) - m_ar_codes.push_back(ar); + if (ed.exec() == QDialog::Rejected) + return; + + m_ar_codes.push_back(std::move(ar)); + } SaveCodes(); UpdateList(); From e08a76f9e2961d45de97838e220e3497ee67094d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 31 Jul 2019 09:08:29 -0400 Subject: [PATCH 3/8] DolphinQt/Config/ARCodeWidget: Call LoadDefaultGameIni() directly This is a static function, so we don't need to go through the instance of SConfig in order to call it. --- Source/Core/DolphinQt/Config/ARCodeWidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/Config/ARCodeWidget.cpp b/Source/Core/DolphinQt/Config/ARCodeWidget.cpp index 4609209bc5..6fbfcb4906 100644 --- a/Source/Core/DolphinQt/Config/ARCodeWidget.cpp +++ b/Source/Core/DolphinQt/Config/ARCodeWidget.cpp @@ -36,7 +36,7 @@ ARCodeWidget::ARCodeWidget(const UICommon::GameFile& game, bool restart_required // will always be stored in GS/${GAMEID}.ini game_ini_local.Load(File::GetUserPath(D_GAMESETTINGS_IDX) + m_game_id + ".ini"); - IniFile game_ini_default = SConfig::GetInstance().LoadDefaultGameIni(m_game_id, m_game_revision); + const IniFile game_ini_default = SConfig::LoadDefaultGameIni(m_game_id, m_game_revision); m_ar_codes = ActionReplay::LoadCodes(game_ini_default, game_ini_local); UpdateList(); From a07d19a2fd1e6a06db56649f4ae6233a358e80dd Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 31 Jul 2019 09:44:16 -0400 Subject: [PATCH 4/8] DolphinQt/Config/ARCodeWidget: Use forward declarations where applicable Avoids propagating headers into scopes where they're not necessary. Also uncovered reliance on an indirect inclusion within CheatsManager.cpp, which is now fixed. --- Source/Core/DolphinQt/CheatsManager.cpp | 1 + Source/Core/DolphinQt/Config/ARCodeWidget.cpp | 2 ++ Source/Core/DolphinQt/Config/ARCodeWidget.h | 7 ++++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/CheatsManager.cpp b/Source/Core/DolphinQt/CheatsManager.cpp index fda9adb909..11daa9d97a 100644 --- a/Source/Core/DolphinQt/CheatsManager.cpp +++ b/Source/Core/DolphinQt/CheatsManager.cpp @@ -21,6 +21,7 @@ #include #include +#include "Core/ActionReplay.h" #include "Core/ConfigManager.h" #include "Core/Core.h" #include "Core/Debugger/PPCDebugInterface.h" diff --git a/Source/Core/DolphinQt/Config/ARCodeWidget.cpp b/Source/Core/DolphinQt/Config/ARCodeWidget.cpp index 6fbfcb4906..7120eaab8a 100644 --- a/Source/Core/DolphinQt/Config/ARCodeWidget.cpp +++ b/Source/Core/DolphinQt/Config/ARCodeWidget.cpp @@ -43,6 +43,8 @@ ARCodeWidget::ARCodeWidget(const UICommon::GameFile& game, bool restart_required OnSelectionChanged(); } +ARCodeWidget::~ARCodeWidget() = default; + void ARCodeWidget::CreateWidgets() { m_warning = new CheatWarningWidget(m_game_id, m_restart_required, this); diff --git a/Source/Core/DolphinQt/Config/ARCodeWidget.h b/Source/Core/DolphinQt/Config/ARCodeWidget.h index a95044645d..48af85ec22 100644 --- a/Source/Core/DolphinQt/Config/ARCodeWidget.h +++ b/Source/Core/DolphinQt/Config/ARCodeWidget.h @@ -10,7 +10,11 @@ #include #include "Common/CommonTypes.h" -#include "Core/ActionReplay.h" + +namespace ActionReplay +{ +struct ARCode; +} namespace UICommon { @@ -28,6 +32,7 @@ class ARCodeWidget : public QWidget Q_OBJECT public: explicit ARCodeWidget(const UICommon::GameFile& game, bool restart_required = true); + ~ARCodeWidget() override; void AddCode(ActionReplay::ARCode code); From 255d2ff2d296307c562057c096a0d77c9c5e06b5 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 31 Jul 2019 09:11:44 -0400 Subject: [PATCH 5/8] DolphinQt/Config/GeckoCodeWidget: Deduplicate ini path We can store this to a local variable to avoid duplicating the same string creation twice. --- Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp b/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp index dda5915fc5..da015cf57b 100644 --- a/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp +++ b/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp @@ -229,12 +229,13 @@ void GeckoCodeWidget::RemoveCode() void GeckoCodeWidget::SaveCodes() { + const auto ini_path = + std::string(File::GetUserPath(D_GAMESETTINGS_IDX)).append(m_game_id).append(".ini"); + IniFile game_ini_local; - game_ini_local.Load(File::GetUserPath(D_GAMESETTINGS_IDX) + m_game_id + ".ini"); - + game_ini_local.Load(ini_path); Gecko::SaveCodes(game_ini_local, m_gecko_codes); - - game_ini_local.Save(File::GetUserPath(D_GAMESETTINGS_IDX) + m_game_id + ".ini"); + game_ini_local.Save(ini_path); } void GeckoCodeWidget::OnContextMenuRequested() From 6002529ece9aa9629169c56d30c3ba39ff98924a Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 31 Jul 2019 09:16:01 -0400 Subject: [PATCH 6/8] DolphinQt/Config/GeckoCodeWidget: Make exec() outcomes explicit Makes it a little more explicit which dialog outcomes we're expecting. While we're at it, we can invert them into guard clauses to unindent code a little bit. --- .../Core/DolphinQt/Config/GeckoCodeWidget.cpp | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp b/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp index da015cf57b..6ed31fe0fd 100644 --- a/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp +++ b/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp @@ -185,33 +185,29 @@ void GeckoCodeWidget::AddCode() CheatCodeEditor ed(this); ed.SetGeckoCode(&code); + if (ed.exec() == QDialog::Rejected) + return; - if (ed.exec()) - { - m_gecko_codes.push_back(std::move(code)); - SaveCodes(); - UpdateList(); - } + m_gecko_codes.push_back(std::move(code)); + SaveCodes(); + UpdateList(); } void GeckoCodeWidget::EditCode() { const auto* item = m_code_list->currentItem(); - if (item == nullptr) return; const int index = item->data(Qt::UserRole).toInt(); CheatCodeEditor ed(this); - ed.SetGeckoCode(&m_gecko_codes[index]); + if (ed.exec() == QDialog::Rejected) + return; - if (ed.exec()) - { - SaveCodes(); - UpdateList(); - } + SaveCodes(); + UpdateList(); } void GeckoCodeWidget::RemoveCode() From 14263ec6dd209dc2ca7be5c1d69b7882c27780e0 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 31 Jul 2019 09:18:37 -0400 Subject: [PATCH 7/8] DolphinQt/Config/GeckoCodeWidget: Call LoadDefaultGameIni() directly This is a static class function, so we don't need to go through the SConfig instance in order to call it. --- Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp b/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp index 6ed31fe0fd..12bf0dd7c9 100644 --- a/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp +++ b/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp @@ -40,7 +40,7 @@ GeckoCodeWidget::GeckoCodeWidget(const UICommon::GameFile& game, bool restart_re // will always be stored in GS/${GAMEID}.ini game_ini_local.Load(File::GetUserPath(D_GAMESETTINGS_IDX) + m_game_id + ".ini"); - IniFile game_ini_default = SConfig::GetInstance().LoadDefaultGameIni(m_game_id, m_game_revision); + const IniFile game_ini_default = SConfig::LoadDefaultGameIni(m_game_id, m_game_revision); m_gecko_codes = Gecko::LoadCodes(game_ini_default, game_ini_local); UpdateList(); From ff8f627499ca3eb8ac5f07cf5ced97f0d8b57ed1 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 31 Jul 2019 09:42:30 -0400 Subject: [PATCH 8/8] DolphinQt/Config/GeckoCodeWidget: Use forward declarations where applicable Avoids propagating headers into scopes where they're not needed. --- Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp | 3 +++ Source/Core/DolphinQt/Config/GeckoCodeWidget.h | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp b/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp index 12bf0dd7c9..4d8d5935b9 100644 --- a/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp +++ b/Source/Core/DolphinQt/Config/GeckoCodeWidget.cpp @@ -19,6 +19,7 @@ #include "Common/IniFile.h" #include "Core/ConfigManager.h" +#include "Core/GeckoCode.h" #include "Core/GeckoCodeConfig.h" #include "DolphinQt/Config/CheatCodeEditor.h" @@ -46,6 +47,8 @@ GeckoCodeWidget::GeckoCodeWidget(const UICommon::GameFile& game, bool restart_re UpdateList(); } +GeckoCodeWidget::~GeckoCodeWidget() = default; + void GeckoCodeWidget::CreateWidgets() { m_warning = new CheatWarningWidget(m_game_id, m_restart_required, this); diff --git a/Source/Core/DolphinQt/Config/GeckoCodeWidget.h b/Source/Core/DolphinQt/Config/GeckoCodeWidget.h index 6a8de9e12d..28fa4aeb44 100644 --- a/Source/Core/DolphinQt/Config/GeckoCodeWidget.h +++ b/Source/Core/DolphinQt/Config/GeckoCodeWidget.h @@ -10,7 +10,6 @@ #include #include "Common/CommonTypes.h" -#include "Core/GeckoCode.h" class CheatWarningWidget; class QLabel; @@ -19,6 +18,11 @@ class QListWidgetItem; class QTextEdit; class QPushButton; +namespace Gecko +{ +class GeckoCode; +} + namespace UICommon { class GameFile; @@ -29,6 +33,7 @@ class GeckoCodeWidget : public QWidget Q_OBJECT public: explicit GeckoCodeWidget(const UICommon::GameFile& game, bool restart_required = true); + ~GeckoCodeWidget() override; signals: void OpenGeneralSettings();