Migrate shared tag to own config, address comments

Migrates the configuration for shared tags to a separate config file, and
addresses CR feedback.

Penumbra/Configuration.cs

@@ -87,8 +87,6 @@ public class Configuration : IPluginConfiguration, ISavable
     public Dictionary<ColorId, uint> Colors { get; set; }
         = Enum.GetValues<ColorId>().ToDictionary(c => c, c => c.Data().DefaultColor);
 
-    public IReadOnlyList<string> SharedTags { get; set; }
-
     /// <summary>
     /// Load the current configuration.
     /// Includes adding new colors and migrating from old versions.

Penumbra/Services/FilenameService.cs

@@ -14,6 +14,7 @@ public class FilenameService(DalamudPluginInterface pi) : IService
     public readonly string EphemeralConfigFile   = Path.Combine(pi.ConfigDirectory.FullName, "ephemeral_config.json");
     public readonly string FilesystemFile        = Path.Combine(pi.ConfigDirectory.FullName, "sort_order.json");
     public readonly string ActiveCollectionsFile = Path.Combine(pi.ConfigDirectory.FullName, "active_collections.json");
+    public readonly string SharedTagFile         = Path.Combine(pi.ConfigDirectory.FullName, "shared_tags.json");
 
     /// <summary> Obtain the path of a collection file given its name.</summary>
     public string CollectionFile(ModCollection collection)

Penumbra/UI/ModsTab/ModPanelDescriptionTab.cs

@@ -36,7 +36,7 @@ public class ModPanelDescriptionTab : ITab
         ImGui.Dummy(ImGuiHelpers.ScaledVector2(2));
 
         ImGui.Dummy(ImGuiHelpers.ScaledVector2(2));
-        var sharedTagsEnabled = _sharedTagManager.SharedTags.Count() > 0;
+        var sharedTagsEnabled = _sharedTagManager.SharedTags.Count > 0;
         var sharedTagButtonOffset = sharedTagsEnabled ? ImGui.GetFrameHeight() + ImGui.GetStyle().FramePadding.X : 0;
         var tagIdx = _localTags.Draw("Local Tags: ",
             "Custom tags you can set personally that will not be exported to the mod data but only set for you.\n"
@@ -48,23 +48,7 @@ public class ModPanelDescriptionTab : ITab
 
         if (sharedTagsEnabled)
         {
-            ImGui.SetCursorPosY(ImGui.GetCursorPosY() - ImGui.GetFrameHeightWithSpacing());
+            _sharedTagManager.DrawAddFromSharedTagsAndUpdateTags(_selector.Selected!.LocalTags, _selector.Selected!.ModTags, true, _selector.Selected!);
-            ImGui.SetCursorPosX(ImGui.GetWindowWidth() - ImGui.GetFrameHeight() - ImGui.GetStyle().FramePadding.X);
-            var sharedTag = _sharedTagManager.DrawAddFromSharedTags(_selector.Selected!.LocalTags, _selector.Selected!.ModTags, true);
-            if (sharedTag.Length > 0)
-            {
-                var index = _selector.Selected!.LocalTags.IndexOf(sharedTag);
-                if (index < 0)
-                {
-                    index = _selector.Selected!.LocalTags.Count;
-                    _modManager.DataEditor.ChangeLocalTag(_selector.Selected, index, sharedTag);
-                }
-                else
-                {
-                    _modManager.DataEditor.ChangeLocalTag(_selector.Selected, index, string.Empty);
-                }
-
-            }
         }
 
         if (_selector.Selected!.ModTags.Count > 0)

Penumbra/UI/ModsTab/ModPanelEditTab.cs

@@ -83,7 +83,7 @@ public class ModPanelEditTab : ITab
             }
 
         UiHelpers.DefaultLineSpace();
-        var sharedTagsEnabled = _sharedTagManager.SharedTags.Count() > 0;
+        var sharedTagsEnabled = _sharedTagManager.SharedTags.Count > 0;
         var sharedTagButtonOffset = sharedTagsEnabled ? ImGui.GetFrameHeight() + ImGui.GetStyle().FramePadding.X : 0;
         var tagIdx = _modTags.Draw("Mod Tags: ", "Edit tags by clicking them, or add new tags. Empty tags are removed.", _mod.ModTags,
             out var editedTag, rightEndOffset: sharedTagButtonOffset);
