Debugger: Added stateLoaded/stateSaved Lua callbacks (+ fixed issues with savestate API & Lua reference cleanup)

Core/Debugger.cpp

@@ -430,7 +430,7 @@ bool Debugger::PrivateProcessRamOperation(MemoryOperationType type, uint16_t &ad
 			}
 
 			addr = _nextReadAddr;
-			value = _memoryManager->DebugRead(addr, false);
+			value = _memoryManager->DebugRead(addr, true);
 			_cpu->SetDebugPC(addr);
 			_nextReadAddr = -1;
 		} else if(_needRewind) {
@@ -445,9 +445,7 @@ bool Debugger::PrivateProcessRamOperation(MemoryOperationType type, uint16_t &ad
 			} else {
 				RewindManager::StartRewinding(true);
 			}
-			addr = _cpu->GetState().PC;
+			UpdateProgramCounter(addr, value);
-			value = _memoryManager->DebugRead(addr, false);
-			_cpu->SetDebugPC(addr);
 			_needRewind = false;
 		}
 		ProcessScriptSaveState(addr, value);
@@ -789,7 +787,7 @@ void Debugger::SetNextStatement(uint16_t addr)
 	if(_currentReadAddr) {
 		_cpu->SetDebugPC(addr);
 		*_currentReadAddr = addr;
-		*_currentReadValue = _memoryManager->DebugRead(addr, false);
+		*_currentReadValue = _memoryManager->DebugRead(addr, true);
 	} else {
 		//Can't change the address right away (CPU is in the middle of an instruction)
 		//Address will change after current instruction is done executing
@@ -1095,24 +1093,34 @@ void Debugger::ResetCounters()
 	_profiler->Reset();
 }
 
+void Debugger::UpdateProgramCounter(uint16_t &addr, uint8_t &value)
+{
+	addr = _cpu->GetState().PC;
+	value = _memoryManager->DebugRead(addr, true);
+	_cpu->SetDebugPC(addr);
+}
+
 void Debugger::ProcessScriptSaveState(uint16_t &addr, uint8_t &value)
 {
 	if(_hasScript) {
 		for(shared_ptr<ScriptHost> &script : _scripts) {
 			if(script->ProcessSavestate()) {
-				addr = _cpu->GetState().PC;
+				//Adjust PC and current addr/value if a state was loaded due to a call to loadSavestateAsync
-				value = _memoryManager->DebugRead(addr, false);
+				UpdateProgramCounter(addr, value);
-				_cpu->SetDebugPC(addr);
 			}
 		}
 	}
 }
 
-void Debugger::ProcessCpuOperation(uint16_t addr, uint8_t &value, MemoryOperationType type)
+void Debugger::ProcessCpuOperation(uint16_t &addr, uint8_t &value, MemoryOperationType type)
 {
 	if(_hasScript) {
 		for(shared_ptr<ScriptHost> &script : _scripts) {
 			script->ProcessCpuOperation(addr, value, type);
+			if(type == MemoryOperationType::ExecOpCode && script->CheckStateLoadedFlag()) {
+				//Adjust PC and current addr/value when a state was loaded during a CpuExec callback
+				UpdateProgramCounter(addr, value);
+			}
 		}
 	}
 }

Core/Debugger.h

@@ -231,8 +231,10 @@ public:
 
 	void ResetCounters();
 
+	void UpdateProgramCounter(uint16_t &addr, uint8_t &value);
+
 	void ProcessScriptSaveState(uint16_t &addr, uint8_t &value);
-	void ProcessCpuOperation(uint16_t addr, uint8_t &value, MemoryOperationType type);
+	void ProcessCpuOperation(uint16_t &addr, uint8_t &value, MemoryOperationType type);
 	void ProcessPpuOperation(uint16_t addr, uint8_t &value, MemoryOperationType type);
 	void ProcessEvent(EventType type);
 };

Core/DebuggerTypes.h

@@ -85,5 +85,7 @@ enum class EventType
 	Irq = 2,
 	StartFrame = 3,
 	EndFrame = 4,
-	CodeBreak = 5
+	CodeBreak = 5,
+	StateLoaded = 6,
+	StateSaved = 7,
 };

Core/LuaApi.cpp

@@ -29,7 +29,7 @@
 #define errorCond(cond, text) if(cond) { luaL_error(lua, text); return 0; }
 #define checkparams() if(!l.CheckParamCount()) { return 0; }
 #define checkminparams(x) if(!l.CheckParamCount(x)) { return 0; }
