Extend translation linting to check translation text fits within widget bounds

[RoosterDragon]

We teach the CheckTranslationReference lint pass to determine the widget bounds of all possible widget trees, and then check the translation text fits within the bounds where it is going to be rendered. This allows translation text that is too long to be flagged, so that either the translation text can be revised or the UI adjusted to ensure the text fits.

As the lint pass is doing a static analysis, results won't be perfect as for example it won't replicate logic that resizes widgets dynamically at runtime. It also hardcodes a lot of knowledge about the function of widgets and logic classes in order to provide a sufficient model for determining widget bounds so we can check sizes.

Existing issues flagged by this pass often look okay in the UI, as we allow text to be rendered outside of the available widget bounds. So the text can appear to be fine even though it's outside the defined area. By fixing the existing sizes in YAML, we now have an English translation that pass the lint. Future translations will now be able to quickly identify issues as we know the English version passes. (This is also useful to help avoid issues where the bounds do matter, e.g. the scroll widget uses the bounds to decide what is in view and needs rendering, so inaccurate bounds can cause items to fail to render when required) Fixes detected by this lint pass were merged in #21309.

To try this lint pass, run the Utility with ra --check-yaml, cnc --check-yaml, d2k --check-yaml or ts --check-yaml to validate that mod.

---

Assists with #10475.

Example by setting button-main-menu-singleplayer = SingleplayerButTheTextIsTooLong:

OpenRA/mods/common/languages/chrome/en.ftl

Line 325 in 0741439

button-main-menu-singleplayer = Singleplayer

Ingame:

Utility output of ra --check-yaml:

Testing translation: en
OpenRA.Utility(1,1): Warning: button-main-menu-singleplayer defined by Button@SINGLEPLAYER_BUTTON in field Text in common|chrome/mainmenu.yaml:49 has value SingleplayerButTheTextIsTooLong in en translation. Text is too large for widget. Text is 240,14. Widget is 140,30.

[pchote]

I'm wondering if it would be cleaner to pull these out into a yaml file?

[pchote]

An easier and IMO better approach would be to move it onto the chrome logic, like we do with hotkey names.

[RoosterDragon]

Yes having an attribute that allows logic classes to specify the dynamic parent->child relationships they are going to set up would be the scalable solution here. Some of those are buried deep in helper methods though, so I don't think it'd be a quick refactoring for all cases.

Would we be okay to defer that as a follow-up, or should we get that in as a pre-req for this PR?

[pchote]

I'm fine with it being a followup.

[pchote]

Would it make sense to add a FontReference attribute that we can reflect over instead? If a widget has multiple text entries with different fonts the attribute could point to the text it applies to.

[RoosterDragon]

Yes, that'd be the scalable approach to solving this. I don't think I ran into anything that uses multiple fonts/texts but yes we could encode that too.

Compared to the relationships, this attribute would be easier to get set up.

[RoosterDragon]

But not that easy, since we need the default values from the class and we're not actually instantiating the widgets. :/

[anvilvapre]

Looking up the translation values everywhere in the classes is a bit messy. Having an underlying layer take care of it would be cleaner.

The translation is now showing up everywhere in each constructor mixing with logic.

Pseudo code

class SomeWidget
{
  struct LocaleResource
  {
   // Would lookup "Scope.SomeWidget.buttonCaption1" from a resource file.
   [LocaleString id="button1Caption"]
   string buttonCaption;
  }

  [LocaleResource id="Scope.SomeWidget"]
  static LocaleResource locale;

  static LocaleResource GetLocale(Mod mod, bool force = false)
  {
    var mod = mod || GetDefaultMod(mod);
     // init once
     return locale;
  }

  void SomeMethod()
  {
     var buttonCaption = GetLocale(GetModFromSomeWhere()).buttonCaption;
  }
}

This would then also allow you to load resources without instantiating the class. I.e. CurentNamespace.Locale.SomeWidgetLocaleResource.cs.

You could even pull the struct outside the class, put all in a Locale namespace. Which your linter could scan.

Also avoid loading translation with each instantation of i.e. widgets. Loading of widgets is already quite dynamic and slow.

[RoosterDragon]

I think you've effectively described the existing translation setup? We're already able to pull the translation text successfully without having to instantiate widgets. It's other references like the fonts and widget IDs that don't have such a mechanism and can't easily add one since their data might be set via code. Whereas translation config is defined in YAML only.

I think the outcome here is that the PR as it stands is pretty much what we're going to get for a "quick win". Replacing the hacks with a proper solution would be a non-trivial undertaking. So I'd say review this on the basis of whether you think these value justifies the amount of hacks required.

[PunkPun]

I don't think this is mergable, for next release we need to make sure translations have proper third party support, and this PR will only delay us from reaching that milestone. If noone's gonna be unhardcoding this lint test then I think a second PR should be split from this with the yaml changes

[RoosterDragon]

Having the lint pass detect some size issues for you still seems to be a net win to me compared to not detecting anything at all. But split out #21309 so we can at least get that merged.

[RoosterDragon]

Have added a commit that allows unhardcoding the parent->child widget relationships. However the code quality is atrocious and there's still a lot of TODOs so converting to a draft PR.

[RoosterDragon]

Ok, updated:

• We now check all languages, not just English.
• ChromeLogic classes that build widget relationships at runtime can now use the DynamicWidgets class to expose those relationships. This allows windows and parent->child widget relationships to be exposed for linting. It can also accept the logicArgs parameter that comes from YAML. This allows relationships that are defined in YAML to be captured as well.

I haven't dealt with unhardcoding all the smaller hacks, but this deals with the main one by allowing the entire widget tree to be discovering without hardcoding it in the linter.

[RoosterDragon]

I've split the changes not related to checking the text size into #21389, so we can get the other accumulated improvements in on their own merit.

[RoosterDragon]

With #21389 merged, the diff in CheckTranslationLint.cs is now much cleaner.