Skip to content

Commit

Permalink
Extend translation linting to check translation text fits within widg…
Browse files Browse the repository at this point in the history
…et bounds.

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. To capture some of the relationships created at runtime, the new DynamicWidgets class allows ChromeLogic classes to encode the windows and parent->child relationships they intend to establish.

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 ca6aa5e.

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.
  • Loading branch information
RoosterDragon committed Apr 24, 2024
1 parent 10d7436 commit d588c4a
Show file tree
Hide file tree
Showing 72 changed files with 1,896 additions and 415 deletions.
34 changes: 14 additions & 20 deletions OpenRA.Game/Game.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,33 +143,27 @@ public static void RemoteDirectConnect(ConnectionTarget endpoint)
OnRemoteDirectConnect(endpoint);
}

// Hacky workaround for orderManager visibility
public static Widget OpenWindow(World world, string widget)
/// <summary>
/// Creates the widget given by <paramref name="id"/> and attaches it to the <paramref name="parent"/>.
/// Prefer to call <see cref="ChromeLogic.DynamicWidgets.LoadWidget"/>.
/// </summary>
public static Widget LoadWidgetUnchecked(string id, Widget parent, WidgetArgs args)
{
return Ui.OpenWindow(widget, new WidgetArgs() { { "world", world }, { "orderManager", OrderManager }, { "worldRenderer", worldRenderer } });
return ModData.WidgetLoader.LoadWidget(ExtendWidgetArgs(args), parent, id);
}

// Who came up with the great idea of making these things
// impossible for the things that want them to access them directly?
public static Widget OpenWindow(string widget, WidgetArgs args)
/// <summary>
/// Returns a new widget args based on <paramref name="args"/>
/// and extended with values for 'orderManager', 'world' and 'worldRenderer'.
/// </summary>
public static WidgetArgs ExtendWidgetArgs(WidgetArgs args)
{
return Ui.OpenWindow(widget, new WidgetArgs(args)
return new WidgetArgs(args)
{
{ "world", worldRenderer.World },
{ "world", worldRenderer?.World },
{ "orderManager", OrderManager },
{ "worldRenderer", worldRenderer },
});
}

// Load a widget with world, orderManager, worldRenderer args, without adding it to the widget tree
public static Widget LoadWidget(World world, string id, Widget parent, WidgetArgs args)
{
return ModData.WidgetLoader.LoadWidget(new WidgetArgs(args)
{
{ "world", world },
{ "orderManager", OrderManager },
{ "worldRenderer", worldRenderer },
}, parent, id);
};
}

public static event Action LobbyInfoChanged = () => { };
Expand Down
124 changes: 109 additions & 15 deletions OpenRA.Game/Widgets/Widget.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,11 @@ public static void CloseWindow()
}
}

public static Widget OpenWindow(string id)
{
return OpenWindow(id, new WidgetArgs());
}

public static Widget OpenWindow(string id, WidgetArgs args)
/// <summary>
/// Loads the widget given by <paramref name="id"/> and pushes it as the current window onto <see cref="Root"/>.
/// Prefer to call <see cref="ChromeLogic.DynamicWidgets.OpenWindow"/>.
/// </summary>
public static Widget OpenWindowUnchecked(string id, WidgetArgs args)
{
var window = Game.ModData.WidgetLoader.LoadWidget(args, Root, id);
if (WindowList.Count > 0)
Expand All @@ -77,15 +76,11 @@ public static Widget CurrentWindow()
return WindowList.Count > 0 ? WindowList.Peek() : null;
}

public static T LoadWidget<T>(string id, Widget parent, WidgetArgs args) where T : Widget
{
if (LoadWidget(id, parent, args) is T widget)
return widget;

throw new InvalidOperationException($"Widget {id} is not of type {typeof(T).Name}");
}