-#define checkstartframe() if(!_context->CheckInStartFrameEvent()) { error("This function cannot be called outside StartFrame event callback"); return 0; }
+#define checksavestateconditions() if(!_context->CheckInStartFrameEvent() && !_context->CheckInExecOpEvent()) { error("This function must be called inside a StartFrame event callback or a CpuExec memory operation callback"); return 0; }
 
 enum class ExecuteCountType
 {
@@ -133,6 +133,8 @@ int LuaApi::GetLibrary(lua_State *lua)
 	lua_pushintvalue(startFrame, EventType::StartFrame);
 	lua_pushintvalue(endFrame, EventType::EndFrame);
 	lua_pushintvalue(codeBreak, EventType::CodeBreak);
+	lua_pushintvalue(stateLoaded, EventType::StateLoaded);
+	lua_pushintvalue(stateSaved, EventType::StateSaved);
 	lua_settable(lua, -3);
 
 	lua_pushliteral(lua, "executeCountType");
@@ -253,7 +255,7 @@ int LuaApi::RegisterEventCallback(lua_State *lua)
 	EventType type = (EventType)l.ReadInteger();
 	int reference = l.GetReference();
 	checkparams();
-	errorCond(type < EventType::Reset || type > EventType::CodeBreak, "the specified type is invalid");
+	errorCond(type < EventType::Reset || type > EventType::StateSaved, "the specified type is invalid");
 	errorCond(reference == LUA_NOREF, "the specified function could not be found");
 	_context->RegisterEventCallback(type, reference);
 	l.Return(reference);
@@ -266,7 +268,7 @@ int LuaApi::UnregisterEventCallback(lua_State *lua)
 	EventType type = (EventType)l.ReadInteger();
 	int reference = l.ReadInteger();
 	checkparams();
-	errorCond(type < EventType::Reset || type > EventType::CodeBreak, "the specified type is invalid");
+	errorCond(type < EventType::Reset || type > EventType::StateSaved, "the specified type is invalid");
 	errorCond(reference == LUA_NOREF, "function reference is invalid");
 	_context->UnregisterEventCallback(type, reference);
 	return l.ReturnCount();
@@ -438,7 +440,7 @@ int LuaApi::Rewind(lua_State *lua)
 	LuaCallHelper l(lua);
 	int seconds = l.ReadInteger();
 	checkparams();
-	checkstartframe();
+	checksavestateconditions();
 	errorCond(seconds <= 0, "seconds must be >= 1");
 	RewindManager::RewindSeconds(seconds);
 	return l.ReturnCount();
@@ -457,7 +459,7 @@ int LuaApi::TakeScreenshot(lua_State *lua)
 int LuaApi::SaveSavestate(lua_State *lua)
 {
 	LuaCallHelper l(lua);
-	checkstartframe();
+	checksavestateconditions();
 	stringstream ss;
 	SaveStateManager::SaveState(ss);
 	l.Return(ss.str());
@@ -469,10 +471,8 @@ int LuaApi::LoadSavestate(lua_State *lua)
 	LuaCallHelper l(lua);
 	string savestate = l.ReadString();
 	checkparams();
-	checkstartframe();
+	checksavestateconditions();
-	stringstream ss;
+	l.Return(_context->LoadState(savestate));
-	ss << savestate;
-	l.Return(SaveStateManager::LoadState(ss, true));
 	return l.ReturnCount();
 }
 
@@ -492,7 +492,7 @@ int LuaApi::LoadSavestateAsync(lua_State *lua)
 	int32_t slot = l.ReadInteger();
 	checkparams();
 	errorCond(slot < 0, "Slot must be >= 0");
-	_context->RequestLoadState(slot);
+	l.Return(_context->RequestLoadState(slot));
 	return l.ReturnCount();
 }
 

Core/LuaScriptingContext.cpp

