Implement macro to feature-test translations in ActionWML

[Slayer95]

There are concerns about new English dialogues and/or hints popping up in the middle of otherwise fully-translated campaigns, so this provides a standard way to check whether given sentences are already translated.

[ProditorMagnus]

Almost all of the macro is Lua body. Why not have it as Lua function then?

[Slayer95]

Almost all of the macro is Lua body. Why not have it as Lua function then?

What should the Lua API look like?

[ProditorMagnus]

As core macro it would also need documentation comment which describes what it does and how to use it. If you have that available it could also be starting point to decide whether to use wml tag, wml macro, or Lua function.

[CelticMinstrel]

new English dialogues

Hints are one thing, but I don't think this can be used for dialogue. The flow of conversation won't make sense if some lines are randomly omitted.

[gfgtdf]

What should the Lua API look like?

My current preference would be something like _("some text").translation_missing() or similar.

I could also make use of it in WC where there are ( for example in the "found a hero" dialogue) Multiple strings for specific heroes and a fallback string for units that don't have a specific dialogue. With something like this, i could make it use the fallback in case that the unit specific variant is not translated (which is afaik currently the case in some languages)

[Slayer95]

Hints are one thing, but I don't think this can be used for dialogue. The flow of conversation won't make sense if some lines are randomly omitted.

Some pieces of dialogue are in fact hints that could be omitted, such as Delfador's in #7291 (granted, that PR poses larger challenges than that piece of advice, but this is a step forward for similar, albeit less complex, cases.)

[CelticMinstrel]

I'm not saying there's no cases, just that this isn't something that can be used for just anything.

My current preference would be something like _("some text").translation_missing() or similar.

I like that model, but I think I'd prefer to invert it – something like _("some text").is_translated(). Maybe is_localized?

[gfgtdf]

I like that model, but I think I'd prefer to invert it

I also think that inverting it makese sense but, i found it quite hard to come up with a function name doesnt sound like like "is this a tstring or a normal string" check.

[CelticMinstrel]

I like that model, but I think I'd prefer to invert it

I also think that inverting it makese sense but, i found it quite hard to come up with a function name doesnt sound like like "is this a tstring or a normal string" check.

If you want to know whether or not it's a t_string, you use wesnoth.type(whatever) == 't_string'. If you know it's a t_string and want to know whether it's localized, you use whatever:is_localized(). I don't think that's all that difficult. If it's a regular Lua string, whatever:is_localized() is an error.

We might also need is_localizable() since not all t_strings even support localization, and it doesn't really make sense to return true from is_localized() if it's just a plain string that was converted to a t_string. I guess a compound t_string would be localized only if all chunks are either localized or non-localizable.

[Slayer95]

wesnoth.type(whatever) == 't_string'

Is this 1.17 API? wesnoth.type is nil in 1.16.

We might also need is_localizable() since not all t_strings even support localization, and it doesn't really make sense to return true from is_localized() if it's just a plain string that was converted to a t_string. I guess a compound t_string would be localized only if all chunks are either localized or non-localizable.

Thanks. Useful remarks for the crash course on t_strings I just got myself into.

I guess a compound t_string would be localized only if all chunks are either localized or non-localizable.

Returning true from non-localizable strings would be misleading. An exception should probably be raised instead. Unless I am severely misunderstanding something. Like, surely there's a reason for this?

wesnoth/src/tstring.cpp

