Create In-Guidebook Errors (#28942)

* Create in-guidebook errors

* Localize client-facing parser error

* Uncomment line

* Missed another localization string

Content.Client/Guidebook/Controls/GuidebookError.xaml

@@ -0,0 +1,41 @@
+<BoxContainer xmlns="https://spacestation14.io"
+              xmlns:gfx="clr-namespace:Robust.Client.Graphics;assembly=Robust.Client"
+              Margin="5 5 5 5"
+              MinHeight="200">
+    <PanelContainer HorizontalExpand="True">
+        <PanelContainer.PanelOverride>
+            <gfx:StyleBoxFlat BorderThickness="2" BorderColor="White" />
+        </PanelContainer.PanelOverride>
+        <BoxContainer Orientation="Vertical">
+
+            <PanelContainer>
+                <PanelContainer.PanelOverride>
+                    <gfx:StyleBoxFlat BorderThickness="0 0 0 1" BackgroundColor="DarkRed" BorderColor="Black" />
+                </PanelContainer.PanelOverride>
+                <Label Margin="5" StyleClasses="bold" Text="{Loc 'guidebook-parser-error'}" />
+            </PanelContainer>
+
+            <OutputPanel Margin="5" MinHeight="75" VerticalExpand="True" Name="Original">
+                <OutputPanel.StyleBoxOverride>
+                    <gfx:StyleBoxFlat BorderThickness="0 0 0 1" BorderColor="Gray"
+                                      ContentMarginLeftOverride="3" ContentMarginRightOverride="3"
+                                      ContentMarginBottomOverride="3" ContentMarginTopOverride="3" />
+                </OutputPanel.StyleBoxOverride>
+            </OutputPanel>
+
+            <Collapsible Margin="5" MinHeight="75" VerticalExpand="True">
+                <CollapsibleHeading Title="{Loc 'guidebook-error-message' }" />
+                <CollapsibleBody VerticalExpand="True">
+                    <OutputPanel Name="Error" VerticalExpand="True" MinHeight="100">
+                        <OutputPanel.StyleBoxOverride>
+                            <gfx:StyleBoxFlat
+                                ContentMarginLeftOverride="3" ContentMarginRightOverride="3"
+                                ContentMarginBottomOverride="3" ContentMarginTopOverride="3" />
+                        </OutputPanel.StyleBoxOverride>
+                    </OutputPanel>
+                </CollapsibleBody>
+            </Collapsible>
+
+        </BoxContainer>
+    </PanelContainer>
+</BoxContainer>

Content.Client/Guidebook/Controls/GuidebookError.xaml.cs

@@ -0,0 +1,23 @@
+using JetBrains.Annotations;
+using Robust.Client.AutoGenerated;
+using Robust.Client.UserInterface.Controls;
+using Robust.Client.UserInterface.XAML;
+
+namespace Content.Client.Guidebook.Controls;
+
+[UsedImplicitly] [GenerateTypedNameReferences]
+public sealed partial class GuidebookError : BoxContainer
+{
+    public GuidebookError()
+    {
+        RobustXamlLoader.Load(this);
+    }
+
+    public GuidebookError(string original, string? error) : this()
+    {
+        Original.AddText(original);
+
+        if (error is not null)
+            Error.AddText(error);
+    }
+}

Content.Client/Guidebook/Controls/GuidebookWindow.xaml.cs

@@ -4,12 +4,10 @@ using Content.Client.UserInterface.ControlExtensions;
 using Content.Client.UserInterface.Controls;
 using Content.Client.UserInterface.Controls.FancyTree;
 using Content.Client.UserInterface.Systems.Info;
-using Content.Shared.CCVar;
 using Content.Shared.Guidebook;
 using Robust.Client.AutoGenerated;
 using Robust.Client.UserInterface.Controls;
 using Robust.Client.UserInterface.XAML;
-using Robust.Shared.Configuration;
 using Robust.Shared.ContentPack;
 using Robust.Shared.Prototypes;
 
@@ -18,15 +16,18 @@ namespace Content.Client.Guidebook.Controls;
 [GenerateTypedNameReferences]
 public sealed partial class GuidebookWindow : FancyWindow, ILinkClickHandler
 {
-    [Dependency] private readonly IResourceManager _resourceManager = default!;
     [Dependency] private readonly DocumentParsingManager _parsingMan = default!;
+    [Dependency] private readonly IResourceManager _resourceManager = default!;
 
     private Dictionary<ProtoId<GuideEntryPrototype>, GuideEntry> _entries = new();
 
+    private readonly ISawmill _sawmill;
+
     public GuidebookWindow()
     {
         RobustXamlLoader.Load(this);
         IoCManager.InjectDependencies(this);
+        _sawmill = Logger.GetSawmill("Guidebook");
 
         Tree.OnSelectedItemChanged += OnSelectionChanged;
 
@@ -36,6 +37,20 @@ public sealed partial class GuidebookWindow : FancyWindow, ILinkClickHandler
         };
     }
 
+    public void HandleClick(string link)
+    {
+        if (!_entries.TryGetValue(link, out var entry))
+            return;
+
+        if (Tree.TryGetIndexFromMetadata(entry, out var index))
+        {
+            Tree.ExpandParentEntries(index.Value);
+            Tree.SetSelectedIndex(index);
+        }
+        else
+            ShowGuide(entry);
+    }
+
     private void OnSelectionChanged(TreeItem? item)
     {
         if (item != null && item.Metadata is GuideEntry entry)
@@ -71,8 +86,9 @@ public sealed partial class GuidebookWindow : FancyWindow, ILinkClickHandler
 
         if (!_parsingMan.TryAddMarkup(EntryContainer, file.ReadToEnd()))
         {
-            EntryContainer.AddChild(new Label() { Text = "ERROR: Failed to parse document." });
+            // The guidebook will automatically display the in-guidebook error if it fails
-            Logger.Error($"Failed to parse contents of guide document {entry.Id}.");
+
+            _sawmill.Error($"Failed to parse contents of guide document {entry.Id}.");
         }
     }
 
@@ -124,8 +140,10 @@ public sealed partial class GuidebookWindow : FancyWindow, ILinkClickHandler
 
                     entry.Children = sortedChildren;
                 }