@@ -11,6 +11,26 @@ LuaScriptingContext::LuaScriptingContext() { }
 LuaScriptingContext::~LuaScriptingContext()
 {
 	if(_lua) {
+		//Cleanup all references, this is required to prevent crashes that can occur when calling lua_close
+		std::unordered_set<int> references;
+		for(int i = (int)CallbackType::CpuRead; i <= (int)CallbackType::PpuWrite; i++) {
+			for(int addr = 0; addr < 0x10000; addr++ ){
+				for(int &ref : _callbacks[i][addr]) {
+					references.emplace(ref);
+				}
+			}
+		}
+
+		for(int i = (int)EventType::Reset; i <= (int)EventType::StateSaved; i++) {
+			for(int &ref : _eventCallbacks[i]) {
+				references.emplace(ref);
+			}
+		}
+
+		for(const int &ref : references) {
+			luaL_unref(_lua, LUA_REGISTRYINDEX, ref);
+		}
+
 		lua_close(_lua);
 		_lua = nullptr;
 	}
@@ -42,6 +62,18 @@ bool LuaScriptingContext::LoadScript(string scriptName, string scriptContent, De
 	return false;
 }
 
+void LuaScriptingContext::UnregisterMemoryCallback(CallbackType type, int startAddr, int endAddr, int reference)
+{
+	ScriptingContext::UnregisterMemoryCallback(type, startAddr, endAddr, reference);
+	luaL_unref(_lua, LUA_REGISTRYINDEX, reference);
+}
+
+void LuaScriptingContext::UnregisterEventCallback(EventType type, int reference)
+{
+	ScriptingContext::UnregisterEventCallback(type, reference);
+	luaL_unref(_lua, LUA_REGISTRYINDEX, reference);
+}
+
 void LuaScriptingContext::InternalCallMemoryCallback(uint16_t addr, uint8_t &value, CallbackType type)
 {
 	LuaApi::SetContext(this);

Core/LuaScriptingContext.h

@@ -19,4 +19,7 @@ public:
 	~LuaScriptingContext();
 
 	bool LoadScript(string scriptName, string scriptContent, Debugger* debugger);
+	
+	void UnregisterMemoryCallback(CallbackType type, int startAddr, int endAddr, int reference) override;
+	void UnregisterEventCallback(EventType type, int reference) override;
 };

Core/SaveStateManager.cpp

@@ -7,6 +7,7 @@
 #include "Console.h"
 #include "EmulationSettings.h"
 #include "VideoDecoder.h"
+#include "Debugger.h"
 
 const uint32_t SaveStateManager::FileFormatVersion;
 atomic<uint32_t> SaveStateManager::_lastIndex(1);
@@ -54,8 +55,6 @@ bool SaveStateManager::LoadState()
 
 void SaveStateManager::SaveState(ostream &stream)
 {
-	Console::Pause();
-
 	uint32_t emuVersion = EmulationSettings::GetMesenVersion();
 	stream.write("MST", 3);
 	stream.write((char*)&emuVersion, sizeof(emuVersion));
@@ -70,7 +69,6 @@ void SaveStateManager::SaveState(ostream &stream)
 	stream.write(romName.c_str(), romName.size());
 
 	Console::SaveState(stream);
-	Console::Resume();
 }
 
 bool SaveStateManager::SaveState(string filepath)
@@ -78,8 +76,16 @@ bool SaveStateManager::SaveState(string filepath)
 	ofstream file(filepath, ios::out | ios::binary);
 
 	if(file) {
+		Console::Pause();
 		SaveState(file);
 		file.close();
+
+		shared_ptr<Debugger> debugger = Console::GetInstance()->GetDebugger(false);
+		if(debugger) {
+			debugger->ProcessEvent(EventType::StateSaved);
+		}
+
+		Console::Resume();
 		return true;
 	}
 	return false;
@@ -141,9 +147,7 @@ bool SaveStateManager::LoadState(istream &stream, bool hashCheckRequired)
 			}
 		}
 
-		Console::Pause();
 		Console::LoadState(stream);
-		Console::Resume();
 
 		return true;
 	}
@@ -157,10 +161,16 @@ bool SaveStateManager::LoadState(string filepath, bool hashCheckRequired)
 	bool result = false;
 
 	if(file.good()) {
+		Console::Pause();
 		if(LoadState(file, hashCheckRequired)) {
 			result = true;
 		}
 		file.close();
+		shared_ptr<Debugger> debugger = Console::GetInstance()->GetDebugger(false);
+		if(debugger) {
+			debugger->ProcessEvent(EventType::StateLoaded);
+		}
+		Console::Resume();
 	} else {
 		MessageManager::DisplayMessage("SaveStates", "SaveStateEmpty");
 	}

Core/ScriptHost.cpp

@@ -62,3 +62,11 @@ bool ScriptHost::ProcessSavestate()
 	}
 	return false;
 }
