ActionReplay: Fix ISOProperties corrupting active code set

ISOProperties loads codes using ActionReplay::LoadCodes which actually applies
the codes to the global state. If a game is running then that games receives
all the codes (and ACTIVE status) from the second game being shown in
ISOProperties which is not desirable.

Source/Core/Core/ActionReplay.cpp

@@ -19,7 +19,10 @@
 // Zero Codes: any code with no address.  These codes are used to do special operations like memory copy, etc
 // -------------------------------------------------------------------------------------------------------------
 
+#include <list>
+#include <mutex>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "Common/CommonTypes.h"
@@ -81,6 +84,9 @@ static std::vector<ARCode> activeCodes;
 static bool logSelf = false;
 static std::vector<std::string> arLog;
 
+static std::mutex s_callbacks_lock;
+static std::list<std::function<void()>> s_callbacks;
+
 struct ARAddr
 {
 	union
@@ -100,16 +106,68 @@ struct ARAddr
 	operator u32() const { return address; }
 };
 
+static void RunCodeChangeCallbacks()
+{
+	std::lock_guard<std::mutex> guard(s_callbacks_lock);
+	for (const auto& cb : s_callbacks)
+		cb();
+}
+
 // ----------------------
 // AR Remote Functions
-void LoadCodes(const IniFile& globalIni, const IniFile& localIni, bool forceLoad)
+void ApplyCodes(const std::vector<ARCode>& codes)
 {
-	// Parses the Action Replay section of a game ini file.
+	if (!SConfig::GetInstance().bEnableCheats)
-	if (!SConfig::GetInstance().bEnableCheats &&
-		!forceLoad)
 		return;
 
-	arCodes.clear();
+	arCodes = codes;
+	UpdateActiveList();
+	RunCodeChangeCallbacks();
+}
+
+void AddCode(const ARCode& code)
+{
+	if (!SConfig::GetInstance().bEnableCheats)
+		return;
+
+	arCodes.push_back(code);
+	if (code.active)
+		UpdateActiveList();
+	RunCodeChangeCallbacks();
+}
+
+void* RegisterCodeChangeCallback(std::function<void()> callback)
+{
+	if (!callback)
+		return nullptr;
+
+	std::lock_guard<std::mutex> guard(s_callbacks_lock);
+	s_callbacks.emplace_back(std::move(callback));
+	return &s_callbacks.back();
+}
+
+void UnregisterCodeChangeCallback(void* token)
+{
+	std::lock_guard<std::mutex> guard(s_callbacks_lock);
+	for (auto i = s_callbacks.begin(); i != s_callbacks.end(); ++i)
+	{
+		if (&*i == token)
+		{
+			s_callbacks.erase(i);
+			break;
+		}
+	}
+}
+
+void LoadAndApplyCodes(const IniFile& globalIni, const IniFile& localIni)
+{
+	ApplyCodes(LoadCodes(globalIni, localIni));
+}
+
+// Parses the Action Replay section of a game ini file.
+std::vector<ARCode> LoadCodes(const IniFile& globalIni, const IniFile& localIni)
+{
+	std::vector<ARCode> codes;
 
 	std::vector<std::string> enabledLines;
 	std::set<std::string> enabledNames;
@@ -146,13 +204,13 @@ void LoadCodes(const IniFile& globalIni, const IniFile& localIni, bool forceLoad
 			{
 				if (currentCode.ops.size())
 				{
-					arCodes.push_back(currentCode);
+					codes.push_back(currentCode);
 					currentCode.ops.clear();
 				}
 				if (encryptedLines.size())
 				{
 					DecryptARCode(encryptedLines, currentCode.ops);
-					arCodes.push_back(currentCode);
+					codes.push_back(currentCode);
 					currentCode.ops.clear();
 					encryptedLines.clear();
 				}
@@ -204,22 +262,16 @@ void LoadCodes(const IniFile& globalIni, const IniFile& localIni, bool forceLoad
 		// Handle the last code correctly.
 		if (currentCode.ops.size())
 		{
-			arCodes.push_back(currentCode);
+			codes.push_back(currentCode);
 		}
 		if (encryptedLines.size())
 		{
 			DecryptARCode(encryptedLines, currentCode.ops);
-			arCodes.push_back(currentCode);
+			codes.push_back(currentCode);
 		}
 	}
 
-	UpdateActiveList();
+	return codes;
-}
-
-void LoadCodes(std::vector<ARCode> &_arCodes, IniFile &globalIni, IniFile& localIni)
-{
-	LoadCodes(globalIni, localIni, true);
-	_arCodes = arCodes;
 }
 
 
@@ -270,6 +322,7 @@ void SetARCode_IsActive(bool active, size_t index)
 	}
 	arCodes[index].active = active;
 	UpdateActiveList();
+	RunCodeChangeCallbacks();
 }
 
 void UpdateActiveList()

