diff --git a/HellionChat/Ui/Components/Sidebar.cs b/HellionChat/Ui/Components/Sidebar.cs index 19b255b..d8b05f4 100644 --- a/HellionChat/Ui/Components/Sidebar.cs +++ b/HellionChat/Ui/Components/Sidebar.cs @@ -103,7 +103,7 @@ internal sealed class Sidebar Plugin.Instance.AutoTellTabsService.MarkGreeted(tab); } - public void Draw(float windowWidth, IList tabs, ref Tab? activeTab) + public void Draw(float windowWidth, IReadOnlyList tabs, ref Tab? activeTab) { LastRenderedGreetedGlyphCount = 0; LastRenderedUnreadDotCount = 0; @@ -147,7 +147,7 @@ internal sealed class Sidebar { DrawSectionHeader( HellionStrings.PinTab_SectionHeader, - Plugin.Instance.AutoTellTabsService.PinnedTempTabCount + TabLifecycleHelpers.CountPinnedPool(tabs, t => _pool.IsOpen(t.Identifier)) ); pinnedHeaderRendered = true; } @@ -155,14 +155,13 @@ internal sealed class Sidebar { DrawSectionHeader( HellionStrings.AutoTellTabs_SectionHeader, - Plugin.Instance.AutoTellTabsService.ActiveTempTabCount + TabLifecycleHelpers.CountUnpinnedPool(tabs, t => _pool.IsOpen(t.Identifier)) ); unpinnedHeaderRendered = true; } DrawRow( tab, - i, expanded, accentRgba, textAbgr, @@ -191,7 +190,6 @@ internal sealed class Sidebar private void DrawRow( Tab tab, - int index, bool expanded, uint accentRgba, uint textAbgr, @@ -202,7 +200,10 @@ internal sealed class Sidebar ref Tab? activeTab ) { - ImGui.PushID(index); + // Identity, not position: ImGui keeps popup state across frames under this + // ID, so an index would re-bind an open context menu to a different tab as + // soon as the list shifts. String, not GetHashCode — hashes collide. + ImGui.PushID(tab.Identifier.ToString()); var origin = ImGui.GetCursorScreenPos(); var avail = ImGui.GetContentRegionAvail().X; diff --git a/HellionChat/Ui/Components/StatusBar.cs b/HellionChat/Ui/Components/StatusBar.cs index 9342c5b..3b867b8 100644 --- a/HellionChat/Ui/Components/StatusBar.cs +++ b/HellionChat/Ui/Components/StatusBar.cs @@ -60,7 +60,7 @@ internal sealed class StatusBar // Single-pass aggregator — same shape as the previous helper so the // build-suite test continues to pin the contract. - internal static (int messages, int tells) AggregateForStatusBar(IList tabs) + internal static (int messages, int tells) AggregateForStatusBar(IReadOnlyList tabs) { int messages = 0, tells = 0; @@ -93,7 +93,7 @@ internal sealed class StatusBar _lastUpdateMs = now; } - public void Draw(Tab? activeTab) + public void Draw(Tab? activeTab, IReadOnlyList tabs) { if (!_fonts.FontsReady) { @@ -105,8 +105,8 @@ internal sealed class StatusBar var now = Environment.TickCount64; if (now - _lastUpdateMs >= UpdateIntervalMs) { - var (messages, tells) = AggregateForStatusBar(Plugin.Config.Tabs); - UpdateCacheIfDue(now, Plugin.Config.Tabs.Count, messages, tells); + var (messages, tells) = AggregateForStatusBar(tabs); + UpdateCacheIfDue(now, tabs.Count, messages, tells); } // Top border via DrawList — ImGui.Separator has too much padding for diff --git a/HellionChat/Ui/Components/ThemeQuickPicker.cs b/HellionChat/Ui/Components/ThemeQuickPicker.cs index 3cb763d..105a76a 100644 --- a/HellionChat/Ui/Components/ThemeQuickPicker.cs +++ b/HellionChat/Ui/Components/ThemeQuickPicker.cs @@ -86,7 +86,12 @@ internal sealed class ThemeQuickPicker ImGui.Separator(); // Snapshot so a worker-thread temp-tab strip can't shift the list mid-loop. - var tabs = Plugin.Config.Tabs.ToList(); + // The copy itself needs the lock, otherwise it tears the same way. Not the + // frame snapshot from MainWindow: reaching it would mean threading a + // parameter through InputBar, which popouts share and which has no tab list. + List tabs; + lock (_plugin.TabsListLock) + tabs = Plugin.Config.Tabs.ToList(); var height = MathF.Min(tabs.Count * RowHeight, MaxSectionHeight); using var child = ImRaii.Child( "##hellion-quick-picker-tabs", diff --git a/HellionChat/Ui/Components/TopTabBar.cs b/HellionChat/Ui/Components/TopTabBar.cs index c7c4e00..59e10c9 100644 --- a/HellionChat/Ui/Components/TopTabBar.cs +++ b/HellionChat/Ui/Components/TopTabBar.cs @@ -16,7 +16,7 @@ internal sealed class TopTabBar _pool = pool; } - public void Draw(IList tabs, ref Tab? activeTab) + public void Draw(IReadOnlyList tabs, ref Tab? activeTab) { var firstDrawn = true; for (var i = 0; i < tabs.Count; i++) @@ -39,7 +39,7 @@ internal sealed class TopTabBar var tabWidth = ImGui.CalcTextSize(tab.Name).X; if ( ImGui.Selectable( - $"{tab.Name}###hellion_toptab_{i}", + $"{tab.Name}###hellion_toptab_{tab.Identifier}", selected, ImGuiSelectableFlags.None, new Vector2(tabWidth, 0) @@ -71,7 +71,7 @@ internal sealed class TopTabBar .AddCircleFilled(new Vector2(max.X - 4f, min.Y + 4f), 3.5f, danger, 12); } - TabContextMenu.Draw(tab, $"toptab_ctx_{i}", _pool); + TabContextMenu.Draw(tab, $"toptab_ctx_{tab.Identifier}", _pool); } ImGui.Separator(); diff --git a/HellionChat/Ui/Windows/MainWindow.cs b/HellionChat/Ui/Windows/MainWindow.cs index 6c5dcac..a89982a 100644 --- a/HellionChat/Ui/Windows/MainWindow.cs +++ b/HellionChat/Ui/Windows/MainWindow.cs @@ -144,7 +144,11 @@ internal sealed class MainWindow : Window, IFocusableChatWindow if (!ReferenceEquals(_activeTab, removed)) return; - var next = Plugin.Config.Tabs.Count > 0 ? Plugin.Config.Tabs[0] : null; + // Framework thread, not the draw frame: needs the current truth, so it takes + // its own lock instead of using the frame snapshot. + Tab? next; + lock (Plugin.Instance.TabsListLock) + next = Plugin.Config.Tabs.Count > 0 ? Plugin.Config.Tabs[0] : null; _activeTab = next; if (next is not null) TabLifecycleHelpers.OnTabActivated(next, removed); @@ -171,7 +175,11 @@ internal sealed class MainWindow : Window, IFocusableChatWindow // deferred (no focus contract) — main-window tabs only. internal void ChangeTabDelta(int delta) { - var tabs = Plugin.Config.Tabs; + // Runs on Framework.Update via the keybind dispatch, not on the draw frame — + // own lock, own copy. Stays a List so IndexOf below keeps working. + List tabs; + lock (Plugin.Instance.TabsListLock) + tabs = Plugin.Config.Tabs.ToList(); if (tabs.Count == 0) return; @@ -256,24 +264,30 @@ internal sealed class MainWindow : Window, IFocusableChatWindow // Primary pool-reset path; InputPreview has a defensive fallback for the MainWindow-closed edge case. _handlerLender.ResetCounter(); + // One snapshot for the whole frame. Everything below reads this instead of + // Config.Tabs, so sidebar, top tabs and status bar see the same list even if + // the worker adds or evicts a tab mid-frame. Deliberately a SHALLOW copy: + // tab identity is compared by reference all over the draw path, so cloning + // would break every ReferenceEquals and Contains. + List tabs; + lock (Plugin.Instance.TabsListLock) + tabs = Plugin.Config.Tabs.ToList(); + // First-frame seed: the active tab defaults to the first persisted // tab so the message list isn't empty on a clean session. - if (_activeTab is null && Plugin.Config.Tabs.Count > 0) + if (_activeTab is null && tabs.Count > 0) { - var seeded = Plugin.Config.Tabs[0]; + var seeded = tabs[0]; _activeTab = seeded; // The seeded Tabs[0] is the likeliest legacy stale-tell carrier // (pre-coupling the detour wrote here); strip it like any activation. TabLifecycleHelpers.OnTabActivated(seeded, null); } - else if (_activeTab is { } active && !Plugin.Config.Tabs.Contains(active)) + else if (_activeTab is { } active && !tabs.Contains(active)) { // Active tab is no longer in the list (e.g. a wholesale config import - // the service repair paths never see). Re-seed on the Draw thread. The - // Contains read shares the pre-existing unsynchronized-Tabs-list - // exposure that spec §6 defers (SaveConfig also strips from the worker - // thread); this adds one more racing read, not a new hazard class. - var reseed = Plugin.Config.Tabs.Count > 0 ? Plugin.Config.Tabs[0] : null; + // the service repair paths never see). Re-seed on the Draw thread. + var reseed = tabs.Count > 0 ? tabs[0] : null; _activeTab = reseed; if (reseed is not null) TabLifecycleHelpers.OnTabActivated(reseed, active); @@ -287,7 +301,7 @@ internal sealed class MainWindow : Window, IFocusableChatWindow // settled, so OnTabActivated fires only on the pop frame. var visibleActive = TabLifecycleHelpers.PickMainActiveTab( _activeTab, - Plugin.Config.Tabs, + tabs, t => _pool.IsOpen(t.Identifier) ); if (!ReferenceEquals(visibleActive, _activeTab)) @@ -309,20 +323,20 @@ internal sealed class MainWindow : Window, IFocusableChatWindow using (var body = ImRaii.Child("##hellion-body", new Vector2(-1f, -statusHeight))) { if (body.Success) - DrawBody(); + DrawBody(tabs); } - _status.Draw(_activeTab); + _status.Draw(_activeTab, tabs); } - private void DrawBody() + private void DrawBody(IReadOnlyList tabs) { var bodyWidth = ImGui.GetContentRegionAvail().X; _honorific.Draw(bodyWidth); if (Plugin.Config.MainWindowLayoutMode == MainWindowLayoutMode.TopTabs) { - _topTabs.Draw(Plugin.Config.Tabs, ref _activeTab); + _topTabs.Draw(tabs, ref _activeTab); using (ImRaii.Group()) { DrawMainArea(); @@ -333,7 +347,7 @@ internal sealed class MainWindow : Window, IFocusableChatWindow // Sidebar layout (default). using (ImRaii.Group()) { - _sidebar.Draw(bodyWidth, Plugin.Config.Tabs, ref _activeTab); + _sidebar.Draw(bodyWidth, tabs, ref _activeTab); } ImGui.SameLine(); diff --git a/HellionChat/Util/TabLifecycleHelpers.cs b/HellionChat/Util/TabLifecycleHelpers.cs index 103f482..b8fb729 100644 --- a/HellionChat/Util/TabLifecycleHelpers.cs +++ b/HellionChat/Util/TabLifecycleHelpers.cs @@ -115,7 +115,30 @@ internal static class TabLifecycleHelpers // row and its pool's section header gates on the first tab actually reached. Pure // + Dalamud-free so the Build-Suite can pin it. // TEST-MIRROR: ../../../Hellion Build test/_Helpers/SidebarRenderOrderTests.cs - internal static List BuildRenderOrder(IList tabs, Func isPoppedOut) + // Section-header counts for the sidebar. They take the same isPoppedOut + // predicate as BuildRenderOrder, which skips popped-out tabs — without it the + // header would claim "(3)" above two rendered rows. AutoTellTabsService keeps + // its own live properties: those gate the pool limits and must not see a + // snapshot. + internal static int CountUnpinnedPool(IReadOnlyList tabs, Func isPoppedOut) + { + var n = 0; + for (var i = 0; i < tabs.Count; i++) + if (IsInUnpinnedPool(tabs[i]) && !isPoppedOut(tabs[i])) + n++; + return n; + } + + internal static int CountPinnedPool(IReadOnlyList tabs, Func isPoppedOut) + { + var n = 0; + for (var i = 0; i < tabs.Count; i++) + if (IsInPinnedPool(tabs[i]) && !isPoppedOut(tabs[i])) + n++; + return n; + } + + internal static List BuildRenderOrder(IReadOnlyList tabs, Func isPoppedOut) { var persistent = new List(tabs.Count); var pinned = new List(); @@ -145,7 +168,7 @@ internal static class TabLifecycleHelpers // TEST-MIRROR: ../../../Hellion Build test/_Helpers/PickMainActiveTabTests.cs internal static Tab? PickMainActiveTab( Tab? current, - IList tabs, + IReadOnlyList tabs, Func isPoppedOut ) {