From efb7b1cecae4d865666125f8beb587df7f9369f2 Mon Sep 17 00:00:00 2001 From: aldelaro5 Date: Fri, 16 Sep 2016 18:08:16 -0400 Subject: [PATCH 1/2] Redo the MemCheck add dialog The old one wasn't very optimal because not only the user would likely want to enter an address instead of a range, but it also made entering just one address confusing (you had to have the same value on both start and end). Also, you should only chose one option between read, write or both, there is no point to not have any. This is why I made more clear how to add an address and it is the default option using radio buttons and I also made the action flags and the flags to be radio buttons. --- .../DolphinWX/Debugger/MemoryCheckDlg.cpp | 127 +++++++++++++----- .../Core/DolphinWX/Debugger/MemoryCheckDlg.h | 20 ++- 2 files changed, 107 insertions(+), 40 deletions(-) diff --git a/Source/Core/DolphinWX/Debugger/MemoryCheckDlg.cpp b/Source/Core/DolphinWX/Debugger/MemoryCheckDlg.cpp index bef78872c1..0f8f1263f9 100644 --- a/Source/Core/DolphinWX/Debugger/MemoryCheckDlg.cpp +++ b/Source/Core/DolphinWX/Debugger/MemoryCheckDlg.cpp @@ -3,8 +3,7 @@ // Refer to the license.txt file included. #include -#include -#include +#include #include #include #include @@ -17,58 +16,116 @@ #include "DolphinWX/Debugger/MemoryCheckDlg.h" #include "DolphinWX/WxUtils.h" -#define TEXT_BOX(text) new wxStaticText(this, wxID_ANY, _(text)) - MemoryCheckDlg::MemoryCheckDlg(CBreakPointWindow* parent) - : wxDialog(parent, wxID_ANY, _("Memory Check")), m_parent(parent) + : wxDialog(parent, wxID_ANY, _("Add a Memory Check")), m_parent(parent) { Bind(wxEVT_BUTTON, &MemoryCheckDlg::OnOK, this, wxID_OK); + Bind(wxEVT_RADIOBUTTON, &MemoryCheckDlg::OnRadioButtonClick, this); - m_pEditStartAddress = new wxTextCtrl(this, wxID_ANY, ""); - m_pEditEndAddress = new wxTextCtrl(this, wxID_ANY, ""); - m_pWriteFlag = new wxCheckBox(this, wxID_ANY, _("Write")); - m_pWriteFlag->SetValue(true); - m_pReadFlag = new wxCheckBox(this, wxID_ANY, _("Read")); + m_textAddress = new wxStaticText(this, wxID_ANY, _("Address")); + m_textStartAddress = new wxStaticText(this, wxID_ANY, _("Start")); + m_textStartAddress->Disable(); + m_textEndAddress = new wxStaticText(this, wxID_ANY, _("End")); + m_textEndAddress->Disable(); + m_pEditAddress = new wxTextCtrl(this, wxID_ANY); + m_pEditStartAddress = new wxTextCtrl(this, wxID_ANY); + m_pEditStartAddress->Disable(); + m_pEditEndAddress = new wxTextCtrl(this, wxID_ANY); + m_pEditEndAddress->Disable(); + m_radioAddress = new wxRadioButton(this, wxID_ANY, _("With an address"), wxDefaultPosition, + wxDefaultSize, wxRB_GROUP); + m_radioRange = new wxRadioButton(this, wxID_ANY, _("Within a range")); + m_radioRead = + new wxRadioButton(this, wxID_ANY, _("Read"), wxDefaultPosition, wxDefaultSize, wxRB_GROUP); + m_radioWrite = new wxRadioButton(this, wxID_ANY, _("Write")); + m_radioReadWrite = new wxRadioButton(this, wxID_ANY, _("Read and write")); + m_radioLog = + new wxRadioButton(this, wxID_ANY, _("Log"), wxDefaultPosition, wxDefaultSize, wxRB_GROUP); + m_radioBreak = new wxRadioButton(this, wxID_ANY, _("Break")); + m_radioBreakLog = new wxRadioButton(this, wxID_ANY, _("Break and log")); + m_radioBreakLog->SetValue(true); - m_log_flag = new wxCheckBox(this, wxID_ANY, _("Log")); - m_log_flag->SetValue(true); - m_break_flag = new wxCheckBox(this, wxID_ANY, _("Break")); + auto* sAddressBox = new wxBoxSizer(wxHORIZONTAL); + sAddressBox->Add(m_textAddress, 0, wxALIGN_CENTER_VERTICAL | wxALL, 5); + sAddressBox->Add(m_pEditAddress, 1, wxALIGN_CENTER_VERTICAL | wxTOP | wxBOTTOM, 10); - wxStaticBoxSizer* sAddressRangeBox = new wxStaticBoxSizer(wxHORIZONTAL, this, _("Address Range")); - sAddressRangeBox->Add(TEXT_BOX("Start"), 0, wxALIGN_CENTER_VERTICAL | wxRIGHT, 5); - sAddressRangeBox->Add(m_pEditStartAddress, 1, wxALIGN_CENTER_VERTICAL | wxRIGHT, 10); - sAddressRangeBox->Add(TEXT_BOX("End"), 0, wxALIGN_CENTER_VERTICAL | wxRIGHT, 5); - sAddressRangeBox->Add(m_pEditEndAddress, 1, wxALIGN_CENTER_VERTICAL); + auto* sAddressRangeBox = new wxBoxSizer(wxHORIZONTAL); + sAddressRangeBox->Add(m_textStartAddress, 0, wxALIGN_CENTER_VERTICAL | wxALL, 5); + sAddressRangeBox->Add(m_pEditStartAddress, 1, wxALIGN_CENTER_VERTICAL | wxTOP | wxBOTTOM, 10); + sAddressRangeBox->Add(m_textEndAddress, 0, wxALIGN_CENTER_VERTICAL | wxALL, 5); + sAddressRangeBox->Add(m_pEditEndAddress, 1, wxALIGN_CENTER_VERTICAL | wxTOP | wxBOTTOM, 5); - wxStaticBoxSizer* sActionBox = new wxStaticBoxSizer(wxVERTICAL, this, _("Action")); - sActionBox->Add(m_pWriteFlag); - sActionBox->Add(m_pReadFlag); + auto* sActions = new wxStaticBoxSizer(wxVERTICAL, this, _("Action")); + sActions->Add(m_radioRead, 0, wxEXPAND); + sActions->Add(m_radioWrite, 0, wxEXPAND); + sActions->Add(m_radioReadWrite, 0, wxEXPAND); + m_radioWrite->SetValue(true); - wxBoxSizer* sFlags = new wxStaticBoxSizer(wxVERTICAL, this, _("Flags")); - sFlags->Add(m_log_flag); - sFlags->Add(m_break_flag); + auto* sFlags = new wxStaticBoxSizer(wxVERTICAL, this, _("Flags")); + sFlags->Add(m_radioLog); + sFlags->Add(m_radioBreak); + sFlags->Add(m_radioBreakLog); - wxBoxSizer* sControls = new wxBoxSizer(wxHORIZONTAL); + auto* sOptionsBox = new wxBoxSizer(wxHORIZONTAL); + sOptionsBox->Add(sActions, 1, wxEXPAND | wxRIGHT | wxTOP | wxBOTTOM, 5); + sOptionsBox->Add(sFlags, 1, wxEXPAND | wxLEFT | wxTOP | wxBOTTOM, 5); + + auto* sControls = new wxBoxSizer(wxVERTICAL); + sControls->Add(m_radioAddress, 0, wxEXPAND); + sControls->Add(sAddressBox, 0, wxEXPAND); + sControls->Add(m_radioRange, 0, wxEXPAND); sControls->Add(sAddressRangeBox, 0, wxEXPAND); - sControls->Add(sActionBox, 0, wxEXPAND); - sControls->Add(sFlags, 0, wxEXPAND); + sControls->Add(sOptionsBox, 0, wxEXPAND); - wxBoxSizer* sMainSizer = new wxBoxSizer(wxVERTICAL); - sMainSizer->Add(sControls, 0, wxEXPAND | wxALL, 5); + auto* sMainSizer = new wxBoxSizer(wxVERTICAL); + sMainSizer->Add(sControls, 0, wxEXPAND | wxTOP | wxRIGHT | wxLEFT, 5); sMainSizer->Add(CreateButtonSizer(wxOK | wxCANCEL), 0, wxEXPAND | wxLEFT | wxRIGHT | wxBOTTOM, 5); SetSizerAndFit(sMainSizer); SetFocus(); + m_pEditAddress->SetFocus(); +} + +void MemoryCheckDlg::OnRadioButtonClick(wxCommandEvent& event) +{ + if (m_radioAddress->GetValue()) + { + m_pEditAddress->Enable(); + m_textAddress->Enable(); + m_pEditStartAddress->Disable(); + m_pEditEndAddress->Disable(); + m_textStartAddress->Disable(); + m_textEndAddress->Disable(); + } + else + { + m_pEditStartAddress->Enable(); + m_textStartAddress->Enable(); + m_textEndAddress->Enable(); + m_pEditEndAddress->Enable(); + m_pEditAddress->Disable(); + m_textAddress->Disable(); + } } void MemoryCheckDlg::OnOK(wxCommandEvent& event) { - wxString StartAddressString = m_pEditStartAddress->GetLineText(0); - wxString EndAddressString = m_pEditEndAddress->GetLineText(0); - bool OnRead = m_pReadFlag->GetValue(); - bool OnWrite = m_pWriteFlag->GetValue(); - bool Log = m_log_flag->GetValue(); - bool Break = m_break_flag->GetValue(); + wxString StartAddressString; + wxString EndAddressString; + if (m_radioAddress->GetValue()) + { + StartAddressString = m_pEditAddress->GetLineText(0); + EndAddressString = m_pEditAddress->GetLineText(0); + } + else + { + StartAddressString = m_pEditStartAddress->GetLineText(0); + EndAddressString = m_pEditEndAddress->GetLineText(0); + } + bool OnRead = m_radioRead->GetValue() || m_radioReadWrite->GetValue(); + bool OnWrite = m_radioWrite->GetValue() || m_radioReadWrite->GetValue(); + bool Log = m_radioLog->GetValue() || m_radioBreakLog->GetValue(); + bool Break = m_radioBreak->GetValue() || m_radioBreakLog->GetValue(); u32 StartAddress, EndAddress; bool EndAddressOK = diff --git a/Source/Core/DolphinWX/Debugger/MemoryCheckDlg.h b/Source/Core/DolphinWX/Debugger/MemoryCheckDlg.h index 135ff489ba..ca3357cf7f 100644 --- a/Source/Core/DolphinWX/Debugger/MemoryCheckDlg.h +++ b/Source/Core/DolphinWX/Debugger/MemoryCheckDlg.h @@ -7,7 +7,8 @@ #include class CBreakPointWindow; -class wxCheckBox; +class wxRadioButton; +class wxStaticText; class wxTextCtrl; class MemoryCheckDlg : public wxDialog @@ -17,12 +18,21 @@ public: private: CBreakPointWindow* m_parent; - wxCheckBox* m_pReadFlag; - wxCheckBox* m_pWriteFlag; - wxCheckBox* m_log_flag; - wxCheckBox* m_break_flag; + wxStaticText* m_textAddress; + wxStaticText* m_textStartAddress; + wxStaticText* m_textEndAddress; + wxRadioButton* m_radioLog; + wxRadioButton* m_radioBreak; + wxRadioButton* m_radioBreakLog; + wxRadioButton* m_radioRead; + wxRadioButton* m_radioWrite; + wxRadioButton* m_radioReadWrite; + wxRadioButton* m_radioAddress; + wxRadioButton* m_radioRange; + wxTextCtrl* m_pEditAddress; wxTextCtrl* m_pEditEndAddress; wxTextCtrl* m_pEditStartAddress; + void OnRadioButtonClick(wxCommandEvent& event); void OnOK(wxCommandEvent& event); }; From 5e8fc4ebd989b9e59cd670a071fd8a2e2f3d6ba6 Mon Sep 17 00:00:00 2001 From: aldelaro5 Date: Fri, 23 Sep 2016 21:15:35 -0400 Subject: [PATCH 2/2] Add consideration for the break flag of memChecks It was never used, even when the code tried to make sure it was initialised and passed correctly. This is a supplementary fix for the memCheck dialog as this option will now work correctly. --- Source/Core/Common/BreakPoints.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/Common/BreakPoints.cpp b/Source/Core/Common/BreakPoints.cpp index e9c8c481cc..a4e5fa64fe 100644 --- a/Source/Core/Common/BreakPoints.cpp +++ b/Source/Core/Common/BreakPoints.cpp @@ -218,8 +218,8 @@ bool TMemCheck::Action(DebugInterface* debug_interface, u32 iValue, u32 addr, bo debug_interface->GetDescription(pc).c_str(), write ? "Write" : "Read", size * 8, size * 2, iValue, addr, debug_interface->GetDescription(addr).c_str()); } - - return true; + if (Break) + return true; } return false; }