+
                 entries.ExceptWith(entry.Children);
             }
+
             rootEntries = entries.ToList();
         }
 
@@ -135,21 +153,25 @@ public sealed partial class GuidebookWindow : FancyWindow, ILinkClickHandler
             .ThenBy(rootEntry => Loc.GetString(rootEntry.Name));
     }
 
-    private void RepopulateTree(List<ProtoId<GuideEntryPrototype>>? roots = null, ProtoId<GuideEntryPrototype>? forcedRoot = null)
+    private void RepopulateTree(List<ProtoId<GuideEntryPrototype>>? roots = null,
+        ProtoId<GuideEntryPrototype>? forcedRoot = null)
     {
         Tree.Clear();
 
         HashSet<ProtoId<GuideEntryPrototype>> addedEntries = new();
 
-        TreeItem? parent = forcedRoot == null ? null : AddEntry(forcedRoot.Value, null, addedEntries);
+        var parent = forcedRoot == null ? null : AddEntry(forcedRoot.Value, null, addedEntries);
         foreach (var entry in GetSortedEntries(roots))
         {
             AddEntry(entry.Id, parent, addedEntries);
         }
+
         Tree.SetAllExpanded(true);
     }
 
-    private TreeItem? AddEntry(ProtoId<GuideEntryPrototype> id, TreeItem? parent, HashSet<ProtoId<GuideEntryPrototype>> addedEntries)
+    private TreeItem? AddEntry(ProtoId<GuideEntryPrototype> id,
+        TreeItem? parent,
+        HashSet<ProtoId<GuideEntryPrototype>> addedEntries)
     {
         if (!_entries.TryGetValue(id, out var entry))
             return null;
@@ -179,22 +201,6 @@ public sealed partial class GuidebookWindow : FancyWindow, ILinkClickHandler
         return item;
     }
 