Source/Core/Core/ActionReplay.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <functional>
 #include <string>
 #include <vector>
 #include "Common/CommonTypes.h"
@@ -31,8 +32,12 @@ struct ARCode
 
 void RunAllActive();
 bool RunCode(const ARCode &arcode);
-void LoadCodes(const IniFile &globalini, const IniFile &localIni, bool forceLoad);
+void ApplyCodes(const std::vector<ARCode>& codes);
-void LoadCodes(std::vector<ARCode> &_arCodes, IniFile &globalini, IniFile &localIni);
+void AddCode(const ARCode& new_code);
+void* RegisterCodeChangeCallback(std::function<void()> callback);
+void  UnregisterCodeChangeCallback(void* token);
+void LoadAndApplyCodes(const IniFile& globalini, const IniFile& localIni);
+std::vector<ARCode> LoadCodes(const IniFile& globalini, const IniFile& localIni);
 size_t GetCodeListSize();
 ARCode GetARCode(size_t index);
 void SetARCode_IsActive(bool active, size_t index);

Source/Core/Core/PatchEngine.cpp

@@ -166,7 +166,7 @@ void LoadPatches()
 	IniFile localIni = SConfig::GetInstance().LoadLocalGameIni();
 
 	LoadPatchSection("OnFrame", onFrame, globalIni, localIni);
-	ActionReplay::LoadCodes(globalIni, localIni, false);
+	ActionReplay::LoadAndApplyCodes(globalIni, localIni);
 
 	// lil silly
 	std::vector<Gecko::GeckoCode> gcodes;

Source/Core/DolphinWX/Cheats/CheatsWindow.cpp

@@ -6,6 +6,7 @@
 #include <cstddef>
 #include <cstdio>
 #include <cstring>
+#include <functional>
 #include <string>
 #include <vector>
 #include <wx/button.h>
@@ -38,12 +39,19 @@
 #include "DolphinWX/Cheats/CreateCodeDialog.h"
 #include "DolphinWX/Cheats/GeckoCodeDiag.h"
 
