From f105a1a68d3e0342ede0fd52d8018fc50faacbbc Mon Sep 17 00:00:00 2001 From: samjviana Date: Sat, 1 Mar 2025 14:36:33 -0300 Subject: [PATCH 1/4] refactor `ImageTagHandler.cs` --- RecurrentErrorLogger.cs | 93 +++++++++++++++ TagHandlers/ImageHandler.cs | 118 ------------------- TagHandlers/ImageTagHandler.cs | 201 +++++++++++++++++++++++++++++++++ UIElements/UITextSnippet.cs | 21 +++- 4 files changed, 310 insertions(+), 123 deletions(-) create mode 100644 RecurrentErrorLogger.cs delete mode 100644 TagHandlers/ImageHandler.cs create mode 100644 TagHandlers/ImageTagHandler.cs diff --git a/RecurrentErrorLogger.cs b/RecurrentErrorLogger.cs new file mode 100644 index 0000000..7012a3b --- /dev/null +++ b/RecurrentErrorLogger.cs @@ -0,0 +1,93 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using Terraria.ModLoader; + +namespace RecipeBrowser { + /// + /// A helper class to log errors while avoiding spamming the log with the same error. + /// + public static class RecurrentErrorLogger { + /// + /// Stores log data for a specific error message. This is used to center data in a thread-safe manner. + /// + private class LogData { + /// + /// A queue of timestamps for each time the logger was called for this message. + /// + public Queue Timestamps { get; } = new Queue(); + + /// + /// The total number of times the logger actually logged the message. + /// + public int TotalCount { get; set; } = 0; + } + + private static Mod mod; + + // TODO: This could be a memory leak if the dictionary grows indefinitely. Consider adding a limit to the number of messages stored or a time limit. + /// + /// Stores the log data for each error message that has been logged. + /// + private static readonly ConcurrentDictionary LogRecords = new ConcurrentDictionary(); + + // TODO: `MaxLogsPerMessage`, `MaxMessagesWithinInterval` and `Interval` could be configurable. Maybe add them as configs to the mod. + /// + /// The maximum number of times that can be logged for a single message. + /// + private const int MaxLogsPerMessage = 5; + /// + /// The maximum number of times that an error can happen within a specific interval before it is logged. + /// + private const int MaxMessagesWithinInterval = 10; + /// + /// The interval in which the error can recur before it is logged. + /// + private static readonly TimeSpan Interval = TimeSpan.FromSeconds(1); + + /// + /// A lock object to ensure that the log data is accessed in a thread-safe manner. + /// + private static readonly object lockObject = new object(); + + /// + /// Logs an error message if the error happens more than a certain number of times within a specific interval. + /// To avoid spamming the log, the error is only logged a maximum number of times. + /// + /// The error message to log. + public static void MaybeLog(string message) { + if (mod == null) { + mod = ModContent.GetInstance(); + } + + LogData data = LogRecords.GetOrAdd(message, _ => new LogData()); + lock (lockObject) { + if (data.TotalCount >= MaxLogsPerMessage) { + return; + } + + data.Timestamps.Enqueue(DateTime.Now); + + if (data.Timestamps.Count > MaxMessagesWithinInterval && IsWithinInterval(data.Timestamps.Peek())) { + mod.Logger.Error($"Error happened more than {MaxMessagesWithinInterval} times within {Interval.TotalSeconds} seconds: {message}"); + + data.TotalCount++; + data.Timestamps.Clear(); + } + + if (data.TotalCount >= MaxLogsPerMessage) { + mod.Logger.Error($"Error logged more than {MaxLogsPerMessage} times, suppressing further logs"); + } + } + } + + /// + /// Checks if the given timestamp is within the interval. + /// + /// The timestamp to check. + /// if the timestamp is within the interval; otherwise, . + private static bool IsWithinInterval(DateTime timestamp) { + return DateTime.Now - timestamp <= Interval; + } + } +} \ No newline at end of file diff --git a/TagHandlers/ImageHandler.cs b/TagHandlers/ImageHandler.cs deleted file mode 100644 index 2e0690f..0000000 --- a/TagHandlers/ImageHandler.cs +++ /dev/null @@ -1,118 +0,0 @@ -using Microsoft.Xna.Framework; -using Microsoft.Xna.Framework.Graphics; -using System.Collections.Generic; -using System.Text; -using Terraria.UI.Chat; -using Newtonsoft.Json.Linq; -using Terraria; -using ReLogic.Graphics; -using System.Net; -using System.IO; -using System; -using ReLogic.Content; -using Terraria.ModLoader; -using System.Globalization; -using Terraria.ModLoader.UI; - -namespace RecipeBrowser.TagHandlers -{ - public class ImageTagHandler : ITagHandler - { - private class ImageSnippet : TextSnippet - { - Asset texture; - Asset Texture - { - get - { - if (texture == null) - { - texture = ModContent.Request(texturePath); - } - return texture; - } - } - private string tooltip; - public int vOffset; - private float otherScale; - - string texturePath; - public ImageSnippet(string texturePath, string tooltip = null, float scale = 1f, float otherScale = 1f) : base("", Color.White, scale) - { - DeleteWhole = true; - this.texturePath = texturePath; - this.tooltip = tooltip; - this.otherScale = otherScale; - } - - public override void OnHover() - { - if (!string.IsNullOrEmpty(tooltip)) - { - // Main.hoverItemName = tooltip; - UICommon.TooltipMouseText(tooltip); - } - } - - public override Color GetVisibleColor() - { - return Color; - } - - public override bool UniqueDraw(bool justCheckingString, out Vector2 size, SpriteBatch spriteBatch, Vector2 position = default(Vector2), Color color = default(Color), float scale = 1f) - { - size = Texture.Size() * new Vector2(otherScale) + new Vector2(0, vOffset); - if (!justCheckingString && color != Color.Black) - { - //size = Texture.Size() * new Vector2(1, scale) + new Vector2(0, vOffset); // TODO: Why was `new Vector2( 1, scale)`?? - //spriteBatch.Draw(Texture, position, color); - spriteBatch.Draw(Texture.Value, position + new Vector2(0, vOffset), null, color, 0, Vector2.Zero, otherScale, SpriteEffects.None, 0); - //if (scale > 1) - // Main.NewText(size); - //size = Vector2.Zero; - } - return true; - } - - public override float GetStringLength(DynamicSpriteFont font) - { - return Texture.Size().X * Scale; - } - } - - TextSnippet ITagHandler.Parse(string text, Color baseColor, string options) - { - // TODO: option for scale or absolute size - // TODO: option for tooltip/translation key - // TODO: option for frame (animated?) - // tTooltip - // f0;0;20;20 - // - string tooltip = null; - float scale = 1f; - int vOffset = 0; - string[] array = options.Split(','); - foreach (var option in array) - { - if (option.Length != 0) - { - if (option[0] == 't') - tooltip = option.Substring(1).Replace(';', ':'); - if (option[0] == 's') - float.TryParse(option.Substring(1), NumberStyles.Float, CultureInfo.InvariantCulture, out scale); // 0.5 is interpreted as 5 instead of 0.5 for users of some cultures, so need InvariantCulture - if (option[0] == 'v') - int.TryParse(option.Substring(1), out vOffset); - } - //return new TextSnippet("<" + text.Replace("\\[", "[").Replace("\\]", "]") + ">", baseColor, 1f); - } - if (ModContent.HasAsset(text)) - { - return new ImageSnippet(text, tooltip, 1f, scale) - { - vOffset = vOffset - }; - } - return new TextSnippet(text); - } - } -} diff --git a/TagHandlers/ImageTagHandler.cs b/TagHandlers/ImageTagHandler.cs new file mode 100644 index 0000000..9c7fa13 --- /dev/null +++ b/TagHandlers/ImageTagHandler.cs @@ -0,0 +1,201 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using Microsoft.Xna.Framework; +using Microsoft.Xna.Framework.Graphics; +using ReLogic.Content; +using ReLogic.Graphics; +using Terraria; +using Terraria.ModLoader; +using Terraria.ModLoader.UI; +using Terraria.UI.Chat; + +namespace RecipeBrowser.TagHandlers { + /// + /// Handles [image/...] tags, allowing to display images/textures where a text would be displayed. + /// + public class ImageTagHandler : ITagHandler { + /// + /// Simple structure to hold the options for the image tag. + /// + private class ImageTagOptions { + public string Tooltip { get; set; } + public float Scale { get; set; } = 1f; + public int VerticalOffset { get; set; } + } + + /// + /// A custom that displays the given texture as an image instead of text. + /// + private class ImageTagSnippet : TextSnippet { + private string Tooltip { get; } + private Asset Texture { get; } + public int VerticalOffset { get; } + + /// + /// Creates a new snippet to display an image in place of text. + /// + /// The path to the texture to be shown. + /// The tooltip to be shown when hovering over the image. + /// The scale of the image. + /// Vertical offset to move the image X rows up or down. + public ImageTagSnippet(string texturePath, string tooltip = null, float scale = 1f, int vOffset = 0) : base("", Color.White, scale) { + DeleteWhole = true; + Tooltip = tooltip; + VerticalOffset = vOffset; + + if (ModContent.HasAsset(texturePath)) { + Texture = ModContent.Request(texturePath); + } else { + ModContent.GetInstance().Logger.Warn($"ImageTagSnippet: Texture not found: {texturePath}"); + RecurrentErrorLogger.MaybeLog($"ImageTagSnippet: Texture not found: {texturePath}"); + } + } + + /// + /// Called when the mouse hovers over the image. Displays the tooltip if one have been set. + /// + public override void OnHover() { + if (!string.IsNullOrWhiteSpace(Tooltip)) { + // TODO: This might lead to conflicts between tooltips, which would make the tooltip flicker. Make sure to handle this case. + UICommon.TooltipMouseText(Tooltip); + } + } + + public override Color GetVisibleColor() { + return Color; + } + + /// + /// + /// This draws the image at a given position with the configured scale and vertical offset. + /// + public override bool UniqueDraw(bool justCheckingString, out Vector2 size, SpriteBatch spriteBatch, Vector2 position = default, Color color = default, float scale = 1f) { + if (Texture == null) { + size = Vector2.Zero; + return false; + } + + Vector2 textureSize = Texture.Size(); + textureSize.X *= Scale; + textureSize.Y *= Scale; + + textureSize.Y += VerticalOffset; + + size = textureSize; + + // TODO: Its not clear why this comparison is done and what it is supposed to do. + if (!justCheckingString && color != Color.Black) { + spriteBatch.Draw(Texture.Value, position + new Vector2(0, VerticalOffset), null, color, 0f, Vector2.Zero, Scale, SpriteEffects.None, 0f); + } + + return true; + } + + /// + /// Returns the correct length of the image based on the configured scale. + /// + /// The font to be used to calculate the length. This is not used in this implementation. + /// The length of the image. + public override float GetStringLength(DynamicSpriteFont font) { + return Texture.Size().X * Scale; + } + } + + /// + /// A dictionary that maps the option key to the corresponding parser function. + /// The parser function is responsible for parsing the option value and setting the corresponding property in the object. + /// It should have the signature: void ParserFunction(string optionValue, ImageTagOptions options) + /// + private static readonly Dictionary> optionParsers = new Dictionary> { + ['t'] = ParseTooltip, + ['s'] = ParseScale, + ['v'] = ParseVerticalOffset + }; + + /// + /// Parses the raw options string into a dictionary of key-value pairs. e.g.: "tBugNet,s0.8,v2" = { 't' => "BugNet", 's' => 0.8f, 'v' => 2 } + /// + /// A comma-separated list of pairs, where the first char is the key, and the rest is the value. + /// + private Dictionary ParseRawOptions(string options) { + Dictionary parsedOptions = new Dictionary(); + + if (string.IsNullOrWhiteSpace(options)) { + return parsedOptions; + } + + string[] pairs = options.Split(','); + foreach (string pair in pairs) { + // TODO: Two characters are always expected, maybe add some logging or error handling. + if (pair.Length < 2) { + continue; + } + + char key = pair[0]; + string value = pair[1..]; + + parsedOptions[key] = value; + } + + return parsedOptions; + } + + /// + /// Handler for the 't' option. Sets the tooltip for the image, that will be shown when hovering over it. + /// + private static void ParseTooltip(string optionValue, ImageTagOptions opts) { + opts.Tooltip = optionValue.Replace(';', ':'); + } + + /// + /// Handler for the 's' option. Sets the scale of the image. + /// + private static void ParseScale(string optionValue, ImageTagOptions opts) { + if (float.TryParse(optionValue, NumberStyles.Float, CultureInfo.InvariantCulture, out float scale)) { + opts.Scale = scale; + } else { + opts.Scale = 1f; + } + } + + /// + /// Handler for the 'v' option. Sets the vertical offset of the image (in rows). + /// + private static void ParseVerticalOffset(string optionValue, ImageTagOptions opts) { + if (int.TryParse(optionValue, out int offset)) { + opts.VerticalOffset = offset; + } else { + opts.VerticalOffset = 0; + } + } + + /// + /// Main method of the handler. Parses the given text and options and converts it into a if possible. + /// + /// The texture path of the image to be displayed. + /// The color of the text. This is not used in this implementation. + /// A comma-separated list of options to configure the image. e.g.: "tBugNet,s0.8,v2" + /// A that draws the image, or falls back to a simple if the image could not be loaded. + public TextSnippet Parse(string text, Color baseColor = default, string options = null) { + ImageTagOptions tagOptions = new ImageTagOptions(); + Dictionary parsedKeyValues = ParseRawOptions(options); + + // Apply each existing parser to the extracted option pairs. + foreach (KeyValuePair parsedOption in parsedKeyValues) { + char key = parsedOption.Key; + string value = parsedOption.Value; + + if (optionParsers.TryGetValue(key, out Action parser)) { + parser(value, tagOptions); + } + } + + try { + return new ImageTagSnippet(text, tagOptions.Tooltip, tagOptions.Scale, tagOptions.VerticalOffset); + } catch { + return new TextSnippet(text); + } + } + } +} \ No newline at end of file diff --git a/UIElements/UITextSnippet.cs b/UIElements/UITextSnippet.cs index b3dd9a4..59b8281 100644 --- a/UIElements/UITextSnippet.cs +++ b/UIElements/UITextSnippet.cs @@ -20,6 +20,10 @@ class UITextSnippet : UIElement private Vector2 _textSize = Vector2.Zero; private bool _isLarge; private Color _color = Color.White; + + private TextSnippet[] _cachedSnippets; + + private string _lastTextParsed; public string Text { @@ -118,19 +122,26 @@ protected override void DrawSelf(SpriteBatch spriteBatch) if (IsMouseHovering) Main.hoverItemName = HoverText; + // This ensures that the text is parsed only once per frame, and only when it needs to be. + if (Text != _lastTextParsed) + { + // Main.NewText($"_lastTextParsed: {_lastTextParsed} != Text: {Text}"); + _lastTextParsed = Text; + _cachedSnippets = ChatManager.ParseMessage(Text, Color.White).ToArray(); + ChatManager.ConvertNormalSnippets(_cachedSnippets); + } + var font = _isLarge ? FontAssets.DeathText : FontAssets.MouseText; int hoveredSnippet = -1; - TextSnippet[] textSnippets = ChatManager.ParseMessage(Text, Color.White).ToArray(); - ChatManager.ConvertNormalSnippets(textSnippets); - ChatManager.DrawColorCodedStringWithShadow(spriteBatch, font.Value, textSnippets, pos, 0f, Vector2.Zero, new Vector2(_textScale), out hoveredSnippet); + ChatManager.DrawColorCodedStringWithShadow(spriteBatch, font.Value, _cachedSnippets, pos, 0f, Vector2.Zero, new Vector2(_textScale), out hoveredSnippet); if (hoveredSnippet > -1) { // annoying click. Main.NewText(hoveredSnippet); - textSnippets[hoveredSnippet].OnHover(); + _cachedSnippets[hoveredSnippet].OnHover(); if (Main.mouseLeft && Main.mouseLeftRelease) { - textSnippets[hoveredSnippet].OnClick(); + _cachedSnippets[hoveredSnippet].OnClick(); } } } From dc4131e468848c6c1902c553b474188e9d2c1614 Mon Sep 17 00:00:00 2001 From: samjviana Date: Sat, 1 Mar 2025 16:47:24 -0300 Subject: [PATCH 2/4] add a texture cache to the `ImageTagSnippet` to improve performance --- TagHandlers/ImageTagHandler.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/TagHandlers/ImageTagHandler.cs b/TagHandlers/ImageTagHandler.cs index 9c7fa13..efe3107 100644 --- a/TagHandlers/ImageTagHandler.cs +++ b/TagHandlers/ImageTagHandler.cs @@ -28,6 +28,7 @@ private class ImageTagOptions { /// A custom that displays the given texture as an image instead of text. /// private class ImageTagSnippet : TextSnippet { + private static Dictionary> _textureCache = new Dictionary>(); private string Tooltip { get; } private Asset Texture { get; } public int VerticalOffset { get; } @@ -44,12 +45,15 @@ public ImageTagSnippet(string texturePath, string tooltip = null, float scale = Tooltip = tooltip; VerticalOffset = vOffset; - if (ModContent.HasAsset(texturePath)) { - Texture = ModContent.Request(texturePath); - } else { - ModContent.GetInstance().Logger.Warn($"ImageTagSnippet: Texture not found: {texturePath}"); - RecurrentErrorLogger.MaybeLog($"ImageTagSnippet: Texture not found: {texturePath}"); + if (!_textureCache.TryGetValue(texturePath, out Asset texture)) { + if (ModContent.HasAsset(texturePath)) { + texture = ModContent.Request(texturePath); + _textureCache[texturePath] = texture; + } else { + RecurrentErrorLogger.MaybeLog($"ImageTagSnippet: Texture not found: {texturePath}"); + } } + Texture = texture; } /// From 1775360fc119ecc98bfa0629dc384ae4eeb32459 Mon Sep 17 00:00:00 2001 From: samjviana Date: Sat, 1 Mar 2025 19:46:00 -0300 Subject: [PATCH 3/4] added a simple error logging to the main parser --- TagHandlers/ImageTagHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TagHandlers/ImageTagHandler.cs b/TagHandlers/ImageTagHandler.cs index efe3107..09719eb 100644 --- a/TagHandlers/ImageTagHandler.cs +++ b/TagHandlers/ImageTagHandler.cs @@ -61,7 +61,6 @@ public ImageTagSnippet(string texturePath, string tooltip = null, float scale = /// public override void OnHover() { if (!string.IsNullOrWhiteSpace(Tooltip)) { - // TODO: This might lead to conflicts between tooltips, which would make the tooltip flicker. Make sure to handle this case. UICommon.TooltipMouseText(Tooltip); } } @@ -198,6 +197,7 @@ public TextSnippet Parse(string text, Color baseColor = default, string options try { return new ImageTagSnippet(text, tagOptions.Tooltip, tagOptions.Scale, tagOptions.VerticalOffset); } catch { + RecurrentErrorLogger.MaybeLog($"ImageTagHandler: Failed to create image snippet for {text}"); return new TextSnippet(text); } } From ad8b21fc2daa56aa4bd76e885e01d43d33ee3821 Mon Sep 17 00:00:00 2001 From: samjviana Date: Sat, 1 Mar 2025 23:06:14 -0300 Subject: [PATCH 4/4] add a error logging to the option pair handling --- TagHandlers/ImageTagHandler.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/TagHandlers/ImageTagHandler.cs b/TagHandlers/ImageTagHandler.cs index 09719eb..67052a2 100644 --- a/TagHandlers/ImageTagHandler.cs +++ b/TagHandlers/ImageTagHandler.cs @@ -87,7 +87,6 @@ public override bool UniqueDraw(bool justCheckingString, out Vector2 size, Sprit size = textureSize; - // TODO: Its not clear why this comparison is done and what it is supposed to do. if (!justCheckingString && color != Color.Black) { spriteBatch.Draw(Texture.Value, position + new Vector2(0, VerticalOffset), null, color, 0f, Vector2.Zero, Scale, SpriteEffects.None, 0f); } @@ -130,8 +129,8 @@ private Dictionary ParseRawOptions(string options) { string[] pairs = options.Split(','); foreach (string pair in pairs) { - // TODO: Two characters are always expected, maybe add some logging or error handling. if (pair.Length < 2) { + RecurrentErrorLogger.MaybeLog($"ImageTagHandler: Invalid option pair: {pair}"); continue; }