-    public void HandleClick(string link)
-    {
-        if (!_entries.TryGetValue(link, out var entry))
-            return;
-
-        if (Tree.TryGetIndexFromMetadata(entry, out var index))
-        {
-            Tree.ExpandParentEntries(index.Value);
-            Tree.SetSelectedIndex(index);
-        }
-        else
-        {
-            ShowGuide(entry);
-        }
-    }
-
     private void HandleFilter()
     {
         var emptySearch = SearchBar.Text.Trim().Length == 0;
@@ -208,6 +214,5 @@ public sealed partial class GuidebookWindow : FancyWindow, ILinkClickHandler
                 element.SetHiddenState(true, SearchBar.Text.Trim());
             }
         }
-
     }
 }

Content.Client/Guidebook/DocumentParsingManager.cs

@@ -1,4 +1,5 @@
 using System.Linq;
+using Content.Client.Guidebook.Controls;
 using Content.Client.Guidebook.Richtext;
 using Content.Shared.Guidebook;
 using Pidgin;
@@ -7,6 +8,7 @@ using Robust.Shared.ContentPack;
 using Robust.Shared.Prototypes;
 using Robust.Shared.Reflection;
 using Robust.Shared.Sandboxing;
+using Robust.Shared.Utility;
 using static Pidgin.Parser;
 
 namespace Content.Client.Guidebook;
@@ -22,8 +24,10 @@ public sealed partial class DocumentParsingManager
     [Dependency] private readonly ISandboxHelper _sandboxHelper = default!;
 
     private readonly Dictionary<string, Parser<char, Control>> _tagControlParsers = new();
-    private Parser<char, Control> _tagParser = default!;
     private Parser<char, Control> _controlParser = default!;
+
+    private ISawmill _sawmill = default!;
+    private Parser<char, Control> _tagParser = default!;
     public Parser<char, IEnumerable<Control>> ControlParser = default!;
 
     public void Initialize()
@@ -32,7 +36,8 @@ public sealed partial class DocumentParsingManager
             .Assert(_tagControlParsers.ContainsKey, tag => $"unknown tag: {tag}")
             .Bind(tag => _tagControlParsers[tag]);
 
-        _controlParser = OneOf(_tagParser, TryHeaderControl, ListControlParser, TextControlParser).Before(SkipWhitespaces);
+        _controlParser = OneOf(_tagParser, TryHeaderControl, ListControlParser, TextControlParser)
+            .Before(SkipWhitespaces);
 
         foreach (var typ in _reflectionManager.GetAllChildren<IDocumentTag>())
         {
@@ -40,6 +45,8 @@ public sealed partial class DocumentParsingManager
         }
 
         ControlParser = SkipWhitespaces.Then(_controlParser.Many());
+
+        _sawmill = Logger.GetSawmill("Guidebook");
     }
 
     public bool TryAddMarkup(Control control, ProtoId<GuideEntryPrototype> entryId, bool log = true)
@@ -68,37 +75,57 @@ public sealed partial class DocumentParsingManager
         }
         catch (Exception e)
         {
-            if (log)
+            _sawmill.Error($"Encountered error while generating markup controls: {e}");
-                Logger.Error($"Encountered error while generating markup controls: {e}");
+
+            control.AddChild(new GuidebookError(text, e.ToStringBetter()));
+
             return false;
         }
 
         return true;
     }
 