+
+bool ScriptHost::CheckStateLoadedFlag()
+{
+	if(_context) {
+		return _context->CheckStateLoadedFlag();
+	}
+	return false;
+}

Core/ScriptHost.h

@@ -23,4 +23,6 @@ public:
 	void ProcessPpuOperation(uint16_t addr, uint8_t &value, MemoryOperationType type);
 	void ProcessEvent(EventType eventType);
 	bool ProcessSavestate();
+
+	bool CheckStateLoadedFlag();
 };

Core/ScriptingContext.cpp

@@ -2,7 +2,7 @@
 #include <algorithm>
 #include "ScriptingContext.h"
 #include "DebuggerTypes.h"
-#include "Console.h"
+#include "SaveStateManager.h"
 
 string ScriptingContext::_log = "";
 
@@ -52,6 +52,18 @@ bool ScriptingContext::CheckInStartFrameEvent()
 	return _inStartFrameEvent;
 }
 
+bool ScriptingContext::CheckInExecOpEvent()
+{
+	return _inExecOpEvent;
+}
+
+bool ScriptingContext::CheckStateLoadedFlag()
+{
+	bool stateLoaded = _stateLoaded;
+	_stateLoaded = false;
+	return stateLoaded;
+}
+
 void ScriptingContext::RegisterMemoryCallback(CallbackType type, int startAddr, int endAddr, int reference)
 {
 	if(endAddr < startAddr) {
@@ -129,7 +141,7 @@ void ScriptingContext::SaveState()
 {
 	if(_saveSlot >= 0) {
 		stringstream ss;
-		Console::SaveState(ss);
+		SaveStateManager::SaveState(ss);
 
 		ss.seekg(0, std::ios::end);
 		uint32_t fileSize = (uint32_t)ss.tellg();
@@ -145,13 +157,27 @@ bool ScriptingContext::LoadState()
 	if(_loadSlot >= 0 && _saveSlotData.find(_loadSlot) != _saveSlotData.end()) {
 		stringstream ss;
 		ss << _saveSlotData[_loadSlot];
-		Console::LoadState(ss);
+		bool result = SaveStateManager::LoadState(ss);
 		_loadSlot = -1;
-		return true;
+		if(result) {
+			_stateLoaded = true;
+		}
+		return result;
 	}
 	return false;
 }
 
+bool ScriptingContext::LoadState(string stateData)
+{
+	stringstream ss;
+	ss << stateData;
+	bool result = SaveStateManager::LoadState(ss);
+	if(result) {
+		_stateLoaded = true;
+	}
+	return result;
+}
+
 bool ScriptingContext::ProcessSavestate()
 {
 	SaveState();

Core/ScriptingContext.h

@@ -30,12 +30,13 @@ private:
 	std::unordered_map<int32_t, string> _saveSlotData;
 	int32_t _saveSlot = -1;
 	int32_t _loadSlot = -1;
+	bool _stateLoaded = false;
 
 protected:
 	string _scriptName;
 
 	vector<int> _callbacks[5][0x10000];
-	vector<int> _eventCallbacks[7];
+	vector<int> _eventCallbacks[8];
 
 	virtual void InternalCallMemoryCallback(uint16_t addr, uint8_t &value, CallbackType type) = 0;
 	virtual int InternalCallEventCallback(EventType type) = 0;
@@ -52,6 +53,7 @@ public:
 	bool RequestLoadState(int slot);
 	void SaveState();
 	bool LoadState();
+	bool LoadState(string stateData);
 	string GetSavestateData(int slot);
 	void ClearSavestateData(int slot);
 	bool ProcessSavestate();
@@ -59,9 +61,11 @@ public:
 	void CallMemoryCallback(uint16_t addr, uint8_t &value, CallbackType type);
 	int CallEventCallback(EventType type);
 	bool CheckInStartFrameEvent();
+	bool CheckInExecOpEvent();
+	bool CheckStateLoadedFlag();
 	
 	void RegisterMemoryCallback(CallbackType type, int startAddr, int endAddr, int reference);
-	void UnregisterMemoryCallback(CallbackType type, int startAddr, int endAddr, int reference);
+	virtual void UnregisterMemoryCallback(CallbackType type, int startAddr, int endAddr, int reference);
 	void RegisterEventCallback(EventType type, int reference);
-	void UnregisterEventCallback(EventType type, int reference);
+	virtual void UnregisterEventCallback(EventType type, int reference);
 };