refactor(honorific): per-method threading banners + warn on unsubscribe-fail
F4.1: replace the block threading comment with per-method banners that read like documentation at the call site. F4.2: TryUnsubscribe now logs Warning instead of Debug — a silent unsubscribe failure leaks a live subscription across plugin reloads. F4.3: CurrentTitle gets a one-line banner matching the same convention.
This commit is contained in:
@@ -27,6 +27,7 @@ internal sealed class HonorificService : IDisposable
|
|||||||
private readonly IFramework _framework;
|
private readonly IFramework _framework;
|
||||||
private bool _versionWarningLogged;
|
private bool _versionWarningLogged;
|
||||||
|
|
||||||
|
// Thread: framework only — IPC delivery + ImGui render both run there.
|
||||||
public HonorificTitleData? CurrentTitle { get; private set; }
|
public HonorificTitleData? CurrentTitle { get; private set; }
|
||||||
public bool IsAvailable { get; private set; }
|
public bool IsAvailable { get; private set; }
|
||||||
public (uint Major, uint Minor)? DetectedApiVersion { get; private set; }
|
public (uint Major, uint Minor)? DetectedApiVersion { get; private set; }
|
||||||
@@ -71,6 +72,7 @@ internal sealed class HonorificService : IDisposable
|
|||||||
TryUnsubscribe(() => _disposing.Unsubscribe(OnDisposing));
|
TryUnsubscribe(() => _disposing.Unsubscribe(OnDisposing));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Thread: framework (scheduled from ctor and OnReady).
|
||||||
private void TryInitialPull()
|
private void TryInitialPull()
|
||||||
{
|
{
|
||||||
try
|
try
|
||||||
@@ -108,6 +110,7 @@ internal sealed class HonorificService : IDisposable
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Thread: framework (Dalamud IPC delivery contract).
|
||||||
private void OnTitleChanged(string json)
|
private void OnTitleChanged(string json)
|
||||||
{
|
{
|
||||||
// Skip updates on version mismatch; subscription stays live for reload.
|
// Skip updates on version mismatch; subscription stays live for reload.
|
||||||
@@ -116,12 +119,13 @@ internal sealed class HonorificService : IDisposable
|
|||||||
CurrentTitle = ParseTitleJson(json);
|
CurrentTitle = ParseTitleJson(json);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Thread: any (Honorific dispatches NotifyReady from its own thread).
|
||||||
private void OnReady()
|
private void OnReady()
|
||||||
{
|
{
|
||||||
// Schedule on framework thread — NotifyReady can dispatch from any thread.
|
|
||||||
_framework.RunOnFrameworkThread(TryInitialPull);
|
_framework.RunOnFrameworkThread(TryInitialPull);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Thread: framework (IPC delivery contract); idempotent — Disposing fires once.
|
||||||
private void OnDisposing()
|
private void OnDisposing()
|
||||||
{
|
{
|
||||||
// Honorific unloading — clear cached state so the header hides next frame.
|
// Honorific unloading — clear cached state so the header hides next frame.
|
||||||
@@ -133,6 +137,8 @@ internal sealed class HonorificService : IDisposable
|
|||||||
DetectedApiVersion = null;
|
DetectedApiVersion = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Thread: framework (called from Dispose, which runs on the framework
|
||||||
|
// cleanup block in Plugin.DisposeAsync).
|
||||||
private void TryUnsubscribe(Action unsubscribe)
|
private void TryUnsubscribe(Action unsubscribe)
|
||||||
{
|
{
|
||||||
try
|
try
|
||||||
@@ -141,18 +147,15 @@ internal sealed class HonorificService : IDisposable
|
|||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
_log.Debug(ex, "Honorific unsubscribe failed (likely already gone).");
|
// Warning not Debug — a silent unsubscribe failure leaks a live
|
||||||
|
// subscription across plugin reloads.
|
||||||
|
_log.Warning(
|
||||||
|
ex,
|
||||||
|
"Honorific unsubscribe failed (likely API break or gate already gone)."
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Threading: IPC events and ImGui both run on the framework thread, so
|
|
||||||
// OnTitleChanged and the render path never race — no volatile/Interlocked
|
|
||||||
// needed as long as Dalamud's framework-thread delivery contract holds.
|
|
||||||
//
|
|
||||||
// Constructor and OnReady are exceptions: they run outside that contract
|
|
||||||
// (plugin-loader thread and Honorific's NotifyReady respectively), so both
|
|
||||||
// use RunOnFrameworkThread to safely reach ObjectTable.LocalPlayer.
|
|
||||||
|
|
||||||
// --- Pure-logic helpers; tested via HellionChat.Tests/Integrations. ---
|
// --- Pure-logic helpers; tested via HellionChat.Tests/Integrations. ---
|
||||||
|
|
||||||
internal static HonorificTitleData? ParseTitleJson(string json)
|
internal static HonorificTitleData? ParseTitleJson(string json)
|
||||||
|
|||||||
Reference in New Issue
Block a user