From 723cd9f0247179ac2eb6bd5d5cdf19f9f10a4e95 Mon Sep 17 00:00:00 2001 From: Matthew Fioravante Date: Wed, 28 Aug 2019 19:01:44 -0400 Subject: [PATCH 1/2] Refactor Interpreter Stack Frame Completion * Rename CommandEnd() to OnFinishStackFrame() This name and it's nearby comment was very misleading. The interpreter has actually always ignored CommandEnd commands. This function was only called when the index ran past end of the stack frame. * Do OnFinishStackFrame() before execution loop. Call this in one place and handle multiple stack pops. Also, don't hang on empty events. * Optimize parallel events Reset the index and never pop the base stack frame. RPG_RT does this and we avoid copying parallel event data every frame. --- src/game_interpreter.cpp | 43 +++++++++++++++++++++------------ src/game_interpreter.h | 6 ++++- src/game_interpreter_battle.cpp | 4 +-- src/game_interpreter_map.cpp | 4 +-- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/game_interpreter.cpp b/src/game_interpreter.cpp index 1e07b4e76c..4d411ae055 100644 --- a/src/game_interpreter.cpp +++ b/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 Game_Interpreter::GetChoices() { diff --git a/src/game_interpreter.h b/src/game_interpreter.h index 5cc3f70840..1132afd83d 100644 --- a/src/game_interpreter.h +++ b/src/game_interpreter.h @@ -158,6 +158,11 @@ class Game_Interpreter static std::vector 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); diff --git a/src/game_interpreter_battle.cpp b/src/game_interpreter_battle.cpp index b69125ad40..eaeaa4fcd6 100644 --- a/src/game_interpreter_battle.cpp +++ b/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]; diff --git a/src/game_interpreter_map.cpp b/src/game_interpreter_map.cpp index 6e8db706dc..0c8e66daa5 100644 --- a/src/game_interpreter_map.cpp +++ b/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]; From 0c54f4d8c97215d925ffb81be739c715d89d5c07 Mon Sep 17 00:00:00 2001 From: Matthew Fioravante Date: Wed, 28 Aug 2019 22:56:06 -0400 Subject: [PATCH 2/2] RPG_RT compatibility for empty parallel events. RPG_RT save games always have an empty base stack frame allocated for parallel map events and common events. We add these to our saves to be compatible, however if we have empty data, we won't create an interpreter object or load any data. --- src/game_commonevent.cpp | 39 +++++++++++++++++++----------- src/game_commonevent.h | 2 -- src/game_event.cpp | 52 ++++++++++++++++++++++++++-------------- 3 files changed, 59 insertions(+), 34 deletions(-) diff --git a/src/game_commonevent.cpp b/src/game_commonevent.cpp index d087dc26c9..e3b58f7b47 100644 --- a/src/game_commonevent.cpp +++ b/src/game_commonevent.cpp @@ -22,27 +22,36 @@ #include "game_interpreter_map.h" #include "main_data.h" #include "reader_util.h" +#include 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)); } diff --git a/src/game_commonevent.h b/src/game_commonevent.h index b04519e69b..41c6aaddb7 100644 --- a/src/game_commonevent.h +++ b/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. */ diff --git a/src/game_event.cpp b/src/game_event.cpp index e79d2180e3..bcd86b8a0b 100644 --- a/src/game_event.cpp +++ b/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();