From f6749d206b087ba28d0789db769b5928e36b178a Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Wed, 27 May 2026 23:42:28 +0200 Subject: [PATCH] =?UTF-8?q?chore(polish):=20cycle-end=20sweep=20=E2=80=94?= =?UTF-8?q?=20drop=20dead=20fields,=20dep-cycle,=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accumulated polish across the v1.7.1 R-Block reviewer findings. Single sweep before Phase-3 Smoke-Gate. Dep-cycle cleanup (Block H + #30): - CommandHelpWindow drops the dead _inputBar ctor-param + discard that was J's speculative prep; this eliminates the InputBar <-> CommandHelpWindow ctor cycle at its root - InputBar replaces Lazy wrapper with direct CommandHelpWindow ctor-param now that the cycle is broken - PluginHostFactory InputBar + CommandHelpWindow DI-regs simplified Dead-field removals: - MessageList drops _themes + _resolver (no reads after H's render-path swap to _chunkRenderer.DrawChunks) - InputBar drops FocusedPreview (no consumer wiring in the new architecture) - InputPreview drops SelectedCursorPos (v1.5.6 letter-by-letter renderer artifact, no callers in R1) - InputPreview drops WhitespaceRegex + partial keyword on class (dead GeneratedRegex with no callers) Visibility fixes: - InputPreview + CommandHelpWindow + DebuggerWindow ctors flip public -> internal for consistency with internal sealed class declarations DI helper extraction: - PluginHostFactory MakePayloadHandler private static helper DRYs the 7-arg list shared between PayloadHandler-singleton and Lender factory ImGui-rendering fix: - MessageList.DrawCompactRow uses SameLine(0f, 0f) — eliminates visible ItemSpacing.X gap between sender-prefix and chunk content Bug fixes: - PayloadHandler.LeftClickPayload drops spurious unsafe keyword (no pointer ops in the method body; v1.5.6 had no unsafe here) - PayloadHandler.StringifyMessage Aggregate seeded with string.Empty to fix empty-sequence crash for pure-icon messages - PayloadHandler.MoveTooltip args==null LogWarning template simplified (?.GetType().Name was always null after the null-check — misleading) - InputBar.SlashCommandCallback drops redundant BufTextLen==0 guard (BufTextSpan handles empty correctly) Comment improvements (WHY-not-WHAT): - ImGuiUtil.cs payload-state cluster comment moved below Buttons array - PayloadHandler: §6.9 trimmed to 1 line, FindCharacterForPayload documented, hq symbol marker restored, MoveTooltip guard documented as defensive v1.7.1 addition, NativeItemTooltips branch explained, §4.2 theme colour swap explained - DebuggerWindow class comment mentions PayloadHandler counters section - InitHostedServices StopAsync explains params-overload semantics - InputBar AppendPending null policy vs SetPendingMessage documented, CommandManager leading-slash assumption noted - PluginHostFactory block comment explains singleton+Lender split Build: 0 warnings, 0 errors. csharpier: clean. Version unchanged. --- .../Hosting/InitHostedServices.cs | 1 + HellionChat/PayloadHandler.cs | 21 +++++----- HellionChat/PluginHostFactory.cs | 41 ++++++++----------- HellionChat/Ui/CommandHelpWindow.cs | 7 +--- HellionChat/Ui/Components/InputBar.cs | 17 ++++---- HellionChat/Ui/Components/MessageList.cs | 15 +------ HellionChat/Ui/Debugger.cs | 5 ++- HellionChat/Ui/InputPreview.cs | 12 ++---- HellionChat/Util/ImGuiUtil.cs | 4 +- 9 files changed, 47 insertions(+), 76 deletions(-) diff --git a/HellionChat/Infrastructure/Hosting/InitHostedServices.cs b/HellionChat/Infrastructure/Hosting/InitHostedServices.cs index a957127..43d1a81 100644 --- a/HellionChat/Infrastructure/Hosting/InitHostedServices.cs +++ b/HellionChat/Infrastructure/Hosting/InitHostedServices.cs @@ -137,6 +137,7 @@ internal sealed class PayloadHandlerInitHostedService( { await Plugin.Framework.RunOnFrameworkThread(() => { + // Single call using the params-overload removes the delegate from all addons it was registered for (ItemDetail + ActionDetail both cleaned in one shot). Plugin.AddonLifecycle.UnregisterListener(payloadHandler.MoveTooltip); }); } diff --git a/HellionChat/PayloadHandler.cs b/HellionChat/PayloadHandler.cs index 1b127b4..46be55b 100644 --- a/HellionChat/PayloadHandler.cs +++ b/HellionChat/PayloadHandler.cs @@ -230,7 +230,7 @@ internal sealed class PayloadHandler .Where(chunk => chunk is TextChunk) .Cast() .Select(text => text.Content) - .Aggregate(string.Concat); + .Aggregate(string.Empty, string.Concat); } private void DrawPlayerPopup(Chunk chunk, PlayerPayload player) @@ -263,8 +263,7 @@ internal sealed class PayloadHandler // Eureka, Bozja and Occult need special handling as tells work different if (!Sheets.IsInForay()) { - // §6.9: build as single string then hand off; replaces v1.5.6's - // incremental LogWindow.Chat += ... pattern + // §6.9: single SetPendingMessage call; v1.5.6 used incremental Chat += writes var builder = $"/tell {player.PlayerName}"; if (world.Value.IsPublic) builder += $"@{world.Value.Name}"; @@ -408,6 +407,7 @@ internal sealed class PayloadHandler ); } + // Returns the first matching IPlayerCharacter in ObjectTable, null if out of render range. private IPlayerCharacter? FindCharacterForPayload(PlayerPayload payload) { foreach (var obj in Plugin.ObjectTable) @@ -449,6 +449,7 @@ internal sealed class PayloadHandler var name = itemRow.Name.ToDalamudString(); if (hq) + // hq symbol name.Payloads.Add(new TextPayload(" ")); else if (payload.Kind == ItemKind.Collectible) name.Payloads.Add(new TextPayload(" ")); @@ -570,12 +571,10 @@ internal sealed class PayloadHandler public unsafe void MoveTooltip(AddonEvent type, AddonArgs args) { + // Defensive guard added in v1.7.1 — AddonLifecycle should never pass null, but be safe. if (args == null) { - _logger.LogWarning( - "MoveTooltip received unexpected AddonArgs type: {ArgsType}", - args?.GetType().Name ?? "" - ); + _logger.LogWarning("MoveTooltip called with null AddonArgs — unexpected, skipping"); return; } @@ -696,6 +695,7 @@ internal sealed class PayloadHandler DoHover(() => HoverStatus(status), hoverSize); break; case ItemPayload item: + // Native tooltip path: set state for MoveTooltip to reposition the game addon next frame. if (Plugin.Config.NativeItemTooltips) { if (!HandleTooltips || HoveredItem != item.RawItemId) @@ -722,13 +722,14 @@ internal sealed class PayloadHandler } } - private void DoHover(Action drawAction, float spacingHorizontal) + private void DoHover(Action drawAction, float tooltipWidth) { - ImGui.SetNextWindowSize(new Vector2(spacingHorizontal, -1f)); + ImGui.SetNextWindowSize(new Vector2(tooltipWidth, -1f)); using (ImRaii.Tooltip()) using (ImRaii.TextWrapPos(0.0f)) using ( + // §4.2: use active theme text colour instead of the former LogWindow.DefaultText static. ImRaii.PushColor( ImGuiCol.Text, ColourUtil.RgbaToVector4(_themes.Active.Colors.TextPrimary) @@ -854,7 +855,7 @@ internal sealed class PayloadHandler } } - private unsafe void LeftClickPayload(Chunk chunk, Payload? payload) + private void LeftClickPayload(Chunk chunk, Payload? payload) { switch (payload) { diff --git a/HellionChat/PluginHostFactory.cs b/HellionChat/PluginHostFactory.cs index c66290b..ea3cdb7 100644 --- a/HellionChat/PluginHostFactory.cs +++ b/HellionChat/PluginHostFactory.cs @@ -130,8 +130,6 @@ internal static class PluginHostFactory sp.GetRequiredService>() )); services.AddSingleton(sp => new Ui.Components.MessageList( - sp.GetRequiredService(), - sp.GetRequiredService(), sp.GetRequiredService(), sp.GetRequiredService() )); @@ -143,7 +141,7 @@ internal static class PluginHostFactory sp.GetRequiredService(), sp.GetRequiredService>(), () => sp.GetRequiredService().SettingsWindow.Toggle(), - new Lazy(() => sp.GetRequiredService()) + sp.GetRequiredService() )); services.AddSingleton(sp => new Ui.Components.Settings.TabSidebar( sp.GetRequiredService() @@ -231,32 +229,15 @@ internal static class PluginHostFactory // Factory-lambdas for ChunkRenderer, PayloadHandler, and Lender // because all three are internal-sealed (ActivatorUtilities can't reflect into // internal ctors) and Lender has an internal ctor by design. + // PayloadHandler registered twice: once as singleton for G/H, once via Lender for per-frame isolation (I/J/K). services.AddSingleton(sp => new Ui.Components.ChunkRenderer( sp.GetRequiredService(), sp.GetRequiredService(), sp.GetRequiredService>(), sp.GetRequiredService() )); - services.AddSingleton(sp => new PayloadHandler( - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService>() - )); - services.AddSingleton(sp => new Lender(() => - new PayloadHandler( - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService(), - sp.GetRequiredService>() - ) - )); + services.AddSingleton(sp => MakePayloadHandler(sp)); + services.AddSingleton(sp => new Lender(() => MakePayloadHandler(sp))); // Block C — Windows. WindowSystem.AddWindow is called from // PluginLifecycle.LoadAsync on the framework thread. @@ -290,7 +271,6 @@ internal static class PluginHostFactory services.AddSingleton(sp => new CommandHelpWindow( sp.GetRequiredService(), sp.GetRequiredService(), - sp.GetRequiredService(), sp.GetRequiredService>() )); services.AddSingleton(sp => new SeStringDebugger(sp.GetRequiredService())); @@ -328,11 +308,22 @@ internal static class PluginHostFactory sp.GetRequiredService() ) ); - services.AddHostedService(sp => new Infrastructure.Hosting.PayloadHandlerInitHostedService( + services.AddHostedService(sp => new PayloadHandlerInitHostedService( sp.GetRequiredService(), sp.GetRequiredService() )); } + + private static PayloadHandler MakePayloadHandler(IServiceProvider sp) => + new( + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService(), + sp.GetRequiredService>() + ); } internal sealed record PluginHostDependencies( diff --git a/HellionChat/Ui/CommandHelpWindow.cs b/HellionChat/Ui/CommandHelpWindow.cs index ceaadae..53ee9c8 100644 --- a/HellionChat/Ui/CommandHelpWindow.cs +++ b/HellionChat/Ui/CommandHelpWindow.cs @@ -14,22 +14,19 @@ internal sealed class CommandHelpWindow : Window { private readonly ChunkRenderer _chunkRenderer; private readonly Windows.MainWindow _mainWindow; - private readonly Components.InputBar _inputBar; private readonly ILogger _logger; private ReadOnlySeString? _commandDescription; - public CommandHelpWindow( + internal CommandHelpWindow( ChunkRenderer chunkRenderer, Windows.MainWindow mainWindow, - Components.InputBar inputBar, ILogger logger ) : base("command help##chat2-commandhelp") { _chunkRenderer = chunkRenderer; _mainWindow = mainWindow; - _inputBar = inputBar; _logger = logger; Flags = @@ -45,8 +42,6 @@ internal sealed class CommandHelpWindow : Window // Logger injected for future diagnostic hooks (no call-sites yet in R2). _ = _logger; - // InputBar injected for future integration (no call-sites yet in R2). - _ = _inputBar; } public void UpdateContent(ReadOnlySeString commandDesc) diff --git a/HellionChat/Ui/Components/InputBar.cs b/HellionChat/Ui/Components/InputBar.cs index f588de5..384d404 100644 --- a/HellionChat/Ui/Components/InputBar.cs +++ b/HellionChat/Ui/Components/InputBar.cs @@ -33,14 +33,13 @@ internal sealed class InputBar private readonly TokenResolver _resolver; private readonly ILogger _logger; private readonly Action _onOpenSettings; - private readonly Lazy _commandHelpWindow; + private readonly CommandHelpWindow _commandHelpWindow; private string _pendingMessage = string.Empty; private bool _isFocused; private bool? _isFocusedOverride; // Test-only; null = honour per-frame Draw() value. public bool Activate; - public bool FocusedPreview; public InputBar( SymbolPicker symbolPicker, @@ -49,7 +48,7 @@ internal sealed class InputBar TokenResolver resolver, ILogger logger, Action onOpenSettings, - Lazy commandHelpWindow + CommandHelpWindow commandHelpWindow ) { _symbolPicker = symbolPicker; @@ -99,6 +98,7 @@ internal sealed class InputBar } } + // Null treated as empty here (matches IsNullOrEmpty guard); contrast with SetPendingMessage which throws to surface PayloadHandler call-site bugs early. public void AppendPending(string suffix) { if (string.IsNullOrEmpty(suffix)) @@ -231,7 +231,7 @@ internal sealed class InputBar ) ) { - _commandHelpWindow.Value.IsOpen = false; + _commandHelpWindow.IsOpen = false; TrySend(activeTab); } _isFocused = ImGui.IsItemFocused(); @@ -241,9 +241,7 @@ internal sealed class InputBar // stays in sync with what the user is typing without a per-frame poll. private int SlashCommandCallback(scoped ref ImGuiInputTextCallbackData data) { - _commandHelpWindow.Value.IsOpen = false; - if (data.BufTextLen == 0) - return 0; + _commandHelpWindow.IsOpen = false; var text = Encoding.UTF8.GetString(data.BufTextSpan); if (!text.StartsWith('/')) @@ -252,12 +250,13 @@ internal sealed class InputBar var spaceIdx = text.IndexOf(' '); var command = spaceIdx > 0 ? text[..spaceIdx] : text; + // Keys in CommandManager.Commands include the leading slash. if (AllCommands.TryGetValue(command, out var textCommand)) - _commandHelpWindow.Value.UpdateContent(textCommand.Description); + _commandHelpWindow.UpdateContent(textCommand.Description); else if ( Plugin.CommandManager.Commands.TryGetValue(command, out var info) && info.ShowInHelp ) - _commandHelpWindow.Value.UpdateContent(info.HelpMessage); + _commandHelpWindow.UpdateContent(info.HelpMessage); return 0; } diff --git a/HellionChat/Ui/Components/MessageList.cs b/HellionChat/Ui/Components/MessageList.cs index 049fce1..f12bf55 100644 --- a/HellionChat/Ui/Components/MessageList.cs +++ b/HellionChat/Ui/Components/MessageList.cs @@ -2,8 +2,6 @@ using System.Globalization; using System.Numerics; using Dalamud.Bindings.ImGui; using Dalamud.Interface.Utility.Raii; -using HellionChat.Themes; -using HellionChat.Ui.StyleEngine; using HellionChat.Util; namespace HellionChat.Ui.Components; @@ -17,8 +15,6 @@ internal sealed class MessageList { private const float CompactRowHeight = 18f; - private readonly ThemeRegistry _themes; - private readonly TokenResolver _resolver; private readonly FontManager _fonts; private readonly ChunkRenderer _chunkRenderer; @@ -35,15 +31,8 @@ internal sealed class MessageList // so MainWindow's draw loop must invoke it explicitly to render right-click context popups. internal void DrawHandlerPopups() => _handler?.Draw(); - public MessageList( - ThemeRegistry themes, - TokenResolver resolver, - FontManager fonts, - ChunkRenderer chunkRenderer - ) + public MessageList(FontManager fonts, ChunkRenderer chunkRenderer) { - _themes = themes; - _resolver = resolver; _fonts = fonts; _chunkRenderer = chunkRenderer; } @@ -105,7 +94,7 @@ internal sealed class MessageList ImGui.TextUnformatted( string.IsNullOrEmpty(sender) ? timestamp : $"{timestamp} {sender}: " ); - ImGui.SameLine(); + ImGui.SameLine(0f, 0f); _chunkRenderer.DrawChunks(message.Content, wrap: true, handler: _handler, lineWidth: 0f); } diff --git a/HellionChat/Ui/Debugger.cs b/HellionChat/Ui/Debugger.cs index 6930d1a..f2c3bc7 100644 --- a/HellionChat/Ui/Debugger.cs +++ b/HellionChat/Ui/Debugger.cs @@ -10,13 +10,14 @@ using Lumina.Text.ReadOnly; namespace HellionChat.Ui; // Dev tool. Reduced to the parts that survive without the legacy chat -// window: current-tab channel state and the vanilla chat channel label. +// window: PayloadHandler counters, current-tab channel state, and the +// vanilla chat channel label. internal sealed class DebuggerWindow : Window, IDisposable { private readonly Plugin Plugin; private readonly PayloadHandler _payloadHandler; - public DebuggerWindow(Plugin plugin, PayloadHandler payloadHandler) + internal DebuggerWindow(Plugin plugin, PayloadHandler payloadHandler) : base("Debugger###chat2-debugger") { Plugin = plugin; diff --git a/HellionChat/Ui/InputPreview.cs b/HellionChat/Ui/InputPreview.cs index 0dd3e72..12fa020 100644 --- a/HellionChat/Ui/InputPreview.cs +++ b/HellionChat/Ui/InputPreview.cs @@ -1,6 +1,5 @@ using System.Numerics; using System.Text; -using System.Text.RegularExpressions; using Dalamud.Bindings.ImGui; using Dalamud.Game.Text; using Dalamud.Game.Text.SeStringHandling; @@ -13,7 +12,7 @@ using Microsoft.Extensions.Logging; namespace HellionChat.Ui; -internal sealed partial class InputPreview : Window +internal sealed class InputPreview : Window { private readonly Components.ChunkRenderer _chunkRenderer; private readonly Lender _handlerLender; @@ -28,9 +27,7 @@ internal sealed partial class InputPreview : Window private int _lastLength; private Message? _previewMessage; - internal int SelectedCursorPos = -1; - - public InputPreview( + internal InputPreview( Components.ChunkRenderer chunkRenderer, Lender handlerLender, Windows.MainWindow mainWindow, @@ -57,7 +54,7 @@ internal sealed partial class InputPreview : Window DisableWindowSounds = true; IsOpen = true; - // Logger injected for future diagnostic hooks (no call-sites yet in R1). + // TODO Polish-Sweep: remove discard once logging call-sites exist _ = _logger; } @@ -187,7 +184,4 @@ internal sealed partial class InputPreview : Window handler.Draw(); } } - - [GeneratedRegex(@"(\s)")] - private static partial Regex WhitespaceRegex(); } diff --git a/HellionChat/Util/ImGuiUtil.cs b/HellionChat/Util/ImGuiUtil.cs index 278d2cb..6714aef 100755 --- a/HellionChat/Util/ImGuiUtil.cs +++ b/HellionChat/Util/ImGuiUtil.cs @@ -616,8 +616,6 @@ internal static class ImGuiUtil } } - // Payload interaction state shared between PostPayload and WrapText. - // Tracks the last hovered payload so hover-leave events can fire correctly. private static readonly ImGuiMouseButton[] Buttons = [ ImGuiMouseButton.Left, @@ -625,6 +623,8 @@ internal static class ImGuiUtil ImGuiMouseButton.Right, ]; + // Payload interaction state shared between PostPayload and WrapText. + // Tracks the last hovered payload so hover-leave events can fire correctly. private static Payload? Hovered; private static Payload? LastLink; private static readonly List<(Vector2, Vector2)> PayloadBounds = [];