Merge pull request #1884 from fmatthew5876/event_stack

Refactor Interpreter Stack Frame Completion

src/game_commonevent.cpp

@@ -22,27 +22,36 @@
 #include "game_interpreter_map.h"
 #include "main_data.h"
 #include "reader_util.h"
+#include <cassert>
 
 Game_CommonEvent::Game_CommonEvent(int common_event_id) :
-	common_event_id(common_event_id) {
+	common_event_id(common_event_id)
+{
+	auto* ce = ReaderUtil::GetElement(Data::commonevents, common_event_id);
 
-	if (GetTrigger() == RPG::EventPage::Trigger_parallel) {
+	if (ce->trigger == RPG::EventPage::Trigger_parallel
+			&& !ce->event_commands.empty()) {
 		interpreter.reset(new Game_Interpreter_Map());
+		interpreter->Push(this);
 	}
+
+
 }
 
 void Game_CommonEvent::SetSaveData(const RPG::SaveEventExecState& data) {
-	if (interpreter && !data.stack.empty()) {
+	// RPG_RT Savegames have empty stacks for parallel events.
+	// We are LSD compatible but don't load these into interpreter.
+	if (!data.stack.empty() && !data.stack.front().commands.empty()) {
+		if (!interpreter) {
+			interpreter.reset(new Game_Interpreter_Map());
+		}
 		interpreter->SetState(data);
 	}
 }
 
 bool Game_CommonEvent::Update() {
 	if (interpreter && IsWaitingBackgroundExecution()) {
-		if (!interpreter->IsRunning()) {
-			interpreter->Clear();
-			interpreter->Push(this);
-		}
+		assert(interpreter->IsRunning());
 		interpreter->Update();
 
 		// Suspend due to async op ...
@@ -85,20 +94,22 @@ RPG::SaveEventExecState Game_CommonEvent::GetSaveData() {
 	if (interpreter) {
 		state = interpreter->GetState();
 	}
+	if (GetTrigger() == RPG::EventPage::Trigger_parallel && state.stack.empty()) {
+		// RPG_RT always stores an empty stack frame for parallel events.
+		state.stack.push_back({});
+	}
 	return state;
 }
 
-bool Game_CommonEvent::IsWaitingExecution(RPG::EventPage::Trigger trigger) const {
+bool Game_CommonEvent::IsWaitingForegroundExecution() const {
 	auto* ce = ReaderUtil::GetElement(Data::commonevents, common_event_id);
-	return ce->trigger == trigger &&
+	return ce->trigger == RPG::EventPage::Trigger_auto_start &&
 		(!ce->switch_flag || Game_Switches.Get(ce->switch_id))
 		&& !ce->event_commands.empty();
 }
 
-bool Game_CommonEvent::IsWaitingForegroundExecution() const {
-	return IsWaitingExecution(RPG::EventPage::Trigger_auto_start);
-}
-
 bool Game_CommonEvent::IsWaitingBackgroundExecution() const {
-	return IsWaitingExecution(RPG::EventPage::Trigger_parallel);
+	auto* ce = ReaderUtil::GetElement(Data::commonevents, common_event_id);
+	return ce->trigger == RPG::EventPage::Trigger_parallel &&
+		(!ce->switch_flag || Game_Switches.Get(ce->switch_id));
 }

src/game_commonevent.h

@@ -105,8 +105,6 @@ class Game_CommonEvent {
 	bool IsAsyncPending() const;
 
 private:
-	bool IsWaitingExecution(RPG::EventPage::Trigger trigger) const;
-
 	int common_event_id;
 
 	/** Interpreter for parallel common events. */

src/game_event.cpp

@@ -55,11 +55,6 @@ Game_Event::Game_Event(int map_id, const RPG::Event& event, const RPG::SaveMapEv
 
 	this->event.ID = data()->ID;
 
-	if (!data()->parallel_event_execstate.stack.empty()) {
-		interpreter.reset(new Game_Interpreter_Map());
-		interpreter->SetState(data()->parallel_event_execstate);
-	}
-
 	Refresh(true);
 }
 
@@ -128,8 +123,14 @@ void Game_Event::Setup(const RPG::EventPage* new_page) {
 	SetLayer(page->layer);
 	data()->overlap_forbidden = page->overlap_forbidden;
 
-	if (!interpreter && GetTrigger() == RPG::EventPage::Trigger_parallel) {
-		interpreter.reset(new Game_Interpreter_Map());
+	if (GetTrigger() == RPG::EventPage::Trigger_parallel) {
+		if (!page->event_commands.empty()) {
+			if (!interpreter) {
+				interpreter.reset(new Game_Interpreter_Map());
+			}
+			interpreter->Clear();
+			interpreter->Push(this);
+		}
 	}
 }
 
@@ -142,10 +143,16 @@ void Game_Event::SetupFromSave(const RPG::EventPage* new_page) {
 
 	original_move_frequency = page->move_frequency;
 
-	// Trigger parallel events when the interpreter wasn't already running
-	// (because it was the middle of a parallel event while saving)
-	if (!interpreter && GetTrigger() == RPG::EventPage::Trigger_parallel) {
-		interpreter.reset(new Game_Interpreter_Map());
+	if (GetTrigger() == RPG::EventPage::Trigger_parallel) {
+		auto& state = data()->parallel_event_execstate;
+		// RPG_RT Savegames have empty stacks for parallel events.
+		// We are LSD compatible but don't load these into interpreter.
+		if (!state.stack.empty() && !state.stack.front().commands.empty()) {
+			if (!interpreter) {
+				interpreter.reset(new Game_Interpreter_Map());
+			}
+			interpreter->SetState(state);
+		}
 	}
 }
 
@@ -501,12 +508,10 @@ bool Game_Event::Update() {
 	// the interpreter will run multiple times per frame.
 	// This results in event waits to finish quicker during collisions as
 	// the wait will tick by 1 each time the interpreter is invoked.
-	if (GetTrigger() == RPG::EventPage::Trigger_parallel) {
+	if (GetTrigger() == RPG::EventPage::Trigger_parallel && interpreter) {
 		assert(interpreter != nullptr);
-		if (!interpreter->IsRunning()) {
-			interpreter->Clear();
-			interpreter->Push(this);
-		}
+		assert(interpreter->IsRunning());
+
 		interpreter->Update();
 
 		// Suspend due to async op ...
@@ -555,9 +560,20 @@ const RPG::EventPage *Game_Event::GetActivePage() const {
 }
 
 const RPG::SaveMapEvent& Game_Event::GetSaveData() {
-	if (interpreter) {
-		data()->parallel_event_execstate = interpreter->GetState();
+	RPG::SaveEventExecState state;
+	if (page && page->trigger == RPG::EventPage::Trigger_parallel) {
+		if (interpreter) {
+			state = interpreter->GetState();
+		}
+
+		if (state.stack.empty()) {
+			// RPG_RT always stores an empty stack frame for parallel events.
+			RPG::SaveEventExecFrame frame;
+			frame.event_id = GetId();
+			state.stack.push_back(std::move(frame));
+		}
 	}
+	data()->parallel_event_execstate = std::move(state);
 	data()->ID = event.ID;
 
 	return *data();

src/game_interpreter.cpp

@@ -264,6 +264,10 @@ void Game_Interpreter::Update(bool reset_loop_count) {
 		loop_count = 0;
 	}
 
+	if (!IsRunning()) {
+		return;
+	}
+
 	for (; loop_count < loop_limit; ++loop_count) {
 		// If something is calling a menu, we're allowed to execute only 1 command per interpreter. So we pass through if loop_count == 0, and stop at 1 or greater.
 		// RPG_RT compatible behavior.
@@ -326,7 +330,7 @@ void Game_Interpreter::Update(bool reset_loop_count) {
 
 		auto* frame = GetFrame();
 		if (frame == nullptr) {
-			return;
+			break;
 		}
 
 		if (continuation) {
@@ -353,11 +357,18 @@ void Game_Interpreter::Update(bool reset_loop_count) {
 		// Previous operations could have modified the stack.
 		// So we need to fetch the frame again.
 		frame = GetFrame();
-
-		if (!frame || frame->commands.empty()) {
+		if (frame == nullptr) {
 			break;
 		}
 
+		// Pop any completed stack frames
+		if (frame->current_command >= (int)frame->commands.size()) {
+			if (!OnFinishStackFrame()) {
+				break;
+			}
+			continue;
+		}
+
 		// Save the frame index before we call events.
 		int current_frame_idx = _state.stack.size() - 1;
 
@@ -682,26 +693,20 @@ bool Game_Interpreter::ExecuteCommand() {
 	}
 }
 
-bool Game_Interpreter::CommandEnd() { // code 10
+bool Game_Interpreter::OnFinishStackFrame() {
 	auto* frame = GetFrame();
 	assert(frame);
 	auto& list = frame->commands;
 
-	// Is this the first event and not a called one?
-	const bool is_original_event = _state.stack.size() == 1;
+	const bool is_base_frame = _state.stack.size() == 1;
 
-	if (main_flag && is_original_event) {
+	if (main_flag && is_base_frame) {
 		Game_Message::SetFaceName("");
 	}
 
-	// FIXME: Hangs in some cases when Autostart events start
-	//if (main_flag) {
-	//	Game_Message::FullClear();
-	//}
-
 	int event_id = frame->event_id;
 
-	if (is_original_event && event_id > 0) {
+	if (is_base_frame && event_id > 0) {
 		Game_Event* evnt = Game_Map::GetEvent(event_id);
 		if (!evnt) {
 			Output::Error("Call stack finished with invalid event id %d. This can be caused by a vehicle teleport?", event_id);
@@ -710,9 +715,17 @@ bool Game_Interpreter::CommandEnd() { // code 10
 		}
 	}
 
-	_state.stack.pop_back();
+	if (!main_flag && is_base_frame) {
+		// Parallel events will never clear the base stack frame. Instead we just
+		// reset the index back to 0 and wait for a frame.
+		// This not only optimizes away copying event code, it's also RPG_RT compatible.
+		frame->current_command = 0;
+	} else {
+		// If a called frame, or base frame of foreground interpreter, pop the stack.
+		_state.stack.pop_back();
+	}
 
-	return true;
+	return !is_base_frame;
 }
 
 std::vector<std::string> Game_Interpreter::GetChoices() {

src/game_interpreter.h

@@ -158,6 +158,11 @@ class Game_Interpreter
 	static std::vector<Game_Actor*> GetActors(int mode, int id);
 	static int ValueOrVariable(int mode, int val);
 
+	/**
+	 * When current frame finishes executing we pop the stack
+	 */
+	bool OnFinishStackFrame();
+
 	/**
 	 * Triggers a game over when all party members are dead.
 	 */
@@ -240,7 +245,6 @@ class Game_Interpreter
 	bool CommandChangeBattleCommands(RPG::EventCommand const& com);
 	bool CommandExitGame(RPG::EventCommand const& com);
 	bool CommandToggleFullscreen(RPG::EventCommand const& com);
-	bool CommandEnd();
 
 	virtual bool DefaultContinuation(RPG::EventCommand const& com);
 	virtual bool ContinuationChoices(RPG::EventCommand const& com);

src/game_interpreter_battle.cpp

@@ -42,9 +42,7 @@ bool Game_Interpreter_Battle::ExecuteCommand() {
 	const auto& list = frame->commands;
 	auto& index = frame->current_command;
 
-	if (index >= list.size()) {
-		return CommandEnd();
-	}
+	assert(index < (int)list.size());
 
 	RPG::EventCommand const& com = list[index];
 

src/game_interpreter_map.cpp

@@ -70,9 +70,7 @@ bool Game_Interpreter_Map::ExecuteCommand() {
 	const auto& list = frame->commands;
 	auto& index = frame->current_command;
 
-	if (index >= list.size()) {
-		return CommandEnd();
-	}
+	assert(index < (int)list.size());
 
 	RPG::EventCommand const& com = list[index];