Update SeStringEvaluator, Payload, AutoTranslatePayload (#2411)

* Handle multiple col- entries in Completion.LookupTable

* Pass correct length to DecodeImpl

* Skip to payload end instead of reading remaining bytes

* Use SeStringEvaluator in AutoTranslatePayload

* Rework range check

- Loops through all entries
- Bumped amount of cols up to 8 for future proofing
- Use Ordinal search
- Actually parse ranges and check if the RowId is allowed

* Only check range if it has ranges

* Fix Group being off by 1

* Add AutoTranslatePayload re-encode test

* Remove redundant checks

* Add some AutoTranslatePayload.Text self-tests

* Remove AutoTranslatePayload text cache

- Lumina lookups aren't that taxing anymore
- Cache wasn't invalidated when the language override changes

Dalamud.Test/Game/Text/SeStringHandling/SeStringTests.cs

@@ -1,6 +1,10 @@
-using System;
+using System;
+using System.IO;
+
 using Dalamud.Configuration;
 using Dalamud.Game.Text.SeStringHandling;
+using Dalamud.Game.Text.SeStringHandling.Payloads;
+
 using Xunit;
 
 namespace Dalamud.Test.Game.Text.SeStringHandling
@@ -50,19 +54,41 @@ namespace Dalamud.Test.Game.Text.SeStringHandling
             var config = new MockConfig { Text = seString };
             PluginConfigurations.SerializeConfig(config);
         }
-        
+
         [Fact]
         public void TestConfigDeserializable()
         {
             var builder = new SeStringBuilder();
             var seString = builder.AddText("Some text").Build();
             var config = new MockConfig { Text = seString };
-            
+
             // This relies on the type information being maintained, which is why we're using these
             // static methods instead of default serialization/deserialization.
             var configSerialized = PluginConfigurations.SerializeConfig(config);
             var configDeserialized = (MockConfig)PluginConfigurations.DeserializeConfig(configSerialized);
             Assert.Equal(config, configDeserialized);
         }
+
+        [Theory]
+        [InlineData(49, 209)]
+        [InlineData(71, 7)]
+        [InlineData(62, 116)]
+        public void TestAutoTranslatePayloadReencode(uint group, uint key)
+        {
+            var payload = new AutoTranslatePayload(group, key);
+
+            Assert.Equal(group, payload.Group);
+            Assert.Equal(key, payload.Key);
+
+            var encoded = payload.Encode();
+            using var stream = new MemoryStream(encoded);
+            using var reader = new BinaryReader(stream);
+            var decodedPayload = Payload.Decode(reader) as AutoTranslatePayload;
+
+            Assert.Equal(group, decodedPayload.Group);
+            Assert.Equal(key, decodedPayload.Key);
+
+            Assert.Equal(encoded, decodedPayload.Encode());
+        }
     }
 }

Dalamud/Game/Text/Evaluator/SeStringEvaluator.cs

@@ -1629,23 +1629,61 @@ internal class SeStringEvaluator : IServiceType, ISeStringEvaluator
             return true;
 
         var isNoun = false;
-        var col = 0;
 