Lines 336 to 338 in c44cc27

	if(!string.empty() && (string[0] == TRANSLATABLE_PART || string[0] == UNTRANSLATABLE_PART)) {
	orig.translatable_ = true;
	} else {

[stevecotton]

Feels like it should be a new ConditionalActionsWML tag, which would allow checking multiple strings to have a coherent conversation:

[if]
    [has_translation]
        text="Hint part 1" # not sure if this should have a _( ) around it, see text below about ghosts
    [/has_translation]
    [and]
        [has_translation]
            text="Hint part 2"
        [/has_translation]
    [/and]

My main question is, what ends up in the .po file? Will old strings become ghosts that still look as if they should be translated, even though they would no longer be used when the new strings are translated? That wouldn't be a problem for translations that had reached 100%, but it would add workload to translations that were catching up.

Slightly worse would be when the ghosts are only there for the if statements. That might be a reason to wrap the [if]...[then][message] up in a macro.

For WC, if it can change which hero is found then it will need multiplayer safety.

[CelticMinstrel]

Is this 1.17 API?

Yes – in 1.16 you would need to check type(whatever) == 'userdata' and getmetatable(whatever) == 't_string'.

Like, surely there's a reason for this?

I think the reason for the code you posted is that a string beginning with UNTRANSLATABLE_PART is an indication that there will be a TRANSLATABLE_PART later on. If the entire thing is untranslatable, it doesn't start with UNTRANSLATABLE_PART, I think.

Returning true from non-localizable strings would be misleading. An exception should probably be raised instead.

I'm not sure… is it really okay for such a function to raise an exception? If it does though there definitely needs to be another function to detect whether it will raise an exception.

not sure if this should have a _( ) around it

It probably shouldn't, but then you also need to specify the textdomain in the tag.

[Slayer95]

My main question is, what ends up in the .po file?

All strings tagged with _, except empty strings. But it's possible to add a directive to ignore the fallback string, if required. Incidentally, that directive, together with #wmlxgettext _"string" would be part a workaround for #7540 .

Will old strings become ghosts that still look as if they should be translated, even though they would no longer be used when the new strings are translated?

That would depend on when it's decided to drop these feature tests. I don't think they should be permanently left in the code. But if people are interested LTS-ing them, then dropping any fallbacks future translation catalogs would be a necessity.

Slightly worse would be when the ghosts are only there for the if statements.

Not sure what you mean. The entire point of feature-testing would be checking for them and then using them. Just checking for them but not using them at all would be just asking for trouble. Certainly, anything can be misused, but that sort of abuse cannot be allowed for mainline campaigns.

reason to wrap the [if]...[then][message] up in a macro.

Indeed, one of the reasons why I proposed this as a macro is so that there is no else branch. Still, that doesn't prevent the daring from figuring something out with [else] and [+then]. I also have come to think that there are indeed legitimate uses for an else branch, such as fallbacks. In that case, the maintenance cost from using fallbacks must be accounted for.

Similarly, because of maintenance cost, if there were a [have_translation] tag, then I don't think having a macro as well would be justified -unless backporting to 1.16 without the tag is found to be useful, but I think that ship has sailed regarding the new strings already added.

I'm not sure… is it really okay for such a function to raise an exception?

Feature-testing should be done in the definition site. Checking in other random places whether an unknown t_string is translated or, worse, a concatenation of them, is something that must be accounted for in order to acommodate the proposed API _().something(), but it's not good practice.

If it does though there definitely needs to be another function to detect whether it will raise an exception.

That's pcall.

not sure if this should have a _( ) around it

An alternative would be _.exists("text"), which wouldn't require specifying the textdomain.

For WC, if it can change which hero is found then it will need multiplayer safety.

Fairly sure the WC case involves changing messages about hero encounters, not heros themselves.

[CelticMinstrel]

An alternative would be _.exists("text"), which wouldn't require specifying the textdomain.

That still specifies a textdomain, as _ represents the catalogue for a single textdomain.

[Slayer95]

But it's possible to add a directive to ignore the fallback string, if required.

Αctually, I don't think the current build workflow would accomodate this. If they are ignored from new catalogs, they will be directly dropped as soon as the build process runs msgmerge.

Also, fallbacks tend to be very generic and pose translation challenges, so I think a #po: comment explaining that the translator should focus on the main string instead, and that the fallback string will not be displayed in that case, would be a better approach.

That still specifies a textdomain, as _ represents the catalogue for a single textdomain.

Right. It needs to be specified somewhere, but _ is already expected to be defined and used, so there wouldn't be further specification.

[stevecotton]

I've been looking at other engines' approach to i18n, and found that Ren'Py its own system, along with great documentation system. Instead of translating msgids to strings, it translates "say msgid" to a function name, and calls it if it exists. The called function can then say zero, one, or many strings. It can also contain code, with an example converting a number to Roman numerals.

Slightly worse would be when the ghosts are only there for the if statements.

Not sure what you mean. The entire point of feature-testing would be checking for them and then using them.

Agreed, but if they're a few lines away from each other then someone can update the ones that are displayed to the user and fail to notice that they're leaving ghosts.