chore(polish): cycle-end sweep — drop dead fields, dep-cycle, comments
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<CommandHelpWindow> 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<T> 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.
This commit is contained in:
@@ -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);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -230,7 +230,7 @@ internal sealed class PayloadHandler
|
||||
.Where(chunk => chunk is TextChunk)
|
||||
.Cast<TextChunk>()
|
||||
.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 ?? "<null>"
|
||||
);
|
||||
_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)
|
||||
{
|
||||
|
||||
@@ -130,8 +130,6 @@ internal static class PluginHostFactory
|
||||
sp.GetRequiredService<ILogger<Ui.Components.Sidebar>>()
|
||||
));
|
||||
services.AddSingleton(sp => new Ui.Components.MessageList(
|
||||
sp.GetRequiredService<ThemeRegistry>(),
|
||||
sp.GetRequiredService<Ui.StyleEngine.TokenResolver>(),
|
||||
sp.GetRequiredService<FontManager>(),
|
||||
sp.GetRequiredService<Ui.Components.ChunkRenderer>()
|
||||
));
|
||||
@@ -143,7 +141,7 @@ internal static class PluginHostFactory
|
||||
sp.GetRequiredService<Ui.StyleEngine.TokenResolver>(),
|
||||
sp.GetRequiredService<ILogger<Ui.Components.InputBar>>(),
|
||||
() => sp.GetRequiredService<Plugin>().SettingsWindow.Toggle(),
|
||||
new Lazy<Ui.CommandHelpWindow>(() => sp.GetRequiredService<Ui.CommandHelpWindow>())
|
||||
sp.GetRequiredService<Ui.CommandHelpWindow>()
|
||||
));
|
||||
services.AddSingleton(sp => new Ui.Components.Settings.TabSidebar(
|
||||
sp.GetRequiredService<FontManager>()
|
||||
@@ -231,32 +229,15 @@ internal static class PluginHostFactory
|
||||
// Factory-lambdas for ChunkRenderer, PayloadHandler, and Lender<PayloadHandler>
|
||||
// because all three are internal-sealed (ActivatorUtilities can't reflect into
|
||||
// internal ctors) and Lender<T> has an internal ctor by design.
|
||||
// PayloadHandler registered twice: once as singleton for G/H, once via Lender<T> for per-frame isolation (I/J/K).
|
||||
services.AddSingleton(sp => new Ui.Components.ChunkRenderer(
|
||||
sp.GetRequiredService<ThemeRegistry>(),
|
||||
sp.GetRequiredService<FontManager>(),
|
||||
sp.GetRequiredService<ILogger<Ui.Components.ChunkRenderer>>(),
|
||||
sp.GetRequiredService<GameFunctions.GameFunctions>()
|
||||
));
|
||||
services.AddSingleton(sp => new PayloadHandler(
|
||||
sp.GetRequiredService<ThemeRegistry>(),
|
||||
sp.GetRequiredService<IpcManager>(),
|
||||
sp.GetRequiredService<GameFunctions.GameFunctions>(),
|
||||
sp.GetRequiredService<Ui.Components.InputBar>(),
|
||||
sp.GetRequiredService<Ui.Windows.MainWindow>(),
|
||||
sp.GetRequiredService<Ui.Components.ChunkRenderer>(),
|
||||
sp.GetRequiredService<ILogger<PayloadHandler>>()
|
||||
));
|
||||
services.AddSingleton(sp => new Lender<PayloadHandler>(() =>
|
||||
new PayloadHandler(
|
||||
sp.GetRequiredService<ThemeRegistry>(),
|
||||
sp.GetRequiredService<IpcManager>(),
|
||||
sp.GetRequiredService<GameFunctions.GameFunctions>(),
|
||||
sp.GetRequiredService<Ui.Components.InputBar>(),
|
||||
sp.GetRequiredService<Ui.Windows.MainWindow>(),
|
||||
sp.GetRequiredService<Ui.Components.ChunkRenderer>(),
|
||||
sp.GetRequiredService<ILogger<PayloadHandler>>()
|
||||
)
|
||||
));
|
||||
services.AddSingleton(sp => MakePayloadHandler(sp));
|
||||
services.AddSingleton(sp => new Lender<PayloadHandler>(() => 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<Ui.Components.ChunkRenderer>(),
|
||||
sp.GetRequiredService<Ui.Windows.MainWindow>(),
|
||||
sp.GetRequiredService<Ui.Components.InputBar>(),
|
||||
sp.GetRequiredService<ILogger<CommandHelpWindow>>()
|
||||
));
|
||||
services.AddSingleton(sp => new SeStringDebugger(sp.GetRequiredService<Plugin>()));
|
||||
@@ -328,11 +308,22 @@ internal static class PluginHostFactory
|
||||
sp.GetRequiredService<Integrations.FailedTellNotifier>()
|
||||
)
|
||||
);
|
||||
services.AddHostedService(sp => new Infrastructure.Hosting.PayloadHandlerInitHostedService(
|
||||
services.AddHostedService(sp => new PayloadHandlerInitHostedService(
|
||||
sp.GetRequiredService<PayloadHandler>(),
|
||||
sp.GetRequiredService<Ui.Components.MessageList>()
|
||||
));
|
||||
}
|
||||
|
||||
private static PayloadHandler MakePayloadHandler(IServiceProvider sp) =>
|
||||
new(
|
||||
sp.GetRequiredService<ThemeRegistry>(),
|
||||
sp.GetRequiredService<IpcManager>(),
|
||||
sp.GetRequiredService<GameFunctions.GameFunctions>(),
|
||||
sp.GetRequiredService<Ui.Components.InputBar>(),
|
||||
sp.GetRequiredService<Ui.Windows.MainWindow>(),
|
||||
sp.GetRequiredService<Ui.Components.ChunkRenderer>(),
|
||||
sp.GetRequiredService<ILogger<PayloadHandler>>()
|
||||
);
|
||||
}
|
||||
|
||||
internal sealed record PluginHostDependencies(
|
||||
|
||||
@@ -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<CommandHelpWindow> _logger;
|
||||
|
||||
private ReadOnlySeString? _commandDescription;
|
||||
|
||||
public CommandHelpWindow(
|
||||
internal CommandHelpWindow(
|
||||
ChunkRenderer chunkRenderer,
|
||||
Windows.MainWindow mainWindow,
|
||||
Components.InputBar inputBar,
|
||||
ILogger<CommandHelpWindow> 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)
|
||||
|
||||
@@ -33,14 +33,13 @@ internal sealed class InputBar
|
||||
private readonly TokenResolver _resolver;
|
||||
private readonly ILogger<InputBar> _logger;
|
||||
private readonly Action _onOpenSettings;
|
||||
private readonly Lazy<CommandHelpWindow> _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<InputBar> logger,
|
||||
Action onOpenSettings,
|
||||
Lazy<CommandHelpWindow> 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;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<PayloadHandler> _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<PayloadHandler> 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();
|
||||
}
|
||||
|
||||
@@ -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 = [];
|
||||
|
||||
Reference in New Issue
Block a user