-        if (ranges.StartsWith("noun"))
+        var colIndex = 0;
-        {
+        Span<int> cols = stackalloc int[8];
-            isNoun = true;
+        cols.Clear();
-        }
+        var hasRanges = false;
-        else if (ranges.StartsWith("col"))
+        var isInRange = false;
-        {
-            var colRangeEnd = ranges.IndexOf(',');
-            if (colRangeEnd == -1)
-                colRangeEnd = ranges.Length;
 
-            col = int.Parse(ranges[4..colRangeEnd]);
+        while (!string.IsNullOrWhiteSpace(ranges))
-        }
+        {
-        else if (ranges.StartsWith("tail"))
+            // find the end of the current entry
+            var entryEnd = ranges.IndexOf(',');
+            if (entryEnd == -1)
+                entryEnd = ranges.Length;
+
+            if (ranges.StartsWith("noun", StringComparison.Ordinal))
+            {
+                isNoun = true;
+            }
+            else if (ranges.StartsWith("col", StringComparison.Ordinal) && colIndex < cols.Length)
+            {
+                cols[colIndex++] = int.Parse(ranges.AsSpan(4, entryEnd - 4));
+            }
+            else if (ranges.StartsWith("tail", StringComparison.Ordinal))
+            {
+                // currently not supported, since there are no known uses
+                context.Builder.Append(payload);
+                return false;
+            }
+            else
+            {
+                var dash = ranges.IndexOf('-');
+
+                hasRanges |= true;
+
+                if (dash == -1)
+                {
+                    isInRange |= int.Parse(ranges.AsSpan(0, entryEnd)) == rowId;
+                }
+                else
+                {
+                    isInRange |= rowId >= int.Parse(ranges.AsSpan(0, dash))
+                        && rowId <= int.Parse(ranges.AsSpan(dash + 1, entryEnd - dash - 1));
+                }
+            }
+
+            // if it's the end of the string, we're done
+            if (entryEnd == ranges.Length)
+                break;
+
+            // else, move to the next entry
+            ranges = ranges[(entryEnd + 1)..].TrimStart();
+        }
+
+        if (hasRanges && !isInRange)
         {
-            // couldn't find any, so we don't handle them :p
             context.Builder.Append(payload);
             return false;
         }
@@ -1663,7 +1701,23 @@ internal class SeStringEvaluator : IServiceType, ISeStringEvaluator
         }
         else if (this.dataManager.GetExcelSheet<RawRow>(context.Language, sheetName).TryGetRow(rowId, out var row))
         {
-            context.Builder.Append(row.ReadStringColumn(col));
+            if (colIndex == 0)
+            {
+                context.Builder.Append(row.ReadStringColumn(0));
+                return true;
+            }
+            else
+            {
+                for (var i = 0; i < colIndex; i++)
+                {
+                    var text = row.ReadStringColumn(cols[i]);
+                    if (!text.IsEmpty)
+                    {
+                        context.Builder.Append(text);
+                        break;
+                    }
+                }
+            }
         }
 
         return true;

Dalamud/Game/Text/SeStringHandling/Payload.cs

@@ -2,11 +2,8 @@ using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
 
-using Dalamud.Data;
 using Dalamud.Game.Text.SeStringHandling.Payloads;
-using Dalamud.Plugin.Services;
 
-using Newtonsoft.Json;
 using Serilog;
 
 // TODOs:
@@ -117,7 +114,7 @@ public abstract partial class Payload
         var chunkType = (SeStringChunkType)reader.ReadByte();
         var chunkLen = GetInteger(reader);
 
-        var packetStart = reader.BaseStream.Position;
+        var expressionsStart = reader.BaseStream.Position;
 
         // any unhandled payload types will be turned into a RawPayload with the exact same binary data
         switch (chunkType)
@@ -208,11 +205,10 @@ public abstract partial class Payload
         }
 
         payload ??= new RawPayload((byte)chunkType);
-        payload.DecodeImpl(reader, reader.BaseStream.Position + chunkLen - 1);
+        payload.DecodeImpl(reader, reader.BaseStream.Position + chunkLen);
 
-        // read through the rest of the packet
+        // skip to the end of the payload, in case the specific payload handler didn't read everything
-        var readBytes = (uint)(reader.BaseStream.Position - packetStart);
+        reader.BaseStream.Seek(expressionsStart + chunkLen + 1, SeekOrigin.Begin); // +1 for the END_BYTE marker
-        reader.ReadBytes((int)(chunkLen - readBytes + 1)); // +1 for the END_BYTE marker
 
         return payload;
     }

Dalamud/Game/Text/SeStringHandling/Payloads/AutoTranslatePayload.cs

@@ -1,14 +1,11 @@
-using System.Collections.Generic;
 using System.IO;
-using System.Linq;
 
