From 8e2d333130de2c2088e873d9edbd1feddefdb4b5 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 16 Jun 2026 00:24:18 +0200 Subject: [PATCH 1/8] fix(toptab): size each tab selectable to its label width --- HellionChat/Ui/Components/TopTabBar.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/HellionChat/Ui/Components/TopTabBar.cs b/HellionChat/Ui/Components/TopTabBar.cs index 767417f..5010335 100644 --- a/HellionChat/Ui/Components/TopTabBar.cs +++ b/HellionChat/Ui/Components/TopTabBar.cs @@ -25,12 +25,17 @@ internal sealed class TopTabBar ImGui.SameLine(); var selected = ReferenceEquals(tab, activeTab); + // Size the selectable to its own label width. A zero width makes ImGui + // stretch the selectable's box to the full remaining window width + // (imgui_widgets.cpp:7378), so in this SameLine row every tab overlaps + // into one giant bar and clicking never lands on the intended tab. + var tabWidth = ImGui.CalcTextSize(tab.Name).X; if ( ImGui.Selectable( $"{tab.Name}###hellion_toptab_{i}", selected, ImGuiSelectableFlags.None, - new Vector2(0, 0) + new Vector2(tabWidth, 0) ) ) { From 7b6871fea421be2def941959b1f2d69b4ad2f7b0 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 16 Jun 2026 00:39:01 +0200 Subject: [PATCH 2/8] feat(autotell): wire temp-tab pop-outs to the channel-popout pool --- HellionChat/AutoTellTabsService.cs | 48 ++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/HellionChat/AutoTellTabsService.cs b/HellionChat/AutoTellTabsService.cs index 830ddbb..ae1bb20 100644 --- a/HellionChat/AutoTellTabsService.cs +++ b/HellionChat/AutoTellTabsService.cs @@ -218,7 +218,7 @@ internal sealed class AutoTellTabsService : IDisposable return null; } - private static Tab? FindTempTab(string name, uint world) + internal static Tab? FindTempTab(string name, uint world) { var byTarget = Plugin.Config.Tabs.FirstOrDefault(t => t.IsTempTab @@ -256,21 +256,20 @@ internal sealed class AutoTellTabsService : IDisposable return; } - // Pop-out-window cleanup is offline while the channel-popout pool - // is rebuilt — Tab.PopOut still flips on/off, the visible window - // disappears once the new pool comes online. - var dropped = victim.Tab; Plugin.Config.Tabs.RemoveAt(victim.Index); - // Re-anchor the UI selection if it pointed at the dropped tab. This runs on - // the PendingMessage worker thread and the repair mutates the re-seeded - // tab's channel via OnTabActivated, so marshal it onto the framework thread - // to serialize with Draw (reference_dalamud_framework_thread) — otherwise a - // half-applied strip could race the input bar's send-routing read. + // Re-anchor the UI selection if it pointed at the dropped tab, and close any + // pop-out window the dropped tab owned. Both run on the PendingMessage worker + // thread and touch window state the Draw path reads (OnTabActivated re-seed + + // the pool's Unbind), so marshal onto the framework thread to serialize with + // Draw (reference_dalamud_framework_thread). TryClose is idempotent: a tab that + // was never popped is a silent no-op. Plugin.Framework.RunOnFrameworkThread(() => - _plugin.MainWindow?.ResetActiveTabIfRemoved(dropped) - ); + { + _plugin.ChannelPopoutPool.TryClose(dropped.Identifier); + _plugin.MainWindow?.ResetActiveTabIfRemoved(dropped); + }); } private void SpawnTempTab((string Name, uint World) partner, Message currentMessage) @@ -282,13 +281,28 @@ internal sealed class AutoTellTabsService : IDisposable tab.AddMessage(currentMessage, unread: true); - // Open as pop-out if configured (set before Tabs.Add for next render-tick) + // Open as pop-out if configured (flag set before Tabs.Add for the next render-tick). if (Plugin.Config.AutoTellTabsOpenAsPopout) { tab.PopOut = true; } Plugin.Config.Tabs.Add(tab); + + // Actually open the pop-out window for the flagged tab — without this the + // flag was dead (a PopOut tab with no window). SpawnTempTab runs on the + // PendingMessage worker thread under _tempTabsLock; TryOpen does + // OnTabActivated + Bind (window state Draw reads), so marshal onto the + // framework thread. If the pool is full, drop the flag so it never claims a + // window it didn't get (flag/window parity). + if (tab.PopOut) + { + Plugin.Framework.RunOnFrameworkThread(() => + { + if (!_plugin.ChannelPopoutPool.TryOpen(tab)) + tab.PopOut = false; + }); + } } private static Tab BuildTempTab(string playerName, uint worldRowId) @@ -427,8 +441,12 @@ internal sealed class AutoTellTabsService : IDisposable .Config.Tabs.Where(t => TabLifecycleHelpers.IsInUnpinnedPool(t) && t.PopOut) .Select(t => t.Identifier) .ToList(); - // Pop-out-window cleanup is offline; see Disconnect path above. - _ = poppedTempTabIds; + + // Close each popped temp tab's window before the tabs leave the list. + // Logout is a framework-thread event (serialized with Draw), so no + // marshalling is needed here, unlike the worker-thread eviction path. + foreach (var id in poppedTempTabIds) + _plugin.ChannelPopoutPool.TryClose(id); Plugin.Config.Tabs.RemoveAll(TabLifecycleHelpers.IsInUnpinnedPool); From 47a49de8c074826d22620c2093b9b90e65e6e073 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 16 Jun 2026 00:50:45 +0200 Subject: [PATCH 3/8] feat(tell-router): auto-open incoming tells per TellAutoOpenMode --- .../Hosting/InitHostedServices.cs | 12 +++ HellionChat/PluginHostFactory.cs | 8 +- HellionChat/Services/TellRouterService.cs | 85 ++++++++++++++++--- 3 files changed, 91 insertions(+), 14 deletions(-) diff --git a/HellionChat/Infrastructure/Hosting/InitHostedServices.cs b/HellionChat/Infrastructure/Hosting/InitHostedServices.cs index 3929753..65d36a8 100644 --- a/HellionChat/Infrastructure/Hosting/InitHostedServices.cs +++ b/HellionChat/Infrastructure/Hosting/InitHostedServices.cs @@ -101,6 +101,18 @@ internal sealed class AutoTellTabsServiceInitHostedService(AutoTellTabsService s public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; } +internal sealed class TellRouterServiceInitHostedService(Services.TellRouterService service) + : IHostedService +{ + public Task StartAsync(CancellationToken cancellationToken) + { + service.Initialize(); + return Task.CompletedTask; + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; +} + // Eager-resolve trigger: resolving FailedTellNotifier in this adapter's ctor // enables its game hook during host startup. StartAsync itself is a no-op. internal sealed class FailedTellNotifierInitHostedService(FailedTellNotifier notifier) diff --git a/HellionChat/PluginHostFactory.cs b/HellionChat/PluginHostFactory.cs index 1fb745f..06a9c32 100644 --- a/HellionChat/PluginHostFactory.cs +++ b/HellionChat/PluginHostFactory.cs @@ -122,7 +122,7 @@ internal static class PluginHostFactory sp.GetRequiredService() )); services.AddSingleton(sp => new Services.TellRouterService( - sp.GetRequiredService(), + sp.GetRequiredService(), sp.GetRequiredService>() )); @@ -369,6 +369,12 @@ internal static class PluginHostFactory services.AddHostedService(sp => new AutoTellTabsServiceInitHostedService( sp.GetRequiredService() )); + // Must come AFTER AutoTell's registration: both subscribe MessageProcessed, + // and AutoTell subscribing first lets the router's IsOpen-guard see the + // already-opened pop-out (FIFO framework-tick ordering, no double-pop). + services.AddHostedService(sp => new TellRouterServiceInitHostedService( + sp.GetRequiredService() + )); services.AddHostedService( sp => new Infrastructure.Hosting.FailedTellNotifierInitHostedService( sp.GetRequiredService() diff --git a/HellionChat/Services/TellRouterService.cs b/HellionChat/Services/TellRouterService.cs index 38d71e0..5f2b989 100644 --- a/HellionChat/Services/TellRouterService.cs +++ b/HellionChat/Services/TellRouterService.cs @@ -1,32 +1,91 @@ -using Dalamud.Game.Chat; -using Dalamud.Plugin.Services; +using HellionChat.Code; +using HellionChat.Util; using Microsoft.Extensions.Logging; namespace HellionChat.Services; -// Skeleton for the upcoming auto-open routing layer. Subscribes to IChatGui -// up front so the DI graph and Plugin.cs registration stay frozen — when -// the routing logic lands, it drops into OnChatMessage without touching -// anything else. +// Routes an incoming tell to the configured TellAutoOpenMode (Off/Sidebar/ +// TopTab/Popout). Decoupled from AutoTellTabsService (Flo decision 2026-06-15): +// that service owns tab CREATION + lifecycle; this only REVEALS/pops the tab it +// finds. Popout guards on pool.IsOpen so it never double-pops a tab the +// AutoTellTabsOpenAsPopout path already opened. Subscribes to the resolved +// MessageManager.MessageProcessed stream (partner already extracted), not the raw +// IChatGui event, and defers the reveal one tick so the tab exists regardless of +// subscriber order. Wired by TellRouterServiceInitHostedService. internal sealed class TellRouterService : IDisposable { - private readonly IChatGui _chatGui; + private readonly MessageManager _messageManager; private readonly ILogger _logger; + private bool _initialized; - public TellRouterService(IChatGui chatGui, ILogger logger) + public TellRouterService(MessageManager messageManager, ILogger logger) { - _chatGui = chatGui; + _messageManager = messageManager; _logger = logger; - _chatGui.ChatMessageUnhandled += OnChatMessage; + } + + public void Initialize() + { + if (_initialized) + return; + + _messageManager.MessageProcessed += OnMessageProcessed; + _initialized = true; + _logger.LogDebug("TellRouterService online; routing incoming tells by TellAutoOpenMode."); } public void Dispose() { - _chatGui.ChatMessageUnhandled -= OnChatMessage; + if (!_initialized) + return; + + _messageManager.MessageProcessed -= OnMessageProcessed; + _initialized = false; } - private void OnChatMessage(IChatMessage message) + private void OnMessageProcessed(Message message) { - // Intentional no-op until the routing implementation lands. + var mode = Plugin.Config.TellAutoOpenMode; + if (mode == TellAutoOpenMode.Off) + return; + + if (message.Code.Type != ChatType.TellIncoming) + return; + + // Partner = sender for an incoming tell. Same payload idiom AutoTellTabs uses + // (AutoTellTabsService.ExtractTellPartner), so the lookup never diverges. + var partner = + ChunkUtil.TryGetPlayerPayload(message.Sender) + ?? ChunkUtil.TryGetPlayerPayload(message.SenderSource); + if (partner == null) + return; + + var name = partner.PlayerName; + var world = partner.World.RowId; + + // Defer the reveal to the next framework tick. AutoTellTabsService also + // handles this MessageProcessed (synchronously); by the next tick the tab + // exists regardless of subscription order, and the reveal (ActivateTab / pool + // mutation) is serialized with Draw (reference_dalamud_framework_thread). + Plugin.Framework.RunOnFrameworkThread(() => + { + var tab = AutoTellTabsService.FindTempTab(name, world); + if (tab == null) + return; // nothing to reveal (auto-tell-tabs off -> no tab created) + + switch (mode) + { + case TellAutoOpenMode.Sidebar: + case TellAutoOpenMode.TopTab: + Plugin.Instance.MainWindow?.ActivateTab(tab); + break; + case TellAutoOpenMode.Popout: + // IsOpen-guard: don't double-pop a tab the AutoTellTabsOpenAsPopout + // path already opened (the two switches stay decoupled). + if (!Plugin.Instance.ChannelPopoutPool.IsOpen(tab.Identifier)) + Plugin.Instance.ChannelPopoutPool.TryOpen(tab); + break; + } + }); } } From 88491902eb47e9334b11932fbc70d3e4f13a2ee7 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 16 Jun 2026 01:04:34 +0200 Subject: [PATCH 4/8] feat(chat): prefill the input bar for context-menu and direct-chat tells --- HellionChat/GameFunctions/Chat.cs | 47 ++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/HellionChat/GameFunctions/Chat.cs b/HellionChat/GameFunctions/Chat.cs index 5a2f542..841e52f 100755 --- a/HellionChat/GameFunctions/Chat.cs +++ b/HellionChat/GameFunctions/Chat.cs @@ -232,9 +232,14 @@ internal sealed unsafe class Chat : IDisposable if (c != '\0' && !char.IsControl(c)) input = c.ToString(); - // Chat-window Activated integration is offline until the - // new chat layer surfaces an Activated entry point. - _ = input; + // Seed the just-typed character into our input field and focus it, + // the same prefill path the inventory item-link below uses. Prefill- + // only — no tab switch (Flo decision 2026-06-15). + if (input != null) + { + Plugin.InputBar.AppendPending(input); + Plugin.InputBar.Activate = true; + } }); } @@ -325,13 +330,18 @@ internal sealed unsafe class Chat : IDisposable { if (playerName != null) { - // Chat-window Activated integration is offline; tell-target - // routing returns when the new chat layer is wired up. - _ = playerName; - _ = worldId; - _ = contentId; - _ = reason; - _ = setChatType; + // Right-click -> Send Tell: prefill our input the same way our own + // "Send Tell" payload menu does (PayloadHandler), then focus. Prefill- + // only — no tab switch, no ChatActivatedArgs revival (Flo decision + // 2026-06-15). The game supplies worldName here, so no sheet lookup. + var tellName = playerName->ToString(); + var tellWorld = worldName != null ? worldName->ToString() : string.Empty; + var tellCommand = $"/tell {tellName}"; + if (!string.IsNullOrEmpty(tellWorld)) + tellCommand += $"@{tellWorld}"; + tellCommand += " "; + Plugin.InputBar.SetPendingMessage(tellCommand); + Plugin.InputBar.Activate = true; } return SetChatLogTellTargetHook!.Original( @@ -361,12 +371,17 @@ internal sealed unsafe class Chat : IDisposable if (playerName != null) { - // Chat-window Activated integration is offline; tell-target - // routing returns when the new chat layer is wired up. - _ = playerName; - _ = worldId; - _ = contentId; - _ = reason; + // In-foray right-click -> Send Tell: same prefill path as the non-foray + // tell. The foray-specific TellSpecial channel routing stays deferred + // (v1.8.1, SetEurekaTellChannel) — prefill-only here (Flo decision 2026-06-15). + var forayName = playerName->ToString(); + var forayWorld = worldName != null ? worldName->ToString() : string.Empty; + var forayCommand = $"/tell {forayName}"; + if (!string.IsNullOrEmpty(forayWorld)) + forayCommand += $"@{forayWorld}"; + forayCommand += " "; + Plugin.InputBar.SetPendingMessage(forayCommand); + Plugin.InputBar.Activate = true; } ContextMenuTellInForayHook!.Original( From 3878869904517250afced5060ae2d68b065ea1c9 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 16 Jun 2026 01:21:05 +0200 Subject: [PATCH 5/8] feat(keybind): cycle tabs and switch channel with pill sync --- HellionChat/GameFunctions/KeybindManager.cs | 30 +++++++++++++++++---- HellionChat/Ui/Windows/MainWindow.cs | 19 +++++++++++++ HellionChat/Util/TabLifecycleHelpers.cs | 11 ++++++++ 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/HellionChat/GameFunctions/KeybindManager.cs b/HellionChat/GameFunctions/KeybindManager.cs index ec7c7ea..e5ee236 100644 --- a/HellionChat/GameFunctions/KeybindManager.cs +++ b/HellionChat/GameFunctions/KeybindManager.cs @@ -501,20 +501,40 @@ internal unsafe class KeybindManager : IDisposable return; Plugin.KeyState[currentBest.Item1] = false; - if (!KeybindsToIntercept.ContainsKey(currentBest.Item2)) + if (!KeybindsToIntercept.TryGetValue(currentBest.Item2, out var info)) return; // Re-surface the chat-activation entry point retired in v1.6.0: a chat-open // keybind shows + focuses the window, restoring it from a user-hide or a - // closed state. Channel/prefill routing from the bind stays out of scope. + // closed state. Plugin.Instance.MainWindow?.ActivateChat(); + + // Direct channel-switch binds (CMD_SAY/PARTY/numbered linkshells/…): switch the + // game channel AND mirror it onto the active tab so the input pill shows the + // real send target (pill-sync, Flo decision 2026-06-15). Rotation binds (REPLY / + // linkshell-cycle, Rotate != None) and the Permanent nuance stay deferred to the + // keybind-routing follow-cycle. + if (info.Channel is { } channel && info.Rotate == RotateMode.None) + { + Plugin.Instance.Functions.Chat.SetChannel(channel); + if (Plugin.Instance.MainWindow?.ActiveTab is { } activeTab) + { + activeTab.CurrentChannel.SetChannel(channel); + activeTab.CurrentChannel.TellTarget = null; + activeTab.CurrentChannel.ResetTempChannel(); + } + } + + // Prefill text binds (CMD_COMMAND seeds "/"): drop the token into our input. + if (info.Text is { } text) + Plugin.Instance.InputBar.SetPendingMessage(text); } - // Tab-cycle dispatch is offline until the new chat layer surfaces a - // ChangeTabDelta entry point and pop-out input bars come back online. + // Cycle the main window's active tab. Pop-out input-bar focus-forward stays + // deferred (no focus contract yet) — main-window tabs only. private void DispatchTabDelta(int delta) { - _ = delta; + Plugin.Instance.MainWindow?.ChangeTabDelta(delta); } private static Keybind GetKeybind(string id) diff --git a/HellionChat/Ui/Windows/MainWindow.cs b/HellionChat/Ui/Windows/MainWindow.cs index 646a9e9..59f095f 100644 --- a/HellionChat/Ui/Windows/MainWindow.cs +++ b/HellionChat/Ui/Windows/MainWindow.cs @@ -160,6 +160,25 @@ internal sealed class MainWindow : Window TabLifecycleHelpers.OnTabActivated(tab, previous); } + // Tab-cycle entry point for the ChatTabForward/Backward keybinds. Empty list is a + // no-op; a null active tab seeds tabs[0]; a single-tab cycle that lands on the + // already-active tab is a no-op (ActivateTab early-returns on the same reference). + // Routes through ActivateTab so the cycle strips stale tell state + re-derives the + // channel exactly like a sidebar/top-tab click. Pop-out focus-forward stays + // deferred (no focus contract) — main-window tabs only. + internal void ChangeTabDelta(int delta) + { + var tabs = Plugin.Config.Tabs; + if (tabs.Count == 0) + return; + + var idx = _activeTab is null ? 0 : tabs.IndexOf(_activeTab); + if (idx < 0) + idx = 0; // active tab not in the list (mid-strip) -> start from the first + + ActivateTab(tabs[TabLifecycleHelpers.WrapTabIndex(idx, delta, tabs.Count)]); + } + // Internal accessors for self-tests so the probes can reach the live // component without exposing them as public surface. internal Components.Sidebar GetSidebarForSelfTest() => _sidebar; diff --git a/HellionChat/Util/TabLifecycleHelpers.cs b/HellionChat/Util/TabLifecycleHelpers.cs index 9c2a6f8..e9edc6f 100644 --- a/HellionChat/Util/TabLifecycleHelpers.cs +++ b/HellionChat/Util/TabLifecycleHelpers.cs @@ -83,4 +83,15 @@ internal static class TabLifecycleHelpers tab.CurrentChannel.TellTarget = null; tab.CurrentChannel.ResetTempChannel(); } + + // Wrap-around tab index for keybind cycling. Pure so the Build-Suite can test the + // wrap math without a live window. count == 0 returns 0 (the caller dead-zones + // before activating); negative deltas wrap correctly via the double-mod. + // TEST-MIRROR: ../../Hellion Build test/_Helpers/TabLifecycleHelpersTests.cs + internal static int WrapTabIndex(int current, int delta, int count) + { + if (count <= 0) + return 0; + return ((current + delta) % count + count) % count; + } } From 6578c10b1377bb8d54dcbd80c801fc25d3e18e3d Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 16 Jun 2026 01:21:05 +0200 Subject: [PATCH 6/8] feat(settings): restore the tab-cycle keybind binder UI --- .../Ui/Components/Settings/Tabs/GeneralTab.cs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/HellionChat/Ui/Components/Settings/Tabs/GeneralTab.cs b/HellionChat/Ui/Components/Settings/Tabs/GeneralTab.cs index 45c5383..b82948f 100644 --- a/HellionChat/Ui/Components/Settings/Tabs/GeneralTab.cs +++ b/HellionChat/Ui/Components/Settings/Tabs/GeneralTab.cs @@ -1,4 +1,5 @@ using Dalamud.Bindings.ImGui; +using HellionChat.Util; namespace HellionChat.Ui.Components.Settings.Tabs; @@ -27,6 +28,23 @@ internal sealed class GeneralTab ); } + if (ImGui.CollapsingHeader("Keybinds", ImGuiTreeNodeFlags.DefaultOpen)) + { + ImGui.TextDisabled("Click a button, then press the key combination. Esc clears."); + DrawKeybind( + "Cycle to next chat tab", + "ChatTabForwardKeybind", + () => Plugin.Config.ChatTabForward, + v => Plugin.Config.ChatTabForward = v + ); + DrawKeybind( + "Cycle to previous chat tab", + "ChatTabBackwardKeybind", + () => Plugin.Config.ChatTabBackward, + v => Plugin.Config.ChatTabBackward = v + ); + } + if (ImGui.CollapsingHeader("Notifications", ImGuiTreeNodeFlags.DefaultOpen)) { DrawToggle( @@ -68,4 +86,27 @@ internal sealed class GeneralTab _plugin.SaveConfig(); } } + + // Wires the already-present ImGuiUtil.KeybindInput capture widget (dead/unwired + // since the v1.6.0 rewrite) back into the settings, so ChatTabForward/Backward + // are bindable again. ConfigKeyBind is a reference type, so a capture (new + // instance) or an Esc-clear (null) changes the reference — persist only then. + private void DrawKeybind( + string label, + string id, + Func get, + Action set + ) + { + ImGui.TextUnformatted(label); + ImGui.SetNextItemWidth(-1); + var keybind = get(); + var before = keybind; + ImGuiUtil.KeybindInput(id, ref keybind); + if (!ReferenceEquals(before, keybind)) + { + set(keybind); + _plugin.SaveConfig(); + } + } } From 49f5119b177d84c88e91b0b4d63c2754a70045ed Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 16 Jun 2026 01:29:59 +0200 Subject: [PATCH 7/8] feat(popout): arm the auto-tell pop-out settings and add the pool self-test --- HellionChat/Plugin.cs | 1 + .../SelfTests/ChannelPopoutBindStep.cs | 119 ++++++++++++++++++ .../Components/Settings/Tabs/ChannelsTab.cs | 18 ++- 3 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 HellionChat/SelfTests/ChannelPopoutBindStep.cs diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index 7f4e746..cc2a810 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -388,6 +388,7 @@ public sealed class Plugin : IAsyncDalamudPlugin new SelfTests.OnOpenMainUiRoutesMainWindowStep(this), new SelfTests.TypingIpcStateStep(this), new SelfTests.ConfigMigrationV23Step(this), + new SelfTests.ChannelPopoutBindStep(this), new SelfTests.HoverSheenAllocStep(this), new SelfTests.HonorificHeaderRenderStep(this), new SelfTests.AboutIntegrationsStatusStep(this), diff --git a/HellionChat/SelfTests/ChannelPopoutBindStep.cs b/HellionChat/SelfTests/ChannelPopoutBindStep.cs new file mode 100644 index 0000000..8d1b987 --- /dev/null +++ b/HellionChat/SelfTests/ChannelPopoutBindStep.cs @@ -0,0 +1,119 @@ +using System.Linq; +using Dalamud.Bindings.ImGui; +using Dalamud.Plugin.SelfTest; + +namespace HellionChat.SelfTests; + +// Exercises the ChannelPopoutPool lifecycle in-game (a behavioural step, not a +// non-null-handle check — feedback_hellion_chat_fontmanager_push_trap). Verifies +// pre-alloc == MaxParallelPopouts, unique slot ids, a TryOpen->IsOpen->TryClose +// round-trip, idempotent TryClose, and capacity-exceeded refusal (warn, no throw). +// The pool is a LIVE DI singleton, so a tester may already have real pop-outs open +// when /xlperf runs; the step tests against the FREE slots (not full capacity) and +// only ever closes ids it opened, so it neither false-REDs on a non-empty pool nor +// disturbs real pop-outs. The pure slot-map math is pinned by PopoutSlotMapTests +// (Build-Suite); this step proves the live wiring on top of it. Every slot reserved +// is released before RunStep returns, so no pop-out is left bound. +internal sealed class ChannelPopoutBindStep : ISelfTestStep +{ + private readonly Plugin _plugin; + + public ChannelPopoutBindStep(Plugin plugin) + { + _plugin = plugin; + } + + public string Name => "Hellion Chat - Channel popout pool lifecycle"; + + public SelfTestStepResult RunStep() + { + var pool = _plugin.ChannelPopoutPool; + var capacity = Plugin.Config.MaxParallelPopouts; + + if (pool.Instances.Count != capacity) + { + ImGui.Text( + $"Expected {capacity} pre-allocated pop-out windows, found {pool.Instances.Count}." + ); + return SelfTestStepResult.Fail; + } + + if (pool.Instances.Select(w => w.SlotIndex).Distinct().Count() != pool.Instances.Count) + { + ImGui.Text("Pop-out windows do not have unique slot indices."); + return SelfTestStepResult.Fail; + } + + // Free slots right now = capacity minus whatever real pop-outs are already + // bound. Testing against this (not capacity) keeps the step state-independent. + var free = capacity - pool.Instances.Count(w => w.Bound is not null); + + // Round-trip on a throwaway tab, only when there's a slot to take. A bare Tab + // has CurrentChannel.Channel == Invalid + an empty SelectedChannels, so the + // pool's OnTabActivated strip is a no-op (no NRE), and the live active tab is + // passed only as `previous`, so it is never mutated. We close before + // returning, so the bound window never reaches a Draw frame. + if (free > 0) + { + var probe = new Tab { Name = "##selftest-popout-probe" }; + if (pool.IsOpen(probe.Identifier)) + { + ImGui.Text("Probe tab already open before TryOpen."); + return SelfTestStepResult.Fail; + } + + if (!pool.TryOpen(probe)) + { + ImGui.Text("TryOpen returned false with a free slot."); + return SelfTestStepResult.Fail; + } + + if (!pool.IsOpen(probe.Identifier)) + { + ImGui.Text("IsOpen is false right after a successful TryOpen."); + pool.TryClose(probe.Identifier); + return SelfTestStepResult.Fail; + } + + pool.TryClose(probe.Identifier); + if (pool.IsOpen(probe.Identifier)) + { + ImGui.Text("IsOpen is still true after TryClose."); + return SelfTestStepResult.Fail; + } + + // Idempotent: closing an already-closed id is a silent no-op. + pool.TryClose(probe.Identifier); + } + + // Capacity guard: fill the remaining free slots, then one more open must be + // refused (warn, no throw). Release everything we opened before reporting. + var fillers = Enumerable + .Range(0, free) + .Select(_ => new Tab { Name = "##selftest-fill" }) + .ToList(); + var opened = fillers.Count(pool.TryOpen); + var overflow = new Tab { Name = "##selftest-overflow" }; + var overflowRejected = !pool.TryOpen(overflow); + + foreach (var filler in fillers) + pool.TryClose(filler.Identifier); + pool.TryClose(overflow.Identifier); + + if (opened != free) + { + ImGui.Text($"Filled only {opened}/{free} free slots before TryOpen refused."); + return SelfTestStepResult.Fail; + } + + if (!overflowRejected) + { + ImGui.Text("Pool accepted an open beyond capacity instead of refusing."); + return SelfTestStepResult.Fail; + } + + return SelfTestStepResult.Pass; + } + + public void CleanUp() { } +} diff --git a/HellionChat/Ui/Components/Settings/Tabs/ChannelsTab.cs b/HellionChat/Ui/Components/Settings/Tabs/ChannelsTab.cs index 5929c6f..7c3bd2b 100644 --- a/HellionChat/Ui/Components/Settings/Tabs/ChannelsTab.cs +++ b/HellionChat/Ui/Components/Settings/Tabs/ChannelsTab.cs @@ -1,5 +1,4 @@ using Dalamud.Bindings.ImGui; -using Dalamud.Interface.Utility.Raii; namespace HellionChat.Ui.Components.Settings.Tabs; @@ -45,12 +44,11 @@ internal sealed class ChannelsTab () => Plugin.Config.AutoTellTabsShowGreetedToggle, v => Plugin.Config.AutoTellTabsShowGreetedToggle = v ); - // Popout is a v1.8.0 teaser — render disabled, do NOT persist. - using (ImRaii.Disabled(true)) - { - var openAsPopout = Plugin.Config.AutoTellTabsOpenAsPopout; - ImGui.Checkbox("Open as popout (lands in v1.8.0)", ref openAsPopout); - } + DrawToggle( + "Open as popout", + () => Plugin.Config.AutoTellTabsOpenAsPopout, + v => Plugin.Config.AutoTellTabsOpenAsPopout = v + ); } if (ImGui.CollapsingHeader("Tell auto-open mode", ImGuiTreeNodeFlags.DefaultOpen)) @@ -75,7 +73,7 @@ internal sealed class ChannelsTab private void DrawTellAutoOpenModeCombo() { - var labels = new[] { "Off", "Sidebar", "Top tab", "Popout (lands in v1.8.0)" }; + var labels = new[] { "Off", "Sidebar", "Top tab", "Popout" }; var values = Enum.GetValues(); var current = Plugin.Config.TellAutoOpenMode; var selected = 0; @@ -91,9 +89,7 @@ internal sealed class ChannelsTab ImGui.SetNextItemWidth(220); if (ImGui.Combo("Tell auto-open mode", ref selected, labels, labels.Length)) { - // Popout (index 3) is a v1.8.0 teaser — revert to previous value - // and skip SaveConfig. - if (selected >= 0 && selected < values.Length && selected != 3) + if (selected >= 0 && selected < values.Length) { Plugin.Config.TellAutoOpenMode = values[selected]; _plugin.SaveConfig(); From 6f71b093317d2bf54e280316244a31ab3f810583 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 16 Jun 2026 09:04:18 +0200 Subject: [PATCH 8/8] fix(closeout): address closure-review findings - gate keybind pill-sync on IsChannelOrExistingLinkshell so an empty linkshell slot no longer desyncs the pill from the real send channel - close manually-popped pop-out windows on logout via an IsOpen filter instead of the PopOut flag (which manual pops never set) - read the router's tell-tab lookup through a lock-wrapped accessor so the framework thread cannot enumerate Config.Tabs mid worker-thread mutation - add a "switch on every tell" toggle (default on) and make the auto-open mode pick the matching layout, so Sidebar vs Top-tab are distinct - comment corrections (stale/contradictory text, TEST-MIRROR path depth) --- HellionChat/AutoTellTabsService.cs | 24 +++++++++++++---- HellionChat/Configuration.cs | 6 +++++ HellionChat/GameFunctions/Chat.cs | 6 ++--- HellionChat/GameFunctions/KeybindManager.cs | 17 ++++++++---- .../SelfTests/ChannelPopoutBindStep.cs | 16 +++++------ HellionChat/Services/TellRouterService.cs | 27 ++++++++++++++++--- .../Components/Settings/Tabs/ChannelsTab.cs | 5 ++++ HellionChat/Ui/Windows/MainWindow.cs | 4 +-- HellionChat/Util/TabLifecycleHelpers.cs | 2 +- 9 files changed, 78 insertions(+), 29 deletions(-) diff --git a/HellionChat/AutoTellTabsService.cs b/HellionChat/AutoTellTabsService.cs index ae1bb20..f66eed5 100644 --- a/HellionChat/AutoTellTabsService.cs +++ b/HellionChat/AutoTellTabsService.cs @@ -239,6 +239,16 @@ internal sealed class AutoTellTabsService : IDisposable ); } + // Lock-protected lookup for the framework-thread caller (TellRouterService). + // Config.Tabs is mutated under _tempTabsLock on the PendingMessage worker thread, + // so a framework-tick reader must take the same lock to avoid enumerating the list + // mid-mutation. + internal Tab? FindTempTabSafe(string name, uint world) + { + lock (_tempTabsLock) + return FindTempTab(name, world); + } + internal void DropOldestTempTab() { // Pinned tabs live in their own bucket (MaxPinnedTempTabs) and are @@ -281,7 +291,8 @@ internal sealed class AutoTellTabsService : IDisposable tab.AddMessage(currentMessage, unread: true); - // Open as pop-out if configured (flag set before Tabs.Add for the next render-tick). + // Flag the tab as a pop-out if configured; the marshalled TryOpen below reads + // that flag to open the real window. if (Plugin.Config.AutoTellTabsOpenAsPopout) { tab.PopOut = true; @@ -438,13 +449,16 @@ internal sealed class AutoTellTabsService : IDisposable var active = _plugin.MainWindow?.ActiveTab; var poppedTempTabIds = Plugin - .Config.Tabs.Where(t => TabLifecycleHelpers.IsInUnpinnedPool(t) && t.PopOut) + .Config.Tabs.Where(t => + TabLifecycleHelpers.IsInUnpinnedPool(t) + && _plugin.ChannelPopoutPool.IsOpen(t.Identifier) + ) .Select(t => t.Identifier) .ToList(); - // Close each popped temp tab's window before the tabs leave the list. - // Logout is a framework-thread event (serialized with Draw), so no - // marshalling is needed here, unlike the worker-thread eviction path. + // Close any pop-out window an unpinned temp tab owns before the tabs leave + // the list. Filtering on the live pool (not the PopOut flag) also catches + // manually right-clicked pop-outs, which never set the flag. foreach (var id in poppedTempTabIds) _plugin.ChannelPopoutPool.TryClose(id); diff --git a/HellionChat/Configuration.cs b/HellionChat/Configuration.cs index 7172db8..3ef8a62 100755 --- a/HellionChat/Configuration.cs +++ b/HellionChat/Configuration.cs @@ -268,6 +268,11 @@ public class Configuration : IPluginConfiguration public bool SettingsWindowOpen; public int MaxParallelPopouts = 8; public TellAutoOpenMode TellAutoOpenMode = TellAutoOpenMode.Sidebar; + + // When true (default) the tell-auto-open router switches the active tab to the + // incoming tell on every message; when false the tab is still created/revealed + // with its unread badge but the active tab is left where the user is reading. + public bool TellAutoOpenSwitchAlways = true; public int SidebarAutoSwitchThresholdPx = 800; // v22 field: MainWindow layout mode (sidebar vs. horizontal top tabs). @@ -428,6 +433,7 @@ public class Configuration : IPluginConfiguration SettingsWindowOpen = other.SettingsWindowOpen; MaxParallelPopouts = other.MaxParallelPopouts; TellAutoOpenMode = other.TellAutoOpenMode; + TellAutoOpenSwitchAlways = other.TellAutoOpenSwitchAlways; SidebarAutoSwitchThresholdPx = other.SidebarAutoSwitchThresholdPx; MainWindowLayoutMode = other.MainWindowLayoutMode; } diff --git a/HellionChat/GameFunctions/Chat.cs b/HellionChat/GameFunctions/Chat.cs index 841e52f..a4e77f1 100755 --- a/HellionChat/GameFunctions/Chat.cs +++ b/HellionChat/GameFunctions/Chat.cs @@ -232,9 +232,9 @@ internal sealed unsafe class Chat : IDisposable if (c != '\0' && !char.IsControl(c)) input = c.ToString(); - // Seed the just-typed character into our input field and focus it, - // the same prefill path the inventory item-link below uses. Prefill- - // only — no tab switch (Flo decision 2026-06-15). + // Seed the just-typed character into our input field and focus it, the + // same InputBar.AppendPending + Activate prefill path inventory item-links + // use. Prefill-only — no tab switch (Flo decision 2026-06-15). if (input != null) { Plugin.InputBar.AppendPending(input); diff --git a/HellionChat/GameFunctions/KeybindManager.cs b/HellionChat/GameFunctions/KeybindManager.cs index e5ee236..1b8f0a0 100644 --- a/HellionChat/GameFunctions/KeybindManager.cs +++ b/HellionChat/GameFunctions/KeybindManager.cs @@ -512,12 +512,19 @@ internal unsafe class KeybindManager : IDisposable // Direct channel-switch binds (CMD_SAY/PARTY/numbered linkshells/…): switch the // game channel AND mirror it onto the active tab so the input pill shows the // real send target (pill-sync, Flo decision 2026-06-15). Rotation binds (REPLY / - // linkshell-cycle, Rotate != None) and the Permanent nuance stay deferred to the - // keybind-routing follow-cycle. + // linkshell-cycle, Rotate != None) are skipped; the temp-vs-permanent distinction + // (v1.5.6's UseTempChannel / info.Permanent) collapses to one permanent-style + // switch here — restoring it is the keybind-routing follow-cycle. if (info.Channel is { } channel && info.Rotate == RotateMode.None) { Plugin.Instance.Functions.Chat.SetChannel(channel); - if (Plugin.Instance.MainWindow?.ActiveTab is { } activeTab) + // Only mirror onto the tab when the game actually accepted the switch — an + // empty linkshell slot leaves the game channel untouched, so the pill must + // stay put rather than show a target the game will not send to. + if ( + Chat.IsChannelOrExistingLinkshell(channel) + && Plugin.Instance.MainWindow?.ActiveTab is { } activeTab + ) { activeTab.CurrentChannel.SetChannel(channel); activeTab.CurrentChannel.TellTarget = null; @@ -530,8 +537,8 @@ internal unsafe class KeybindManager : IDisposable Plugin.Instance.InputBar.SetPendingMessage(text); } - // Cycle the main window's active tab. Pop-out input-bar focus-forward stays - // deferred (no focus contract yet) — main-window tabs only. + // Pop-out input-bar focus-forward stays deferred (no focus contract yet) — + // main-window tabs only. private void DispatchTabDelta(int delta) { Plugin.Instance.MainWindow?.ChangeTabDelta(delta); diff --git a/HellionChat/SelfTests/ChannelPopoutBindStep.cs b/HellionChat/SelfTests/ChannelPopoutBindStep.cs index 8d1b987..9965110 100644 --- a/HellionChat/SelfTests/ChannelPopoutBindStep.cs +++ b/HellionChat/SelfTests/ChannelPopoutBindStep.cs @@ -4,16 +4,12 @@ using Dalamud.Plugin.SelfTest; namespace HellionChat.SelfTests; -// Exercises the ChannelPopoutPool lifecycle in-game (a behavioural step, not a -// non-null-handle check — feedback_hellion_chat_fontmanager_push_trap). Verifies -// pre-alloc == MaxParallelPopouts, unique slot ids, a TryOpen->IsOpen->TryClose -// round-trip, idempotent TryClose, and capacity-exceeded refusal (warn, no throw). -// The pool is a LIVE DI singleton, so a tester may already have real pop-outs open -// when /xlperf runs; the step tests against the FREE slots (not full capacity) and -// only ever closes ids it opened, so it neither false-REDs on a non-empty pool nor -// disturbs real pop-outs. The pure slot-map math is pinned by PopoutSlotMapTests -// (Build-Suite); this step proves the live wiring on top of it. Every slot reserved -// is released before RunStep returns, so no pop-out is left bound. +// In-game behavioural check of the ChannelPopoutPool lifecycle (not a non-null-handle +// check — feedback_hellion_chat_fontmanager_push_trap): pre-alloc count, unique slot +// ids, a TryOpen->IsOpen->TryClose round-trip, idempotent close, and capacity refusal. +// The pool is a live DI singleton, so the step works against the FREE slots (not full +// capacity) and only closes ids it opened — it neither false-REDs on a non-empty pool +// nor disturbs real pop-outs. Pure slot-map math is pinned by PopoutSlotMapTests. internal sealed class ChannelPopoutBindStep : ISelfTestStep { private readonly Plugin _plugin; diff --git a/HellionChat/Services/TellRouterService.cs b/HellionChat/Services/TellRouterService.cs index 5f2b989..ce081d0 100644 --- a/HellionChat/Services/TellRouterService.cs +++ b/HellionChat/Services/TellRouterService.cs @@ -9,7 +9,7 @@ namespace HellionChat.Services; // that service owns tab CREATION + lifecycle; this only REVEALS/pops the tab it // finds. Popout guards on pool.IsOpen so it never double-pops a tab the // AutoTellTabsOpenAsPopout path already opened. Subscribes to the resolved -// MessageManager.MessageProcessed stream (partner already extracted), not the raw +// MessageManager.MessageProcessed stream (a resolved Message), not the raw // IChatGui event, and defers the reveal one tick so the tab exists regardless of // subscriber order. Wired by TellRouterServiceInitHostedService. internal sealed class TellRouterService : IDisposable @@ -69,7 +69,9 @@ internal sealed class TellRouterService : IDisposable // mutation) is serialized with Draw (reference_dalamud_framework_thread). Plugin.Framework.RunOnFrameworkThread(() => { - var tab = AutoTellTabsService.FindTempTab(name, world); + // Lock-safe lookup: AutoTellTabs mutates Config.Tabs under its lock on the + // worker thread, so we read through its guarded accessor, not the static. + var tab = Plugin.Instance.AutoTellTabsService?.FindTempTabSafe(name, world); if (tab == null) return; // nothing to reveal (auto-tell-tabs off -> no tab created) @@ -77,7 +79,26 @@ internal sealed class TellRouterService : IDisposable { case TellAutoOpenMode.Sidebar: case TellAutoOpenMode.TopTab: - Plugin.Instance.MainWindow?.ActivateTab(tab); + // Switching to the tab on every tell is user-gated + // (TellAutoOpenSwitchAlways, default on); when off the tab still + // appears with its unread badge but the active tab is left alone. + // The mode also picks the layout, so Sidebar vs TopTab are actually + // distinct outcomes, not the same ActivateTab. + if (Plugin.Config.TellAutoOpenSwitchAlways) + { + var wantLayout = + mode == TellAutoOpenMode.TopTab + ? MainWindowLayoutMode.TopTabs + : MainWindowLayoutMode.Sidebar; + if (Plugin.Config.MainWindowLayoutMode != wantLayout) + { + Plugin.Config.MainWindowLayoutMode = wantLayout; + Plugin.Instance.SaveConfig(); + } + + Plugin.Instance.MainWindow?.ActivateTab(tab); + } + break; case TellAutoOpenMode.Popout: // IsOpen-guard: don't double-pop a tab the AutoTellTabsOpenAsPopout diff --git a/HellionChat/Ui/Components/Settings/Tabs/ChannelsTab.cs b/HellionChat/Ui/Components/Settings/Tabs/ChannelsTab.cs index 7c3bd2b..b23658d 100644 --- a/HellionChat/Ui/Components/Settings/Tabs/ChannelsTab.cs +++ b/HellionChat/Ui/Components/Settings/Tabs/ChannelsTab.cs @@ -54,6 +54,11 @@ internal sealed class ChannelsTab if (ImGui.CollapsingHeader("Tell auto-open mode", ImGuiTreeNodeFlags.DefaultOpen)) { DrawTellAutoOpenModeCombo(); + DrawToggle( + "Switch to the tab on every tell", + () => Plugin.Config.TellAutoOpenSwitchAlways, + v => Plugin.Config.TellAutoOpenSwitchAlways = v + ); } if (ImGui.CollapsingHeader("Sidebar")) diff --git a/HellionChat/Ui/Windows/MainWindow.cs b/HellionChat/Ui/Windows/MainWindow.cs index 59f095f..524d3ad 100644 --- a/HellionChat/Ui/Windows/MainWindow.cs +++ b/HellionChat/Ui/Windows/MainWindow.cs @@ -161,8 +161,8 @@ internal sealed class MainWindow : Window } // Tab-cycle entry point for the ChatTabForward/Backward keybinds. Empty list is a - // no-op; a null active tab seeds tabs[0]; a single-tab cycle that lands on the - // already-active tab is a no-op (ActivateTab early-returns on the same reference). + // no-op; a null active tab seeds the index to 0; a single-tab cycle that lands on + // the already-active tab is a no-op (ActivateTab early-returns on the same reference). // Routes through ActivateTab so the cycle strips stale tell state + re-derives the // channel exactly like a sidebar/top-tab click. Pop-out focus-forward stays // deferred (no focus contract) — main-window tabs only. diff --git a/HellionChat/Util/TabLifecycleHelpers.cs b/HellionChat/Util/TabLifecycleHelpers.cs index e9edc6f..dc87ae2 100644 --- a/HellionChat/Util/TabLifecycleHelpers.cs +++ b/HellionChat/Util/TabLifecycleHelpers.cs @@ -87,7 +87,7 @@ internal static class TabLifecycleHelpers // Wrap-around tab index for keybind cycling. Pure so the Build-Suite can test the // wrap math without a live window. count == 0 returns 0 (the caller dead-zones // before activating); negative deltas wrap correctly via the double-mod. - // TEST-MIRROR: ../../Hellion Build test/_Helpers/TabLifecycleHelpersTests.cs + // TEST-MIRROR: ../../../Hellion Build test/_Helpers/TabLifecycleHelpersTests.cs internal static int WrapTabIndex(int current, int delta, int count) { if (count <= 0)