From a166cf24812451f62680eac971b6f50173653084 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 13 May 2018 13:53:49 -0400 Subject: [PATCH 1/3] PatchEngine: Give Patch and PatchEntry default member initializers Avoids potentially using the values uninitialized. While we're at it, also drop the prefixed underscores from one of the constructors. --- Source/Core/Core/PatchEngine.h | 14 +++++++------- Source/Core/DolphinQt2/Config/NewPatchDialog.cpp | 6 +----- Source/Core/DolphinWX/PatchAddEdit.cpp | 4 ++-- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/Source/Core/Core/PatchEngine.h b/Source/Core/Core/PatchEngine.h index b1e7b2ba02..11889cf35d 100644 --- a/Source/Core/Core/PatchEngine.h +++ b/Source/Core/Core/PatchEngine.h @@ -24,19 +24,19 @@ extern const char* PatchTypeStrings[]; struct PatchEntry { - PatchEntry() {} - PatchEntry(PatchType _t, u32 _addr, u32 _value) : type(_t), address(_addr), value(_value) {} - PatchType type; - u32 address; - u32 value; + PatchEntry() = default; + PatchEntry(PatchType t, u32 addr, u32 value_) : type(t), address(addr), value(value_) {} + PatchType type = PatchType::PATCH_8BIT; + u32 address = 0; + u32 value = 0; }; struct Patch { std::string name; std::vector entries; - bool active; - bool user_defined; // False if this code is shipped with Dolphin. + bool active = false; + bool user_defined = false; // False if this code is shipped with Dolphin. }; int GetSpeedhackCycles(const u32 addr); diff --git a/Source/Core/DolphinQt2/Config/NewPatchDialog.cpp b/Source/Core/DolphinQt2/Config/NewPatchDialog.cpp index 3e871b842a..33fef87db5 100644 --- a/Source/Core/DolphinQt2/Config/NewPatchDialog.cpp +++ b/Source/Core/DolphinQt2/Config/NewPatchDialog.cpp @@ -79,11 +79,7 @@ void NewPatchDialog::ConnectWidgets() void NewPatchDialog::AddEntry() { - PatchEngine::PatchEntry entry; - entry.type = PatchEngine::PATCH_8BIT; - entry.address = entry.value = 0; - - m_patch.entries.push_back(entry); + m_patch.entries.emplace_back(); m_entry_layout->addWidget(CreateEntry(static_cast(m_patch.entries.size() - 1))); } diff --git a/Source/Core/DolphinWX/PatchAddEdit.cpp b/Source/Core/DolphinWX/PatchAddEdit.cpp index e58b90a913..78d22094f7 100644 --- a/Source/Core/DolphinWX/PatchAddEdit.cpp +++ b/Source/Core/DolphinWX/PatchAddEdit.cpp @@ -42,7 +42,7 @@ void CPatchAddEdit::CreateGUIControls(int _selection) if (_selection == -1) { tempEntries.clear(); - tempEntries.emplace_back(PatchEngine::PATCH_8BIT, 0x00000000, 0x00000000); + tempEntries.emplace_back(); } else { @@ -165,7 +165,7 @@ void CPatchAddEdit::AddEntry(wxCommandEvent& event) if (!UpdateTempEntryData(itCurEntry)) return; - PatchEngine::PatchEntry peEmptyEntry(PatchEngine::PATCH_8BIT, 0x00000000, 0x00000000); + PatchEngine::PatchEntry peEmptyEntry; ++itCurEntry; currentItem++; itCurEntry = tempEntries.insert(itCurEntry, peEmptyEntry); From 0995cfef6a8a59fd3e2c7c03297001e8a6c3fb8d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 13 May 2018 14:10:58 -0400 Subject: [PATCH 2/3] PatchEngine: Make PatchType an enum class Makes the enum strongly typed. A function for retrieving the string representation of the enum is also added, which allows hiding the array that contains all of the strings from view (i.e. we operate on the API, not the exposed internals). This also allows us to bounds check any querying for the strings. --- Source/Core/Core/PatchEngine.cpp | 27 ++++++++++++------- Source/Core/Core/PatchEngine.h | 20 +++++++------- .../Core/DolphinQt2/Config/NewPatchDialog.cpp | 12 ++++----- .../Core/DolphinQt2/Config/PatchesWidget.cpp | 2 +- .../DolphinWX/ISOProperties/ISOProperties.cpp | 7 ++--- Source/Core/DolphinWX/PatchAddEdit.cpp | 25 +++++++++-------- 6 files changed, 53 insertions(+), 40 deletions(-) diff --git a/Source/Core/Core/PatchEngine.cpp b/Source/Core/Core/PatchEngine.cpp index 8ded8c4c3c..a272bd0f0c 100644 --- a/Source/Core/Core/PatchEngine.cpp +++ b/Source/Core/Core/PatchEngine.cpp @@ -9,6 +9,8 @@ #include "Core/PatchEngine.h" #include +#include +#include #include #include #include @@ -26,15 +28,20 @@ namespace PatchEngine { -const char* PatchTypeStrings[] = { +constexpr std::array s_patch_type_strings{{ "byte", "word", "dword", -}; +}}; static std::vector onFrame; static std::map speedHacks; +const char* PatchTypeAsString(PatchType type) +{ + return s_patch_type_strings.at(static_cast(type)); +} + void LoadPatchSection(const std::string& section, std::vector& patches, IniFile& globalIni, IniFile& localIni) { @@ -97,8 +104,10 @@ void LoadPatchSection(const std::string& section, std::vector& patches, I success &= TryParse(items[0], &pE.address); success &= TryParse(items[2], &pE.value); - pE.type = PatchType(std::find(PatchTypeStrings, PatchTypeStrings + 3, items[1]) - - PatchTypeStrings); + const auto iter = + std::find(s_patch_type_strings.begin(), s_patch_type_strings.end(), items[1]); + pE.type = PatchType(std::distance(s_patch_type_strings.begin(), iter)); + success &= (pE.type != (PatchType)3); if (success) { @@ -173,13 +182,13 @@ static void ApplyPatches(const std::vector& patches) u32 value = entry.value; switch (entry.type) { - case PATCH_8BIT: - PowerPC::HostWrite_U8((u8)value, addr); + case PatchType::Patch8Bit: + PowerPC::HostWrite_U8(static_cast(value), addr); break; - case PATCH_16BIT: - PowerPC::HostWrite_U16((u16)value, addr); + case PatchType::Patch16Bit: + PowerPC::HostWrite_U16(static_cast(value), addr); break; - case PATCH_32BIT: + case PatchType::Patch32Bit: PowerPC::HostWrite_U32(value, addr); break; default: diff --git a/Source/Core/Core/PatchEngine.h b/Source/Core/Core/PatchEngine.h index 11889cf35d..7e3ae628db 100644 --- a/Source/Core/Core/PatchEngine.h +++ b/Source/Core/Core/PatchEngine.h @@ -13,20 +13,18 @@ class IniFile; namespace PatchEngine { -enum PatchType +enum class PatchType { - PATCH_8BIT, - PATCH_16BIT, - PATCH_32BIT, + Patch8Bit, + Patch16Bit, + Patch32Bit, }; -extern const char* PatchTypeStrings[]; - struct PatchEntry { PatchEntry() = default; PatchEntry(PatchType t, u32 addr, u32 value_) : type(t), address(addr), value(value_) {} - PatchType type = PatchType::PATCH_8BIT; + PatchType type = PatchType::Patch8Bit; u32 address = 0; u32 value = 0; }; @@ -39,6 +37,8 @@ struct Patch bool user_defined = false; // False if this code is shipped with Dolphin. }; +const char* PatchTypeAsString(PatchType type); + int GetSpeedhackCycles(const u32 addr); void LoadPatchSection(const std::string& section, std::vector& patches, IniFile& globalIni, IniFile& localIni); @@ -52,15 +52,15 @@ inline int GetPatchTypeCharLength(PatchType type) int size = 8; switch (type) { - case PatchEngine::PATCH_8BIT: + case PatchType::Patch8Bit: size = 2; break; - case PatchEngine::PATCH_16BIT: + case PatchType::Patch16Bit: size = 4; break; - case PatchEngine::PATCH_32BIT: + case PatchType::Patch32Bit: size = 8; break; } diff --git a/Source/Core/DolphinQt2/Config/NewPatchDialog.cpp b/Source/Core/DolphinQt2/Config/NewPatchDialog.cpp index 33fef87db5..0dfc94a474 100644 --- a/Source/Core/DolphinQt2/Config/NewPatchDialog.cpp +++ b/Source/Core/DolphinQt2/Config/NewPatchDialog.cpp @@ -161,24 +161,24 @@ QGroupBox* NewPatchDialog::CreateEntry(int index) connect(byte, &QRadioButton::toggled, [this, index](bool checked) { if (checked) - m_patch.entries[index].type = PatchEngine::PATCH_8BIT; + m_patch.entries[index].type = PatchEngine::PatchType::Patch8Bit; }); connect(word, &QRadioButton::toggled, [this, index](bool checked) { if (checked) - m_patch.entries[index].type = PatchEngine::PATCH_16BIT; + m_patch.entries[index].type = PatchEngine::PatchType::Patch16Bit; }); connect(dword, &QRadioButton::toggled, [this, index](bool checked) { if (checked) - m_patch.entries[index].type = PatchEngine::PATCH_32BIT; + m_patch.entries[index].type = PatchEngine::PatchType::Patch32Bit; }); auto entry_type = m_patch.entries[index].type; - byte->setChecked(entry_type == PatchEngine::PATCH_8BIT); - word->setChecked(entry_type == PatchEngine::PATCH_16BIT); - dword->setChecked(entry_type == PatchEngine::PATCH_32BIT); + byte->setChecked(entry_type == PatchEngine::PatchType::Patch8Bit); + word->setChecked(entry_type == PatchEngine::PatchType::Patch16Bit); + dword->setChecked(entry_type == PatchEngine::PatchType::Patch32Bit); offset->setText( QStringLiteral("%1").arg(m_patch.entries[index].address, 10, 16, QLatin1Char('0'))); diff --git a/Source/Core/DolphinQt2/Config/PatchesWidget.cpp b/Source/Core/DolphinQt2/Config/PatchesWidget.cpp index b7c8a4218a..6b063d0136 100644 --- a/Source/Core/DolphinQt2/Config/PatchesWidget.cpp +++ b/Source/Core/DolphinQt2/Config/PatchesWidget.cpp @@ -138,7 +138,7 @@ void PatchesWidget::SavePatches() for (const auto& entry : patch.entries) { lines.push_back(StringFromFormat("0x%08X:%s:0x%08X", entry.address, - PatchEngine::PatchTypeStrings[entry.type], entry.value)); + PatchEngine::PatchTypeAsString(entry.type), entry.value)); } } diff --git a/Source/Core/DolphinWX/ISOProperties/ISOProperties.cpp b/Source/Core/DolphinWX/ISOProperties/ISOProperties.cpp index 9de36914ad..2c63253a43 100644 --- a/Source/Core/DolphinWX/ISOProperties/ISOProperties.cpp +++ b/Source/Core/DolphinWX/ISOProperties/ISOProperties.cpp @@ -755,9 +755,10 @@ void CISOProperties::PatchList_Save() lines.push_back("$" + p.name); for (const PatchEngine::PatchEntry& entry : p.entries) { - std::string temp = StringFromFormat("0x%08X:%s:0x%08X", entry.address, - PatchEngine::PatchTypeStrings[entry.type], entry.value); - lines.push_back(temp); + std::string temp = + StringFromFormat("0x%08X:%s:0x%08X", entry.address, + PatchEngine::PatchTypeAsString(entry.type), entry.value); + lines.push_back(std::move(temp)); } } ++index; diff --git a/Source/Core/DolphinWX/PatchAddEdit.cpp b/Source/Core/DolphinWX/PatchAddEdit.cpp index 78d22094f7..1ad155b2bc 100644 --- a/Source/Core/DolphinWX/PatchAddEdit.cpp +++ b/Source/Core/DolphinWX/PatchAddEdit.cpp @@ -68,11 +68,14 @@ void CPatchAddEdit::CreateGUIControls(int _selection) EntrySelection->SetRange(0, (int)tempEntries.size() - 1); EntrySelection->SetValue((int)tempEntries.size() - 1); - wxArrayString wxArrayStringFor_EditPatchType; + wxArrayString patch_types; for (int i = 0; i < 3; ++i) - wxArrayStringFor_EditPatchType.Add(StrToWxStr(PatchEngine::PatchTypeStrings[i])); - EditPatchType = new wxRadioBox(this, wxID_ANY, _("Type"), wxDefaultPosition, wxDefaultSize, - wxArrayStringFor_EditPatchType, 3, wxRA_SPECIFY_COLS); + { + patch_types.Add( + StrToWxStr(PatchEngine::PatchTypeAsString(static_cast(i)))); + } + EditPatchType = + new wxRadioBox(this, wxID_ANY, _("Type"), wxDefaultPosition, wxDefaultSize, patch_types); EditPatchType->SetSelection((int)tempEntries.at(0).type); wxStaticText* EditPatchValueText = new wxStaticText(this, wxID_ANY, _("Value:")); @@ -206,7 +209,7 @@ void CPatchAddEdit::UpdateEntryCtrls(PatchEngine::PatchEntry pE) sbEntry->GetStaticBox()->SetLabel( wxString::Format(_("Entry %d/%d"), currentItem, (int)tempEntries.size())); EditPatchOffset->SetValue(wxString::Format("%08X", pE.address)); - EditPatchType->SetSelection(pE.type); + EditPatchType->SetSelection(static_cast(pE.type)); EditPatchValue->SetValue( wxString::Format("%0*X", PatchEngine::GetPatchTypeCharLength(pE.type), pE.value)); } @@ -217,19 +220,19 @@ bool CPatchAddEdit::UpdateTempEntryData(std::vector::it bool parsed_ok = true; if (EditPatchOffset->GetValue().ToULong(&value, 16)) - (*iterEntry).address = value; + iterEntry->address = value; else parsed_ok = false; - PatchEngine::PatchType tempType = (*iterEntry).type = - (PatchEngine::PatchType)EditPatchType->GetSelection(); + const auto tempType = iterEntry->type = + static_cast(EditPatchType->GetSelection()); if (EditPatchValue->GetValue().ToULong(&value, 16)) { - (*iterEntry).value = value; - if (tempType == PatchEngine::PATCH_8BIT && value > 0xff) + iterEntry->value = value; + if (tempType == PatchEngine::PatchType::Patch8Bit && value > 0xff) parsed_ok = false; - else if (tempType == PatchEngine::PATCH_16BIT && value > 0xffff) + else if (tempType == PatchEngine::PatchType::Patch16Bit && value > 0xffff) parsed_ok = false; } else From 5fd8cec7ea961559a4e5c6b66420a55144c60a76 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 13 May 2018 15:17:38 -0400 Subject: [PATCH 3/3] PatchEngine: Add s_ prefix to file-scope variables Brings the translation unit in line with the convention used elsewhere in the codebase. --- Source/Core/Core/PatchEngine.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Source/Core/Core/PatchEngine.cpp b/Source/Core/Core/PatchEngine.cpp index a272bd0f0c..f1ab5432bd 100644 --- a/Source/Core/Core/PatchEngine.cpp +++ b/Source/Core/Core/PatchEngine.cpp @@ -34,8 +34,8 @@ constexpr std::array s_patch_type_strings{{ "dword", }}; -static std::vector onFrame; -static std::map speedHacks; +static std::vector s_on_frame; +static std::map s_speed_hacks; const char* PatchTypeAsString(PatchType type) { @@ -141,7 +141,7 @@ static void LoadSpeedhacks(const std::string& section, IniFile& ini) success &= TryParse(value, &cycles); if (success) { - speedHacks[address] = (int)cycles; + s_speed_hacks[address] = static_cast(cycles); } } } @@ -149,11 +149,11 @@ static void LoadSpeedhacks(const std::string& section, IniFile& ini) int GetSpeedhackCycles(const u32 addr) { - std::map::const_iterator iter = speedHacks.find(addr); - if (iter == speedHacks.end()) + const auto iter = s_speed_hacks.find(addr); + if (iter == s_speed_hacks.end()) return 0; - else - return iter->second; + + return iter->second; } void LoadPatches() @@ -162,7 +162,7 @@ void LoadPatches() IniFile globalIni = SConfig::GetInstance().LoadDefaultGameIni(); IniFile localIni = SConfig::GetInstance().LoadLocalGameIni(); - LoadPatchSection("OnFrame", onFrame, globalIni, localIni); + LoadPatchSection("OnFrame", s_on_frame, globalIni, localIni); ActionReplay::LoadAndApplyCodes(globalIni, localIni); Gecko::SetActiveCodes(Gecko::LoadCodes(globalIni, localIni)); @@ -238,7 +238,7 @@ bool ApplyFramePatches() return false; } - ApplyPatches(onFrame); + ApplyPatches(s_on_frame); // Run the Gecko code handler Gecko::RunCodeHandler(); @@ -249,8 +249,8 @@ bool ApplyFramePatches() void Shutdown() { - onFrame.clear(); - speedHacks.clear(); + s_on_frame.clear(); + s_speed_hacks.clear(); ActionReplay::ApplyCodes({}); Gecko::Shutdown(); }