-using Dalamud.Data;
+using Dalamud.Game.Text.Evaluator;
 
-using Lumina.Excel.Sheets;
+using Lumina.Text.Payloads;
 using Lumina.Text.ReadOnly;
 
 using Newtonsoft.Json;
-using Serilog;
 
 namespace Dalamud.Game.Text.SeStringHandling.Payloads;
 
@@ -17,7 +14,7 @@ namespace Dalamud.Game.Text.SeStringHandling.Payloads;
 /// </summary>
 public class AutoTranslatePayload : Payload, ITextProvider
 {
-    private string? text;
+    private ReadOnlySeString payload;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="AutoTranslatePayload"/> class.
@@ -34,6 +31,14 @@ public class AutoTranslatePayload : Payload, ITextProvider
         // TODO: friendlier ctor? not sure how to handle that given how weird the tables are
         this.Group = group;
         this.Key = key;
+
+        var ssb = Lumina.Text.SeStringBuilder.SharedPool.Get();
+        this.payload = ssb.BeginMacro(MacroCode.Fixed)
+                     .AppendUIntExpression(group - 1)
+                     .AppendUIntExpression(key)
+                     .EndMacro()
+                     .ToReadOnlySeString();
+        Lumina.Text.SeStringBuilder.SharedPool.Return(ssb);
     }
 
     /// <summary>
@@ -41,6 +46,7 @@ public class AutoTranslatePayload : Payload, ITextProvider
     /// </summary>
     internal AutoTranslatePayload()
     {
+        this.payload = default; // parsed by DecodeImpl
     }
     
     /// <summary>
@@ -68,8 +74,13 @@ public class AutoTranslatePayload : Payload, ITextProvider
     {
         get
         {
-            // wrap the text in the colored brackets that is uses in-game, since those are not actually part of any of the payloads
+            if (this.Group is 100 or 200)
-            return this.text ??= $"{(char)SeIconChar.AutoTranslateOpen} {this.Resolve()} {(char)SeIconChar.AutoTranslateClose}";
+            {
+                return Service<SeStringEvaluator>.Get().Evaluate(this.payload).ToString();
+            }
+
+            // wrap the text in the colored brackets that are used in-game, since those are not actually part of any of the fixed macro payload
+            return $"{(char)SeIconChar.AutoTranslateOpen} {Service<SeStringEvaluator>.Get().Evaluate(this.payload)} {(char)SeIconChar.AutoTranslateClose}";
         }
     }
 
@@ -85,95 +96,25 @@ public class AutoTranslatePayload : Payload, ITextProvider
     /// <inheritdoc/>
     protected override byte[] EncodeImpl()
     {
-        var keyBytes = MakeInteger(this.Key);
+        return this.payload.Data.ToArray();
-
-        var chunkLen = keyBytes.Length + 2;
-        var bytes = new List<byte>()
-        {
-            START_BYTE,
-            (byte)SeStringChunkType.AutoTranslateKey, (byte)chunkLen,
-            (byte)this.Group,
-        };
-        bytes.AddRange(keyBytes);
-        bytes.Add(END_BYTE);
-
-        return bytes.ToArray();
     }
 
     /// <inheritdoc/>
     protected override void DecodeImpl(BinaryReader reader, long endOfStream)
     {
-        // this seems to always be a bare byte, and not following normal integer encoding
+        var body = reader.ReadBytes((int)(endOfStream - reader.BaseStream.Position));
-        // the values in the table are all <70 so this is presumably ok
+        var rosps = new ReadOnlySePayloadSpan(ReadOnlySePayloadType.Macro, MacroCode.Fixed, body.AsSpan());
-        this.Group = reader.ReadByte();
 
-        this.Key = GetInteger(reader);
+        var span = rosps.EnvelopeByteLength <= 512 ? stackalloc byte[rosps.EnvelopeByteLength] : new byte[rosps.EnvelopeByteLength];
-    }
+        rosps.WriteEnvelopeTo(span);
+        this.payload = new ReadOnlySeString(span);
 
