Extract (transient) text notifications

[Mailaender]

This prepares #19872 for localization including some hard-coded but mostly trait defined strings.

[PunkPun]

I'm thinking it would be much cleaner to just translate inside AddTransientLine and AddFeedbackLine. We should try to embed translation support wherever possible.

As for AddSystemLine, there's still a bunch of untranslated strings for it and it is accessible via lua unlike the aforementioned ones

[Mailaender]

I tried that at first, but those are static methods that need a lot of context for translation like Map, ModData and the translation arguments also which line to use for empty vs. not, so I quickly abandoned that.

[pchote]

See discussion in #20387 (comment). This is currently continuing the trend of unwanted duplication 😞

[Mailaender]

I agree. If you have a suggestion to make this easily feasible, then I am all ears.

[PunkPun]

I tried that at first, but those are static methods that need a lot of context for translation like Map, ModData and the translation arguments also which line to use for empty vs. not, so I quickly abandoned that.

Perhaps it's wrong for Map to contain the Translate method. The choice of wether to translate from map Translations or engine Translations should NOT be done on a case by case basis. This is just asking for trouble.

[PunkPun]

Hmm, though that would allow maps to overwrite UI translations. It may be something we want, or may be something we want to avoid.

I tried that at first, but those are static methods that need a lot of context for translation like Map, ModData and the translation arguments also which line to use for empty vs. not, so I quickly abandoned that.

Incase we want these separate, then these static methods can be split. One instance would take Map as an argument and another ModData

[pchote]

Perhaps it's wrong for Map to contain the Translate method. The choice of wether to translate from map Translations or engine Translations should NOT be done on a case by case basis. This is just asking for trouble.

Maps can define new actors / lobby options / other things that use strings that aren't defined in the base mod. They may also adjust balance in ways that need to be reflected in e.g. sidebar tooltips. IMO there is no choice here: Maps do need to be able to override translations.

[Mailaender]

We have mod hard-coded and map defined (like these trait notifications). The new API is now less flexible and does not allow mixing them. I still fail to see the benefit so I kept it a separate commit we can easily revert later.

[pchote]

Where is this new API described and why is it less flexible?

[PunkPun]

Maps can define new actors / lobby options / other things that use strings that aren't defined in the base mod. They may also adjust balance in ways that need to be reflected in e.g. sidebar tooltips. IMO there is no choice here: Maps do need to be able to override translations.

This is a misunderstanding. The question is about, should only some translations be overwritable, or all?

[pchote]

I agree. If you have a suggestion to make this easily feasible, then I am all ears.

https://github.com/OpenRA/OpenRA/blob/bleed/OpenRA.Game/ModData.cs#L134-L144 Is where we pass map and mod state to the rest of the static objects. Do the same here.

[pchote]

Maps can define new actors / lobby options / other things that use strings that aren't defined in the base mod. They may also adjust balance in ways that need to be reflected in e.g. sidebar tooltips. IMO there is no choice here: Maps do need to be able to override translations.

This is a misunderstanding. The question is about, should only some translations be overwritable, or all?

All.

[PunkPun]

LGTM, will just need the last commit squashed.

As for some of the structure inconsistencies, it will be easier to solve them in the followup PR which localises rest of system lines. We will also have to decide how we want to handle lua notifications

[PunkPun]

Needs a rebase

[penev92]

What is the idea behind the optional flag on the TranslationReference attribute?

[PunkPun]

In runtime there are null checks to prevent some logic from executing, but the linter is unaware of such checks. I suppose this flag is the method of conveying the info to the linter.

[penev92]

So basically the same as FieldLoader.Require, but for use by the translation linting?

[PunkPun]

LGTM

[PunkPun]

Changelog