Fix issues reported by zoeontheloose + Undefined Behaviour in ControlSwitches

src/game_interpreter.cpp

@@ -1059,37 +1059,39 @@ bool Game_Interpreter::CommandInputNumber(lcf::rpg::EventCommand const& com) { /
 }
 
 bool Game_Interpreter::CommandControlSwitches(lcf::rpg::EventCommand const& com) { // code 10210
-	{
-		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, 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 val = com.parameters[3];
+	int mode = com.parameters[3];
 
-		if (start == end) {
-			if (val < 2) {
-				Main_Data::game_switches->Set(start, val == 0);
-			} else {
-				Main_Data::game_switches->Flip(start);
-			}
-			Game_Map::SetNeedRefreshForSwitchChange(start);
+	if (start == end) {
+		if (mode == 0 || mode == 1) {
+			Main_Data::game_switches->Set(start, mode == 0);
+		} else if (mode == 2) {
+			Main_Data::game_switches->Flip(start);
 		} else {
-			if (val < 2) {
-				Main_Data::game_switches->SetRange(start, end, val == 0);
-			} else {
-				Main_Data::game_switches->FlipRange(start, end);
-			}
-			Game_Map::SetNeedRefresh(true);
+			Output::Debug("ControlSwitch: Unknown mode {}", mode);
 		}
+		Game_Map::SetNeedRefreshForSwitchChange(start);
+	} else {
+		if (mode == 0 || mode == 1) {
+			Main_Data::game_switches->SetRange(start, end, mode == 0);
+		} else if (mode == 2) {
+			Main_Data::game_switches->FlipRange(start, end);
+		} else {
+			Output::Debug("ControlSwitch: Unknown mode {}", mode);
+		}
+		Game_Map::SetNeedRefresh(true);
 	}
 	return true;
 }
@@ -1545,19 +1547,20 @@ std::vector<Game_Actor*> Game_Interpreter::GetActors(int mode, int id) {
 	return actors;
 }
 
-Game_Character* Game_Interpreter::GetCharacter(int event_id, std::string_view origin) const {
+Game_Character* Game_Interpreter::GetCharacter(int event_id, std::string_view origin, bool silent) const {
 	if (event_id == Game_Character::CharThisEvent) {
 		event_id = GetThisEventId();
 		// Is a common event
 		if (event_id == 0) {
 			// With no map parent
+			// Still reported even when "silent" as this would hide a bug
 			Output::Warning("{}: Can't use ThisEvent in common event: Not called from a map event", origin);
 			return nullptr;
 		}
 	}
 
 	Game_Character* ch = Game_Character::GetCharacter(event_id, event_id);
-	if (!ch) {
+	if (!ch && !silent) {
 		Output::Warning("{}: Unknown event with id {}", origin, event_id);
 	}
 	return ch;
@@ -2809,10 +2812,7 @@ bool Game_Interpreter::CommandShowPicture(lcf::rpg::EventCommand const& com) { /
 			pic_id = ValueOrVariable(com.parameters[17], pic_id);
 		}
 		if (com.parameters[19] != 0) {
-			int var = 0;
-			if (Main_Data::game_variables->IsValid(com.parameters[19])) {
-				var = Main_Data::game_variables->Get(com.parameters[19]);
-			}
+			int var = Main_Data::game_variables->Get(com.parameters[19]);
 			params.name = PicPointerPatch::ReplaceName(params.name, var, com.parameters[18]);
 		}
 
@@ -3623,9 +3623,14 @@ bool Game_Interpreter::CommandConditionalBranch(lcf::rpg::EventCommand const& co
 			chara_id = ValueOrVariable(com.parameters[3], chara_id);
 		}
 
-		character = GetCharacter(chara_id, "ConditionalBranch");
-		if (character != NULL) {
-			result = character->GetFacing() == com.parameters[2];
+		if (Player::IsPatchManiac() && com.parameters[4] == 1) {
+			// Existance check
+			result = GetCharacter(chara_id, "ConditionalBranch", true) != nullptr;
+		} else {
+			character = GetCharacter(chara_id, "ConditionalBranch");
+			if (character) {
+				result = character->GetFacing() == com.parameters[2];
+			}
 		}
 		break;
 	}
@@ -4674,7 +4679,31 @@ bool Game_Interpreter::CommandManiacShowStringPicture(lcf::rpg::EventCommand con
 	}
 
 	params.system_name = components[2];
+	uint32_t var_id = 0;
+	auto mode = delims[1] - 1;
+	if (mode > 0) {
+		if (!ManiacPatch::DecodeStringToInt(params.system_name, var_id)) {
+			Output::Warning("ShowStringPic: Bad system name arg (id={}, arg={})", pic_id, components[2]);
+			return true;
+		}
+
+		params.system_name = Main_Data::game_strings->GetWithMode(
+			params.system_name, mode, var_id, *Main_Data::game_variables
+		);
+	}
+
 	text.font_name = components[3];
+	mode = delims[2] - 1;
+	if (mode > 0) {
+		if (!ManiacPatch::DecodeStringToInt(text.font_name, var_id)) {
+			Output::Warning("ShowStringPic: Bad font name arg (id={}, arg={})", pic_id, components[3]);
+			return true;
+		}
+
+		text.font_name = Main_Data::game_strings->GetWithMode(
+			text.font_name, mode, var_id, *Main_Data::game_variables
+		);
+	}
 
 	params.texts = {text};
 

src/game_interpreter.h

@@ -104,7 +104,7 @@ class Game_Interpreter : public Game_BaseInterpreterContext
 	lcf::rpg::SaveEventExecState GetSaveState();
 
 	/** @return Game_Character of the passed event_id */
-	Game_Character* GetCharacter(int event_id, std::string_view origin) const override;
+	Game_Character* GetCharacter(int event_id, std::string_view origin, bool silent = false) const override;
 
 	/** @return the event_id of the current frame */
 	int GetCurrentEventId() const;

src/game_interpreter_shared.cpp

@@ -78,6 +78,10 @@ inline bool Game_Interpreter_Shared::DecodeTargetEvaluationMode(lcf::rpg::EventC
 				return true;
 			}
 			break;
+		default:
+			id_0 = 0;
+			id_1 = 0;
+			break;
 	}
 
 	if constexpr (validate_patches) {

src/game_interpreter_shared.h

@@ -252,7 +252,7 @@ class Game_BaseInterpreterContext {
 	virtual ~Game_BaseInterpreterContext() {}
 
 	virtual int GetThisEventId() const = 0;
-	virtual Game_Character* GetCharacter(int event_id, std::string_view origin) const = 0;
+	virtual Game_Character* GetCharacter(int event_id, std::string_view origin, bool silent = false) const = 0;
 
 	virtual const lcf::rpg::SaveEventExecState& GetState() const = 0;
 	virtual const lcf::rpg::SaveEventExecFrame& GetFrame() const = 0;

src/game_strings.cpp

@@ -92,7 +92,7 @@ int Game_Strings::ToNum(Str_Params params, int var_id, Game_Variables& variables
 	if (params.hex)
 		num = static_cast<int>(std::strtol(it->second.c_str(), nullptr, 16));
 	else
-		num = static_cast<int>(std::strtol(it->second.c_str(), nullptr, 0));
+		num = static_cast<int>(std::strtol(it->second.c_str(), nullptr, 10));
 
 	variables.Set(var_id, num);
 

src/maniac_patch.cpp

@@ -834,6 +834,50 @@ std::string_view ManiacPatch::GetLcfDescription(int data_type, int id, bool is_d
 	return {};
 }
 
+bool ManiacPatch::DecodeStringToInt(std::string_view str, uint32_t& out) {
+	/*
+	Is a custom 5 bit encoding:
+
+	For the rightmost character c the value is:
+	c - A + 1
+
+	For all other characters c with index i the value is:
+	(c - a + 1) << (i * 5)
+
+	Due to integer overflow the max string length is 6.
+	*/
+
+	if (str.empty() || str.size() > 6) {
+		out = 0;
+		return false;
+	}
+
+	auto in_range = [](uint32_t value) {
+		return value < 32;
+	};
+
+	out = (str.back() - 'A' + 1);
+
+	if (!in_range(out)) {
+		return false;
+	}
+
+	str.remove_suffix(1);
+
+	for (size_t i = 0; i < str.size(); ++i) {
+		uint32_t result = (str[i] - 'a' + 1);
+
+		if (!in_range(result)) {
+			return false;
+		}
+
+		int chidx = str.size() - i;
+		out += (result << (chidx * 5));
+	}
+
+	return true;
+}
+
 bool ManiacPatch::GlobalSave::Load() {
 	if (!Player::IsPatchManiac()) {
 		return true;

src/maniac_patch.h

@@ -38,6 +38,17 @@ namespace ManiacPatch {
 	std::string_view GetLcfName(int data_type, int id, bool is_dynamic);
 	std::string_view GetLcfDescription(int data_type, int id, bool is_dynamic);
 
+	/**
+	 * Decodes an encoded string as used by the ShowStringPicture to the number
+	 * representation.
+	 *
+	 * @param str String to decode
+	 * @param out The number represented by the string
+	 *
+	 * @return Whether the decoding was successful (this function will reject malformed strings)
+	 */
+	bool DecodeStringToInt(std::string_view str, uint32_t& out);
+
 	namespace GlobalSave {
 		/**
 		* Attempts to load Save.lgs from the save directory.