merge: Hellion Chat 0.5.1 — Backlog Sweep

2026-05-02 21:34:50 +02:00
parent e66ae1f5b4 4ba5004322
commit bfacf150a3
16 changed files with 202 additions and 55 deletions
@@ -4,7 +4,7 @@
             0.1.0 is our bootstrap release; the underlying Chat 2 base is
             called out in the yaml changelog so users can see what it
             derives from. -->
-        <Version>0.5.0</Version>
+        <Version>0.5.1</Version>
        <ImplicitUsings>enable</ImplicitUsings>
        <!-- HellionChat fork: assembly is renamed so Dalamud uses
             pluginConfigs/HellionChat instead of pluginConfigs/ChatTwo,
@@ -35,20 +35,20 @@ public static class EmoteCache
        public Emote Emote { get; set; }

        [JsonPropertyName("id")]
-        public string Id { get; set; }
+        public required string Id { get; set; }
    }

    [Serializable]
    public struct Emote()
    {
        [JsonPropertyName("id")]
-        public string Id { get; set; }
+        public required string Id { get; set; }

        [JsonPropertyName("code")]
-        public string Code { get; set; }
+        public required string Code { get; set; }

        [JsonPropertyName("imageType")]
-        public string ImageType { get; set; }
+        public required string ImageType { get; set; }
    }

    public enum LoadingState
@@ -66,7 +66,7 @@ public static class EmoteCache

    public static string[] SortedCodeArray = [];