public static Widget LoadWidget(string id, Widget parent, WidgetArgs args)
/// <summary>
/// Creates the widget given by <paramref name="id"/> and attaches it to the <paramref name="parent"/>.
/// Prefer to call <see cref="ChromeLogic.DynamicWidgets.LoadWidget"/>.
/// </summary>
public static Widget LoadWidgetUnchecked(string id, Widget parent, WidgetArgs args)
{
return Game.ModData.WidgetLoader.LoadWidget(args, parent, id);
}
Expand Down Expand Up @@ -180,6 +175,100 @@ public class ChromeLogic : IDisposable
public virtual void BecameHidden() { }
public virtual void BecameVisible() { }
protected virtual void Dispose(bool disposing) { }

/// <summary>
/// Classes deriving from <see cref="ChromeLogic"/> should create a nested class derived from <see cref="DynamicWidgets"/>.
/// Any child widgets the logic creates at runtime should be created via this class. This supports linting.
/// A constructor marked with <see cref="ObjectCreator.UseCtorAttribute"/> can accept a parameter named 'logicArgs'
/// of type <see cref="Dictionary{TKey, TValue}"/> with key <see cref="string"/> and value <see cref="MiniYaml"/>.
/// </summary>
public abstract class DynamicWidgets
{
// Can't be an IReadOnlySet<string> as mono doesn't have that.
public static readonly ISet<string> EmptySet =
new HashSet<string>();
public static readonly IReadOnlyDictionary<string, string> EmptyDictionary =
new Dictionary<string, string>();
public static readonly IReadOnlyDictionary<string, IReadOnlyCollection<string>> EmptyLookup =
new Dictionary<string, IReadOnlyCollection<string>>();

/// <summary>
/// For each widget that will be opened as a window at runtime, specific the widget ID.
/// This supports linting.
/// </summary>
public abstract ISet<string> WindowWidgetIds { get; }

/// <summary>
/// For each child widget ID to be created at runtime, specify the parent widget ID in this lookup.
/// This supports linting.
/// </summary>
public abstract IReadOnlyDictionary<string, string> ParentWidgetIdForChildWidgetId { get; }

/// <summary>
/// Like <see cref="ParentWidgetIdForChildWidgetId"/> but for any widgets created outside the widget tree passed to the logic class.
/// e.g. loading a widget onto something located on <see cref="Ui.Root"/>.
/// A blank parent ID indicates the parent *is* <see cref="Ui.Root"/>.
/// </summary>
public virtual IReadOnlyDictionary<string, string> OutOfTreeParentWidgetIdForChildWidgetId { get; } = EmptyDictionary;

/// <summary>
/// For each child panel template to be created at runtime, specify the parent dropdown widget IDs in this lookup.
/// This supports linting.
/// </summary>
public virtual IReadOnlyDictionary<string, IReadOnlyCollection<string>> ParentDropdownWidgetIdsFromPanelWidgetId { get; } = EmptyLookup;

/// <summary>
/// Creates the widget given by <paramref name="id"/> and opens it as a window.
/// The ID must be listed in <see cref="WindowWidgetIds"/>.
/// The widgets args will be extended with values for 'orderManager', 'world' and 'worldRenderer'.
/// </summary>
public Widget OpenWindow(string id, WidgetArgs args)
{
if (!WindowWidgetIds.Contains(id))
throw new ArgumentException($"'{id}' was not found in the {nameof(WindowWidgetIds)} collection.");
return Ui.OpenWindowUnchecked(id, Game.ExtendWidgetArgs(args));
}

/// <summary>
/// Creates the widget given by <paramref name="childWidgetId"/>.
/// The parent widget ID is determined via the <see cref="ParentWidgetIdForChildWidgetId"/> lookup.
/// This child will be attached to this parent widget, which is located within <paramref name="rootWidget"/>.
/// The widgets args will be extended with values for 'orderManager', 'world' and 'worldRenderer'.
/// </summary>
public Widget LoadWidget(Widget rootWidget, string childWidgetId, WidgetArgs args)
{
if (!ParentWidgetIdForChildWidgetId.TryGetValue(childWidgetId, out var parentId))
throw new ArgumentException($"'{childWidgetId}' was not found in the " +
$"{nameof(ParentWidgetIdForChildWidgetId)} lookup.");
return Ui.LoadWidgetUnchecked(childWidgetId, rootWidget.Get(parentId), Game.ExtendWidgetArgs(args));
}

public void ValidateWidgetOutOfTree(
Widget parentWidget,
string childWidgetId)
{
if (!OutOfTreeParentWidgetIdForChildWidgetId.TryGetValue(childWidgetId, out var parentId))
throw new ArgumentException($"'{childWidgetId}' was not found in the " +
$"{nameof(OutOfTreeParentWidgetIdForChildWidgetId)} lookup.");

if (parentWidget.Id != parentId)
throw new ArgumentException($"{nameof(parentWidget)} must be the parent of {nameof(childWidgetId)} " +
$"as listed in {nameof(OutOfTreeParentWidgetIdForChildWidgetId)} " +
$"but the listed ID {parentId} does not match the given ID {parentWidget.Id}");
}

/// <summary>
/// Like <see cref="LoadWidget"/>, but for any widgets created outside the widget tree passed to the logic class.
/// </summary>
public Widget LoadWidgetOutOfTree(Widget rootWidget, string childWidgetId, WidgetArgs args)
{
if (!OutOfTreeParentWidgetIdForChildWidgetId.TryGetValue(childWidgetId, out var parentId))
throw new ArgumentException($"'{childWidgetId}' was not found in the " +
$"{nameof(OutOfTreeParentWidgetIdForChildWidgetId)} lookup.");
var parentWidget = parentId == "" ? Ui.Root : rootWidget.Get(parentId);
return Ui.LoadWidgetUnchecked(childWidgetId, parentWidget, Game.ExtendWidgetArgs(args));
}
}
}

