[Editor] Fix return of `EditorTranslationParserPlugin._parse_file`

Merged `_get_comments` into `_parse_file` and changed to using a
returned `Array[PackedStringArray]` instead.

doc/classes/EditorTranslationParserPlugin.xml

@@ -5,8 +5,7 @@
 	</brief_description>
 	<description>
 		[EditorTranslationParserPlugin] is invoked when a file is being parsed to extract strings that require translation. To define the parsing and string extraction logic, override the [method _parse_file] method in script.
-		Add the extracted strings to argument [code]msgids[/code] or [code]msgids_context_plural[/code] if context or plural is used.
-		When adding to [code]msgids_context_plural[/code], you must add the data using the format [code]["A", "B", "C"][/code], where [code]A[/code] represents the extracted string, [code]B[/code] represents the context, and [code]C[/code] represents the plural version of the extracted string. If you want to add only context but not plural, put [code]""[/code] for the plural slot. The idea is the same if you only want to add plural but not context. See the code below for concrete examples.
+		The return value should be an [Array] of [PackedStringArray]s, one for each extracted translatable string. Each entry should contain [code][msgid, msgctxt, msgid_plural, comment][/code], where all except [code]msgid[/code] are optional. Empty strings will be ignored.
 		The extracted strings will be written into a POT file selected by user under "POT Generation" in "Localization" tab in "Project Settings" menu.
 		Below shows an example of a custom parser that extracts strings from a CSV file to write into a POT.
 		[codeblocks]
@@ -14,14 +13,17 @@
 		@tool
 		extends EditorTranslationParserPlugin
 
-		func _parse_file(path, msgids, msgids_context_plural):
+		func _parse_file(path):
+		    var ret: Array[PackedStringArray] = []
 		    var file = FileAccess.open(path, FileAccess.READ)
 		    var text = file.get_as_text()
 		    var split_strs = text.split(",", false)
 		    for s in split_strs:
-		        msgids.append(s)
+		        msgids.append(PackedStringArray([s]))
 		        #print("Extracted string: " + s)
 
+		    return ret
+
 		func _get_recognized_extensions():
 		    return ["csv"]
 		[/gdscript]
@@ -31,16 +33,18 @@
 		[Tool]
 		public partial class CustomParser : EditorTranslationParserPlugin
 		{
-		    public override void _ParseFile(string path, Godot.Collections.Array&lt;string&gt; msgids, Godot.Collections.Array&lt;Godot.Collections.Array&gt; msgidsContextPlural)
+		    public override Godot.Collections.Array&lt;string[]&gt; _ParseFile(string path)
 		    {
+		        Godot.Collections.Array&lt;string[]&gt; ret;
 		        using var file = FileAccess.Open(path, FileAccess.ModeFlags.Read);
 		        string text = file.GetAsText();
 		        string[] splitStrs = text.Split(",", allowEmpty: false);
 		        foreach (string s in splitStrs)
 		        {
-		            msgids.Add(s);
+		            ret.Add([s]);
 		            //GD.Print($"Extracted string: {s}");
 		        }
+		        return ret;
 		    }
 
 		    public override string[] _GetRecognizedExtensions()
@@ -50,29 +54,29 @@
 		}
 		[/csharp]
 		[/codeblocks]
-		To add a translatable string associated with context or plural, add it to [code]msgids_context_plural[/code]:
+		To add a translatable string associated with a context, plural, or comment:
 		[codeblocks]
 		[gdscript]
-		# This will add a message with msgid "Test 1", msgctxt "context", and msgid_plural "test 1 plurals".
-		msgids_context_plural.append(["Test 1", "context", "test 1 plurals"])
+		# This will add a message with msgid "Test 1", msgctxt "context", msgid_plural "test 1 plurals", and comment "test 1 comment".
+		ret.append(PackedStringArray(["Test 1", "context", "test 1 plurals", "test 1 comment"]))
 		# This will add a message with msgid "A test without context" and msgid_plural "plurals".
-		msgids_context_plural.append(["A test without context", "", "plurals"])
+		ret.append(PackedStringArray(["A test without context", "", "plurals"]))
 		# This will add a message with msgid "Only with context" and msgctxt "a friendly context".
