From daa800c8b16d3946988c457ef537185874fb7fa7 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Fri, 8 May 2026 19:46:11 +0200 Subject: [PATCH] Apply code-quality fixes to Plugin.cs IAsyncDalamudPlugin refactor I-1: rewrite property-shape comment to reflect that all properties (not just Phase-2 ones) moved to { get; private set; } = null!;. I-3: drop plan-jargon (Q1=A / Q3=B / Task 5) from source comments; replace with durable rationale and a version-anchored TODO for the FontManager.BuildFontsAsync follow-up. I-4: remove German-word leak ("pflicht") from English comment in DisposeAsync. M-5: wrap each cleanup line inside Framework.RunOnFrameworkThread with CaptureFailure so a single Dispose throw no longer strands subsequent cleanup. Drops the inline try/swallow on SetChatInteractable. Mirrors Lightless DisposeFrameworkBoundServicesAsync pattern. --- HellionChat/Plugin.cs | 52 ++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index 0d05778..dc14cf3 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -49,9 +49,10 @@ public sealed class Plugin : IAsyncDalamudPlugin public readonly WindowSystem WindowSystem = new(PluginName); - // v1.4.3: Phase-2 services need private setters now that LoadAsync - // owns their construction. Phase-1-only services (Commands, Functions, - // Ipc, ExtraChat, TypingIpc) keep their setters for symmetry. + // v1.4.3: properties moved from { get; } to { get; private set; } = null!; + // because LoadAsync now owns construction of the Phase-2 services. + // Phase-1 services use the same shape for consistency, even though + // they're still allocated in the ctor. public SettingsWindow SettingsWindow { get; private set; } = null!; public ChatLogWindow ChatLogWindow { get; private set; } = null!; public DbViewer DbViewer { get; private set; } = null!; @@ -486,14 +487,13 @@ public sealed class Plugin : IAsyncDalamudPlugin try { - // Group A: Font + Theme parallel (Q1=A). Both are CPU-bound, - // independent, and dominate the load-time profile. Everything - // else stays sequential to keep ordering simple. - // Q3=B transition: BuildFonts() is sync today; Task 5 converts - // FontManager itself to BuildFontsAsync. + // Group A: Font + Theme parallel — both CPU-bound, independent, and + // dominate the load-time profile. Everything else stays sequential to + // keep ordering simple. var fontTask = Task.Run(() => { FontManager = new FontManager(); + // TODO(v1.4.x): replace with FontManager.BuildFontsAsync(cancellationToken) FontManager.BuildFonts(); }, cancellationToken); @@ -620,7 +620,7 @@ public sealed class Plugin : IAsyncDalamudPlugin }); // Auto-Tell-Tabs unsubscribes from MessageProcessed before MessageManager - // goes away. Pure-memory cleanup, no framework-thread pflicht. + // goes away. Pure-memory cleanup, no framework-thread requirement. failure = CaptureFailure(failure, () => AutoTellTabsService?.Dispose()); // v1.4.0 F6.2 — MessageManager has its own async dispose path @@ -636,35 +636,41 @@ public sealed class Plugin : IAsyncDalamudPlugin // framework thread. WindowSystem mutations and IPC subscriber // disposes touch Dalamud state that's only safe from the framework. // Worker-thread DisposeAsync would race the next Draw tick. - failure = await CaptureFailureAsync(failure, async () => + // Per-line CaptureFailure so a single throw can't strand the lines + // behind it; mirrors Lightless DisposeFrameworkBoundServicesAsync. + try { await Framework.RunOnFrameworkThread(() => { // Game-Functions first — other services may still query // chat-interactable state during their Dispose. - try { GameFunctions.GameFunctions.SetChatInteractable(true); } catch { /* swallowed */ } + failure = CaptureFailure(failure, () => GameFunctions.GameFunctions.SetChatInteractable(true)); // IPC subscribers — dispose before windows so any final // event firing from the IPC source can't reach a half-torn // ChatLogWindow. - HonorificService?.Dispose(); - TypingIpc?.Dispose(); - ExtraChat?.Dispose(); - Ipc?.Dispose(); + failure = CaptureFailure(failure, () => HonorificService?.Dispose()); + failure = CaptureFailure(failure, () => TypingIpc?.Dispose()); + failure = CaptureFailure(failure, () => ExtraChat?.Dispose()); + failure = CaptureFailure(failure, () => Ipc?.Dispose()); // Windows — RemoveAllWindows first, then per-window Dispose. // Order matches the pre-v1.4.3 Dispose body byte-for-byte. // CommandHelpWindow and FirstRunWizard don't implement // IDisposable; their resources are reclaimed via WindowSystem. - WindowSystem?.RemoveAllWindows(); - ChatLogWindow?.Dispose(); - DbViewer?.Dispose(); - InputPreview?.Dispose(); - SettingsWindow?.Dispose(); - DebuggerWindow?.Dispose(); - SeStringDebugger?.Dispose(); + failure = CaptureFailure(failure, () => WindowSystem?.RemoveAllWindows()); + failure = CaptureFailure(failure, () => ChatLogWindow?.Dispose()); + failure = CaptureFailure(failure, () => DbViewer?.Dispose()); + failure = CaptureFailure(failure, () => InputPreview?.Dispose()); + failure = CaptureFailure(failure, () => SettingsWindow?.Dispose()); + failure = CaptureFailure(failure, () => DebuggerWindow?.Dispose()); + failure = CaptureFailure(failure, () => SeStringDebugger?.Dispose()); }).ConfigureAwait(false); - }).ConfigureAwait(false); + } + catch (Exception ex) + { + failure ??= ex; + } // Pure-memory cleanups — no Framework / UI / IPC touch, so they // run on whatever thread DisposeAsync resumes on.