@@ -92,23 +92,7 @@ public class ModPanelEditTab : ITab
 
         if (sharedTagsEnabled)
         {
-            ImGui.SetCursorPosY(ImGui.GetCursorPosY() - ImGui.GetFrameHeightWithSpacing());
+            _sharedTagManager.DrawAddFromSharedTagsAndUpdateTags(_selector.Selected!.LocalTags, _selector.Selected!.ModTags, false, _selector.Selected!);
-            ImGui.SetCursorPosX(ImGui.GetWindowWidth() - ImGui.GetFrameHeight() - ImGui.GetStyle().FramePadding.X);
-            var sharedTag = _sharedTagManager.DrawAddFromSharedTags(_selector.Selected!.LocalTags, _selector.Selected!.ModTags, false);
-            if (sharedTag.Length > 0)
-            {
-                var index = _selector.Selected!.ModTags.IndexOf(sharedTag);
-                if (index < 0)
-                {
-                    index = _selector.Selected!.ModTags.Count;
-                    _modManager.DataEditor.ChangeModTag(_selector.Selected, index, sharedTag);
-                }
-                else
-                {
-                    _modManager.DataEditor.ChangeModTag(_selector.Selected, index, string.Empty);
-                }
-
-            }
         }
 
         UiHelpers.DefaultLineSpace();

Penumbra/UI/SharedTagManager.cs

@@ -1,19 +1,22 @@
-using System;
+using Dalamud.Interface;
-using System.Collections.Generic;
+using Dalamud.Interface.Internal.Notifications;
-using System.Linq;
+using Dalamud.Utility;
-using System.Text;
-using System.Threading.Tasks;
-using Dalamud.Interface;
 using ImGuiNET;
+using Newtonsoft.Json;
 using OtterGui;
+using OtterGui.Classes;
 using OtterGui.Raii;
-using OtterGui.Widgets;
+using Penumbra.Mods.Manager;
-using Penumbra.Mods;
+using Penumbra.Services;
 using Penumbra.UI.Classes;
+using ErrorEventArgs = Newtonsoft.Json.Serialization.ErrorEventArgs;
 
 namespace Penumbra.UI;
