fix(tell): couple CurrentTab to the active tab, retire LastTab
Plugin.CurrentTab now delegates to MainWindow.ActiveTab (fallback Tabs[0]) instead of the never-assigned LastTab index, so the game hooks, unread tracking, notification sounds, InputDisabled and Foray/Eureka paths all operate on the tab the user actually has selected. The dead LastTab/WantedTab fields and both WantedTab writes are removed. A reference-based MainWindow.ResetActiveTabIfRemoved repairs the active-tab reference on eviction/logout (immune to the SaveConfig temp-tab strip window). The worker-thread eviction path marshals it onto the framework thread so the strip mutation serializes with Draw; logout is already framework-thread. The Draw-seed gains a lazy re-seed for a wholesale config swap. Adds CurrentTabCouplingStep (headless) and the interactive CurrentTabGuidedStep self-test (step count 30 -> 32).
This commit is contained in:
@@ -260,13 +260,17 @@ internal sealed class AutoTellTabsService : IDisposable
|
||||
// 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 active tab to avoid silent switch when tab is dropped
|
||||
if (victim.Index <= _plugin.LastTab)
|
||||
{
|
||||
_plugin.WantedTab = 0;
|
||||
}
|
||||
// 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.
|
||||
Plugin.Framework.RunOnFrameworkThread(() =>
|
||||
_plugin.MainWindow?.ResetActiveTabIfRemoved(dropped)
|
||||
);
|
||||
}
|
||||
|
||||
private void SpawnTempTab((string Name, uint World) partner, Message currentMessage)
|
||||
@@ -417,11 +421,7 @@ internal sealed class AutoTellTabsService : IDisposable
|
||||
{
|
||||
// Pinned TempTabs must survive char-switch — that's the whole point
|
||||
// of pinning. Only unpinned ones get stripped.
|
||||
var lastIndex = _plugin.LastTab;
|
||||
var lastIndexValid = lastIndex >= 0 && lastIndex < Plugin.Config.Tabs.Count;
|
||||
var currentWasUnpinnedTempTab =
|
||||
lastIndexValid
|
||||
&& TabLifecycleHelpers.IsInUnpinnedPool(Plugin.Config.Tabs[lastIndex]);
|
||||
var active = _plugin.MainWindow?.ActiveTab;
|
||||
|
||||
var poppedTempTabIds = Plugin
|
||||
.Config.Tabs.Where(t => TabLifecycleHelpers.IsInUnpinnedPool(t) && t.PopOut)
|
||||
@@ -432,12 +432,13 @@ internal sealed class AutoTellTabsService : IDisposable
|
||||
|
||||
Plugin.Config.Tabs.RemoveAll(TabLifecycleHelpers.IsInUnpinnedPool);
|
||||
|
||||
// Force switch to tab 0 if active tab was an unpinned temp tab or
|
||||
// index is now out of range. Pinned tabs survive — no switch needed.
|
||||
var stillValid = lastIndex >= 0 && lastIndex < Plugin.Config.Tabs.Count;
|
||||
if (currentWasUnpinnedTempTab || !stillValid)
|
||||
// Re-anchor the UI selection if the active tab was one of the stripped
|
||||
// unpinned temp tabs (reference predicate, not an index). Logout is a
|
||||
// framework-thread event, so this is already serialized with Draw — no
|
||||
// marshalling needed here, unlike the worker-thread eviction path.
|
||||
if (active is { } a && TabLifecycleHelpers.IsInUnpinnedPool(a))
|
||||
{
|
||||
_plugin.WantedTab = 0;
|
||||
_plugin.MainWindow?.ResetActiveTabIfRemoved(a);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+8
-11
@@ -182,17 +182,12 @@ public sealed class Plugin : IAsyncDalamudPlugin
|
||||
|
||||
internal DateTime GameStarted { get; }
|
||||
|
||||
// Tab management lives here rather than in ChatLogWindow for access reasons.
|
||||
internal int LastTab { get; set; }
|
||||
internal int? WantedTab { get; set; }
|
||||
internal Tab CurrentTab
|
||||
{
|
||||
get
|
||||
{
|
||||
var i = LastTab;
|
||||
return i > -1 && i < Config.Tabs.Count ? Config.Tabs[i] : new Tab();
|
||||
}
|
||||
}
|
||||
// Couples "current tab" to the real UI selection. The chat hooks are
|
||||
// installed before MainWindow is Phase-1 resolved, so the null-conditional
|
||||
// fallback to Tabs[0] is load-bearing — it keeps the pre-coupling behavior
|
||||
// in that early window rather than being merely defensive.
|
||||
internal Tab CurrentTab =>
|
||||
MainWindow?.ActiveTab ?? (Config.Tabs.Count > 0 ? Config.Tabs[0] : new Tab());
|
||||
|
||||
public Plugin()
|
||||
{
|
||||
@@ -406,6 +401,8 @@ public sealed class Plugin : IAsyncDalamudPlugin
|
||||
new SelfTests.SidebarSectionHeaderStep(this),
|
||||
new SelfTests.ScrollSnapDecisionStep(this),
|
||||
new SelfTests.TellResetOnActivateStep(),
|
||||
new SelfTests.CurrentTabCouplingStep(this),
|
||||
new SelfTests.CurrentTabGuidedStep(this),
|
||||
]);
|
||||
|
||||
// Re-surface the wizard for existing users when a major UX
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
using Dalamud.Bindings.ImGui;
|
||||
using Dalamud.Plugin.SelfTest;
|
||||
|
||||
namespace HellionChat.SelfTests;
|
||||
|
||||
// F2: CurrentTab is coupled to MainWindow.ActiveTab (no longer the fixed index-0
|
||||
// Tabs lookup). Asserts ReferenceEquals between the two, with false-green
|
||||
// defenses: (1) empty-config exercises the getter's fallback; (2) null ActiveTab
|
||||
// opens the window so the Draw-seed sets it and retries via Waiting (bounded so a
|
||||
// never-drawn window cannot hang a batch); (3) a victim tab at index 0 makes a
|
||||
// regressed index-0 getter return the victim (!= ActiveTab) and fail. Also checks
|
||||
// the ResetActiveTabIfRemoved reference no-op branch.
|
||||
internal sealed class CurrentTabCouplingStep : ISelfTestStep
|
||||
{
|
||||
private readonly Plugin _plugin;
|
||||
private bool _forcedOpen;
|
||||
private int _waitFrames;
|
||||
|
||||
public CurrentTabCouplingStep(Plugin plugin)
|
||||
{
|
||||
_plugin = plugin;
|
||||
}
|
||||
|
||||
public string Name => "Hellion Chat - CurrentTab couples to active tab";
|
||||
|
||||
public SelfTestStepResult RunStep()
|
||||
{
|
||||
// Empty-config edge: actually exercise the getter's empty-fallback (it must
|
||||
// return a fresh Tab, not null/throw) rather than an unconditional pass.
|
||||
if (Plugin.Config.Tabs.Count == 0)
|
||||
{
|
||||
if (_plugin.CurrentTab is null)
|
||||
{
|
||||
ImGui.Text("Empty-config getter returned null instead of a fallback Tab.");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
ImGui.Text("No tabs configured; getter returns the empty-fallback Tab.");
|
||||
return SelfTestStepResult.Pass;
|
||||
}
|
||||
|
||||
// /xlperf usually runs without the window drawn, so ActiveTab can be null
|
||||
// on the first pass. Open the window so the Draw-seed sets it, retry next
|
||||
// frame, and assert unconditionally once it is non-null. Bounded so a
|
||||
// never-drawn window cannot hang a batch run.
|
||||
if (_plugin.MainWindow.ActiveTab is null)
|
||||
{
|
||||
if (!_plugin.MainWindow.IsOpen)
|
||||
{
|
||||
_plugin.MainWindow.Toggle();
|
||||
_forcedOpen = true;
|
||||
}
|
||||
|
||||
if (++_waitFrames > 300)
|
||||
{
|
||||
RestoreWindow();
|
||||
ImGui.Text(
|
||||
"MainWindow never drew a seed within 300 frames; coupling not asserted."
|
||||
);
|
||||
return SelfTestStepResult.Pass;
|
||||
}
|
||||
|
||||
ImGui.Text("Opening window so the draw-seed can set ActiveTab; retrying...");
|
||||
return SelfTestStepResult.Waiting;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
// Insert a victim at index 0: a regressed index-0 getter would return
|
||||
// THIS instead of ActiveTab, so ReferenceEquals would catch it.
|
||||
var victim = new Tab { Name = "selftest-coupling-victim" };
|
||||
Plugin.Config.Tabs.Insert(0, victim);
|
||||
try
|
||||
{
|
||||
if (!ReferenceEquals(_plugin.CurrentTab, _plugin.MainWindow.ActiveTab))
|
||||
{
|
||||
ImGui.Text("CurrentTab is not the same reference as ActiveTab");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
if (ReferenceEquals(_plugin.CurrentTab, victim))
|
||||
{
|
||||
ImGui.Text("CurrentTab returned the index-0 victim (getter still index-based)");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
// Reference no-op: resetting against a tab that is NOT the active
|
||||
// one must leave the active reference untouched.
|
||||
var activeBefore = _plugin.MainWindow.ActiveTab;
|
||||
_plugin.MainWindow.ResetActiveTabIfRemoved(victim);
|
||||
if (!ReferenceEquals(_plugin.MainWindow.ActiveTab, activeBefore))
|
||||
{
|
||||
ImGui.Text("ResetActiveTabIfRemoved changed the active tab on a non-match");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
return SelfTestStepResult.Pass;
|
||||
}
|
||||
finally
|
||||
{
|
||||
Plugin.Config.Tabs.Remove(victim);
|
||||
}
|
||||
}
|
||||
finally
|
||||
{
|
||||
RestoreWindow();
|
||||
}
|
||||
}
|
||||
|
||||
private void RestoreWindow()
|
||||
{
|
||||
if (_forcedOpen && _plugin.MainWindow.IsOpen)
|
||||
_plugin.MainWindow.Toggle();
|
||||
_forcedOpen = false;
|
||||
}
|
||||
|
||||
public void CleanUp()
|
||||
{
|
||||
RestoreWindow();
|
||||
_waitFrames = 0;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,141 @@
|
||||
using Dalamud.Bindings.ImGui;
|
||||
using Dalamud.Plugin.SelfTest;
|
||||
using HellionChat.Code;
|
||||
using HellionChat.GameFunctions.Types;
|
||||
|
||||
namespace HellionChat.SelfTests;
|
||||
|
||||
// F2 (guided): interactive, fires NO synthetic probes. Shows the full measured
|
||||
// state every frame so a result is observable, not a guess, and walks the user
|
||||
// through the real switch-away-and-back flow. It verifies the PRIVACY-relevant
|
||||
// effect, keyed on the tab type:
|
||||
// - a NORMAL tab carrying a game-side tell must lose its RUNTIME target
|
||||
// (CurrentChannel.TellTarget) on switch-away-and-back (the F1 strip), so a
|
||||
// typed line can't /tell the old partner;
|
||||
// - a BOUND auto-tell tab keeps its partner by design (leg1) — its binding is
|
||||
// Tab.TellTarget and is deliberately untouched by the strip.
|
||||
// The channel label is intentionally NOT asserted: a tell tab re-derives back to
|
||||
// Tell after the strip (spec TR-7); only the target matters for privacy.
|
||||
internal sealed class CurrentTabGuidedStep : ISelfTestStep
|
||||
{
|
||||
private readonly Plugin _plugin;
|
||||
|
||||
// 0 = waiting for a tell; 1 = tell seen, waiting to switch AWAY; 2 = switched
|
||||
// away, waiting to come BACK to the tracked tab.
|
||||
private int _phase;
|
||||
private Tab? _tellTab;
|
||||
private bool _wasBound;
|
||||
private string _seenPartner = "";
|
||||
|
||||
public CurrentTabGuidedStep(Plugin plugin)
|
||||
{
|
||||
_plugin = plugin;
|
||||
}
|
||||
|
||||
public string Name => "Hellion Chat - Tell target cleared on tab switch (guided)";
|
||||
|
||||
public SelfTestStepResult RunStep()
|
||||
{
|
||||
var active = _plugin.CurrentTab;
|
||||
var cc = active.CurrentChannel;
|
||||
var bound = active.TellTarget?.IsSet() == true;
|
||||
var runtime = cc.TellTarget?.IsSet() == true;
|
||||
|
||||
// Live diagnostics every frame — a result is never a guess.
|
||||
ImGui.Text($"Active tab : {active.Name}");
|
||||
ImGui.Text($"Channel : {cc.Channel}");
|
||||
ImGui.Text($"Runtime target : {DescribeTarget(cc.TellTarget)}");
|
||||
ImGui.Text($"Tab-bound (leg1): {(bound ? $"yes -> {active.TellTarget!.Name}" : "no")}");
|
||||
if (_tellTab is not null)
|
||||
ImGui.Text(
|
||||
$"Tracking '{_tellTab.Name}' (bound: {_wasBound}, partner: {_seenPartner})"
|
||||
);
|
||||
ImGui.Separator();
|
||||
|
||||
if (ImGui.Button("Skip##guided-tellflow"))
|
||||
{
|
||||
ImGui.Text("Skipped by user — not verified.");
|
||||
return SelfTestStepResult.Pass;
|
||||
}
|
||||
|
||||
// Restart cleanly if the tracked tab is evicted mid-flow.
|
||||
if (_tellTab is not null && !Plugin.Config.Tabs.Contains(_tellTab))
|
||||
{
|
||||
ImGui.Text(">> Tracked tab was removed; restarting.");
|
||||
Reset();
|
||||
}
|
||||
|
||||
if (_phase == 0)
|
||||
{
|
||||
ImGui.Text(">> Step 1: get a tab into Tell — /tell from a normal tab (stay on it),");
|
||||
ImGui.Text(" or open an auto-tell tab. Watch the lines above update.");
|
||||
if (cc.Channel == InputChannel.Tell && (runtime || bound))
|
||||
{
|
||||
_tellTab = active;
|
||||
_wasBound = bound;
|
||||
_seenPartner = bound ? active.TellTarget!.Name : cc.TellTarget!.Name;
|
||||
_phase = 1;
|
||||
}
|
||||
|
||||
return SelfTestStepResult.Waiting;
|
||||
}
|
||||
|
||||
if (_phase == 1)
|
||||
{
|
||||
ImGui.Text(">> Step 2: now click AWAY to a different tab.");
|
||||
if (!ReferenceEquals(active, _tellTab))
|
||||
_phase = 2;
|
||||
|
||||
return SelfTestStepResult.Waiting;
|
||||
}
|
||||
|
||||
// _phase == 2: switched away; wait to come BACK, then check the target.
|
||||
ImGui.Text($">> Step 3: now click BACK onto '{_tellTab!.Name}'.");
|
||||
if (!ReferenceEquals(active, _tellTab))
|
||||
return SelfTestStepResult.Waiting;
|
||||
|
||||
if (_wasBound)
|
||||
{
|
||||
// leg1: the binding lives on Tab.TellTarget and must survive the strip.
|
||||
if (_tellTab.TellTarget?.IsSet() == true)
|
||||
{
|
||||
ImGui.Text(
|
||||
"PASS: bound auto-tell tab kept its partner (leg1 — the conversation stays)."
|
||||
);
|
||||
return SelfTestStepResult.Pass;
|
||||
}
|
||||
|
||||
ImGui.Text(
|
||||
$"FAIL: bound tab LOST partner '{_seenPartner}' — leg1 was wrongly stripped."
|
||||
);
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
// non-bound: the stale RUNTIME target must be gone (the privacy strip).
|
||||
if (_tellTab.CurrentChannel.TellTarget?.IsSet() != true)
|
||||
{
|
||||
ImGui.Text(
|
||||
$"PASS: stale partner '{_seenPartner}' cleared — a typed line won't /tell them."
|
||||
);
|
||||
return SelfTestStepResult.Pass;
|
||||
}
|
||||
|
||||
ImGui.Text(
|
||||
"FAIL: stale runtime partner still bound after switch-away-and-back — privacy leak."
|
||||
);
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
private static string DescribeTarget(TellTarget? t) =>
|
||||
t?.IsSet() == true ? $"{t.Name} (World {t.World})" : "none";
|
||||
|
||||
private void Reset()
|
||||
{
|
||||
_phase = 0;
|
||||
_tellTab = null;
|
||||
_wasBound = false;
|
||||
_seenPartner = "";
|
||||
}
|
||||
|
||||
public void CleanUp() => Reset();
|
||||
}
|
||||
@@ -119,6 +119,22 @@ internal sealed class MainWindow : Window
|
||||
|
||||
public Tab? ActiveTab => _activeTab;
|
||||
|
||||
// Re-anchors the active-tab reference when the tab it points at is removed
|
||||
// (eviction / logout). Reference compare, so it is immune to the SaveConfig
|
||||
// temp-tab strip window where a tab is briefly absent from Config.Tabs; the
|
||||
// re-seeded tab runs through OnTabActivated so a programmatic switch strips
|
||||
// stale tell state the way a click would.
|
||||
internal void ResetActiveTabIfRemoved(Tab removed)
|
||||
{
|
||||
if (!ReferenceEquals(_activeTab, removed))
|
||||
return;
|
||||
|
||||
var next = Plugin.Config.Tabs.Count > 0 ? Plugin.Config.Tabs[0] : null;
|
||||
_activeTab = next;
|
||||
if (next is not null)
|
||||
TabLifecycleHelpers.OnTabActivated(next, removed);
|
||||
}
|
||||
|
||||
// Internal accessors for self-tests so the probes can reach the live
|
||||
// component without exposing them as public surface.
|
||||
internal Components.Sidebar GetSidebarForSelfTest() => _sidebar;
|
||||
@@ -163,6 +179,18 @@ internal sealed class MainWindow : Window
|
||||
// (pre-coupling the detour wrote here); strip it like any activation.
|
||||
TabLifecycleHelpers.OnTabActivated(seeded, null);
|
||||
}
|
||||
else if (_activeTab is { } active && !Plugin.Config.Tabs.Contains(active))
|
||||
{
|
||||
// Active tab is no longer in the list (e.g. a wholesale config import
|
||||
// the service repair paths never see). Re-seed on the Draw thread. The
|
||||
// Contains read shares the pre-existing unsynchronized-Tabs-list
|
||||
// exposure that spec §6 defers (SaveConfig also strips from the worker
|
||||
// thread); this adds one more racing read, not a new hazard class.
|
||||
var reseed = Plugin.Config.Tabs.Count > 0 ? Plugin.Config.Tabs[0] : null;
|
||||
_activeTab = reseed;
|
||||
if (reseed is not null)
|
||||
TabLifecycleHelpers.OnTabActivated(reseed, active);
|
||||
}
|
||||
|
||||
var statusHeight = Components.StatusBar.Height;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user