Move context and plural support to Translation

- `TranslationPO` is now an empty class. It exists for compatibility.
- `OptimizedTranslation` stays the same, no context or plural support.

Author: timothyqiu

core/io/translation_loader_po.cpp

@@ -31,15 +31,16 @@
 #include "translation_loader_po.h"
 
 #include "core/io/file_access.h"
-#include "core/string/translation_po.h"
+#include "core/string/translation.h"
 
 Ref<Resource> TranslationLoaderPO::load_translation(Ref<FileAccess> f, Error *r_error) {
 	if (r_error) {
 		*r_error = ERR_FILE_CORRUPT;
 	}
 
 	const String path = f->get_path();
-	Ref<TranslationPO> translation = Ref<TranslationPO>(memnew(TranslationPO));
+	Ref<Translation> translation;
+	translation.instantiate();
 	String config;
 
 	uint32_t magic = f->get_32();
@@ -229,7 +230,7 @@ Ref<Resource> TranslationLoaderPO::load_translation(Ref<FileAccess> f, Error *r_
 					if (p_start != -1) {
 						int p_end = config.find_char('\n', p_start);
 						translation->set_plural_rules_override(config.substr(p_start, p_end - p_start));
-						plural_forms = translation->get_plural_forms();
+						plural_forms = translation->get_nplurals();
 					}
 				}
 

core/io/translation_loader_po.h

@@ -32,7 +32,6 @@
 
 #include "core/io/file_access.h"
 #include "core/io/resource_loader.h"
-#include "core/string/translation.h"
 
 class TranslationLoaderPO : public ResourceFormatLoader {
 	GDSOFTCLASS(TranslationLoaderPO, ResourceFormatLoader);

core/string/optimized_translation.cpp

@@ -44,11 +44,27 @@ struct CompressedString {
 
 void OptimizedTranslation::generate(const Ref<Translation> &p_from) {
 	// This method compresses a Translation instance.
-	// Right now, it doesn't handle context or plurals, so Translation subclasses using plurals or context (i.e TranslationPO) shouldn't be compressed.
+	// Right now, it doesn't handle context or plurals.
 #ifdef TOOLS_ENABLED
 	ERR_FAIL_COND(p_from.is_null());
+
 	List<StringName> keys;
-	p_from->get_message_list(&keys);
+	{
+		List<StringName> raw_keys;
+		p_from->get_message_list(&raw_keys);
+
+		for (const StringName &key : raw_keys) {
+			const String key_str = key.operator String();
+			int p = key_str.find_char(0x04);
+			if (p == -1) {
+				keys.push_back(key);
+			} else {
+				const String &msgctxt = key_str.substr(0, p);
+				const String &msgid = key_str.substr(p + 1);
+				WARN_PRINT(vformat("OptimizedTranslation does not support context, ignoring message '%s' with context '%s'.", msgid, msgctxt));
+			}
+		}
+	}
 
 	int size = Math::larger_prime(keys.size());
 

core/string/translation.cpp

@@ -34,42 +34,102 @@
 #include "core/string/plural_rules.h"
 #include "core/string/translation_server.h"
 
+void _check_for_incompatibility(const String &p_msgctxt, const String &p_msgid) {
+	// Gettext PO and MO files use an empty untranslated string without context
+	// to store metadata.
+	if (p_msgctxt.is_empty() && p_msgid.is_empty()) {
+		WARN_PRINT("Both context and the untranslated string are empty. This may cause issues with the translation system and external tools.");
+	}
+
+	// The EOT character (0x04) is used as a separator between context and
+	// untranslated string in the MO file format. This convention is also used
+	// by `get_message_list()`.
+	//
+	// It's unusual to have this character in the context or untranslated
+	// string. But it doesn't do any harm as long as you are aware of this when
+	// using the relevant APIs and tools.
+	if (p_msgctxt.contains_char(0x04)) {
+		WARN_PRINT(vformat("Found EOT character (0x04) within context '%s'. This may cause issues with the translation system and external tools.", p_msgctxt));
+	}
+	if (p_msgid.contains_char(0x04)) {
+		WARN_PRINT(vformat("Found EOT character (0x04) within untranslated string '%s'. This may cause issues with the translation system and external tools.", p_msgid));
+	}
+}
+
 Dictionary Translation::_get_messages() const {
 	Dictionary d;
-	for (const KeyValue<StringName, StringName> &E : translation_map) {
-		d[E.key] = E.value;
+	for (const KeyValue<MessageKey, Vector<StringName>> &E : translation_map) {
+		const Array &storage_key = { E.key.msgctxt, E.key.msgid };
+
+		Array storage_value;
+		storage_value.resize(E.value.size());
+		for (int i = 0; i < E.value.size(); i++) {
+			storage_value[i] = E.value[i];
+		}
+		d[storage_key] = storage_value;
 	}
 	return d;
 }
 
-Vector<String> Translation::_get_message_list() const {
-	Vector<String> msgs;
-	msgs.resize(translation_map.size());
-	int idx = 0;
-	for (const KeyValue<StringName, StringName> &E : translation_map) {
-		msgs.set(idx, E.key);
-		idx += 1;
-	}
+void Translation::_set_messages(const Dictionary &p_messages) {
+	translation_map.clear();
 
-	return msgs;
+	for (const KeyValue<Variant, Variant> &kv : p_messages) {
+		switch (kv.key.get_type()) {
+			// Old version, no context or plural support.
+			case Variant::STRING_NAME: {
+				const MessageKey msg_key = { StringName(), kv.key };
+				_check_for_incompatibility(msg_key.msgctxt, msg_key.msgid);
+				translation_map[msg_key] = { kv.value };
+			} break;
+
+			// Current version.
+			case Variant::ARRAY: {
+				const Array &storage_key = kv.key;
+				const MessageKey msg_key = { storage_key[0], storage_key[1] };
+
+				const Array &storage_value = kv.value;
+				ERR_CONTINUE_MSG(storage_value.is_empty(), vformat("No translated strings for untranslated string '%s' with context '%s'.", msg_key.msgid, msg_key.msgctxt));
+
+				Vector<StringName> msgstrs;
+				msgstrs.resize(storage_value.size());
+				for (int i = 0; i < storage_value.size(); i++) {
+					msgstrs.write[i] = storage_value[i];
+				}
+
+				_check_for_incompatibility(msg_key.msgctxt, msg_key.msgid);
+				translation_map[msg_key] = msgstrs;
+			} break;
+
+			default: {
+				WARN_PRINT(vformat("Invalid key type in messages dictionary: %s.", Variant::get_type_name(kv.key.get_type())));
+				continue;
+			}
+		}
+	}
 }
 
-Vector<String> Translation::get_translated_message_list() const {
-	Vector<String> msgs;
-	msgs.resize(translation_map.size());
+Vector<String> Translation::_get_message_list() const {
+	List<StringName> msgstrs;
+	get_message_list(&msgstrs);
+
+	Vector<String> keys;
+	keys.resize(msgstrs.size());
 	int idx = 0;
-	for (const KeyValue<StringName, StringName> &E : translation_map) {
-		msgs.set(idx, E.value);
-		idx += 1;
+	for (const StringName &msgstr : msgstrs) {
+		keys.write[idx++] = msgstr;
 	}
-
-	return msgs;
+	return keys;
 }
 
-void Translation::_set_messages(const Dictionary &p_messages) {
-	for (const KeyValue<Variant, Variant> &kv : p_messages) {
-		translation_map[kv.key] = kv.value;
+Vector<String> Translation::get_translated_message_list() const {
+	Vector<String> msgstrs;
+	for (const KeyValue<MessageKey, Vector<StringName>> &E : translation_map) {
+		for (const StringName &msgstr : E.value) {
+			msgstrs.push_back(msgstr);
+		}
 	}
+	return msgstrs;
 }
 
 void Translation::set_locale(const String &p_locale) {
@@ -82,13 +142,21 @@ void Translation::set_locale(const String &p_locale) {
 }
 
 void Translation::add_message(const StringName &p_src_text, const StringName &p_xlated_text, const StringName &p_context) {
-	translation_map[p_src_text] = p_xlated_text;
+	_check_for_incompatibility(p_context, p_src_text);
+	translation_map[{ p_context, p_src_text }] = { p_xlated_text };
 }
 
 void Translation::add_plural_message(const StringName &p_src_text, const Vector<String> &p_plural_xlated_texts, const StringName &p_context) {
-	WARN_PRINT("Translation class doesn't handle plural messages. Calling add_plural_message() on a Translation instance is probably a mistake. \nUse a derived Translation class that handles plurals, such as TranslationPO class");
 	ERR_FAIL_COND_MSG(p_plural_xlated_texts.is_empty(), "Parameter vector p_plural_xlated_texts passed in is empty.");
-	translation_map[p_src_text] = p_plural_xlated_texts[0];
+
+	Vector<StringName> msgstrs;
+	msgstrs.resize(p_plural_xlated_texts.size());
+	for (int i = 0; i < p_plural_xlated_texts.size(); i++) {
+		msgstrs.write[i] = p_plural_xlated_texts[i];
+	}
+
+	_check_for_incompatibility(p_context, p_src_text);
+	translation_map[{ p_context, p_src_text }] = msgstrs;
 }
 
 StringName Translation::get_message(const StringName &p_src_text, const StringName &p_context) const {
@@ -97,16 +165,13 @@ StringName Translation::get_message(const StringName &p_src_text, const StringNa
 		return ret;
 	}
 
-	if (p_context != StringName()) {
-		WARN_PRINT("Translation class doesn't handle context. Using context in get_message() on a Translation instance is probably a mistake. \nUse a derived Translation class that handles context, such as TranslationPO class");
-	}
-
-	HashMap<StringName, StringName>::ConstIterator E = translation_map.find(p_src_text);
-	if (!E) {
+	const Vector<StringName> *msgstrs = translation_map.getptr({ p_context, p_src_text });
+	if (msgstrs == nullptr) {
 		return StringName();
 	}
 
-	return E->value;
+	DEV_ASSERT(!msgstrs->is_empty()); // Should be prevented when adding messages.
+	return msgstrs->get(0);
 }
 
 StringName Translation::get_plural_message(const StringName &p_src_text, const StringName &p_plural_text, int p_n, const StringName &p_context) const {
@@ -115,21 +180,30 @@ StringName Translation::get_plural_message(const StringName &p_src_text, const S
 		return ret;
 	}
 
-	WARN_PRINT("Translation class doesn't handle plural messages. Calling get_plural_message() on a Translation instance is probably a mistake. \nUse a derived Translation class that handles plurals, such as TranslationPO class");
-	return get_message(p_src_text);
-}
+	ERR_FAIL_COND_V_MSG(p_n < 0, StringName(), "N passed into translation to get a plural message should not be negative. For negative numbers, use singular translation please. Search \"gettext PO Plural Forms\" online for details on translating negative numbers.");
 
-void Translation::erase_message(const StringName &p_src_text, const StringName &p_context) {
-	if (p_context != StringName()) {
-		WARN_PRINT("Translation class doesn't handle context. Using context in erase_message() on a Translation instance is probably a mistake. \nUse a derived Translation class that handles context, such as TranslationPO class");
+	const Vector<StringName> *msgstrs = translation_map.getptr({ p_context, p_src_text });
+	if (msgstrs == nullptr) {
+		return StringName();
 	}
 
-	translation_map.erase(p_src_text);
+	const int index = _get_plural_rules()->evaluate(p_n);
+	ERR_FAIL_INDEX_V_MSG(index, msgstrs->size(), StringName(), "Plural index returned or number of plural translations is not valid.");
+	return msgstrs->get(index);
+}
+
+void Translation::erase_message(const StringName &p_src_text, const StringName &p_context) {
+	translation_map.erase({ p_context, p_src_text });
 }
 
 void Translation::get_message_list(List<StringName> *r_messages) const {
-	for (const KeyValue<StringName, StringName> &E : translation_map) {
-		r_messages->push_back(E.key);
+	for (const KeyValue<MessageKey, Vector<StringName>> &E : translation_map) {
+		if (E.key.msgctxt.is_empty()) {
+			r_messages->push_back(E.key.msgid);
+		} else {
+			// Separated by the EOT character. Compatible with the MO file format.
+			r_messages->push_back(vformat("%s\x04%s", E.key.msgctxt, E.key.msgid));
+		}
 	}
 }
 
@@ -175,6 +249,10 @@ String Translation::get_plural_rules_override() const {
 	return plural_rules_override;
 }
 
+int Translation::get_nplurals() const {
+	return _get_plural_rules()->get_nplurals();
+}
+
 void Translation::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("set_locale", "locale"), &Translation::set_locale);
 	ClassDB::bind_method(D_METHOD("get_locale"), &Translation::get_locale);
@@ -195,7 +273,7 @@ void Translation::_bind_methods() {
 	GDVIRTUAL_BIND(_get_message, "src_message", "context");
 
 	ADD_PROPERTY(PropertyInfo(Variant::DICTIONARY, "messages", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NO_EDITOR | PROPERTY_USAGE_INTERNAL), "_set_messages", "_get_messages");
-	ADD_PROPERTY(PropertyInfo(Variant::STRING, "locale"), "set_locale", "get_locale");
+	ADD_PROPERTY(PropertyInfo(Variant::STRING, "locale", PROPERTY_HINT_LOCALE_ID), "set_locale", "get_locale");
 	ADD_PROPERTY(PropertyInfo(Variant::STRING, "plural_rules_override"), "set_plural_rules_override", "get_plural_rules_override");
 }
 

core/string/translation.h

@@ -41,12 +41,29 @@ class Translation : public Resource {
 	RES_BASE_EXTENSION("translation");
 
 	String locale = "en";
-	HashMap<StringName, StringName> translation_map;
+
+	struct MessageKey {
+		StringName msgctxt;
+		StringName msgid;
+
+		// Required to use this struct as a key in HashMap.
+		static uint32_t hash(const MessageKey &p_key) {
+			uint32_t h = hash_murmur3_one_32(HashMapHasherDefault::hash(p_key.msgctxt));
+			return hash_fmix32(hash_murmur3_one_32(HashMapHasherDefault::hash(p_key.msgid), h));
+		}
+		bool operator==(const MessageKey &p_key) const {
+			return msgctxt == p_key.msgctxt && msgid == p_key.msgid;
+		}
+	};
+
+	HashMap<MessageKey, Vector<StringName>, MessageKey> translation_map;
 
 	mutable PluralRules *plural_rules_cache = nullptr;
 	String plural_rules_override;
 
 	virtual Vector<String> _get_message_list() const;
+
+	// For data storage.
 	virtual Dictionary _get_messages() const;
 	virtual void _set_messages(const Dictionary &p_messages);
 
@@ -74,5 +91,8 @@ class Translation : public Resource {
 	void set_plural_rules_override(const String &p_rules);
 	String get_plural_rules_override() const;
 
+	// This method is not exposed to scripting intentionally. It is only used by TranslationLoaderPO and tests.
+	int get_nplurals() const;
+
 	~Translation();
 };