public struct WidgetBounds
Expand All @@ -202,6 +291,11 @@ public WidgetBounds(int x, int y, int width, int height)
{
return new Rectangle(X, Y, Width, Height);
}

public override readonly string ToString()
{
return $"{X},{Y},{Width},{Height}";
}
}

public abstract class Widget
Expand Down
24 changes: 19 additions & 5 deletions OpenRA.Mods.Cnc/Widgets/Logic/PreReleaseWarningPrompt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
#endregion

using System.Collections.Generic;
using OpenRA.Mods.Common.Widgets;
using OpenRA.Widgets;

Expand All @@ -18,20 +19,33 @@ public class PreReleaseWarningPrompt : ChromeLogic
{
static bool promptAccepted;

public class PreReleaseWarningPromptDynamicWidgets : DynamicWidgets
{
public override ISet<string> WindowWidgetIds { get; } = EmptySet;
public override IReadOnlyDictionary<string, string> ParentWidgetIdForChildWidgetId { get; } = EmptyDictionary;
public override IReadOnlyDictionary<string, string> OutOfTreeParentWidgetIdForChildWidgetId =>
new Dictionary<string, string>
{
{ "MAINMENU", "" },
};
}

readonly PreReleaseWarningPromptDynamicWidgets dynamicWidgets = new();

[ObjectCreator.UseCtor]
public PreReleaseWarningPrompt(Widget widget, World world, ModData modData)
public PreReleaseWarningPrompt(Widget widget, ModData modData)
{
if (!promptAccepted && modData.Manifest.Metadata.Version != "{DEV_VERSION}")
widget.Get<ButtonWidget>("CONTINUE_BUTTON").OnClick = () => ShowMainMenu(world);
widget.Get<ButtonWidget>("CONTINUE_BUTTON").OnClick = ShowMainMenu;
else
ShowMainMenu(world);
ShowMainMenu();
}

static void ShowMainMenu(World world)
void ShowMainMenu()
{
promptAccepted = true;
Ui.ResetAll();
Game.LoadWidget(world, "MAINMENU", Ui.Root, new WidgetArgs());
dynamicWidgets.LoadWidgetOutOfTree(Ui.Root, "MAINMENU", new WidgetArgs());
}
}
}

0 comments on commit d588c4a

Please sign in to comment.