fix(tell): restore outgoing tell routing from the input bar
Input-bar tells went out as a bare "/t" without the target, so the game
rejected them with "you must add the World name". Rebuild the full
"/tell name@world" from the 1.5.6 target chain in a pure BuildOutgoing:
- leg2/leg3 gated on current == Tell so a stale tell target on a Say tab
can't send a say line silently as /tell (CORR-1)
- world-resolve gate: an unresolvable world falls back to the channel
prefix, never "/tell name@ text" (COMP-1)
- ResetTempChannel after the send, tell-only
Also clear the runtime tell state on PromoteToPermanent so a promoted tab
can't route a typed line to the old partner, and surface the tell partner
("-> name@world") in the channel pill so a misfire stays visible. Adds
tell-routing and pill-transparency self tests.
This commit is contained in:
@@ -496,9 +496,12 @@ internal sealed class AutoTellTabsService : IDisposable
|
||||
return;
|
||||
}
|
||||
|
||||
tab.IsTempTab = false;
|
||||
tab.IsPinned = false;
|
||||
tab.TellTarget = TellTarget.Empty();
|
||||
// Drops the temp/pin flags, the persisted tell target AND the runtime
|
||||
// channel's tell state. The runtime-channel clear is the CORR-1 guard —
|
||||
// see StripTellBindingOnPromote; clearing Tab.TellTarget alone would leave
|
||||
// CurrentChannel.Channel == Tell + a stale target and route a typed line
|
||||
// silently as /tell to the old partner.
|
||||
TabLifecycleHelpers.StripTellBindingOnPromote(tab);
|
||||
_logger.LogDebug($"[Pin] Promoted tab '{tab.Name}' to permanent (tell-binding dropped)");
|
||||
_plugin.SaveConfig();
|
||||
}
|
||||
|
||||
@@ -387,6 +387,8 @@ public sealed class Plugin : IAsyncDalamudPlugin
|
||||
new SelfTests.MainWindowFlagsStep(this),
|
||||
new SelfTests.SenderNameReformatStep(this),
|
||||
new SelfTests.DisclosureArmStep(this),
|
||||
new SelfTests.TellRoutingBuildStep(this),
|
||||
new SelfTests.TellPillTransparencyStep(this),
|
||||
]);
|
||||
|
||||
// Re-surface the wizard for existing users when a major UX
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
using System;
|
||||
using Dalamud.Bindings.ImGui;
|
||||
using Dalamud.Plugin.SelfTest;
|
||||
using HellionChat.Code;
|
||||
using HellionChat.GameFunctions.Types;
|
||||
using HellionChat.Ui.Components;
|
||||
|
||||
namespace HellionChat.SelfTests;
|
||||
|
||||
// v1.8.4: proves the channel pill names the tell partner in the stale-/reply-tell
|
||||
// state on a NORMAL tab. A game-side tell or reply writes {Channel=Tell, TellTarget}
|
||||
// onto the active tab's CurrentChannel even when Tab.TellTarget is empty, so the
|
||||
// isTell pill branch is false. Before the transparency fix the pill showed only
|
||||
// "Tell (Outgoing)" and hid WHO the next typed line would reach — while BuildOutgoing's
|
||||
// leg2/leg3 would still /tell that partner. The pill must mirror the exact send target
|
||||
// (and only when the world resolves, matching the COMP-1 gate) so the user can see and
|
||||
// avoid a misfire. Restores 1.5.6 transparency. Pure label resolution, no send.
|
||||
internal sealed class TellPillTransparencyStep : ISelfTestStep
|
||||
{
|
||||
private readonly Plugin plugin;
|
||||
|
||||
public TellPillTransparencyStep(Plugin plugin)
|
||||
{
|
||||
this.plugin = plugin;
|
||||
}
|
||||
|
||||
public string Name => "Hellion Chat - tell pill transparency";
|
||||
|
||||
public SelfTestStepResult RunStep()
|
||||
{
|
||||
// Same deterministic resolvable-world pick as the routing SelfTest.
|
||||
uint validWorldId = 0;
|
||||
var worldName = string.Empty;
|
||||
foreach (var world in Sheets.WorldSheet)
|
||||
{
|
||||
if (world.IsPublic && !string.IsNullOrEmpty(world.Name.ToString()))
|
||||
{
|
||||
validWorldId = world.RowId;
|
||||
worldName = world.Name.ToString();
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (validWorldId == 0)
|
||||
{
|
||||
ImGui.Text(
|
||||
"No resolvable public world in the sheet — cannot build the stale-tell case"
|
||||
);
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
// Stale-/reply-tell shape on a normal tab: current==Tell, Tab.TellTarget
|
||||
// empty (so isTell is false), CurrentChannel.TellTarget a resolvable partner.
|
||||
var tab = new Tab();
|
||||
tab.CurrentChannel.Channel = InputChannel.Tell;
|
||||
tab.CurrentChannel.TellTarget = new TellTarget(
|
||||
"Partner",
|
||||
validWorldId,
|
||||
0,
|
||||
TellReason.Direct
|
||||
);
|
||||
|
||||
// isTell is false here (no IsTempTab + Tab.TellTarget) — exactly the case the
|
||||
// fix targets, where the old pill collapsed to "Tell (Outgoing)".
|
||||
var label = InputBar.TestResolvePillLabelForSelfTest(tab, false);
|
||||
|
||||
if (!label.Contains("Partner", StringComparison.Ordinal))
|
||||
{
|
||||
ImGui.Text($"Pill hid the tell partner in the stale-tell state: '{label}'");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
if (!label.Contains(worldName, StringComparison.Ordinal))
|
||||
{
|
||||
ImGui.Text(
|
||||
$"Pill omitted the partner world: '{label}' (expected to contain '{worldName}')"
|
||||
);
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
return SelfTestStepResult.Pass;
|
||||
}
|
||||
|
||||
public void CleanUp() { }
|
||||
}
|
||||
@@ -0,0 +1,140 @@
|
||||
using System;
|
||||
using Dalamud.Bindings.ImGui;
|
||||
using Dalamud.Plugin.SelfTest;
|
||||
using HellionChat.Code;
|
||||
using HellionChat.GameFunctions.Types;
|
||||
using HellionChat.Util;
|
||||
|
||||
namespace HellionChat.SelfTests;
|
||||
|
||||
// v1.8.4: proves the restored tell routing in InputBar.BuildOutgoing turns a
|
||||
// tell tab's TellTarget into a full "/tell name@world" instead of the bare "/t"
|
||||
// the channel prefix would produce. Drives the pure routing via the test hook,
|
||||
// so it never reaches ChatBox.SendMessageUnsafe (no real chat line) — the actual
|
||||
// outgoing send stays in-game smoke only. Three cases:
|
||||
// - Positive: a Tell tab with a TellTarget whose world resolves in the Lumina
|
||||
// sheet must report wasTell and build the "/tell name@world " prefix.
|
||||
// - Negative (COMP-1): the same shape but a world id that does NOT resolve must
|
||||
// report wasTell == false and must NOT build a /tell, so an unresolvable world
|
||||
// falls back to the channel-prefix path instead of emitting "/tell Name@ text"
|
||||
// (which the game rejects with "you must add the World name").
|
||||
// - Promote guard (CORR-1): a tell tab run through the real promote mutation
|
||||
// (StripTellBindingOnPromote) must NOT route a typed line as /tell to the old
|
||||
// partner anymore — the regression guard for the promoted-tab privacy leak.
|
||||
internal sealed class TellRoutingBuildStep : ISelfTestStep
|
||||
{
|
||||
private readonly Plugin plugin;
|
||||
|
||||
public TellRoutingBuildStep(Plugin plugin)
|
||||
{
|
||||
this.plugin = plugin;
|
||||
}
|
||||
|
||||
public string Name => "Hellion Chat - tell routing build";
|
||||
|
||||
public SelfTestStepResult RunStep()
|
||||
{
|
||||
var input = this.plugin.InputBar;
|
||||
if (input is null)
|
||||
{
|
||||
ImGui.Text("Plugin.InputBar is null");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
// Pull a resolvable world straight from the sheet instead of hard-coding an
|
||||
// id — world RowIds shift between patches, so a literal could silently rot.
|
||||
uint validWorldId = 0;
|
||||
foreach (var world in Sheets.WorldSheet)
|
||||
{
|
||||
if (world.IsPublic && !string.IsNullOrEmpty(world.Name.ToString()))
|
||||
{
|
||||
validWorldId = world.RowId;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (validWorldId == 0)
|
||||
{
|
||||
ImGui.Text("No resolvable public world in the sheet — cannot build the positive case");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
// Positive: a Tell tab with a resolvable target builds the full /tell prefix.
|
||||
var tellTab = new Tab();
|
||||
tellTab.CurrentChannel.Channel = InputChannel.Tell;
|
||||
tellTab.TellTarget = new TellTarget("Testchar", validWorldId, 0, TellReason.Direct);
|
||||
|
||||
var (toSend, wasTell) = input.TestBuildOutgoingForSelfTest(tellTab, "ping");
|
||||
if (!wasTell)
|
||||
{
|
||||
ImGui.Text(
|
||||
"Positive: BuildOutgoing reported wasTell == false for a resolvable tell tab"
|
||||
);
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
var expectedPrefix = $"/tell Testchar@{tellTab.TellTarget.ToWorldString()} ";
|
||||
if (!toSend.StartsWith(expectedPrefix, StringComparison.Ordinal))
|
||||
{
|
||||
ImGui.Text($"Positive: expected prefix '{expectedPrefix}', got '{toSend}'");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
// Negative (COMP-1): a world id that does not resolve must NOT become a /tell.
|
||||
var missTab = new Tab();
|
||||
missTab.CurrentChannel.Channel = InputChannel.Tell;
|
||||
missTab.TellTarget = new TellTarget("Testchar", uint.MaxValue, 0, TellReason.Direct);
|
||||
|
||||
var (missSend, missWasTell) = input.TestBuildOutgoingForSelfTest(missTab, "ping");
|
||||
if (missWasTell)
|
||||
{
|
||||
ImGui.Text("Negative COMP-1: wasTell == true for a world id that does not resolve");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
if (missSend.StartsWith("/tell ", StringComparison.Ordinal))
|
||||
{
|
||||
ImGui.Text($"Negative COMP-1: built a /tell for an unresolvable world: '{missSend}'");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
// Promote guard (CORR-1): build the pre-promote leak shape — a pinned tell
|
||||
// tab whose CurrentChannel still carries Channel=Tell + a resolvable target
|
||||
// — run the REAL promote mutation, then BuildOutgoing must not produce a
|
||||
// /tell to the old partner. If StripTellBindingOnPromote ever stops clearing
|
||||
// the runtime channel, this turns red.
|
||||
var promoteTab = new Tab();
|
||||
promoteTab.IsTempTab = true;
|
||||
promoteTab.IsPinned = true;
|
||||
promoteTab.Channel = InputChannel.Tell;
|
||||
promoteTab.TellTarget = new TellTarget("Oldpartner", validWorldId, 0, TellReason.Direct);
|
||||
promoteTab.CurrentChannel.Channel = InputChannel.Tell;
|
||||
promoteTab.CurrentChannel.TellTarget = promoteTab.TellTarget.Clone();
|
||||
|
||||
TabLifecycleHelpers.StripTellBindingOnPromote(promoteTab);
|
||||
|
||||
var (promotedSend, promotedWasTell) = input.TestBuildOutgoingForSelfTest(
|
||||
promoteTab,
|
||||
"ping"
|
||||
);
|
||||
if (promotedWasTell)
|
||||
{
|
||||
ImGui.Text(
|
||||
"Promote guard (CORR-1): a promoted tab still routes as /tell to the old partner"
|
||||
);
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
if (promotedSend.StartsWith("/tell ", StringComparison.Ordinal))
|
||||
{
|
||||
ImGui.Text(
|
||||
$"Promote guard (CORR-1): built a /tell to the old partner: '{promotedSend}'"
|
||||
);
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
return SelfTestStepResult.Pass;
|
||||
}
|
||||
|
||||
public void CleanUp() { }
|
||||
}
|
||||
@@ -8,6 +8,7 @@ using Dalamud.Interface.Utility.Raii;
|
||||
using HellionChat._Helpers;
|
||||
using HellionChat.Code;
|
||||
using HellionChat.GameFunctions;
|
||||
using HellionChat.GameFunctions.Types;
|
||||
using HellionChat.Resources;
|
||||
using HellionChat.Themes;
|
||||
using HellionChat.Ui;
|
||||
@@ -201,6 +202,34 @@ internal sealed class InputBar
|
||||
// saved default and is null for most non-FC tabs, which produced
|
||||
// the "—" placeholder users saw.
|
||||
var current = tab?.CurrentChannel?.Channel ?? InputChannel.Invalid;
|
||||
|
||||
// Privacy transparency: a game-side tell or reply writes {Channel=Tell,
|
||||
// TellTarget} onto the active tab's CurrentChannel even on a NORMAL tab
|
||||
// (Tab.TellTarget stays empty, so the isTell branch above is false). In
|
||||
// that state BuildOutgoing's leg2/leg3 would route the next typed line as
|
||||
// /tell to that partner — but the bare "Tell" label hid WHO. Mirror the
|
||||
// exact leg2/leg3 source (current==Tell, TempTellTarget ?? TellTarget) AND
|
||||
// the COMP-1 world-resolve gate, so the pill names the partner ONLY when a
|
||||
// /tell would actually be built; an unresolvable world sends no /tell and
|
||||
// falls through to the plain label below. Read-only — no routing effect.
|
||||
// 1.5.6 showed the partner name here; this restores that transparency.
|
||||
if (current == InputChannel.Tell)
|
||||
{
|
||||
// Mirror BuildOutgoing's exact target chain for the tell channel (leg1
|
||||
// Tab.TellTarget first, then leg2/leg3 CurrentChannel) so the pill names
|
||||
// precisely who the next line would reach — no drift between shown and sent.
|
||||
var ccTarget =
|
||||
tab is not null && tab.TellTarget.IsSet()
|
||||
? tab.TellTarget
|
||||
: tab?.CurrentChannel?.TempTellTarget ?? tab?.CurrentChannel?.TellTarget;
|
||||
if (ccTarget is not null && ccTarget.IsSet())
|
||||
{
|
||||
var world = ccTarget.ToWorldString();
|
||||
if (!string.IsNullOrEmpty(world))
|
||||
return $"→ {ccTarget.Name}@{world}";
|
||||
}
|
||||
}
|
||||
|
||||
if (current != InputChannel.Invalid)
|
||||
return current.ToChatType().Name();
|
||||
|
||||
@@ -382,20 +411,11 @@ internal sealed class InputBar
|
||||
}
|
||||
_disclosureArmedBuffer = null;
|
||||
|
||||
// Slash commands route through verbatim — the game's chat parser
|
||||
// handles /tell, /fc, /hellion etc. on its own. Other text gets
|
||||
// the active channel's prefix so the line lands on the channel
|
||||
// the user is reading instead of the game-side default.
|
||||
string toSend;
|
||||
if (text.StartsWith('/'))
|
||||
{
|
||||
toSend = text;
|
||||
}
|
||||
else
|
||||
{
|
||||
var current = activeTab?.CurrentChannel?.Channel ?? InputChannel.Invalid;
|
||||
toSend = current == InputChannel.Invalid ? text : $"{current.Prefix()} {text}";
|
||||
}
|
||||
// Route the trimmed buffer into the exact send string. BuildOutgoing is
|
||||
// pure (no send, no field write) so the SelfTest can exercise the tell
|
||||
// routing without firing a real chat line; the wasTell flag drives the
|
||||
// post-send ResetTempChannel below.
|
||||
var (toSend, wasTell) = BuildOutgoing(activeTab, text);
|
||||
|
||||
try
|
||||
{
|
||||
@@ -415,6 +435,13 @@ internal sealed class InputBar
|
||||
}
|
||||
ChatBox.SendMessageUnsafe(bytes);
|
||||
_pendingMessage = string.Empty;
|
||||
|
||||
// 1.5.6 parity (1d3b429:ChatLogWindow.cs:1558): clear the temp channel
|
||||
// after a tell so a one-off /tell doesn't stick to the tab. Tell-only,
|
||||
// so Say/Party/FC stay untouched. A no-op in today's input-bar path
|
||||
// (TempTellTarget is inert), kept for an eventual temp-channel revival.
|
||||
if (wasTell)
|
||||
activeTab?.CurrentChannel?.ResetTempChannel();
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
@@ -422,6 +449,52 @@ internal sealed class InputBar
|
||||
}
|
||||
}
|
||||
|
||||
// Pure routing: turns the trimmed buffer into the bytes-source string and
|
||||
// reports whether it became a tell. No send, no field mutation — the
|
||||
// ResetTempChannel side-effect lives in TrySend, gated by wasTell, so this
|
||||
// stays exercisable from the SelfTest. Slash input is verbatim (the game
|
||||
// parser owns /tell, /fc, …); everything else gets the channel prefix,
|
||||
// except a tell tab, which needs the full "/tell name@world" because
|
||||
// InputChannel.Tell.Prefix() is only "/t" and would drop the target.
|
||||
private (string toSend, bool wasTell) BuildOutgoing(Tab? activeTab, string text)
|
||||
{
|
||||
if (text.StartsWith('/'))
|
||||
return (text, false);
|
||||
|
||||
var current = activeTab?.CurrentChannel?.Channel ?? InputChannel.Invalid;
|
||||
|
||||
// 1.5.6 tell-target chain (1d3b429:ChatLogWindow.cs:1543-1546).
|
||||
TellTarget? target = null;
|
||||
if (activeTab is not null && activeTab.TellTarget.IsSet())
|
||||
{
|
||||
// leg1 — unconditional: a freshly spawned temp tab carries its target
|
||||
// only here, with CurrentChannel still Invalid until a sidebar/top-bar
|
||||
// click runs EnsureCurrentChannel. A current==Tell gate would miss it.
|
||||
target = activeTab.TellTarget;
|
||||
}
|
||||
else if (current == InputChannel.Tell)
|
||||
{
|
||||
// leg2/leg3 — gated on Tell (CORR-1): CurrentChannel.TellTarget is NOT
|
||||
// channel-bound. After a game-side tell, switching the pill to Say leaves
|
||||
// the tell target standing (SetChannel only sets Channel), so without this
|
||||
// gate a say line would silently go out as /tell — a privacy misfire.
|
||||
target =
|
||||
activeTab?.CurrentChannel?.TempTellTarget ?? activeTab?.CurrentChannel?.TellTarget;
|
||||
}
|
||||
|
||||
// One world lookup, reused by the gate and the string build (ToTargetString
|
||||
// would resolve the sheet twice). The !IsNullOrEmpty(world) check is the
|
||||
// COMP-1 guard: IsSet() only proves World > 0, not that the id resolves in
|
||||
// the Lumina sheet. A miss yields an empty world, and "/tell Name@ text" is
|
||||
// exactly what the game rejects with "you must add the World name". On a miss
|
||||
// we fall through to the channel-prefix path.
|
||||
var world = target?.ToWorldString();
|
||||
if (target != null && target.IsSet() && !string.IsNullOrEmpty(world))
|
||||
return ($"/tell {target.Name}@{world} {text}", true);
|
||||
|
||||
return (current == InputChannel.Invalid ? text : $"{current.Prefix()} {text}", false);
|
||||
}
|
||||
|
||||
private void DrawQuickButtons()
|
||||
{
|
||||
using (_fonts.FontAwesome.Push())
|
||||
@@ -480,6 +553,20 @@ internal sealed class InputBar
|
||||
// so a SelfTest leaves no residual arm state.
|
||||
internal void TestResetDisclosureForSelfTest() => _disclosureArmedBuffer = null;
|
||||
|
||||
// Test-only hook; do not call from production code. Exposes the pure routing
|
||||
// so the tell SelfTest can assert the string + wasTell flag without ever
|
||||
// reaching ChatBox.SendMessageUnsafe (no real chat line).
|
||||
internal (string toSend, bool wasTell) TestBuildOutgoingForSelfTest(
|
||||
Tab? activeTab,
|
||||
string text
|
||||
) => BuildOutgoing(activeTab, text);
|
||||
|
||||
// Test-only hook; do not call from production code. Exposes the pure pill-label
|
||||
// resolution so the tell-transparency SelfTest can assert the partner name is
|
||||
// shown in the stale-/reply-tell state. Static (ResolvePillLabel is static).
|
||||
internal static string TestResolvePillLabelForSelfTest(Tab? tab, bool isTell) =>
|
||||
ResolvePillLabel(tab, isTell);
|
||||
|
||||
private void DrawAutoCompletePopup()
|
||||
{
|
||||
if (_autoCompleteInfo == null)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
using HellionChat.Code;
|
||||
using HellionChat.GameFunctions.Types;
|
||||
|
||||
namespace HellionChat.Util;
|
||||
|
||||
@@ -32,4 +33,27 @@ internal static class TabLifecycleHelpers
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Drops a temp/pinned tell tab's binding when it is promoted to a permanent
|
||||
// tab. Beyond the obvious IsTempTab/IsPinned/Tab.TellTarget reset, this also
|
||||
// clears the RUNTIME channel's tell state — that part is the CORR-1 guard:
|
||||
// a spawned tell tab carries CurrentChannel.Channel == Tell plus a resolvable
|
||||
// CurrentChannel.TellTarget, and neither is touched by clearing Tab.TellTarget
|
||||
// alone. Without this clear the input bar would route a normal typed line on
|
||||
// the promoted tab silently as /tell to the OLD partner (a privacy misfire the
|
||||
// current==Tell routing gate cannot catch, because current here really IS
|
||||
// Tell). Channel -> Invalid so the next sidebar/top-bar click re-derives the
|
||||
// channel from SelectedChannels via EnsureCurrentChannel like any normal tab;
|
||||
// the worst residual is a "/t" with no target, which the game rejects without
|
||||
// sending (same safe class as the COMP-1 fall-through, no silent send).
|
||||
internal static void StripTellBindingOnPromote(Tab tab)
|
||||
{
|
||||
tab.IsTempTab = false;
|
||||
tab.IsPinned = false;
|
||||
tab.TellTarget = TellTarget.Empty();
|
||||
tab.Channel = null;
|
||||
tab.CurrentChannel.SetChannel(InputChannel.Invalid);
|
||||
tab.CurrentChannel.TellTarget = null;
|
||||
tab.CurrentChannel.ResetTempChannel();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user