From 9a957f1d4b0d033f9d97ff2f54b2cdd4f18ffada Mon Sep 17 00:00:00 2001
From: glx22 <glx@openttd.org>
Date: Sun, 5 Mar 2023 17:00:16 +0100
Subject: [PATCH] Change: [Script] Improve ScriptText validation error messages

---
 src/game/game_text.cpp         | 14 ++++++++++++++
 src/game/game_text.hpp         |  1 +
 src/script/api/script_text.cpp | 15 ++++++++-------
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/src/game/game_text.cpp b/src/game/game_text.cpp
index 57ef1f97ec..e3585de2e1 100644
--- a/src/game/game_text.cpp
+++ b/src/game/game_text.cpp
@@ -354,6 +354,20 @@ const StringParams &GetGameStringParams(uint id)
 	return _current_data->string_params[id];
 }
 
+/**
+ * Get the name of a particular game string.
+ * @param id The ID of the game string.
+ * @return The name of the string.
+ */
+const std::string &GetGameStringName(uint id)
+{
+	/* The name for STR_UNDEFINED. */
+	static const std::string undefined = "STR_UNDEFINED";
+
+	if (id >= _current_data->string_names.size()) return undefined;
+	return _current_data->string_names[id];
+}
+
 /**
  * Register the current translation to the Squirrel engine.
  * @param engine The engine to update/
diff --git a/src/game/game_text.hpp b/src/game/game_text.hpp
index 7aebf885e3..0f4c49698d 100644
--- a/src/game/game_text.hpp
+++ b/src/game/game_text.hpp
@@ -29,6 +29,7 @@ using StringParamsList = std::vector<StringParams>;
 
 const char *GetGameStringPtr(uint id);
 const StringParams &GetGameStringParams(uint id);
+const std::string &GetGameStringName(uint id);
 void RegisterGameTranslation(class Squirrel *engine);
 void ReconsiderGameScriptLanguage();
 
diff --git a/src/script/api/script_text.cpp b/src/script/api/script_text.cpp
index e634e14d43..03c00fdae5 100644
--- a/src/script/api/script_text.cpp
+++ b/src/script/api/script_text.cpp
@@ -161,7 +161,7 @@ const std::string ScriptText::GetEncodedText()
 	static char buf[1024];
 	int param_count = 0;
 	this->_GetEncodedText(buf, lastof(buf), param_count);
-	if (param_count > SCRIPT_TEXT_MAX_PARAMETERS) throw Script_FatalError("A string had too many parameters");
+	if (param_count > SCRIPT_TEXT_MAX_PARAMETERS) throw Script_FatalError(fmt::format("{}: Too many parameters", GetGameStringName(this->string)));
 	return buf;
 }
 
@@ -171,30 +171,31 @@ char *ScriptText::_GetEncodedText(char *p, char *lastofp, int &param_count)
 	p += seprintf(p, lastofp, "%X", this->string);
 
 	const StringParams &params = GetGameStringParams(this->string);
+	const std::string &name = GetGameStringName(this->string);
 	int cur_idx = 0;
 
 	for (const StringParam &cur_param : params) {
-		if (cur_idx >= this->paramc) throw Script_FatalError("Not enough string parameters");
+		if (cur_idx >= this->paramc) throw Script_FatalError(fmt::format("{}: Not enough parameters", name));
 
 		switch (cur_param.type) {
 			case StringParam::RAW_STRING:
-				if (!std::holds_alternative<std::string>(this->param[cur_idx])) throw Script_FatalError("Wrong string parameter type");
+				if (!std::holds_alternative<std::string>(this->param[cur_idx])) throw Script_FatalError(fmt::format("{}: Parameter {} expects a raw string", name, cur_idx));
 				p += seprintf(p, lastofp, ":\"%s\"", std::get<std::string>(this->param[cur_idx++]).c_str());
 				break;
 
 			case StringParam::STRING: {
-				if (!std::holds_alternative<ScriptTextRef>(this->param[cur_idx])) throw Script_FatalError("Wrong string parameter type");
+				if (!std::holds_alternative<ScriptTextRef>(this->param[cur_idx])) throw Script_FatalError(fmt::format("{}: Parameter {} expects a substring", name, cur_idx));
 				int count = 1; // 1 because the string id is included in consumed parameters
 				p += seprintf(p, lastofp, ":");
 				p = std::get<ScriptTextRef>(this->param[cur_idx++])->_GetEncodedText(p, lastofp, count);
-				if (count != cur_param.consumes) throw Script_FatalError("Substring doesn't consume the expected amount of parameters.");
+				if (count != cur_param.consumes) throw Script_FatalError(fmt::format("{}: Parameter {} substring consumes {}, but expected {} to be consumed", name, cur_idx, count - 1, cur_param.consumes - 1));
 				break;
 			}
 
 			default:
-				if (cur_idx + cur_param.consumes > this->paramc) throw Script_FatalError("Not enough string parameters");
+				if (cur_idx + cur_param.consumes > this->paramc) throw Script_FatalError(fmt::format("{}: Not enough parameters", name));
 				for (int i = 0; i < cur_param.consumes; i++) {
-					if (!std::holds_alternative<SQInteger>(this->param[cur_idx])) throw Script_FatalError("Wrong string parameter type");
+					if (!std::holds_alternative<SQInteger>(this->param[cur_idx])) throw Script_FatalError(fmt::format("{}: Parameter {} expects an integer", name, cur_idx));
 					p += seprintf(p, lastofp,":" OTTD_PRINTFHEX64, std::get<SQInteger>(this->param[cur_idx++]));
 				}
 		}