Initial Work on Using Translations Via LcfTrans

[sorlok]

DO NOT MERGE: This is for discussion only.

This is a draft PR; see Issue #797 for the initial discussion. Further code discussion will go here.

[mateofio]

Doing all this text conversion on the fly is going to slow down games that don't use this feature. It also adds a lot of complexity over the code and you're bound to miss a few cases.

Can't we do it on startup? LIke on game initialization, retranslate the whole database. On load map, retranslate all strings in the map data. On load save game, retranslate all strings in the save game etc..

Maybe we can add a nice autogenerated hook to liblcf that lets you iterate over all string members of all lcf struct structs.

[sorlok]

Hmm, I was thinking the performance hit wouldn't be that big, but then again for things like Nintendo 3DS and Android it might start to matter. Translating the DB on the fly whenever the translation is selected makes sense, although we'd need to reload the original LCF if the translation was changed at runtime (so: slower to switch translations, but that's actually fine).

Do you know if the LCF data structure can be modified at runtime? I.e., variables are not "const", and nothing is cached that could interfere with changing the data at runtime?

If a hook is added to iterate over all strings, it would make sense to have it include the "contexts" as well --and LcfTrans should probably use it so that both code paths are aligned. Ideally there would be a similar function to iterate + change all strings.

[mateofio]

Hmm, I was thinking the performance hit wouldn't be that big, but then again for things like Nintendo 3DS and Android it might start to matter.

It's not just performance, there are going to be endless whack-a-mole style bugs where we forget to translate a string or 2 here and there.

The key here is that for something like this we have make it systematic so we solve the problem once and only once. Sprinkling Tr() everywhere is just not going to work.

Translating the DB on the fly whenever the translation is selected makes sense, although we'd need to reload the original LCF if the translation was changed at runtime (so: slower to switch translations, but that's actually fine).

QuickSave method:

If you want to support hot reload of translations, then as discussed in irc, one way to go is if you change the translation we can do an automatic quicksave/quickload and reload all the game structures. Then the translation logic is localized to one place and always works. I think it's perfectly acceptable to have to wait for an automatic reload if you change the translation.

Do you know if the LCF data structure can be modified at runtime? I.e., variables are not "const", and nothing is cached that could interfere with changing the data at runtime?

Right now they can, but later they will be behind a const facade #2285 . Still this could be modified to allow translation or if we do the quicksave/quickload approach it doesn't matter.

Also, some of the save structures are copied into the various classes so getting at all of them isn't really viable.

If a hook is added to iterate over all strings, it would make sense to have it include the "contexts" as well --and LcfTrans should probably use it so that both code paths are aligned. Ideally there would be a similar function to iterate + change all strings.

The hook can be a very general template <typename F> ForEachString(lcf::rpg::Object&, const F& f) auto generated and provided by liblcf for each struct.

Like this one:
https://github.com/EasyRPG/Player/blob/master/src/utils.h#L367

Liblcf just provides a mechanism to call a lambda function on all string members, then it's up to player to supply the translate function into the lambda.

[Ghabry]

Prereplacing will work for most of the cases. The only one that will probably fail is the ShowMessage command because it is multi-line and has other funny corner cases that are out-of-scope right now like "the translation has 5 lines but the original is 4". But except for this one edge-case it will work.

This hook callback function will then receive a context. For flexibility it could be the name of the field, so for a lcf::Skill.name the context is "name".
The callback function is responsible for prefixing the "skill." (and postfix a ".1" for ID 1) to get the entire context.

I will think about a solution as I don't expect sorlok to dig into our lcf code generator ;)

[Ghabry]

@fmatthew5876
You thought of something like this?

https://github.com/EasyRPG/liblcf/compare/master...Ghabry:foreachstring?expand=1

usage

lcf::Data::actors[0].ForEachString([](const std::string& val, const std::string& ctx) {
	return Translate(val, "actor." + ctx);
});