-    private Parser<char, Control> CreateTagControlParser(string tagId, Type tagType, ISandboxHelper sandbox) => Map(
+    private Parser<char, Control> CreateTagControlParser(string tagId, Type tagType, ISandboxHelper sandbox)
+    {
+        return Map(
                 (args, controls) =>
+                {
+                    try
                     {
                         var tag = (IDocumentTag) sandbox.CreateInstance(tagType);
                         if (!tag.TryParseTag(args, out var control))
                         {
-                Logger.Error($"Failed to parse {tagId} args");
+                            _sawmill.Error($"Failed to parse {tagId} args");
-                return new Control();
+                            return new GuidebookError(args.ToString() ?? tagId, $"Failed to parse {tagId} args");
                         }
 
                         foreach (var child in controls)
                         {
                             control.AddChild(child);
                         }
+
                         return control;
+                    }
+                    catch (Exception e)
+                    {
+                        var output = args.Aggregate(string.Empty,
+                            (current, pair) => current + $"{pair.Key}=\"{pair.Value}\" ");
+
+                        _sawmill.Error($"Tag: {tagId} \n Arguments: {output}/>");
+                        return new GuidebookError($"Tag: {tagId}\nArguments: {output}", e.ToString());
+                    }
                 },
                 ParseTagArgs(tagId),
-        TagContentParser(tagId)).Labelled($"{tagId} control");
+                TagContentParser(tagId))
+            .Labelled($"{tagId} control");
+    }
 
     // Parse a bunch of controls until we encounter a matching closing tag.
-    private Parser<char, IEnumerable<Control>> TagContentParser(string tag) =>
+    private Parser<char, IEnumerable<Control>> TagContentParser(string tag)
-    OneOf(
+    {
+        return OneOf(
             Try(ImmediateTagEnd).ThenReturn(Enumerable.Empty<Control>()),
             TagEnd.Then(_controlParser.Until(TryTagTerminator(tag)).Labelled($"{tag} children"))
         );
     }
+}

Content.Client/Guidebook/DocumentParsingManager.static.cs

@@ -1,4 +1,5 @@
 using System.Linq;
+using Content.Client.Guidebook.Controls;
 using Pidgin;
 using Robust.Client.UserInterface;
 using Robust.Client.UserInterface.Controls;