-    public static async void LoadData()
+    public static async Task LoadData()
    {
        if (State is not LoadingState.Unloaded)
            return;
@@ -40,6 +40,36 @@ tags:
  - Replacement
  - Privacy
 changelog: |-
+  **Hellion Chat 0.5.1 — Backlog Sweep**
+
+  Pure hardening and polish. No new features. Eight backlog items
+  from the v0.5.0 codebase review collected into one patch:
+
+  - Cleanup preview now flags itself as out-of-date when the user
+    edits the whitelist after the last refresh, and the refresh
+    button is visually emphasised in that state
+  - Greeted Auto-Tell-Tabs now also dim their selection and hover
+    backgrounds in the sidebar, not just the text
+  - Performance section in the General tab moves to the standard
+    HelpMarker tooltip pattern instead of a wall-of-text description
+  - Tabs and Database settings tabs pull their display name from
+    HellionStrings instead of the upstream Language bundle, so all
+    eight tabs share one i18n source
+  - FontChooser results are now marshalled onto the framework thread
+    via Plugin.Framework.Run instead of being written to settings
+    state directly from the threadpool
+  - EmoteCache.LoadData drops async void and the four CS8618 build
+    warnings the build has been carrying since v0.4.0
+  - All MessageStore SQL paths that fed dynamic value lists into
+    interpolated SQL now use named parameter bindings via a new
+    BindIntList helper. Same behaviour, defence against future
+    user-input regressions
+
+  Configuration version is unchanged at 10. No migration. Existing
+  installs upgrade silently.
+
+  Based on Chat 2 1.35.3 (upstream Infiziert90/ChatTwo, EUPL-1.2).
+
  **Hellion Chat 0.5.0 — Settings UX polish**

  The settings window has been pulled apart and rebuilt around eight
@@ -239,6 +239,9 @@ internal class MessageStore : IDisposable

    private bool ColumnExists(string table, string column)
    {
+        // PRAGMA does not accept SQLite parameter bindings. The table name is
+        // a compile-time constant fed in from internal call sites, so the
+        // interpolation cannot be reached from any user-controlled path.
        using var cmd = Connection.CreateCommand();
        cmd.CommandText = $"PRAGMA table_info({table});";
        using var reader = cmd.ExecuteReader();
@@ -298,8 +301,10 @@ internal class MessageStore : IDisposable
    {
        Plugin.Log.Information($"Setting version {version}");
        using var cmd = Connection.CreateCommand();
-        // Parameters aren't supported for PRAGMA queries, and you can't set the
-        // version with a pragma_ function.
+        // PRAGMA does not accept SQLite parameter bindings, and there is no
+        // pragma_ function variant that can set the version either. The
+        // version is a compile-time int from the migration sequence, never
+        // user input.
        cmd.CommandText = $"PRAGMA user_version = {version};";
        cmd.ExecuteNonQuery();
    }
@@ -346,31 +351,44 @@ internal class MessageStore : IDisposable
                throw new ArgumentOutOfRangeException(nameof(chatTypeDaysMap), "Negative retention is not allowed.");

        var nowMs = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds();
-        var clauses = new List<string>();
-        foreach (var (type, days) in chatTypeDaysMap)
-        {
-            var cutoff = nowMs - days * 86400000L;
-            clauses.Add($"(ChatType = {type} AND Date < {cutoff})");
-        }

-        // Catch-all for channels without an explicit override. "0" is treated
-        // as "do not delete by default" — without an explicit user override,
-        // unmapped channels stay forever instead of getting wiped immediately.
-        if (defaultDays > 0)
-        {
-            var cutoff = nowMs - defaultDays * 86400000L;
-            var explicitTypes = chatTypeDaysMap.Count > 0
-                ? string.Join(",", chatTypeDaysMap.Keys)
-                : "-1"; // empty list would produce invalid SQL
-            clauses.Add($"(ChatType NOT IN ({explicitTypes}) AND Date < {cutoff})");
-        }
-
-        if (clauses.Count == 0)
+        if (chatTypeDaysMap.Count == 0 && defaultDays <= 0)
            return 0;

        long deleted;
        using (var cmd = Connection.CreateCommand())
        {
+            var clauses = new List<string>();
+            var index = 0;
+            foreach (var (type, days) in chatTypeDaysMap)
+            {
+                var cutoff = nowMs - days * 86400000L;
+                var typeParam = $"$type{index}";
+                var cutoffParam = $"$cutoff{index}";
+                cmd.Parameters.AddWithValue(typeParam, type);
+                cmd.Parameters.AddWithValue(cutoffParam, cutoff);
+                clauses.Add($"(ChatType = {typeParam} AND Date < {cutoffParam})");
+                index++;
+            }
+
+            // Catch-all for channels without an explicit override. "0" is
+            // treated as "do not delete by default" — without an explicit
+            // user override, unmapped channels stay forever instead of
+            // getting wiped immediately.
+            if (defaultDays > 0)
+            {
+                var defaultCutoff = nowMs - defaultDays * 86400000L;
+                cmd.Parameters.AddWithValue("$defaultCutoff", defaultCutoff);
+
+                var explicitPlaceholders = chatTypeDaysMap.Count > 0
+                    ? BindIntList(cmd, "explicit", chatTypeDaysMap.Keys)
+                    : "-1"; // empty list would produce invalid SQL
+                clauses.Add($"(ChatType NOT IN ({explicitPlaceholders}) AND Date < $defaultCutoff)");
+            }
+
+            if (clauses.Count == 0)
+                return 0;
+
            cmd.CommandText = $"DELETE FROM messages WHERE {string.Join(" OR ", clauses)};";
            cmd.CommandTimeout = 600;
            deleted = cmd.ExecuteNonQuery();
@@ -395,11 +413,11 @@ internal class MessageStore : IDisposable
            throw new InvalidOperationException("CleanupRetainOnly requires at least one allowed ChatType. Use ClearMessages for a full wipe.");
        }

-        var inList = string.Join(",", allowedTypes);
        long deleted;
        using (var cmd = Connection.CreateCommand())
        {
-            cmd.CommandText = $"DELETE FROM messages WHERE ChatType NOT IN ({inList});";
+            var placeholders = BindIntList(cmd, "ct", allowedTypes);
+            cmd.CommandText = $"DELETE FROM messages WHERE ChatType NOT IN ({placeholders});";
            cmd.CommandTimeout = 600;
            deleted = cmd.ExecuteNonQuery();
        }
@@ -512,15 +530,16 @@ internal class MessageStore : IDisposable
        DateTimeOffset? from,
        DateTimeOffset? to)
    {
+        var cmd = Connection.CreateCommand();
+
        var clauses = new List<string> { "deleted = false" };
        if (chatTypes is { Count: > 0 })
-            clauses.Add($"ChatType IN ({string.Join(",", chatTypes)})");
+            clauses.Add($"ChatType IN ({BindIntList(cmd, "exct", chatTypes)})");
        if (from is not null)
            clauses.Add("Date >= $From");
        if (to is not null)
            clauses.Add("Date <= $To");

-        var cmd = Connection.CreateCommand();
        cmd.CommandText = @"
            SELECT
                Id,
@@ -693,16 +712,17 @@ internal class MessageStore : IDisposable

    internal long CountDateRange(DateTime after, DateTime before, IEnumerable<byte> channels, ulong? receiver = null)
    {
+        using var cmd = Connection.CreateCommand();
+
        List<string> whereClauses = ["deleted = false"];
        if (receiver != null)
            whereClauses.Add("Receiver = $Receiver");

        whereClauses.Add("Date BETWEEN $After AND $Before");
-        whereClauses.Add($"ChatType IN ({string.Join(", ", channels)})");
+        whereClauses.Add($"ChatType IN ({BindIntList(cmd, "cdr", channels.Select(c => (int)c))})");

        var whereClause = "WHERE " + string.Join(" AND ", whereClauses);

-        using var cmd = Connection.CreateCommand();
        // Select last N messages by date DESC, but reverse the order to get
        // them in ascending order.
        cmd.CommandText = @"
@@ -722,16 +742,17 @@ internal class MessageStore : IDisposable

    internal MessageEnumerator GetDateRange(DateTime after, DateTime before, IEnumerable<byte> channels, ulong? receiver = null)
    {
+        var cmd = Connection.CreateCommand();
+
        List<string> whereClauses = ["deleted = false"];
        if (receiver != null)
            whereClauses.Add("Receiver = $Receiver");

        whereClauses.Add("Date BETWEEN $After AND $Before");
-        whereClauses.Add($"ChatType IN ({string.Join(", ", channels)})");
+        whereClauses.Add($"ChatType IN ({BindIntList(cmd, "gdr", channels.Select(c => (int)c))})");

        var whereClause = $"WHERE {string.Join(" AND ", whereClauses)}";

-        var cmd = Connection.CreateCommand();
        // Select last N messages by date DESC, but reverse the order to get
        // them in ascending order.
        cmd.CommandText = @"
@@ -763,16 +784,17 @@ internal class MessageStore : IDisposable

    internal MessageEnumerator GetPagedDateRange(DateTime after, DateTime before, IEnumerable<byte> channels, ulong? receiver = null, int page = 0)
    {
+        var cmd = Connection.CreateCommand();
+
        List<string> whereClauses = ["deleted = false"];
        if (receiver != null)
            whereClauses.Add("Receiver = $Receiver");

        whereClauses.Add("Date BETWEEN $After AND $Before");
-        whereClauses.Add($"ChatType IN ({string.Join(", ", channels)})");
+        whereClauses.Add($"ChatType IN ({BindIntList(cmd, "pdr", channels.Select(c => (int)c))})");

        var whereClause = $"WHERE {string.Join(" AND ", whereClauses)}";

-        var cmd = Connection.CreateCommand();
        // Select last N messages by date DESC, but reverse the order to get
        // them in ascending order.
        cmd.CommandText = @"
@@ -806,6 +828,24 @@ internal class MessageStore : IDisposable

        return new MessageEnumerator(cmd.ExecuteReader());
    }
+
+    // Build "$prefix0,$prefix1,..." placeholder list and bind values to
+    // the command. SQLite has no native array parameter, so we generate
+    // the list at runtime and bind each entry under its own name. Used
+    // for IN-clauses and similar dynamic-arity SQL fragments.
+    private static string BindIntList(SqliteCommand cmd, string prefix, IEnumerable<int> values)
+    {
+        var names = new List<string>();
+        var index = 0;
+        foreach (var value in values)
+        {
+            var name = $"${prefix}{index}";
+            cmd.Parameters.AddWithValue(name, value);
+            names.Add(name);
+            index++;
+        }
+        return string.Join(",", names);
+    }
 }

 internal class MessageEnumerator(DbDataReader reader) : IEnumerable<Message>, IDisposable, IAsyncDisposable
@@ -241,7 +241,7 @@ public sealed class Plugin : IDalamudPlugin
            Interface.UiBuilder.OpenMainUi += OpenMainUi;

            if (Config.ShowEmotes)
-                Task.Run(EmoteCache.LoadData);
+                _ = EmoteCache.LoadData(); // Fire-and-forget intentional, exceptions are caught inside

            #if !DEBUG
            // Avoid 300ms hitch when sending first message by preloading the
@@ -64,6 +64,7 @@ internal class HellionStrings
    internal static string Cleanup_Heading => Get(nameof(Cleanup_Heading));
    internal static string Cleanup_Help_Intro => Get(nameof(Cleanup_Help_Intro));
    internal static string Cleanup_Help_SavedNote => Get(nameof(Cleanup_Help_SavedNote));
+    internal static string Cleanup_Preview_Stale => Get(nameof(Cleanup_Preview_Stale));
    internal static string Retention_Help_SavedNote => Get(nameof(Retention_Help_SavedNote));
    internal static string Cleanup_RefreshPreview => Get(nameof(Cleanup_RefreshPreview));
    internal static string Cleanup_NoPreview => Get(nameof(Cleanup_NoPreview));
@@ -196,6 +197,8 @@ internal class HellionStrings
    internal static string Settings_Tab_Appearance => Get(nameof(Settings_Tab_Appearance));
    internal static string Settings_Tab_Window => Get(nameof(Settings_Tab_Window));
    internal static string Settings_Tab_Chat => Get(nameof(Settings_Tab_Chat));
+    internal static string Settings_Tab_Tabs => Get(nameof(Settings_Tab_Tabs));
+    internal static string Settings_Tab_Database => Get(nameof(Settings_Tab_Database));
    internal static string Settings_Tab_Information => Get(nameof(Settings_Tab_Information));

    // Hellion Chat — General-Tab section headings
@@ -81,6 +81,9 @@
  <data name="Retention_Help_SavedNote" xml:space="preserve">
    <value>Der manuelle Lauf nutzt deine GESPEICHERTE Retention-Policy, nicht die Slider-Werte oben. Klicke zuerst Speichern, wenn der Lauf deine aktuellen Änderungen anwenden soll.</value>
  </data>
+  <data name="Cleanup_Preview_Stale" xml:space="preserve">
+    <value>Vorschau veraltet, deine Whitelist hat sich seit dem letzten Aktualisieren geändert. Klicke Aktualisieren, um neu zu berechnen.</value>
+  </data>
  <data name="Cleanup_RefreshPreview" xml:space="preserve">
    <value>Vorschau aktualisieren</value>
  </data>
@@ -445,6 +448,12 @@
  <data name="Settings_Tab_Chat" xml:space="preserve">
    <value>Chat</value>
  </data>
+  <data name="Settings_Tab_Tabs" xml:space="preserve">
+    <value>Kanäle</value>
+  </data>
+  <data name="Settings_Tab_Database" xml:space="preserve">
+    <value>Datenbank</value>
+  </data>
  <data name="Settings_Tab_Information" xml:space="preserve">
    <value>Über</value>
  </data>
@@ -81,6 +81,9 @@
  <data name="Retention_Help_SavedNote" xml:space="preserve">
    <value>The manual sweep uses your SAVED retention policy, not the slider values above. Click Save first if you want the run to apply your current edits.</value>
  </data>
+  <data name="Cleanup_Preview_Stale" xml:space="preserve">
+    <value>Preview is out of date — your whitelist has changed since the last refresh. Click Refresh to recalculate.</value>
+  </data>
  <data name="Cleanup_RefreshPreview" xml:space="preserve">
    <value>Refresh preview</value>
  </data>
@@ -445,6 +448,12 @@
  <data name="Settings_Tab_Chat" xml:space="preserve">
    <value>Chat</value>
  </data>
+  <data name="Settings_Tab_Tabs" xml:space="preserve">
+    <value>Tabs</value>
+  </data>
+  <data name="Settings_Tab_Database" xml:space="preserve">
+    <value>Database</value>
+  </data>
  <data name="Settings_Tab_Information" xml:space="preserve">
    <value>Information</value>
  </data>
@@ -1374,8 +1374,18 @@ public sealed class ChatLogWindow : Window
                    {
                        // Dim the tab name once the user marked the partner
                        // as greeted, so a glance at the sidebar tells them
-                        // who still needs attention.
+                        // who still needs attention. Selectable has no idle
+                        // background slot in ImGui, so the dim only applies
+                        // to the selected and hovered states — the text dim
+                        // alone signals greeted in the idle state.
+                        var headerBase = ImGui.GetColorU32(ImGuiCol.Header);
+                        var hoverBase = ImGui.GetColorU32(ImGuiCol.HeaderHovered);
+                        var dimHeader = (headerBase & 0xFF000000u) | ((headerBase & 0x00FEFEFEu) >> 1);
+                        var dimHover = (hoverBase & 0xFF000000u) | ((hoverBase & 0x00FEFEFEu) >> 1);
+
                        using (ImRaii.PushColor(ImGuiCol.Text, ImGui.GetColorU32(ImGuiCol.TextDisabled)))
+                        using (ImRaii.PushColor(ImGuiCol.Header, dimHeader))
+                        using (ImRaii.PushColor(ImGuiCol.HeaderHovered, dimHover))
                        {
                            clicked = ImGui.Selectable(selectableLabel, isCurrentTab);
                        }
@@ -177,7 +177,7 @@ public sealed class SettingsWindow : Dalamud.Interface.Windowing.Window
            GameFunctions.GameFunctions.SetChatInteractable(true);

        if (Plugin.Config.ShowEmotes)
-            Task.Run(EmoteCache.LoadData);
+            _ = EmoteCache.LoadData(); // Fire-and-forget intentional, exceptions are caught inside

        Initialise();
    }
@@ -147,7 +147,9 @@ internal sealed class Appearance : ISettingsTab
                globalChooser?.ResultTask.ContinueWith(r =>
                {
                    if (r.IsCompletedSuccessfully)
-                        Mutable.GlobalFontV2 = r.Result;
+                    {
+                        Plugin.Framework.Run(() => Mutable.GlobalFontV2 = r.Result);
+                    }
                });
                ImGui.SameLine();
                if (ImGui.Button("Reset##global"))
@@ -164,7 +166,9 @@ internal sealed class Appearance : ISettingsTab
                japaneseChooser?.ResultTask.ContinueWith(r =>
                {
                    if (r.IsCompletedSuccessfully)
-                        Mutable.JapaneseFontV2 = r.Result;
+                    {
+                        Plugin.Framework.Run(() => Mutable.JapaneseFontV2 = r.Result);
+                    }
                });
                ImGui.SameLine();
                if (ImGui.Button("Reset##japanese"))
@@ -179,7 +183,9 @@ internal sealed class Appearance : ISettingsTab
                italicChooser?.ResultTask.ContinueWith(r =>
                {
                    if (r.IsCompletedSuccessfully)
-                        Mutable.ItalicFontV2 = r.Result;
+                    {
+                        Plugin.Framework.Run(() => Mutable.ItalicFontV2 = r.Result);
+                    }
                });
                ImGui.SameLine();
                if (ImGui.Button("Reset##italic"))
@@ -16,7 +16,7 @@ internal sealed class Database : ISettingsTab
    private Plugin Plugin { get; }
    private Configuration Mutable { get; }

-    public string Name => Language.Options_Database_Tab + "###tabs-database";
+    public string Name => HellionStrings.Settings_Tab_Database + "###tabs-database";

    internal Database(Plugin plugin, Configuration mutable)
    {
@@ -1,5 +1,6 @@
 using ChatTwo.Resources;
 using ChatTwo.Util;
+using Dalamud.Interface.Utility;
 using Dalamud.Interface.Utility.Raii;
 using Dalamud.Bindings.ImGui;

@@ -81,10 +82,12 @@ internal sealed class General : ISettingsTab

        using (ImRaii.PushIndent(ImGui.GetStyle().IndentSpacing, false))
        {
-            if (ImGuiUtil.InputIntVertical(Language.Options_MaxLinesToShow_Name, Language.Options_MaxLinesToShow_Description, ref Mutable.MaxLinesToRender))
+            ImGui.SetNextItemWidth(200f * ImGuiHelpers.GlobalScale);
+            if (ImGui.InputInt(Language.Options_MaxLinesToShow_Name, ref Mutable.MaxLinesToRender))
            {
                Mutable.MaxLinesToRender = Math.Clamp(Mutable.MaxLinesToRender, 1, 10_000);
            }
+            ImGuiUtil.HelpMarker(Language.Options_MaxLinesToShow_Description);
        }
    }

@@ -3,6 +3,7 @@ using ChatTwo.Export;
 using ChatTwo.Privacy;
 using ChatTwo.Resources;
 using ChatTwo.Util;
+using Dalamud.Interface.Colors;
 using Dalamud.Interface.ImGuiNotification;
 using Dalamud.Interface.Utility;
 using Dalamud.Interface.Utility.Raii;
@@ -55,6 +56,8 @@ internal sealed class Privacy : ISettingsTab
    private long CleanupKeepCount;
    private long CleanupDeleteCount;
    private bool CleanupRunning;
+    private bool CleanupPreviewStale;
+    private HashSet<ChatType>? CleanupPreviewSnapshot;

    // The retention-running state lives on Plugin so the auto-sweep and
    // this manual button see the same flag. UI reads stay lock-free
@@ -484,6 +487,21 @@ internal sealed class Privacy : ISettingsTab

            ImGui.Spacing();

+            // Drift-detection between the snapshot taken at last refresh
+            // and the current Mutable whitelist. Cleanup itself runs on
+            // the SAVED policy (Cleanup_Help_SavedNote covers that), but
+            // the user usually expects "the preview reflects what I just
+            // ticked" — so we surface the divergence instead of silently
+            // showing stale numbers.
+            if (CleanupPreviewSnapshot is not null
+                && !CleanupPreviewSnapshot.SetEquals(Mutable.PrivacyPersistChannels))
+            {
+                CleanupPreviewStale = true;
+            }
+
+            using (var emphasis = CleanupPreviewStale
+                       ? ImRaii.PushColor(ImGuiCol.Button, ImGuiColors.HealerGreen with { W = 0.6f })
+                       : null)
            using (ImRaii.Disabled(CleanupRunning))
            {
                if (ImGui.Button(HellionStrings.Cleanup_RefreshPreview))
@@ -496,10 +514,22 @@ internal sealed class Privacy : ISettingsTab
                return;
            }

+            if (CleanupPreviewStale)
+            {
+                ImGui.Spacing();
+                ImGuiUtil.HelpText(HellionStrings.Cleanup_Preview_Stale);
+            }
+
            ImGui.Spacing();
-            ImGuiUtil.HelpText(string.Format(HellionStrings.Cleanup_TotalStored, CleanupKeepCount + CleanupDeleteCount));
-            ImGuiUtil.HelpText(string.Format(HellionStrings.Cleanup_WillKeep, CleanupKeepCount));
-            ImGuiUtil.HelpText(string.Format(HellionStrings.Cleanup_WillDelete, CleanupDeleteCount));
+
+            using (var staleColor = CleanupPreviewStale
+                       ? ImRaii.PushColor(ImGuiCol.Text, ImGuiColors.DalamudGrey)
+                       : null)
+            {
+                ImGuiUtil.HelpText(string.Format(HellionStrings.Cleanup_TotalStored, CleanupKeepCount + CleanupDeleteCount));
+                ImGuiUtil.HelpText(string.Format(HellionStrings.Cleanup_WillKeep, CleanupKeepCount));
+                ImGuiUtil.HelpText(string.Format(HellionStrings.Cleanup_WillDelete, CleanupDeleteCount));
+            }

            using (var tree = ImRaii.TreeNode(HellionStrings.Cleanup_Breakdown))
            {
@@ -555,6 +585,13 @@ internal sealed class Privacy : ISettingsTab
                else
                    CleanupDeleteCount += count;
            }
+
+            // Snapshot the whitelist as it stood at preview-time so the
+            // render pass can flag the user about subsequent edits. Only
+            // updated on success — if the preview throws, the previous
+            // snapshot stays in place so stale-detection keeps working.
+            CleanupPreviewSnapshot = new HashSet<ChatType>(Mutable.PrivacyPersistChannels);
+            CleanupPreviewStale = false;
        }
        catch (Exception e)
        {
@@ -13,7 +13,7 @@ internal sealed class Tabs : ISettingsTab
    private Plugin Plugin { get; }
    private Configuration Mutable { get; }

-    public string Name => Language.Options_Tabs_Tab + "###tabs-tabs";
+    public string Name => HellionStrings.Settings_Tab_Tabs + "###tabs-tabs";

    private int ToOpen = -2;