(not PR'ing yet, do this after StringView and DBString)

[mateofio]

@fmatthew5876
You thought of something like this?

https://github.com/EasyRPG/liblcf/compare/master...Ghabry:foreachstring?expand=1

usage

lcf::Data::actors[0].ForEachString([](const std::string& val, const std::string& ctx) {
	return Translate(val, "actor." + ctx);
});

(not PR'ing yet, do this after StringView and DBString)

yeah exactly, something like that. (use const F&, not F&&)

[sorlok]

I removed all the old runtime changes (except message boxes), so most of the translation now happens in ForEachString(). Still a lot of things to fix, but I made a brief demo of Yume 2kki:
https://drive.google.com/file/d/18gn6QgDXShxdExXfWrT1kZ6wl-JJKjdH/view?usp=sharing

Unzip it so that "translations" appears in the game's Project directory, then you can select "en" from the menu. I copied the existing translation strings/pictures from wolfen's translation --this is a tech demo, so please don't redistribute beyond this thread.

The cool thing about doing translations this way is that wolfen's translation was made for 0.112b, but you can run my link on 0.113e (or later) since it's just doing string substitution. I can definitely see the value of a system like this for projects that receive updates.

By the way, I think I will switch my demo game to something else. Yume 2kki is really interesting, but it doesn't have a lot of "standard" RPG Maker stuff (like combat effects, states, or some menu items), so it doesn't really stress the software. Suggestions are welcome (or maybe I'll just do Don Miguel's demo).

[Ghabry]

Yeah I made the "-u" update switch mostly for Yume2kki. The updates are so fast that normal translators are always behind because it is so cumbersome to update this. The 0.112 to 0.113 is actually quite interesting for testing the merge algorithm:
One game dev of Yume2kki left and they asked to remove everything they contributed from the game. Really nice to test the update+stale feature 👍

---

Btw a tinygettext replacement: Here is the Po parser from lcftrans (used by the update feature so you can consider it tested and working now ;)). (I renamed the class to Dictionary because you already use Translation). Only needs: The starts_with converted to StringView startswith and maybe helpers to find entries. Otherwise is just "call fromPo" and then magic happens.

class Entry {
public:
	std::string original; // msgid
	std::string translation; // msgstr
	std::string context; // msgctxt
};

class Dictionary
{
public:
	const std::vector<Entry>& GetEntries() const;

	static Dictionary FromPo(std::istream& in);

private:
	std::vector<Entry> entries;
};

const std::vector<Entry>& Dictionary::GetEntries() const {
	return entries;
}

Dictionary Dictionary::FromPo(std::istream& in) {
	// Super simple parser.
	// Only parses msgstr, msgid and msgctx

	Dictionary t;

	std::string line;
	bool found_header = false;
	bool parse_item = false;

	Entry e;

	auto extract_string = [&line](int offset) {
		std::stringstream out;
		bool slash = false;
		bool first_quote = false;

		for (char c : line.substr(offset)) {
			if (c == ' ' && !first_quote) {
				continue;
			} else if (c == '"' && !first_quote) {
				first_quote = true;
				continue;
			}

			if (!slash && c == '\\') {
				slash = true;
			} else if (slash) {
				slash = false;
				switch (c) {
					case '\\':
						out << c;
						break;
					case 'n':
						out << '\n';
						break;
					case '"':
						out << '"';
						break;
					default:
						std::cerr << "Parse error " << line << " (" << c << ")\n";
						break;
				}
			} else {
				// no-slash
				if (c == '"') {
					// done
					return out.str();
				}
				out << c;
			}
		}

		std::cerr << "Parse error: Unterminated line" << line << "\n";
		return out.str();
	};

	auto read_msgstr = [&]() {
		// Parse multiply lines until empty line or comment
		e.translation = extract_string(6);

		while (std::getline(in, line, '\n')) {
			if (line.empty() || starts_with(line, "#")) {
				break;
			}
			e.translation += extract_string(0);
		}

		parse_item = false;
		t.addEntry(e);
	};

	auto read_msgid = [&]() {
		// Parse multiply lines until empty line or msgstr is encountered
		e.original = extract_string(5);

		while (std::getline(in, line, '\n')) {
			if (line.empty() || starts_with(line, "msgstr")) {
				read_msgstr();
				return;
			}
			e.original += extract_string(0);
		}
	};

	while (std::getline(in, line, '\n')) {
		if (!found_header) {
			if (starts_with(line, "msgstr")) {
				found_header = true;
			}
			continue;
		}

		if (!parse_item) {
			if (starts_with(line, "msgctxt")) {
				e.context = extract_string(7);

				parse_item = true;
			} else if (starts_with(line, "msgid")) {
				parse_item = true;
				read_msgid();
			}
		} else {
			if (starts_with(line, "msgid")) {
				read_msgid();
			} else if (starts_with(line, "msgstr")) {
				read_msgstr();
			}
		}
	}

	return t;
}

[sorlok]

Awesome, thanks! I'll integrate the PO reader.

[Ghabry]

You can't use dynamic cast. It requires rtti which results in binary bloat (and I forgot to disable rtti+exceptions in the web player. Huge waste of space).
You can use Game_Battle::IsBattleRunning.
Though there is currently no good way to tell if an event is a common event. I'm thinking about a solution (maybe custom lcf chunks that contain the event type)

[Ghabry]

From an esthethic POV I like that the Language selection is part of the title scene and just spawns a new command box :).
Also nice fade effect after language selection (guess this is just the Title Scene::Pop/Push ^^')

Works nice in Yume2kki!

[Ghabry]

I recorded a video here: Nice job!

yume2kki-translation-0.6.3.zip

@sorlok
One problem I noticed is that it won't work when the language directory is not full lower case (English doesnt work, english does) but I guess there will be some Ini file later to solve this (and use this string instead).

I general imo the folder should be "Languages". In other user interfaces you usually see the word "Languages", not "Translations". (same for the folder -> "languages")

E.g. for each language a "Meta.ini" file:

[Language]
Name=English
Code=en
Description=Shiny translation by Team TheEnglishSpeakazzzz <- Could be shown as a one-line help window at the top

[sorlok]

You can't use dynamic cast. It requires rtti which results in binary bloat (and I forgot to disable rtti+exceptions in the web player. Huge waste of space).
You can use Game_Battle::IsBattleRunning.
Though there is currently no good way to tell if an event is a common event. I'm thinking about a solution (maybe custom lcf chunks that contain the event type)

Removed the dynamic cast. Right now I just scan both maps and common events (since there's no good way to tell). That will work fine for now.

[sorlok]

Languages should load correctly now regardless of case. Also, the directory was renamed to "Languages", and the Meta.ini file works as described. There is also Help text shown.

[Ghabry]

You can't use exceptions. They are disabled on the same performs as rtti ;).
Also this is bad style. You never throw an exception and you do a "catch all". What do you want to catch there? Fix the code bug instead of hiding it.

[fdelapena]

From an esthethic POV I like that the Language selection is part of the title scene and just spawns a new command box :).
Also nice fade effect after language selection (guess this is just the Title Scene::Pop/Push ^^')

Works nice in Yume2kki!

Just as a note, some games ( #2035 (comment) ) will require additional tweaks to integrate additional options.

A possible approach to work around this without modifying the original game data:

From the built-in system graphics in Player, get the blinking arrow sprites used for scrollable window selectables and add them when there are more than three options (e.g. Translations, Options), then when the selectable goes further the quit, display the built-in system window over the original with the 3 original selectors (with some built-in names) + the additional ones. When to override with built-in system graphics? It's likely 99% safe for games without empty strings in new game and quit game. It's unlikely games exist with transparent color for picture, selectable inner content and window background (it could be checked if some game actually uses this trick, though, if worth).

Some games skip the title scene (AEP patches and modern 2k[3]E) which will require some alternate approach on how to access translations without that scene. What about injecting it when there's is an open video options call from those games?

[sorlok]

Example export of RPG Advocate's Aurora's Tear translation is now "done":
https://drive.google.com/file/d/1qUSQwWjoPLr7dKYBpvqDdqyMhQqR5eXW/view?usp=sharing

...however, note that this is still not a 1.0 translation, since the way Choice text is stored will change in LcfTrans (to be independent of the Message text). Once that happens, I'll update this.

[sorlok]

Aurora's Tear translation is now at 1.0:
https://drive.google.com/file/d/15uqWzc125y0J8XldIv7e60XhKSF6Qapi/view?usp=sharing

Keep in mind that RPG Advocate's translation was not 100% to begin with --this simply copies over everything they worked on.

I need to rebase this branch and do a minor code cleanup, then it will be ready to merge.

[sorlok]

Hi all, I am currently on a bit of a vacation, so updates on this will be slow. I will try to do a final pass sometime in the next few weeks.

[sorlok]

Ok, I managed to get some time to look at this. I've rebased it and recompiled, and it all works as expected. Note that @fmatthew5876's outstanding request was dealt with some time ago (we modify the LDB/Maps at load instead of when strings are requested). So this Pull Request should be good to go!

[sorlok]

@fmatthew5876 , @Ghabry (just bumping this up in case you missed it).

[Ghabry]

sorry, I'm kinda overloaded with work since a month, so I can't really focus on Player right now... So had no time to check this yet again :/

[sorlok]

No worries, I'll check again in a few weeks. Best of luck!

[Ghabry]

Well for me this all works excellent. I want to look at RewriteEventCommandMessage again later. This function is really complecated, maybe can be simplified ^^

Please make the requested changes in a new commit

[Ghabry]

lacks @window documentation.
Also in general your naming convention again: centerVertical -> center_vertical

[sorlok]

Fixed missing documentation.
Fixed all the camel case names I could find.

[Ghabry]

lang_str

[sorlok]

Fixed

[Ghabry]

don't align the "=", just move them to the left next to the var name.

[sorlok]

Fixed

[Ghabry]

Applies to remove and add: Is this still used? I see them checked in ifs but never assigned?

[sorlok]

These strings are supposed to be added by the translator. Here's how it works; let's say I have a message box:

msgid ""
"Hero: Hello there!"

...and I want that message box to be removed from the translation entirely. In that case, I replace it with:

msgstr ""
"__EASY_RPG_CMD:REMOVE_MSGBOX__"

Let's say, however, that in the target language it makes more sense for the other party to shout at the hero first. So I need a new message box to appear BEFORE the current one. I'll replace it with:

msgstr ""
"Townsperson: Mr. Hero, hello!"
"__EASY_RPG_CMD:ADD_MSGBOX__"
"Hero: Hello there!"

This will ADD a message box to the event list, so now we'll have two message boxes (rather than a single two-line message box).

They are magic strings, unfortunately, but I think they're pretty unlikely to appear in source games. (And I can't think of a more robust way to do this that interacts cleanly with gettext).

[Ghabry]

ah now I get it so that's some kind of in-band signaling. Will see if I can find a way to improve this.

For "insert new page" we already have the the form feed "\f" (needed for some battle messages) but I guess this will need changes to the po parser.

For "delete message" one could use gettext flags. They are marked via #,, something like #, easyrpg-delete (see https://www.gnu.org/software/gettext/manual/html_node/PO-Files.html). Will also need parser changes (to ensure lcftrans keeps them)

[sorlok]

Yep, that's right.

I think I might prefer to keep the current strings, since they are very self-descriptive for translators, and I don't think that "\f" or gettext flags will clean up our own processing code. But this is certainly up for debate.

[Ghabry]

at least \f will not easily work because our code filters away control characters... Have to sleep about this ^^'

[Ghabry]

Is a getter, call it GetTranslationDir

[sorlok]

Fixed

[Ghabry]

Wrong doc comments, should be /** */

[sorlok]

Fixed all usages of ///

[Ghabry]

should be lang_dir, lang_name, lang_desc

[sorlok]

Fixed

[Ghabry]

lang_id

[sorlok]

Fixed

[Ghabry]

translation_root_dir, current_language

[sorlok]

Fixed

[Ghabry]

Marking this as "Reviewing later", can you explain me first how this TRCUST_REMOVEMSG stuff works? Considering that this is never pushed anywhere I'm confused.

[sorlok]

See the earlier comment about the translator adding the strings into the translated text.

Given that, the basic idea is to take a CommandIterator on the command stream, and then .insert() and .remove() as needed to get the requested translation. It's fairly generic around MessageBoxes, and a little bit stricter around Choices. I think the comments do a reasonable job of explaining what's going on, but let me know if there are some areas that are not clear and I'll tidy up the docs.

I did a pass to simplify the function (a month ago); it's possible it can be simplified a little further, or we could encapsulate some of these activities (e.g., "trim down to 4 lines") into their own tiny functions, if that helps.

[sorlok]

Thanks for the feedback; all items were fixed (and I added two major comments describing how add/remove work).

I also rebased, and loading maps now crashes on Linux (see below), but since it does this for new games with translations turned off, I think it is probably not related to my minor code cleanup:

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff503e8b1 in __GI_abort () at abort.c:79
#2  0x00007ffff5693957 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff5699ae6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff5699b21 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00005555556d022c in nonstd::span_lite::detail::report_contract_violation(char const*) ()
#6  0x00005555556d05da in nonstd::span_lite::span<unsigned char const, 18446744073709551615ul>::operator[](unsigned long) const ()
#7  0x00005555556cea61 in TilemapLayer::CreateTileCache(std::vector<short, std::allocator<short> > const&) ()
#8  0x00005555556cfeaf in TilemapLayer::SetPassable(std::vector<unsigned char, std::allocator<unsigned char> >) ()
#9  0x00005555556a71e1 in Tilemap::SetPassableDown(std::vector<unsigned char, std::allocator<unsigned char> >) ()
#10 0x00005555556a6d71 in Spriteset_Map::OnTilemapSpriteReady(FileRequestResult*) ()

[Ghabry]

try updating to the latest liblcf version. Here it works.

[sorlok]

try updating to the latest liblcf version. Here it works.

Yep, that fixed it, thanks. (I had to do a clean rebuild of both sources.)

[sorlok]

I've been thinking about __EASY_RPG_CMD:ADD_MSGBOX__ and __EASY_RPG_CMD:REMOVE_MSGBOX__ these past few days, and I think I prefer to keep them as strings. Reasoning:

• Translators may not be coders; if they see "\f", it will be hard for them to find a good search result on Google.
• They're somewhat self-documenting: if we add other "EASY_RPG_CMD's" in the future, it will be simple to search for them.
  • I'm not planning this, but adding EASY_RPG_CMD's for inserting all types of Event Commands would allow translators greater freedom when dealing with custom menus/combat and RTL or top-to-bottom languages.
• I don't like having two different mechanisms for add (\f) and remove (gettext flags)
• I like keeping our .po parser simple (no need for gettext flags)
• I like keeping tools simple (a .po editor might treat \f differently because it's an escape character)

The final decision is of course up to EasyRPG devs.

[Ghabry]

Maybe make the commands less clunky by using something like <easyrpg:new_page> and <easyrpg:delete_page>. Then it looks a bit like XML :D.
But in general I'm by now fine with this solution.

[sorlok]

That sounds good; I'll change the commands to XML-like later today.

[sorlok]

Changes made; we now use <easyrpg:new_page>and <easyrpg:delete_page>. (I also rebased it.)

The sample translation of Aurora's Tear has also been updated to match:
https://drive.google.com/file/d/1NjsOs96j-nhkyooE7YQL3t9qzR67iKJs/view?usp=sharing

As far as I'm concerned, this feature is ready.

Changes have been addressed with ForEachString() as suggested

[Ghabry]

Except for Scene_Title this is minimally invasive as all rewrites are on Database/Map load.

<easyrpg:delete_page> and <easyrpg:new_page> also work as intended and help alot when the target language has a much shorter (delete unneeded messages) or a much longer (add more pages to handle e.g. a "Actor name" on Line 1) translation.

Imo this is a really good job and the only way to find more issues is testing this in the wild. 👍