-		msgids_context_plural.append(["Only with context", "a friendly context", ""])
+		ret.append(PackedStringArray(["Only with context", "a friendly context"]))
 		[/gdscript]
 		[csharp]
-		// This will add a message with msgid "Test 1", msgctxt "context", and msgid_plural "test 1 plurals".
-		msgidsContextPlural.Add(["Test 1", "context", "test 1 Plurals"]);
+		// This will add a message with msgid "Test 1", msgctxt "context", msgid_plural "test 1 plurals", and comment "test 1 comment".
+		ret.Add(["Test 1", "context", "test 1 plurals", "test 1 comment"]);
 		// This will add a message with msgid "A test without context" and msgid_plural "plurals".
-		msgidsContextPlural.Add(["A test without context", "", "plurals"]);
+		ret.Add(["A test without context", "", "plurals"]);
 		// This will add a message with msgid "Only with context" and msgctxt "a friendly context".
-		msgidsContextPlural.Add(["Only with context", "a friendly context", ""]);
+		ret.Add(["Only with context", "a friendly context"]);
 		[/csharp]
 		[/codeblocks]
 		[b]Note:[/b] If you override parsing logic for standard script types (GDScript, C#, etc.), it would be better to load the [code]path[/code] argument using [method ResourceLoader.load]. This is because built-in scripts are loaded as [Resource] type, not [FileAccess] type. For example:
 		[codeblocks]
 		[gdscript]
-		func _parse_file(path, msgids, msgids_context_plural):
+		func _parse_file(path):
 		    var res = ResourceLoader.load(path, "Script")
 		    var text = res.source_code
 		    # Parsing logic.
@@ -81,7 +85,7 @@
 		    return ["gd"]
 		[/gdscript]
 		[csharp]
-		public override void _ParseFile(string path, Godot.Collections.Array&lt;string&gt; msgids, Godot.Collections.Array&lt;Godot.Collections.Array&gt; msgidsContextPlural)
+		public override Godot.Collections.Array&lt;string[]&gt; _ParseFile(string path)
 		{
 		    var res = ResourceLoader.Load&lt;Script&gt;(path, "Script");
 		    string text = res.SourceCode;
@@ -99,14 +103,6 @@
 	<tutorials>
 	</tutorials>
 	<methods>
-		<method name="_get_comments" qualifiers="virtual">
-			<return type="void" />
-			<param index="0" name="msgids_comment" type="String[]" />
-			<param index="1" name="msgids_context_plural_comment" type="String[]" />
-			<description>
-				If overridden, called after [method _parse_file] to get comments for the parsed entries. This method should fill the arrays with the same number of elements and in the same order as [method _parse_file].
-			</description>
-		</method>
 		<method name="_get_recognized_extensions" qualifiers="virtual const">
 			<return type="PackedStringArray" />
 			<description>
@@ -114,10 +110,8 @@
 			</description>
 		</method>
 		<method name="_parse_file" qualifiers="virtual">
-			<return type="void" />
+			<return type="PackedStringArray[]" />
 			<param index="0" name="path" type="String" />
-			<param index="1" name="msgids" type="String[]" />
-			<param index="2" name="msgids_context_plural" type="Array[]" />
 			<description>
 				Override this method to define a custom parsing logic to extract the translatable strings.
 			</description>

editor/editor_translation_parser.cpp

@@ -36,47 +36,41 @@
 
 EditorTranslationParser *EditorTranslationParser::singleton = nullptr;
 
-Error EditorTranslationParserPlugin::parse_file(const String &p_path, Vector<String> *r_ids, Vector<Vector<String>> *r_ids_ctx_plural) {
+Error EditorTranslationParserPlugin::parse_file(const String &p_path, Vector<Vector<String>> *r_translations) {
+	TypedArray<PackedStringArray> ret;
+
+	if (GDVIRTUAL_CALL(_parse_file, p_path, ret)) {
+		// Copy over entries directly.
+		for (const PackedStringArray translation : ret) {
+			r_translations->push_back(translation);
+		}
+
+		return OK;
+	}
+
+#ifndef DISABLE_DEPRECATED
 	TypedArray<String> ids;
 	TypedArray<Array> ids_ctx_plural;
 
-	if (GDVIRTUAL_CALL(_parse_file, p_path, ids, ids_ctx_plural)) {
+	if (GDVIRTUAL_CALL(_parse_file_bind_compat_99297, p_path, ids, ids_ctx_plural)) {
 		// Add user's extracted translatable messages.
 		for (int i = 0; i < ids.size(); i++) {
-			r_ids->append(ids[i]);
+			r_translations->push_back({ ids[i] });
 		}
 
 		// Add user's collected translatable messages with context or plurals.
 		for (int i = 0; i < ids_ctx_plural.size(); i++) {
 			Array arr = ids_ctx_plural[i];
-			ERR_FAIL_COND_V_MSG(arr.size() != 3, ERR_INVALID_DATA, "Array entries written into `msgids_context_plural` in `parse_file()` method should have the form [\"message\", \"context\", \"plural message\"]");
+			ERR_FAIL_COND_V_MSG(arr.size() != 3, ERR_INVALID_DATA, "Array entries written into `msgids_context_plural` in `_parse_file` method should have the form [\"message\", \"context\", \"plural message\"]");
 
-			Vector<String> id_ctx_plural;
-			id_ctx_plural.push_back(arr[0]);
-			id_ctx_plural.push_back(arr[1]);
-			id_ctx_plural.push_back(arr[2]);
-			r_ids_ctx_plural->append(id_ctx_plural);
+			r_translations->push_back({ arr[0], arr[1], arr[2] });
 		}
 		return OK;
-	} else {
-		ERR_PRINT("Custom translation parser plugin's \"func parse_file(path, extracted_strings)\" is undefined.");
-		return ERR_UNAVAILABLE;
 	}
-}
-
-void EditorTranslationParserPlugin::get_comments(Vector<String> *r_ids_comment, Vector<String> *r_ids_ctx_plural_comment) {
-	TypedArray<String> ids_comment;
-	TypedArray<String> ids_ctx_plural_comment;
-
-	if (GDVIRTUAL_CALL(_get_comments, ids_comment, ids_ctx_plural_comment)) {
-		for (int i = 0; i < ids_comment.size(); i++) {
-			r_ids_comment->append(ids_comment[i]);
-		}
+#endif // DISABLE_DEPRECATED
 
-		for (int i = 0; i < ids_ctx_plural_comment.size(); i++) {
-			r_ids_ctx_plural_comment->append(ids_ctx_plural_comment[i]);
-		}
-	}
+	ERR_PRINT("Custom translation parser plugin's \"_parse_file\" is undefined.");
+	return ERR_UNAVAILABLE;
 }
 
 void EditorTranslationParserPlugin::get_recognized_extensions(List<String> *r_extensions) const {
@@ -91,9 +85,12 @@ void EditorTranslationParserPlugin::get_recognized_extensions(List<String> *r_ex
 }
 
 void EditorTranslationParserPlugin::_bind_methods() {
-	GDVIRTUAL_BIND(_parse_file, "path", "msgids", "msgids_context_plural");
-	GDVIRTUAL_BIND(_get_comments, "msgids_comment", "msgids_context_plural_comment");
+	GDVIRTUAL_BIND(_parse_file, "path");
 	GDVIRTUAL_BIND(_get_recognized_extensions);
+
+#ifndef DISABLE_DEPRECATED
+	GDVIRTUAL_BIND_COMPAT(_parse_file_bind_compat_99297, "path", "msgids", "msgids_context_plural");
+#endif
 }
 
 /////////////////////////

editor/editor_translation_parser.h

@@ -42,13 +42,15 @@ class EditorTranslationParserPlugin : public RefCounted {
 protected:
 	static void _bind_methods();
 
-	GDVIRTUAL3(_parse_file, String, TypedArray<String>, TypedArray<Array>)
-	GDVIRTUAL2(_get_comments, TypedArray<String>, TypedArray<String>)
+	GDVIRTUAL1R(TypedArray<PackedStringArray>, _parse_file, String)
 	GDVIRTUAL0RC(Vector<String>, _get_recognized_extensions)
 
+#ifndef DISABLE_DEPRECATED
+	GDVIRTUAL3_COMPAT(_parse_file_bind_compat_99297, _parse_file, String, TypedArray<String>, TypedArray<Array>)
+#endif
+
 public:
-	virtual Error parse_file(const String &p_path, Vector<String> *r_ids, Vector<Vector<String>> *r_ids_ctx_plural);
-	virtual void get_comments(Vector<String> *r_ids_comment, Vector<String> *r_ids_ctx_plural_comment);
+	virtual Error parse_file(const String &p_path, Vector<Vector<String>> *r_translations);
 	virtual void get_recognized_extensions(List<String> *r_extensions) const;
 };
 

editor/plugins/packed_scene_translation_parser_plugin.cpp

@@ -38,7 +38,7 @@ void PackedSceneEditorTranslationParserPlugin::get_recognized_extensions(List<St
 	ResourceLoader::get_recognized_extensions_for_type("PackedScene", r_extensions);
 }
 
-Error PackedSceneEditorTranslationParserPlugin::parse_file(const String &p_path, Vector<String> *r_ids, Vector<Vector<String>> *r_ids_ctx_plural) {
+Error PackedSceneEditorTranslationParserPlugin::parse_file(const String &p_path, Vector<Vector<String>> *r_translations) {
 	// Parse specific scene Node's properties (see in constructor) that are auto-translated by the engine when set. E.g Label's text property.
 	// These properties are translated with the tr() function in the C++ code when being set or updated.
 
@@ -52,9 +52,9 @@ Error PackedSceneEditorTranslationParserPlugin::parse_file(const String &p_path,
 	ERR_FAIL_COND_V_MSG(packed_scene.is_null(), ERR_FILE_UNRECOGNIZED, vformat("'%s' is not a valid PackedScene resource.", p_path));
 	Ref<SceneState> state = packed_scene->get_state();
 
-	Vector<String> parsed_strings;
 	Vector<Pair<NodePath, bool>> atr_owners;
 	Vector<String> tabcontainer_paths;
+
 	for (int i = 0; i < state->get_node_count(); i++) {
 		String node_type = state->get_node_type(i);
 		String parent_path = state->get_node_path(i, true);
@@ -122,10 +122,9 @@ Error PackedSceneEditorTranslationParserPlugin::parse_file(const String &p_path,
 
 			if (auto_translating && !tabcontainer_paths.is_empty() && ClassDB::is_parent_class(node_type, "Control") &&
 					parent_path == tabcontainer_paths[tabcontainer_paths.size() - 1]) {
-				parsed_strings.push_back(state->get_node_name(i));
+				r_translations->push_back({ state->get_node_name(i) });
 			}
 		}
-
 		if (!auto_translating) {
 			continue;
 		}
@@ -151,11 +150,7 @@ Error PackedSceneEditorTranslationParserPlugin::parse_file(const String &p_path,
 
 				String extension = s->get_language()->get_extension();
 				if (EditorTranslationParser::get_singleton()->can_parse(extension)) {
-					Vector<String> temp;
-					Vector<Vector<String>> ids_context_plural;
-					EditorTranslationParser::get_singleton()->get_parser(extension)->parse_file(s->get_path(), &temp, &ids_context_plural);
-					parsed_strings.append_array(temp);
-					r_ids_ctx_plural->append_array(ids_context_plural);
+					EditorTranslationParser::get_singleton()->get_parser(extension)->parse_file(s->get_path(), r_translations);
 				}
 			} else if (node_type == "FileDialog" && property_name == "filters") {
 				// Extract FileDialog's filters property with values in format "*.png ; PNG Images","*.gd ; GDScript Files".
@@ -163,21 +158,19 @@ Error PackedSceneEditorTranslationParserPlugin::parse_file(const String &p_path,
 				for (int k = 0; k < str_values.size(); k++) {
 					String desc = str_values[k].get_slice(";", 1).strip_edges();
 					if (!desc.is_empty()) {
-						parsed_strings.push_back(desc);
+						r_translations->push_back({ desc });
 					}
 				}
 			} else if (property_value.get_type() == Variant::STRING) {
 				String str_value = String(property_value);
 				// Prevent reading text containing only spaces.
 				if (!str_value.strip_edges().is_empty()) {
-					parsed_strings.push_back(str_value);
+					r_translations->push_back({ str_value });
 				}
 			}
 		}
 	}
 
-	r_ids->append_array(parsed_strings);
-
 	return OK;
 }
 

editor/plugins/packed_scene_translation_parser_plugin.h

@@ -42,7 +42,7 @@ class PackedSceneEditorTranslationParserPlugin : public EditorTranslationParserP
 	HashMap<String, Vector<String>> exception_list;
 
 public:
-	virtual Error parse_file(const String &p_path, Vector<String> *r_ids, Vector<Vector<String>> *r_ids_ctx_plural) override;
+	virtual Error parse_file(const String &p_path, Vector<Vector<String>> *r_translations) override;
 	bool match_property(const String &p_property_name, const String &p_node_type);
 	virtual void get_recognized_extensions(List<String> *r_extensions) const override;
 

editor/pot_generator.cpp

@@ -67,31 +67,24 @@ void POTGenerator::generate_pot(const String &p_file) {
 
 	// Collect all translatable strings according to files order in "POT Generation" setting.
 	for (int i = 0; i < files.size(); i++) {
-		Vector<String> msgids;
-		Vector<Vector<String>> msgids_context_plural;
-
-		Vector<String> msgids_comment;
-		Vector<String> msgids_context_plural_comment;
+		Vector<Vector<String>> translations;
 
 		const String &file_path = files[i];
 		String file_extension = file_path.get_extension();
 
 		if (EditorTranslationParser::get_singleton()->can_parse(file_extension)) {
-			EditorTranslationParser::get_singleton()->get_parser(file_extension)->parse_file(file_path, &msgids, &msgids_context_plural);
-			EditorTranslationParser::get_singleton()->get_parser(file_extension)->get_comments(&msgids_comment, &msgids_context_plural_comment);
+			EditorTranslationParser::get_singleton()->get_parser(file_extension)->parse_file(file_path, &translations);
 		} else {
 			ERR_PRINT("Unrecognized file extension " + file_extension + " in generate_pot()");
 			return;
 		}
 
-		for (int j = 0; j < msgids_context_plural.size(); j++) {
-			const Vector<String> &entry = msgids_context_plural[j];
-			const String &comment = (j < msgids_context_plural_comment.size()) ? msgids_context_plural_comment[j] : String();
-			_add_new_msgid(entry[0], entry[1], entry[2], file_path, comment);
-		}
-		for (int j = 0; j < msgids.size(); j++) {
-			const String &comment = (j < msgids_comment.size()) ? msgids_comment[j] : String();
-			_add_new_msgid(msgids[j], "", "", file_path, comment);
+		for (const Vector<String> &translation : translations) {
+			ERR_CONTINUE(translation.is_empty());
+			const String &msgctxt = (translation.size() > 1) ? translation[1] : String();
+			const String &msgid_plural = (translation.size() > 2) ? translation[2] : String();
+			const String &comment = (translation.size() > 3) ? translation[3] : String();
+			_add_new_msgid(translation[0], msgctxt, msgid_plural, file_path, comment);
 		}
 	}
 

misc/extension_api_validation/4.3-stable.expected

@@ -326,3 +326,11 @@ GH-102796
 Validate extension JSON: Error: Field 'classes/GraphEdit/signals/frame_rect_changed/arguments/1': type changed value in new API, from "Vector2" to "Rect2".
 
 Previous type was incorrect. No compatibility system for signal arguments.
+
+
+GH-99297
+--------
+Validate extension JSON: Error: Field 'classes/EditorTranslationParserPlugin/methods/_parse_file/arguments': size changed value in new API, from 3 to 1.
+Validate extension JSON: JSON file: Field was added in a way that breaks compatibility 'classes/EditorTranslationParserPlugin/methods/_parse_file': return_value
+
+Returning by argument reference is not safe in extensions, changed to returning as an Array and merged with `get_comments`. Compatibility method registered.

modules/gdscript/editor/gdscript_translation_parser_plugin.cpp

@@ -39,7 +39,7 @@ void GDScriptEditorTranslationParserPlugin::get_recognized_extensions(List<Strin
 	GDScriptLanguage::get_singleton()->get_recognized_extensions(r_extensions);
 }
 
-Error GDScriptEditorTranslationParserPlugin::parse_file(const String &p_path, Vector<String> *r_ids, Vector<Vector<String>> *r_ids_ctx_plural) {
+Error GDScriptEditorTranslationParserPlugin::parse_file(const String &p_path, Vector<Vector<String>> *r_translations) {
 	// Extract all translatable strings using the parsed tree from GDScriptParser.
 	// The strategy is to find all ExpressionNode and AssignmentNode from the tree and extract strings if relevant, i.e
 	// Search strings in ExpressionNode -> CallNode -> tr(), set_text(), set_placeholder() etc.
@@ -49,11 +49,7 @@ Error GDScriptEditorTranslationParserPlugin::parse_file(const String &p_path, Ve
 	Ref<Resource> loaded_res = ResourceLoader::load(p_path, "", ResourceFormatLoader::CACHE_MODE_REUSE, &err);
 	ERR_FAIL_COND_V_MSG(err, err, "Failed to load " + p_path);
 
-	ids = r_ids;
-	ids_ctx_plural = r_ids_ctx_plural;
-
-	ids_comment.clear();
-	ids_ctx_plural_comment.clear();
+	translations = r_translations;
 
 	Ref<GDScript> gdscript = loaded_res;
 	String source_code = gdscript->get_source_code();
@@ -77,11 +73,6 @@ Error GDScriptEditorTranslationParserPlugin::parse_file(const String &p_path, Ve
 	return OK;
 }
 
-void GDScriptEditorTranslationParserPlugin::get_comments(Vector<String> *r_ids_comment, Vector<String> *r_ids_ctx_plural_comment) {
-	r_ids_comment->append_array(ids_comment);
-	r_ids_ctx_plural_comment->append_array(ids_ctx_plural_comment);
-}
-
 bool GDScriptEditorTranslationParserPlugin::_is_constant_string(const GDScriptParser::ExpressionNode *p_expression) {
 	ERR_FAIL_NULL_V(p_expression, false);
 	return p_expression->is_constant && p_expression->reduced_value.is_string();
@@ -135,8 +126,7 @@ void GDScriptEditorTranslationParserPlugin::_add_id(const String &p_id, int p_li
 		return;
 	}
 
-	ids->push_back(p_id);
-	ids_comment.push_back(comment);
+	translations->push_back({ p_id, String(), String(), comment });
 }
 
 void GDScriptEditorTranslationParserPlugin::_add_id_ctx_plural(const Vector<String> &p_id_ctx_plural, int p_line) {
@@ -146,8 +136,7 @@ void GDScriptEditorTranslationParserPlugin::_add_id_ctx_plural(const Vector<Stri
 		return;
 	}
 
-	ids_ctx_plural->push_back(p_id_ctx_plural);
-	ids_ctx_plural_comment.push_back(comment);
+	translations->push_back({ p_id_ctx_plural[0], p_id_ctx_plural[1], p_id_ctx_plural[2], comment });
 }
 
 void GDScriptEditorTranslationParserPlugin::_traverse_class(const GDScriptParser::ClassNode *p_class) {

modules/gdscript/editor/gdscript_translation_parser_plugin.h

@@ -43,11 +43,7 @@ class GDScriptEditorTranslationParserPlugin : public EditorTranslationParserPlug
 
 	const HashMap<int, GDScriptTokenizer::CommentData> *comment_data = nullptr;
 
-	Vector<String> *ids = nullptr;
-	Vector<Vector<String>> *ids_ctx_plural = nullptr;
-
-	Vector<String> ids_comment;
-	Vector<String> ids_ctx_plural_comment;
+	Vector<Vector<String>> *translations = nullptr;
 
 	// List of patterns used for extracting translation strings.
 	StringName tr_func = "tr";
@@ -81,8 +77,7 @@ class GDScriptEditorTranslationParserPlugin : public EditorTranslationParserPlug
 	void _extract_fd_filter_array(const GDScriptParser::ExpressionNode *p_expression);
 
 public:
-	virtual Error parse_file(const String &p_path, Vector<String> *r_ids, Vector<Vector<String>> *r_ids_ctx_plural) override;
-	virtual void get_comments(Vector<String> *r_ids_comment, Vector<String> *r_ids_ctx_plural_comment) override;
+	virtual Error parse_file(const String &p_path, Vector<Vector<String>> *r_translations) override;
 	virtual void get_recognized_extensions(List<String> *r_extensions) const override;
 
 	GDScriptEditorTranslationParserPlugin();