@@ -14,10 +15,9 @@ public sealed partial class DocumentParsingManager
 {
     private const string ListBullet = "  › ";
 
-    #region Text Parsing
+    // Parser that consumes a - and then just parses normal rich text with some prefix text (a bullet point).
-    #region Basic Text Parsing
+    private static readonly Parser<char, char> TryEscapedChar = Try(Char('\\')
-    // Try look for an escaped character. If found, skip the escaping slash and return the character.
+        .Then(OneOf(
-    private static readonly Parser<char, char> TryEscapedChar = Try(Char('\\').Then(OneOf(
             Try(Char('<')),
             Try(Char('>')),
             Try(Char('\\')),
@@ -31,7 +31,8 @@ public sealed partial class DocumentParsingManager
 
     private static readonly Parser<char, Unit> SkipNewline = Whitespace.SkipUntil(Char('\n'));
 
-    private static readonly Parser<char, char> TrySingleNewlineToSpace = Try(SkipNewline).Then(SkipWhitespaces).ThenReturn(' ');
+    private static readonly Parser<char, char> TrySingleNewlineToSpace =
+        Try(SkipNewline).Then(SkipWhitespaces).ThenReturn(' ');
 
     private static readonly Parser<char, char> TextChar = OneOf(
         TryEscapedChar, // consume any backslashed being used to escape text
@@ -39,67 +40,117 @@ public sealed partial class DocumentParsingManager
         Any // just return the character.
     );
 
-    // like TextChar, but not skipping whitespace around newlines
     private static readonly Parser<char, char> QuotedTextChar = OneOf(TryEscapedChar, Any);
 
-    // Quoted text
+    private static readonly Parser<char, string> QuotedText =
-    private static readonly Parser<char, string> QuotedText = Char('"').Then(QuotedTextChar.Until(Try(Char('"'))).Select(string.Concat)).Labelled("quoted text");
+        Char('"').Then(QuotedTextChar.Until(Try(Char('"'))).Select(string.Concat)).Labelled("quoted text");
-    #endregion
+
+    private static readonly Parser<char, Unit> TryStartList =
+        Try(SkipNewline.Then(SkipWhitespaces).Then(Char('-'))).Then(SkipWhitespaces);
 
-    #region rich text-end markers
-    private static readonly Parser<char, Unit> TryStartList = Try(SkipNewline.Then(SkipWhitespaces).Then(Char('-'))).Then(SkipWhitespaces);
     private static readonly Parser<char, Unit> TryStartTag = Try(Char('<')).Then(SkipWhitespaces);
-    private static readonly Parser<char, Unit> TryStartParagraph = Try(SkipNewline.Then(SkipNewline)).Then(SkipWhitespaces);
-    private static readonly Parser<char, Unit> TryLookTextEnd = Lookahead(OneOf(TryStartTag, TryStartList, TryStartParagraph, Try(Whitespace.SkipUntil(End))));
-    #endregion
 
-    // parses text characters until it hits a text-end
+    private static readonly Parser<char, Unit> TryStartParagraph =
-    private static readonly Parser<char, string> TextParser = TextChar.AtLeastOnceUntil(TryLookTextEnd).Select(string.Concat);
+        Try(SkipNewline.Then(SkipNewline)).Then(SkipWhitespaces);
 
-    private static readonly Parser<char, Control> TextControlParser = Try(Map(text =>
+    private static readonly Parser<char, Unit> TryLookTextEnd =
+        Lookahead(OneOf(TryStartTag, TryStartList, TryStartParagraph, Try(Whitespace.SkipUntil(End))));
+
+    private static readonly Parser<char, string> TextParser =
+        TextChar.AtLeastOnceUntil(TryLookTextEnd).Select(string.Concat);
+
+    private static readonly Parser<char, Control> TextControlParser = Try(Map<char, string, Control>(text =>
                 {
-        var rt = new RichTextLabel()
+                    var rt = new RichTextLabel
                     {
                         HorizontalExpand = true,
-            Margin = new Thickness(0, 0, 0, 15.0f),
+                        Margin = new Thickness(0, 0, 0, 15.0f)
                     };
 
                     var msg = new FormattedMessage();
                     // THANK YOU RICHTEXT VERY COOL
                     // (text doesn't default to white).
                     msg.PushColor(Color.White);
-        msg.AddMarkup(text);
+
+                    // If the parsing fails, don't throw an error and instead make an inline error message
+                    string? error;
+                    if (!msg.TryAddMarkup(text, out error))
+                    {
+                        Logger.GetSawmill("Guidebook").Error("Failed to parse RichText in Guidebook");
+
+                        return new GuidebookError(text, error);
+                    }
+
                     msg.Pop();
                     rt.SetMessage(msg);
                     return rt;
-    }, TextParser).Cast<Control>()).Labelled("richtext");
+                },
-    #endregion
+                TextParser)
+            .Cast<Control>())
+        .Labelled("richtext");
 
-    #region Headers
+    private static readonly Parser<char, Control> HeaderControlParser = Try(Char('#'))
-    private static readonly Parser<char, Control> HeaderControlParser = Try(Char('#')).Then(SkipWhitespaces.Then(Map(text => new Label()
+        .Then(SkipWhitespaces.Then(Map(text => new Label
                 {
                     Text = text,
                     StyleClasses = { "LabelHeadingBigger" }
-    }, AnyCharExcept('\n').AtLeastOnceString()).Cast<Control>())).Labelled("header");
+                },
+                AnyCharExcept('\n').AtLeastOnceString())
+            .Cast<Control>()))
+        .Labelled("header");
 
-    private static readonly Parser<char, Control> SubHeaderControlParser = Try(String("##")).Then(SkipWhitespaces.Then(Map(text => new Label()
+    private static readonly Parser<char, Control> SubHeaderControlParser = Try(String("##"))
+        .Then(SkipWhitespaces.Then(Map(text => new Label
                 {
                     Text = text,
                     StyleClasses = { "LabelHeading" }
-    }, AnyCharExcept('\n').AtLeastOnceString()).Cast<Control>())).Labelled("subheader");
+                },
+                AnyCharExcept('\n').AtLeastOnceString())
+            .Cast<Control>()))
+        .Labelled("subheader");
 
     private static readonly Parser<char, Control> TryHeaderControl = OneOf(SubHeaderControlParser, HeaderControlParser);
+
+    private static readonly Parser<char, Control> ListControlParser = Try(Char('-'))
+        .Then(SkipWhitespaces)
+        .Then(Map(
+                control => new BoxContainer
+                {
+                    Children = { new Label { Text = ListBullet, VerticalAlignment = VAlignment.Top }, control },
+                    Orientation = LayoutOrientation.Horizontal
+                },
+                TextControlParser)
+            .Cast<Control>())
+        .Labelled("list");
+
+    #region Text Parsing
+
+    #region Basic Text Parsing
+
+    // Try look for an escaped character. If found, skip the escaping slash and return the character.
+
+
+    // like TextChar, but not skipping whitespace around newlines
+
+
+    // Quoted text
+
     #endregion
 
-    // Parser that consumes a - and then just parses normal rich text with some prefix text (a bullet point).
+    #region rich text-end markers
-    private static readonly Parser<char, Control> ListControlParser = Try(Char('-')).Then(SkipWhitespaces).Then(Map(
+
-        control => new BoxContainer()
+    #endregion
-        {
+
-            Children = { new Label() { Text = ListBullet, VerticalAlignment = VAlignment.Top, }, control },
+    // parses text characters until it hits a text-end
-            Orientation = LayoutOrientation.Horizontal,
+
-        }, TextControlParser).Cast<Control>()).Labelled("list");
+    #endregion
+
+    #region Headers
+
+    #endregion
 
     #region Tag Parsing
+
     // closing brackets for tags
     private static readonly Parser<char, Unit> TagEnd = Char('>').Then(SkipWhitespaces);
     private static readonly Parser<char, Unit> ImmediateTagEnd = String("/>").Then(SkipWhitespaces);
@@ -107,20 +158,24 @@ public sealed partial class DocumentParsingManager
     private static readonly Parser<char, Unit> TryLookTagEnd = Lookahead(OneOf(Try(TagEnd), Try(ImmediateTagEnd)));
 
     //parse tag argument key. any normal text character up until we hit a "="
-    private static readonly Parser<char, string> TagArgKey = LetterOrDigit.Until(Char('=')).Select(string.Concat).Labelled("tag argument key");
+    private static readonly Parser<char, string> TagArgKey =
+        LetterOrDigit.Until(Char('=')).Select(string.Concat).Labelled("tag argument key");
 
     // parser for a singular tag argument. Note that each TryQuoteOrChar will consume a whole quoted block before the Until() looks for whitespace
-    private static readonly Parser<char, (string, string)> TagArgParser = Map((key, value) => (key, value), TagArgKey, QuotedText).Before(SkipWhitespaces);
+    private static readonly Parser<char, (string, string)> TagArgParser =
+        Map((key, value) => (key, value), TagArgKey, QuotedText).Before(SkipWhitespaces);
 
     // parser for all tag arguments
-    private static readonly Parser<char, IEnumerable<(string, string)>> TagArgsParser = TagArgParser.Until(TryLookTagEnd);
+    private static readonly Parser<char, IEnumerable<(string, string)>> TagArgsParser =
+        TagArgParser.Until(TryLookTagEnd);
 
     // parser for an opening tag.
     private static readonly Parser<char, string> TryOpeningTag =
         Try(Char('<'))
             .Then(SkipWhitespaces)
             .Then(TextChar.Until(OneOf(Whitespace.SkipAtLeastOnce(), TryLookTagEnd)))
-        .Select(string.Concat).Labelled($"opening tag");
+            .Select(string.Concat)
+            .Labelled("opening tag");
 
     private static Parser<char, Dictionary<string, string>> ParseTagArgs(string tag)
     {
@@ -138,5 +193,6 @@ public sealed partial class DocumentParsingManager
             .Then(TagEnd)
             .Labelled($"closing {tag} tag");
     }
+
     #endregion
 }

Resources/Locale/en-US/guidebook/guidebook.ftl

@@ -3,6 +3,8 @@ guidebook-placeholder-text = Select an entry.
 guidebook-placeholder-text-2 = If you're new, head over to "New? Start here!"
 guidebook-filter-placeholder-text = Filter items
 
+guidebook-parser-error = Parser Error
+guidebook-error-message = Error Message
 
 guidebook-monkey-unspin = Unspin Monkey
 guidebook-monkey-disco = Disco Monkey