-    private static ReadOnlySeString ResolveTextCommand(TextCommand command)
+        if (rosps.TryGetExpression(out var expr1, out var expr2)
-    {
+            && expr1.TryGetUInt(out var group)
-        // TextCommands prioritize the `Alias` field, if it not empty
+            && expr2.TryGetUInt(out var key))
-        // Example for this is /rangerpose2l which becomes /blackrangerposeb in chat
-        return !command.Alias.IsEmpty ? command.Alias : command.Command;
-    }
-
-    private string Resolve()
-    {
-        string value = null;
-
-        var excelModule = Service<DataManager>.Get().Excel;
-        var completionSheet = excelModule.GetSheet<Completion>();
-
-        // try to get the row in the Completion table itself, because this is 'easiest'
-        // The row may not exist at all (if the Key is for another table), or it could be the wrong row
-        // (again, if it's meant for another table)
-
-        if (completionSheet.GetRowOrDefault(this.Key) is { } completion && completion.Group == this.Group)
         {
-            // if the row exists in this table and the group matches, this is actually the correct data
+            this.Group = group + 1;
-            value = completion.Text.ExtractText();
+            this.Key = key;
         }
-        else
-        {
-            try
-            {
-                // we need to get the linked table and do the lookup there instead
-                // in this case, there will only be one entry for this group id
-                var row = completionSheet.First(r => r.Group == this.Group);
-                // many of the names contain valid id ranges after the table name, but we don't need those
-                var actualTableName = row.LookupTable.ExtractText().Split('[')[0];
-
-                var name = actualTableName switch
-                {
-                    "Action" => excelModule.GetSheet<Lumina.Excel.Sheets.Action>().GetRow(this.Key).Name,
-                    "ActionComboRoute" => excelModule.GetSheet<ActionComboRoute>().GetRow(this.Key).Name,
-                    "BuddyAction" => excelModule.GetSheet<BuddyAction>().GetRow(this.Key).Name,
-                    "ClassJob" => excelModule.GetSheet<ClassJob>().GetRow(this.Key).Name,
-                    "Companion" => excelModule.GetSheet<Companion>().GetRow(this.Key).Singular,
-                    "CraftAction" => excelModule.GetSheet<CraftAction>().GetRow(this.Key).Name,
-                    "GeneralAction" => excelModule.GetSheet<GeneralAction>().GetRow(this.Key).Name,
-                    "GuardianDeity" => excelModule.GetSheet<GuardianDeity>().GetRow(this.Key).Name,
-                    "MainCommand" => excelModule.GetSheet<MainCommand>().GetRow(this.Key).Name,
-                    "Mount" => excelModule.GetSheet<Mount>().GetRow(this.Key).Singular,
-                    "Pet" => excelModule.GetSheet<Pet>().GetRow(this.Key).Name,
-                    "PetAction" => excelModule.GetSheet<PetAction>().GetRow(this.Key).Name,
-                    "PetMirage" => excelModule.GetSheet<PetMirage>().GetRow(this.Key).Name,
-                    "PlaceName" => excelModule.GetSheet<PlaceName>().GetRow(this.Key).Name,
-                    "Race" => excelModule.GetSheet<Race>().GetRow(this.Key).Masculine,
-                    "TextCommand" => AutoTranslatePayload.ResolveTextCommand(excelModule.GetSheet<TextCommand>().GetRow(this.Key)),
-                    "Tribe" => excelModule.GetSheet<Tribe>().GetRow(this.Key).Masculine,
-                    "Weather" => excelModule.GetSheet<Weather>().GetRow(this.Key).Name,
-                    _ => throw new Exception(actualTableName),
-                };
-
-                value = name.ExtractText();
-            }
-            catch (Exception e)
-            {
-                Log.Error(e, $"AutoTranslatePayload - failed to resolve: {this.Type} - Group: {this.Group}, Key: {this.Key}");
-            }
-        }
-
-        return value;
     }
 }