+namespace
+{
+wxDEFINE_EVENT(DOLPHIN_EVT_UPDATE_CHEAT_LIST, wxThreadEvent);
+}
+
 wxCheatsWindow::wxCheatsWindow(wxWindow* const parent)
 	: wxDialog(parent, wxID_ANY, "", wxDefaultPosition, wxDefaultSize, wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER | wxMAXIMIZE_BOX | wxMINIMIZE_BOX | wxDIALOG_NO_PARENT)
 {
 	// Create the GUI controls
 	Init_ChildControls();
 
+	m_ar_callback_token = ActionReplay::RegisterCodeChangeCallback(std::bind(&wxCheatsWindow::OnActionReplayModified, this));
+
 	// load codes
 	UpdateGUI();
 
@@ -54,6 +62,7 @@ wxCheatsWindow::wxCheatsWindow(wxWindow* const parent)
 
 wxCheatsWindow::~wxCheatsWindow()
 {
+	ActionReplay::UnregisterCodeChangeCallback(m_ar_callback_token);
 	main_frame->g_CheatsWindow = nullptr;
 }
 
@@ -126,7 +135,7 @@ void wxCheatsWindow::Init_ChildControls()
 	button_cancel->Bind(wxEVT_BUTTON, &wxCheatsWindow::OnEvent_ButtonClose_Press, this);
 
 	Bind(wxEVT_CLOSE_WINDOW, &wxCheatsWindow::OnEvent_Close, this);
-	Bind(UPDATE_CHEAT_LIST_EVENT, &wxCheatsWindow::OnEvent_CheatsList_Update, this);
+	Bind(DOLPHIN_EVT_UPDATE_CHEAT_LIST, &wxCheatsWindow::OnEvent_CheatsList_Update, this);
 
 	wxStdDialogButtonSizer* const sButtons = new wxStdDialogButtonSizer();
 	sButtons->AddButton(m_button_apply);
@@ -232,24 +241,30 @@ void wxCheatsWindow::OnEvent_CheatsList_ItemToggled(wxCommandEvent& WXUNUSED(eve
 	{
 		if ((int)code_index.uiIndex == index)
 		{
+			m_ar_ignore_callback = true;
 			ActionReplay::SetARCode_IsActive(m_checklistbox_cheats_list->IsChecked(index), code_index.index);
 		}
 	}
 }
 
-void wxCheatsWindow::OnEvent_CheatsList_Update(wxCommandEvent& event)
+void wxCheatsWindow::OnEvent_CheatsList_Update(wxThreadEvent&)
 {
+	if (m_ar_ignore_callback)
+	{
+		m_ar_ignore_callback = false;
+		return;
+	}
 	Load_ARCodes();
 }
 
+void wxCheatsWindow::OnActionReplayModified()
+{
+	// NOTE: This is an arbitrary thread context
+	GetEventHandler()->QueueEvent(new wxThreadEvent(DOLPHIN_EVT_UPDATE_CHEAT_LIST));
+}
+
 void wxCheatsWindow::OnEvent_ApplyChanges_Press(wxCommandEvent& ev)
 {
-	// Apply AR Code changes
-	for (const ARCodeIndex& code_index : m_index_list)
-	{
-		ActionReplay::SetARCode_IsActive(m_checklistbox_cheats_list->IsChecked(code_index.uiIndex), code_index.index);
-	}
-
 	// Apply Gecko Code changes
 	Gecko::SetActiveCodes(m_geckocode_panel->GetCodes());
 
@@ -265,11 +280,15 @@ void wxCheatsWindow::OnEvent_ApplyChanges_Press(wxCommandEvent& ev)
 
 void wxCheatsWindow::OnEvent_ButtonUpdateLog_Press(wxCommandEvent& WXUNUSED(event))
 {
+	wxBeginBusyCursor();
+	m_textctrl_log->Freeze();
 	m_textctrl_log->Clear();
 	for (const std::string& text : ActionReplay::GetSelfLog())
 	{
 		m_textctrl_log->AppendText(StrToWxStr(text));
 	}
+	m_textctrl_log->Thaw();
+	wxEndBusyCursor();
 }
 
 void wxCheatsWindow::OnEvent_CheckBoxEnableLogging_StateChange(wxCommandEvent& WXUNUSED(event))

Source/Core/DolphinWX/Cheats/CheatsWindow.h

@@ -72,6 +72,10 @@ private:
 	IniFile m_gameini_local;
 	std::string m_gameini_local_path;
 
+	// ActionReplay::UnregisterCodeChangeCallback handle
+	void* m_ar_callback_token = nullptr;
+	bool  m_ar_ignore_callback = false;
+
 	void Init_ChildControls();
 
 	void Load_ARCodes();
@@ -86,7 +90,8 @@ private:
 	// Cheats List
 	void OnEvent_CheatsList_ItemSelected(wxCommandEvent& event);
 	void OnEvent_CheatsList_ItemToggled(wxCommandEvent& event);
-	void OnEvent_CheatsList_Update(wxCommandEvent& event);
+	void OnEvent_CheatsList_Update(wxThreadEvent& event);
+	void OnActionReplayModified();
 
 	// Apply Changes Button
 	void OnEvent_ApplyChanges_Press(wxCommandEvent& event);

Source/Core/DolphinWX/Cheats/CreateCodeDialog.cpp

@@ -14,9 +14,6 @@
 #include "DolphinWX/WxUtils.h"
 #include "DolphinWX/Cheats/CreateCodeDialog.h"
 
-// Fired when an ActionReplay code is created.
-wxDEFINE_EVENT(UPDATE_CHEAT_LIST_EVENT, wxCommandEvent);
-
 CreateCodeDialog::CreateCodeDialog(wxWindow* const parent, const u32 address)
 	: wxDialog(parent, wxID_ANY, _("Create AR Code"))
 	, m_code_address(address)
@@ -80,24 +77,21 @@ void CreateCodeDialog::PressOK(wxCommandEvent& ev)
 	// create the new code
 	ActionReplay::ARCode new_cheat;
 	new_cheat.active = false;
+	new_cheat.user_defined = true;
 	new_cheat.name = WxStrToStr(code_name);
-	const ActionReplay::AREntry new_entry(m_code_address, code_value);
+	new_cheat.ops.emplace_back(ActionReplay::AREntry(m_code_address, code_value));
-	new_cheat.ops.push_back(new_entry);
+	ActionReplay::AddCode(new_cheat);
 
 	// pretty hacky - add the code to the gameini
+	// FIXME: The save logic should be ActionReplay since it mirrors the parser
 	{
-	CISOProperties isoprops(GameListItem(SConfig::GetInstance().m_LastFilename, std::unordered_map<std::string, std::string>()), this);
+	CISOProperties isoprops(GameListItem(SConfig::GetInstance().m_LastFilename, {}), this);
 	// add the code to the isoproperties arcode list
 	isoprops.AddARCode(new_cheat);
 	// save the gameini
 	isoprops.SaveGameConfig();
-	isoprops.ActionReplayList_Load(); // loads the new arcodes
-	//ActionReplay::UpdateActiveList();
 	}
 
-	// Propagate back to the parent frame to update the cheat list.
-	GetEventHandler()->AddPendingEvent(wxCommandEvent(UPDATE_CHEAT_LIST_EVENT));
-
 	Close();
 }
 

Source/Core/DolphinWX/Cheats/CreateCodeDialog.h

@@ -12,8 +12,6 @@
 class wxCheckBox;
 class wxTextCtrl;
 
-wxDECLARE_EVENT(UPDATE_CHEAT_LIST_EVENT, wxCommandEvent);
-
 class CreateCodeDialog final : public wxDialog
 {
 public:

Source/Core/DolphinWX/ISOProperties.cpp

@@ -95,6 +95,7 @@ BEGIN_EVENT_TABLE(CISOProperties, wxDialog)
 	EVT_MENU(IDM_EXTRACTDOL, CISOProperties::OnExtractDataFromHeader)
 	EVT_MENU(IDM_CHECKINTEGRITY, CISOProperties::CheckPartitionIntegrity)
 	EVT_CHOICE(ID_LANG, CISOProperties::OnChangeBannerLang)
+	EVT_CHECKLISTBOX(ID_CHEATS_LIST, CISOProperties::OnActionReplayCodeChecked)
 END_EVENT_TABLE()
 
 CISOProperties::CISOProperties(const GameListItem& game_list_item, wxWindow* parent, wxWindowID id, const wxString& title, const wxPoint& position, const wxSize& size, long style)
@@ -1314,6 +1315,11 @@ void CISOProperties::ListSelectionChanged(wxCommandEvent& event)
 	}
 }
 
+void CISOProperties::OnActionReplayCodeChecked(wxCommandEvent& event)
+{
+	arCodes[event.GetSelection()].active = Cheats->IsChecked(event.GetSelection());
+}
+
 void CISOProperties::PatchList_Load()
 {
 	onFrame.clear();
@@ -1399,9 +1405,8 @@ void CISOProperties::PatchButtonClicked(wxCommandEvent& event)
 
 void CISOProperties::ActionReplayList_Load()
 {
-	arCodes.clear();
 	Cheats->Clear();
-	ActionReplay::LoadCodes(arCodes, GameIniDefault, GameIniLocal);
+	arCodes = ActionReplay::LoadCodes(GameIniDefault, GameIniLocal);
 
 	u32 index = 0;
 	for (const ActionReplay::ARCode& arCode : arCodes)
@@ -1422,8 +1427,7 @@ void CISOProperties::ActionReplayList_Save()
 	u32 cheats_chkbox_count = Cheats->GetCount();
 	for (const ActionReplay::ARCode& code : arCodes)
 	{
-		// Check the index against the count because of the hacky way codes are added from the "Cheat Search" dialog
+		if (code.active)
-		if ((index < cheats_chkbox_count) && Cheats->IsChecked(index))
 			enabledLines.push_back("$" + code.name);
 
 		// Do not save default cheats.

Source/Core/DolphinWX/ISOProperties.h

@@ -208,6 +208,7 @@ private:
 	void OnComputeMD5Sum(wxCommandEvent& event);
 	void OnShowDefaultConfig(wxCommandEvent& event);
 	void ListSelectionChanged(wxCommandEvent& event);
+	void OnActionReplayCodeChecked(wxCommandEvent& event);
 	void PatchButtonClicked(wxCommandEvent& event);
 	void ActionReplayButtonClicked(wxCommandEvent& event);
 	void RightClickOnBanner(wxMouseEvent& event);