Refactored several Interpreter related code segments:

- Implemented template method "DecodeTargetEvaluationMode". (This logic is shared between ControlVariables, ControlSwitches & ControlStrings but the actual implementation for several special access modes differs between implementations. Support for these modes is evaluated at compile time.) - Moved some common helper functions into a new file "game_interpreter_shared.cpp" (CheckOperator, ValueOrVariable / CommandStringOrVariable & variants) - Refactored "ValueOrVariable" & "ValueOrVariableBitfield" into template methods - Moved some commonly used member function declarations which are needed for certain command argument evaluation methods from Game_Interpreter into a new abstract base class (namespace "Game_Interpreter_Shared" is agnostic to the actual implementation of the interpreter)
florianessl · May 30, 2024 · 407342d · 407342d
1 parent ec941b0
commit 407342d
Show file tree

Hide file tree

Showing 13 changed files with 422 additions and 201 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -177,6 +177,8 @@ add_library(${PROJECT_NAME} OBJECT
 	src/game_interpreter.h
 	src/game_interpreter_map.cpp
 	src/game_interpreter_map.h
+	src/game_interpreter_shared.cpp
+	src/game_interpreter_shared.h
 	src/game_map.cpp
 	src/game_map.h
 	src/game_message.cpp

diff --git a/Makefile.am b/Makefile.am
@@ -157,6 +157,8 @@ libeasyrpg_player_a_SOURCES = \
 	src/game_interpreter_control_variables.h \
 	src/game_interpreter_map.cpp \
 	src/game_interpreter_map.h \
+	src/game_interpreter_shared.cpp \
+	src/game_interpreter_shared.h \
 	src/game_map.cpp \
 	src/game_map.h \
 	src/game_message.cpp \

diff --git a/src/game_event.cpp b/src/game_event.cpp
@@ -256,33 +256,9 @@ bool Game_Event::AreConditionsMet(const lcf::rpg::EventPage& page) {
 			return false;
 		}
 	} else {
-		if (page.condition.flags.variable) {
-			switch (page.condition.compare_operator) {
-			case 0: // ==
-				if (!(Main_Data::game_variables->Get(page.condition.variable_id) == page.condition.variable_value))
-					return false;
-				break;
-			case 1: // >=
-				if (!(Main_Data::game_variables->Get(page.condition.variable_id) >= page.condition.variable_value))
-					return false;
-				break;
-			case 2: // <=
-				if (!(Main_Data::game_variables->Get(page.condition.variable_id) <= page.condition.variable_value))
-					return false;
-				break;
-			case 3: // >
-				if (!(Main_Data::game_variables->Get(page.condition.variable_id) > page.condition.variable_value))
-					return false;
-				break;
-			case 4: // <
-				if (!(Main_Data::game_variables->Get(page.condition.variable_id) < page.condition.variable_value))
-					return false;
-				break;
-			case 5: // !=
-				if (!(Main_Data::game_variables->Get(page.condition.variable_id) != page.condition.variable_value))
-					return false;
-				break;
-			}
+		if (page.condition.flags.variable && page.condition.compare_operator >= 0 && page.condition.compare_operator <= 5) {
+			if (!Game_Interpreter_Shared::CheckOperator(Main_Data::game_variables->Get(page.condition.variable_id), page.condition.variable_value, page.condition.compare_operator))
+				return false;
 		}
 	}
 

diff --git a/src/game_interpreter.cpp b/src/game_interpreter.cpp
@@ -69,6 +69,8 @@
 #include "algo.h"
 #include "rand.h"
 
+using namespace Game_Interpreter_Shared;
+
 enum BranchSubcommand {
 	eOptionBranchElse = 1
 };
@@ -1055,12 +1057,20 @@ bool Game_Interpreter::CommandInputNumber(lcf::rpg::EventCommand const& com) { /
 }
 
 bool Game_Interpreter::CommandControlSwitches(lcf::rpg::EventCommand const& com) { // code 10210
-	if (com.parameters[0] >= 0 && com.parameters[0] <= 2) {
-		// Param0: 0: Single, 1: Range, 2: Indirect
-		// For Range set end to param 2, otherwise to start, this way the loop runs exactly once
+	{
+		int start, end;
+		bool target_eval_result = DecodeTargetEvaluationMode<
+			/* validate_patches */ true,
+			/* support_range_indirect */ false,
+			/* support_expressions */ false,
+			/* support_bitmask */ false,
+			/* support_scopes */ false
+		>(com, start, end);
+		if (!target_eval_result) {
+			Output::Warning("ControlSwitches: Unsupported target evaluation mode {}", com.parameters[0]);
+			return true;
+		}
 
-		int start = com.parameters[0] == 2 ? Main_Data::game_variables->Get(com.parameters[1]) : com.parameters[1];
-		int end = com.parameters[0] == 1 ? com.parameters[2] : start;
 		int val = com.parameters[3];
 
 		if (start == end) {
@@ -1079,7 +1089,6 @@ bool Game_Interpreter::CommandControlSwitches(lcf::rpg::EventCommand const& com)
 			Game_Map::SetNeedRefresh(true);
 		}
 	}
-
 	return true;
 }
 
@@ -1263,41 +1272,20 @@ bool Game_Interpreter::CommandControlVariables(lcf::rpg::EventCommand const& com
 			return true;
 	}
 
-	int target = com.parameters[0];
-	if (target >= 0 && target <= 4) {
-		// For Range set end to param 2, otherwise to start, this way the loop runs exactly once
-
-		int start, end;
-		if (target == 0) {
-			// Single
-			start = com.parameters[1];
-			end = start;
-		} else if (target == 1) {
-			// Range
-			start = com.parameters[1];
-			end = com.parameters[2];
-		} else if (target == 2) {
-			// Indirect
-			start = Main_Data::game_variables->Get(com.parameters[1]);
-			end = start;
-		} else if (target == 3 && Player::IsPatchManiac()) {
-			// Range Indirect (Maniac)
-			start = Main_Data::game_variables->Get(com.parameters[1]);
-			end = Main_Data::game_variables->Get(com.parameters[2]);
-		} else if (target == 4 && Player::IsPatchManiac()) {
-			// Expression (Maniac)
-			int idx = com.parameters[1];
-			start = ManiacPatch::ParseExpression(MakeSpan(com.parameters).subspan(idx + 1, com.parameters[idx]), *this);
-			end = start;
-		} else {
-			return true;
-		}
-
-		if (Player::IsPatchManiac() && end < start) {
-			// Vanilla does not support end..start, Maniac does
-			std::swap(start, end);
-		}
+	int start, end;
+	bool target_eval_result = DecodeTargetEvaluationMode<
+		/* validate_patches */ true,
+		/* support_range_indirect */ true,
+		/* support_expressions */ true,
+		/* support_bitmask */ false,
+		/* support_scopes */ false
+	>(com, start, end);
+	if (!target_eval_result) {
+		Output::Warning("ControlVariables: Unsupported target evaluation mode {}", com.parameters[0]);
+		return true;
+	}
 
+	{
 		int operation = com.parameters[3];
 		if (EP_UNLIKELY(operation >= 6 && !Player::IsPatchManiac())) {
 			Output::Warning("ControlVariables: Unsupported operation {}", operation);
@@ -1742,78 +1730,6 @@ bool Game_Interpreter::CommandChangeLevel(lcf::rpg::EventCommand const& com) { /
 	return true;
 }
 
-int Game_Interpreter::ValueOrVariable(int mode, int val) {
-	if (mode == 0) {
-		return val;
-	} else if (mode == 1) {
-		return Main_Data::game_variables->Get(val);
-	} else if (Player::IsPatchManiac()) {
-		// Maniac Patch does not implement all modes for all commands
-		// For simplicity it is enabled for all here
-		if (mode == 2) {
-			// Variable indirect
-			return Main_Data::game_variables->GetIndirect(val);
-		} else if (mode == 3) {
-			// Switch (F = 0, T = 1)
-			return Main_Data::game_switches->GetInt(val);
-		} else if (mode == 4) {
-			// Switch through Variable (F = 0, T = 1)
-			return Main_Data::game_switches->GetInt(Main_Data::game_variables->Get(val));
-		}
-	}
-	return -1;
-}
-
-int Game_Interpreter::ValueOrVariableBitfield(int mode, int shift, int val) {
-	return ValueOrVariable((mode & (0xF << shift * 4)) >> shift * 4, val);
-}
-
-int Game_Interpreter::ValueOrVariableBitfield(lcf::rpg::EventCommand const& com, int mode_idx, int shift, int val_idx) {
-	assert(com.parameters.size() > val_idx);
-
-	if (!Player::IsPatchManiac()) {
-		return com.parameters[val_idx];
-	}
-
-	assert(mode_idx != val_idx);
-
-	if (com.parameters.size() > std::max(mode_idx, val_idx)) {
-		int mode = com.parameters[mode_idx];
-		return ValueOrVariableBitfield(com.parameters[mode_idx], shift, com.parameters[val_idx]);
-	}
-
-	return com.parameters[val_idx];
-}
-
-StringView Game_Interpreter::CommandStringOrVariable(lcf::rpg::EventCommand const& com, int mode_idx, int val_idx) {
-	if (!Player::IsPatchManiac()) {
-		return com.string;
-	}
-
-	assert(mode_idx != val_idx);
-
-	if (com.parameters.size() > std::max(mode_idx, val_idx)) {
-		return Main_Data::game_strings->GetWithMode(ToString(com.string), com.parameters[mode_idx], com.parameters[val_idx], *Main_Data::game_variables);
-	}
-
-	return com.string;
-}
-
-StringView Game_Interpreter::CommandStringOrVariableBitfield(lcf::rpg::EventCommand const& com, int mode_idx, int shift, int val_idx) {
-	if (!Player::IsPatchManiac()) {
-		return com.string;
-	}
-
-	assert(mode_idx != val_idx);
-
-	if (com.parameters.size() >= std::max(mode_idx, val_idx) + 1) {
-		int mode = com.parameters[mode_idx];
-		return Main_Data::game_strings->GetWithMode(ToString(com.string), (mode & (0xF << shift * 4)) >> shift * 4, com.parameters[val_idx], *Main_Data::game_variables);
-	}
-
-	return com.string;
-}
-
 bool Game_Interpreter::CommandChangeParameters(lcf::rpg::EventCommand const& com) { // Code 10430
 	int value = OperateValue(
 		com.parameters[2],
@@ -4805,18 +4721,9 @@ bool Game_Interpreter::CommandManiacControlStrings(lcf::rpg::EventCommand const&
 	//		Flags, such as: extract, hex... There is also an edge case where the last argument of exRep is here
 	//
 	//*parameters 4..n - arguments
-	int string_mode = com.parameters[0] & 15;
-	int string_id_0 = com.parameters[1];
-	int string_id_1 = com.parameters[2]; //for ranges
-
-	int is_range = string_mode & 1;
-
-	if (string_mode >= 2) {
-		string_id_0 = Main_Data::game_variables->Get(string_id_0);
-	}
-	if (string_mode == 3) {
-		string_id_1 = Main_Data::game_variables->Get(string_id_1);
-	}
+	bool is_range = com.parameters[0] & 1;
+	int string_id_0, string_id_1;
+	DecodeTargetEvaluationMode<false, true, false, true, false>(com, string_id_0, string_id_1);
 
 	int op = (com.parameters[3] >>  0) & 255;
 	int fn = (com.parameters[3] >>  8) & 255;
@@ -5149,25 +5056,6 @@ bool Game_Interpreter::IsWaitingForWaitCommand() const {
 	return (_state.wait_time > 0) || _state.wait_key_enter;
 }
 
-bool Game_Interpreter::CheckOperator(int val, int val2, int op) const {
-	switch (op) {
-		case 0:
-			return val == val2;
-		case 1:
-			return val >= val2;
-		case 2:
-			return val <= val2;
-		case 3:
-			return val > val2;
-		case 4:
-			return val < val2;
-		case 5:
-			return val != val2;
-		default:
-			return false;
-	}
-}
-
 bool Game_Interpreter::ManiacCheckContinueLoop(int val, int val2, int type, int op) const {
 	switch (type) {
 		case 0: // Infinite loop

diff --git a/src/game_interpreter.h b/src/game_interpreter.h
@@ -25,6 +25,7 @@
 #include "async_handler.h"
 #include "game_character.h"
 #include "game_actor.h"
+#include "game_interpreter_shared.h"
 #include <lcf/dbarray.h>
 #include <lcf/rpg/fwd.h>
 #include <lcf/rpg/eventcommand.h>
@@ -40,7 +41,7 @@ class PendingMessage;
 /**
  * Game_Interpreter class
  */
-class Game_Interpreter
+class Game_Interpreter : public Game_BaseInterpreterContext
 {
 public:
 	using Cmd = lcf::rpg::EventCommand::Code;
@@ -87,13 +88,13 @@ class Game_Interpreter
 	lcf::rpg::SaveEventExecState GetState() const;
 
 	/** @return Game_Character of the passed event_id */
-	Game_Character* GetCharacter(int event_id) const;
+	Game_Character* GetCharacter(int event_id) const override;
 
 	/** @return the event_id of the current frame */
 	int GetCurrentEventId() const;
 
 	/** @return the event_id used by "ThisEvent" in commands */
-	int GetThisEventId() const;
+	int GetThisEventId() const override;
 
 	/** @return the event_id of the event at the base of the call stack */
 	int GetOriginalEventId() const;
@@ -112,7 +113,7 @@ class Game_Interpreter
 	static constexpr int call_stack_limit = 1000;
 	static constexpr int subcommand_sentinel = 255;
 
-	const lcf::rpg::SaveEventExecFrame& GetFrame() const;
+	const lcf::rpg::SaveEventExecFrame& GetFrame() const override;
 	lcf::rpg::SaveEventExecFrame& GetFrame();
 	const lcf::rpg::SaveEventExecFrame* GetFramePtr() const;
 	lcf::rpg::SaveEventExecFrame* GetFramePtr();
@@ -168,12 +169,6 @@ class Game_Interpreter
 	 * @param id actor ID (mode = 1) or variable ID (mode = 2).
 	 */
 	static std::vector<Game_Actor*> GetActors(int mode, int id);
-	static int ValueOrVariable(int mode, int val);
-	static int ValueOrVariableBitfield(int mode, int shift, int val);
-	// Range checked, conditional version (slower) of ValueOrVariableBitfield
-	static int ValueOrVariableBitfield(lcf::rpg::EventCommand const& com, int mode_idx, int shift, int val_idx);
-	static StringView CommandStringOrVariable(lcf::rpg::EventCommand const& com, int mode_idx, int val_idx);
-	static StringView CommandStringOrVariableBitfield(lcf::rpg::EventCommand const& com, int mode_idx, int shift, int val_idx);
 
 	/**
 	 * When current frame finishes executing we pop the stack
@@ -334,7 +329,6 @@ class Game_Interpreter
 		void toSave(lcf::rpg::SaveEventExecState& save) const;
 	};
 
-	bool CheckOperator(int val, int val2, int op) const;
 	bool ManiacCheckContinueLoop(int val, int val2, int type, int op) const;
 	int ManiacBitmask(int value, int mask) const;
 

diff --git a/src/game_interpreter_control_variables.cpp b/src/game_interpreter_control_variables.cpp
@@ -15,6 +15,7 @@
  * along with EasyRPG Player. If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "game_interpreter_shared.h"
 #include "game_interpreter_control_variables.h"
 #include "game_actors.h"
 #include "game_enemyparty.h"
@@ -154,7 +155,7 @@ int ControlVariables::Party(int op, int party_idx) {
 	return ControlVariables::Actor(op, actor->GetId());
 }
 
-int ControlVariables::Event(int op, int event_id, const Game_Interpreter& interpreter) {
+int ControlVariables::Event(int op, int event_id, const Game_BaseInterpreterContext& interpreter) {
 	auto character = interpreter.GetCharacter(event_id);
 	if (character) {
 		switch (op) {

diff --git a/src/game_interpreter_control_variables.h b/src/game_interpreter_control_variables.h
@@ -21,14 +21,14 @@
 
 #include <lcf/rpg/eventcommand.h>
 
-class Game_Interpreter;
+class Game_BaseInterpreterContext;
 
 namespace ControlVariables {
 	int Random(int value, int value2);
 	int Item(int op, int item);
 	int Actor(int op, int actor_id);
 	int Party(int op, int party_idx);
-	int Event(int op, int event_id, const Game_Interpreter& interpreter);
+	int Event(int op, int event_id, const Game_BaseInterpreterContext& interpreter);
 	int Other(int op);
 	int Enemy(int op, int enemy_idx);
 	int Pow(int arg1, int arg2);

diff --git a/src/game_interpreter_map.cpp b/src/game_interpreter_map.cpp
@@ -76,6 +76,8 @@ enum InnSubcommand {
 	eOptionInnNoStay = 1,
 };
 
+using namespace Game_Interpreter_Shared;
+
 void Game_Interpreter_Map::SetState(const lcf::rpg::SaveEventExecState& save) {
 	Clear();
 	_state = save;