Dalamud/Game/Text/SeStringHandling/Payloads/RawPayload.cs

@@ -95,14 +95,12 @@ public class RawPayload : Payload
     /// <inheritdoc/>
     protected override byte[] EncodeImpl()
     {
-        var chunkLen = this.data.Length + 1;
-
         var bytes = new List<byte>()
         {
             START_BYTE,
             this.chunkType,
-            (byte)chunkLen,
         };
+        bytes.AddRange(MakeInteger((uint)this.data.Length)); // chunkLen
         bytes.AddRange(this.data);
 
         bytes.Add(END_BYTE);
@@ -113,6 +111,6 @@ public class RawPayload : Payload
     /// <inheritdoc/>
     protected override void DecodeImpl(BinaryReader reader, long endOfStream)
     {
-        this.data = reader.ReadBytes((int)(endOfStream - reader.BaseStream.Position + 1));
+        this.data = reader.ReadBytes((int)(endOfStream - reader.BaseStream.Position));
     }
 }

Dalamud/Interface/Internal/Windows/SelfTest/Steps/SeStringEvaluatorSelfTestStep.cs

@@ -1,6 +1,9 @@
 using Dalamud.Bindings.ImGui;
+using Dalamud.Configuration.Internal;
 using Dalamud.Game.ClientState;
 using Dalamud.Game.Text.Evaluator;
+using Dalamud.Game.Text.SeStringHandling.Payloads;
+
 using Lumina.Text.ReadOnly;
 
 namespace Dalamud.Interface.Internal.Windows.SelfTest.Steps;
@@ -75,6 +78,55 @@ internal class SeStringEvaluatorSelfTestStep : ISelfTestStep
                     return SelfTestStepResult.Waiting;
                 }
 
+                this.step++;
+                break;
+
+            case 2:
+                ImGui.Text("Checking AutoTranslatePayload.Text results..."u8);
+
+                var config = Service<DalamudConfiguration>.Get();
+                var originalLanguageOverride = config.LanguageOverride;
+
+                Span<(string Language, uint Group, uint Key, string ExpectedText)> tests = [
+                    ("en", 49u, 209u, " albino karakul "), // Mount
+                    ("en", 62u, 116u, " /echo "), // TextCommand - testing Command
+                    ("en", 62u, 143u, " /dutyfinder "), // TextCommand - testing Alias over Command
+                    ("en", 65u, 67u, " Minion of Light "), // Companion - testing noun handling for the german language (special case)
+                    ("en", 71u, 7u, " Phantom Geomancer "), // MKDSupportJob
+
+                    ("de", 49u, 209u, " Albino-Karakul "), // Mount
+                    ("de", 62u, 115u, " /freiegesellschaft "), // TextCommand - testing Alias over Command
+                    ("de", 62u, 116u, " /echo "), // TextCommand - testing Command
+                    ("de", 65u, 67u, " Begleiter des Lichts "), // Companion - testing noun handling for the german language (special case)
+                    ("de", 71u, 7u, " Phantom-Geomant "), // MKDSupportJob
+                ];
+
+                try
+                {
+                    foreach (var (language, group, key, expectedText) in tests)
+                    {
+                        config.LanguageOverride = language;
+
+                        var payload = new AutoTranslatePayload(group, key);
+
+                        if (payload.Text != expectedText)
+                        {
+                            ImGui.Text($"Test failed for Group {group}, Key {key}");
+                            ImGui.Text($"Expected: {expectedText}");
+                            ImGui.Text($"Got: {payload.Text}");
+
+                            if (ImGui.Button("Continue"u8))
+                                return SelfTestStepResult.Fail;
+
+                            return SelfTestStepResult.Waiting;
+                        }
+                    }
+                }
+                finally
+                {
+                    config.LanguageOverride = originalLanguageOverride;
+                }
+
                 return SelfTestStepResult.Pass;
         }