-public sealed class SharedTagManager
+public sealed class SharedTagManager : ISavable
 {
+    private readonly ModManager _modManager;
+    private readonly SaveService _saveService;
+
     private static uint _tagButtonAddColor = ColorId.SharedTagAdd.Value();
     private static uint _tagButtonRemoveColor = ColorId.SharedTagRemove.Value();
 
@@ -22,11 +25,66 @@ public sealed class SharedTagManager
     private const string PopupContext = "SharedTagsPopup";
     private bool _isPopupOpen = false;
 
+    // Operations on this list assume that it is sorted and will keep it sorted if that is the case.
+    // The list also gets re-sorted when first loaded from config in case the config was modified.
+    [JsonRequired]
+    private readonly List<string> _sharedTags = [];
+    [JsonIgnore]
+    public IReadOnlyList<string> SharedTags => _sharedTags;
 
-    public IReadOnlyList<string> SharedTags { get; internal set; } = Array.Empty<string>();
+    public int ConfigVersion = 1;
 
-    public SharedTagManager()
+    public SharedTagManager(ModManager modManager, SaveService saveService)
     {
+        _modManager = modManager;
+        _saveService = saveService;
+        Load();
+    }
+
+    public string ToFilename(FilenameService fileNames)
+    {
+        return fileNames.SharedTagFile;
+    }
+
+    public void Save(StreamWriter writer)
+    {
+        using var jWriter = new JsonTextWriter(writer) { Formatting = Formatting.Indented };
+        var serializer = new JsonSerializer { Formatting = Formatting.Indented };
+        serializer.Serialize(jWriter, this);
+    }
+
+    public void Save()
+        => _saveService.DelaySave(this, TimeSpan.FromSeconds(5));
+
+    private void Load()
+    {
+        static void HandleDeserializationError(object? sender, ErrorEventArgs errorArgs)
+        {
+            Penumbra.Log.Error(
+                $"Error parsing shared tags Configuration at {errorArgs.ErrorContext.Path}, using default or migrating:\n{errorArgs.ErrorContext.Error}");
+            errorArgs.ErrorContext.Handled = true;
+        }
+
+        if (!File.Exists(_saveService.FileNames.SharedTagFile))
+            return;
+
+        try
+        {
+            var text = File.ReadAllText(_saveService.FileNames.SharedTagFile);
+            JsonConvert.PopulateObject(text, this, new JsonSerializerSettings
+            {
+                Error = HandleDeserializationError,
+            });
+
+            // Any changes to this within this class should keep it sorted, but in case someone went in and manually changed the JSON, run a sort on initial load.
+            _sharedTags.Sort();
+        }
+        catch (Exception ex)
+        {
+            Penumbra.Messager.NotificationMessage(ex,
+                "Error reading shared tags Configuration, reverting to default.",
+                "Error reading shared tags Configuration", NotificationType.Error);
+        }
     }
 
     public void ChangeSharedTag(int tagIdx, string tag)
@@ -34,32 +92,74 @@ public sealed class SharedTagManager
         if (tagIdx < 0 || tagIdx > SharedTags.Count)
             return;
 
-        if (tagIdx == SharedTags.Count) // Adding a new tag
+        // In the case of editing a tag, remove what's there prior to doing an insert.
+        if (tagIdx != SharedTags.Count)
         {
-            SharedTags = SharedTags.Append(tag).Distinct().Where(tag => tag.Length > 0).OrderBy(a => a).ToArray();
+            _sharedTags.RemoveAt(tagIdx);
         }
-        else // Editing an existing tag
+
+        if (!string.IsNullOrEmpty(tag))
         {
-            var tmpTags = SharedTags.ToArray();
+            // Taking advantage of the fact that BinarySearch returns the complement of the correct sorted position for the tag.
-            tmpTags[tagIdx] = tag;
+            var existingIdx = _sharedTags.BinarySearch(tag);
-            SharedTags = tmpTags.Distinct().Where(tag => tag.Length > 0).OrderBy(a => a).ToArray();
+            if (existingIdx < 0)
+                _sharedTags.Insert(~existingIdx, tag);
+        }
+
+        Save();
+    }
+
+    public void DrawAddFromSharedTagsAndUpdateTags(IReadOnlyCollection<string> localTags, IReadOnlyCollection<string> modTags, bool editLocal, Mods.Mod mod)
+    {
+        ImGui.SetCursorPosY(ImGui.GetCursorPosY() - ImGui.GetFrameHeightWithSpacing());
+        ImGui.SetCursorPosX(ImGui.GetWindowWidth() - ImGui.GetFrameHeight() - ImGui.GetStyle().FramePadding.X);
+
+        var sharedTag = DrawAddFromSharedTags(localTags, modTags, editLocal);
+
+        if (sharedTag.Length > 0)
+        {
+            var index = editLocal ? mod.LocalTags.IndexOf(sharedTag) : mod.ModTags.IndexOf(sharedTag);
+
+            if (editLocal)
+            {
+                if (index < 0)
+                {
+                    index = mod.LocalTags.Count;
+                    _modManager.DataEditor.ChangeLocalTag(mod, index, sharedTag);
+                }
+                else
+                {
+                    _modManager.DataEditor.ChangeLocalTag(mod, index, string.Empty);
+                }
+            } else
+            {
+                if (index < 0)
+                {
+                    index = mod.ModTags.Count;
+                    _modManager.DataEditor.ChangeModTag(mod, index, sharedTag);
+                }
+                else
+                {
+                    _modManager.DataEditor.ChangeModTag(mod, index, string.Empty);
+                }
+            }
+
         }
     }
 
     public string DrawAddFromSharedTags(IReadOnlyCollection<string> localTags, IReadOnlyCollection<string> modTags, bool editLocal)
     {
-        var tagToAdd = "";
+        var tagToAdd = string.Empty;
         if (ImGuiUtil.DrawDisabledButton(FontAwesomeIcon.Tags.ToIconString(), new Vector2(ImGui.GetFrameHeight()), "Add Shared Tag... (Right-click to close popup)",
             false, true) || _isPopupOpen)
             return DrawSharedTagsPopup(localTags, modTags, editLocal);
 
-
         return tagToAdd;
     }
 
     private string DrawSharedTagsPopup(IReadOnlyCollection<string> localTags, IReadOnlyCollection<string> modTags, bool editLocal)
     {
-        var selected = "";
+        var selected = string.Empty;
         if (!ImGui.IsPopupOpen(PopupContext))
         {
             ImGui.OpenPopup(PopupContext);
@@ -75,16 +175,15 @@ public sealed class SharedTagManager
         if (!popup)
             return selected;
 
-        ImGui.Text("Shared Tags");
+        ImGui.TextUnformatted("Shared Tags");
         ImGuiUtil.HoverTooltip("Right-click to close popup");
         ImGui.Separator();
 
-        foreach (var tag in SharedTags)
+        foreach (var (tag, idx) in SharedTags.WithIndex())
         {
-            if (DrawColoredButton(localTags, modTags, tag, editLocal))
+            if (DrawColoredButton(localTags, modTags, tag, editLocal, idx))
             {
                 selected = tag;
-                return selected;
             }
             ImGui.SameLine();
         }
@@ -97,8 +196,10 @@ public sealed class SharedTagManager
         return selected;
     }
 
-    private static bool DrawColoredButton(IReadOnlyCollection<string> localTags, IReadOnlyCollection<string> modTags, string buttonLabel, bool editLocal)
+    private static bool DrawColoredButton(IReadOnlyCollection<string> localTags, IReadOnlyCollection<string> modTags, string buttonLabel, bool editLocal, int index)
     {
+        var ret = false;
+
         var isLocalTagPresent = localTags.Contains(buttonLabel);
         var isModTagPresent = modTags.Contains(buttonLabel);
 
@@ -116,21 +217,24 @@ public sealed class SharedTagManager
         if (buttonWidth + ImGui.GetStyle().ItemSpacing.X >= ImGui.GetContentRegionAvail().X)
             ImGui.NewLine();
 
-        // Trimmed tag names can collide, but the full tags are guaranteed distinct so use the full tag as the ID to avoid an ImGui moment.
+        // Trimmed tag names can collide, and while tag names are currently distinct this may not always be the case. As such use the index to avoid an ImGui moment.
-        ImRaii.PushId(buttonLabel);
+        using var id = ImRaii.PushId(index);
 
         if (editLocal && isModTagPresent || !editLocal && isLocalTagPresent)
         {
             using var alpha = ImRaii.PushStyle(ImGuiStyleVar.Alpha, 0.5f);
             ImGui.Button(displayedLabel);
-            alpha.Pop();
+        }
-            return false;
+        else
+        {
+            using (ImRaii.PushColor(ImGuiCol.Button, isLocalTagPresent || isModTagPresent ? _tagButtonRemoveColor : _tagButtonAddColor))
+            {
+                if (ImGui.Button(displayedLabel))
+                    ret = true;
+            }
         }
 
-        using (ImRaii.PushColor(ImGuiCol.Button, isLocalTagPresent || isModTagPresent ? _tagButtonRemoveColor : _tagButtonAddColor))
+        return ret;
-        {
-            return ImGui.Button(displayedLabel);
-        }
     }
 
     private static string TrimButtonTextToWidth(string fullText, float maxWidth)
@@ -156,5 +260,4 @@ public sealed class SharedTagManager
     {
         return ImGui.CalcTextSize(text).X + 2 * ImGui.GetStyle().FramePadding.X;
     }
-
 }

Penumbra/UI/Tabs/SettingsTab.cs

@@ -73,8 +73,6 @@ public class SettingsTab : ITab
         if (_compactor.CanCompact)
             _compactor.Enabled = _config.UseFileSystemCompression;
         _sharedTagManager = sharedTagConfig;
-        if (sharedTagConfig.SharedTags.Count == 0 && _config.SharedTags != null)
-            sharedTagConfig.SharedTags = _config.SharedTags;
     }
 
     public void DrawHeader()
@@ -916,14 +914,12 @@ public class SettingsTab : ITab
             return;
 
         var tagIdx = _sharedTags.Draw("Shared Tags: ",
-            "Tags that can be added/removed from mods with 1 click.", _sharedTagManager.SharedTags,
+            "Predefined tags that can be added or removed from mods with a single click.", _sharedTagManager.SharedTags,
             out var editedTag);
 
         if (tagIdx >= 0)
         {
             _sharedTagManager.ChangeSharedTag(tagIdx, editedTag);
-            _config.SharedTags = _sharedTagManager.SharedTags;
-            _config.Save();
         }
     }
 }