Author Topic: Refactoring translation  (Read 15977 times)

Offline karvanit

  • Captain
  • ***
  • Posts: 94
    • View Profile
Re: Refactoring translation
« Reply #15 on: December 03, 2012, 12:50:35 pm »
I have now added a new PR, containing just the minimum code changes, without changing all the states to use the 'tr()' functions.

Offline SupSuper

  • Lazy Developer
  • Administrator
  • Commander
  • *****
  • Posts: 2162
    • View Profile
Re: Refactoring translation
« Reply #16 on: December 04, 2012, 10:57:15 pm »
Ok... I'm still really confused. :? Could you walk me through your thought process behind this. For example:
- Why did you encapsulate strings inside a LocalizedText?
- What's OX_REQUIRED_RESULT?
- Why should the program crash on a blank string?
- Should PluralityRules really be a programmer concern instead of a translator concern? It's gonna have to be maintained everytime a new language comes in, all this for one plural word.

Offline karvanit

  • Captain
  • ***
  • Posts: 94
    • View Profile
Re: Refactoring translation
« Reply #17 on: December 04, 2012, 11:50:28 pm »
- Why did you encapsulate strings inside a LocalizedText?

Because this allows me to add the 'arg()' function and allow call chaining for argument substitution. It also makes it clear what is translated and what isn't. To take full advantage of this we should make all user-visible functions accept LocalizedText arguments so that translator-unfriendly code would trigger compilation errors. But that's a very big change for a small gain in the way the game is written right now.
The proposed code has no overhead and allows readable translation code, without changing too much. It just stops string concatenation with '+', requiring wstringstream instead. Which is good, since this indicates that proper translation-aware code should be substituted there (using LocalizedText::arg for most cases).

- What's OX_REQUIRED_RESULT?
It is hides compiler specific code to warn about unused results of "expensive" functions, eg code like
Code: [Select]
_game->getLanguage()->getString("STR_ID").arg("Example"); https://Nothing is done with this.

VS

setText(_game->getLanguage()->getString("STR_ID").arg("Example")); https:// This is ok, we are using the result.
- Why should the program crash on a blank string?
Ok, crashing is kind of overkill, but that assert only triggered when someone wanted a translation for the empty ID, so it is a logical error in the code, or in the rule set.

- Should PluralityRules really be a programmer concern instead of a translator concern? It's gonna have to be maintained everytime a new language comes in, all this for one plural word.
PluralityRules is used for the generic code to support all different types of plural forms (not just 2 as in English).
It allows the programmers to call a single function and have the translators provide the proper forms.
And the implementation is such that for most languages only a single line needs to be added (if they are the same as another supported language). Only a language that has different rules needs more code, and even then it's a simple enough class that can be written by anyone, if he/she understands the language in question (or has the rules provided by someone who does).

PS: I agree that the current game code does not allow the new translation code to show it's true potential, and it may seem overkill. But I think that using it will allow better translations as the rest of the game eventually gets re-written to take advantage of it. If we don't have the translation code, these possibilities will never be taken, and the translations will always be sub-par, no matter how hard the translators try.
« Last Edit: December 05, 2012, 12:05:05 am by karvanit »

Offline SupSuper

  • Lazy Developer
  • Administrator
  • Commander
  • *****
  • Posts: 2162
    • View Profile
Re: Refactoring translation
« Reply #18 on: December 05, 2012, 03:47:51 am »
I guess that makes sense. I've always been more of a fan of simplicity over robustness, probably would've just written something like this for args and leave it at that (pseudo-code):
Code: [Select]
Language::getString(id, ...)
{
s = _strings.find(id)
va_list args;
va_start(args);
for (int i = 0; i < count; i++)
{
stringstream key << "{" << i << "}";
stringstream arg << args[i];
replace(s, key, arg);
}
va_end(args);
return s;
}

As for the empty string dilemma, I think there's cases where you just wanna add an empty string, since that "if" was added later. Like imagine you're looping through an array of IDs and adding corresponding strings to a list, and you just wanna introduce some blank lines in there, or something.