fix(vtank): slice 7 round E item D-5 — refresh Advanced Options on open

RefreshAdvancedOptions' own doc comment lists every real mutation site
that must call it (category toggle, edit/apply, selection change,
profile load) but ShowAdvancedOptions itself was missing — opening the
popup only called LoadAdvancedOptionDraft. A setting changed while the
popup was closed (/vt opt set, one of the Options tab's own direct
checkboxes, or a profile load) left the cached AdvancedOptionNames/
AdvancedOptionValueColumn on whatever they held from construction or
the popup's last open, until some in-popup interaction happened to
refresh them.

Mutation shown to fail first: reverting the source fix (git checkout,
patch saved and reapplied) reproduced the stale column — toggling
CombatEnabled then opening the popup still showed the pre-toggle value.

MossTank suite 721 -> 722; App markup/plugin filter holds 244/244.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-07 18:16:36 +02:00
parent 64b62fea33
commit 1a0b0e014c
2 changed files with 38 additions and 0 deletions

View file

@ -610,9 +610,19 @@ internal sealed partial class MossTankPanel
return value.ToDisplayString();
}
// D-5 (round E architecture re-check): RefreshAdvancedOptions' own doc
// comment already lists every real mutation site that must call it —
// category toggle, edit/apply, selection change, and profile load —
// but opening the popup itself was missing, so a value changed
// between closes (/vt opt set, one of the Options tab's own direct
// checkboxes, or a profile load that happened while the popup was
// already closed) stayed on whatever _advancedOptionValueColumn held
// from construction or the popup's last open, until some in-popup
// interaction happened to refresh it.
public Action ShowAdvancedOptions => () =>
{
_advancedOptionsVisible = true;
RefreshAdvancedOptions();
LoadAdvancedOptionDraft();
};
public Action HideAdvancedOptions => () => _advancedOptionsVisible = false;

View file

@ -1392,6 +1392,34 @@ public sealed class MossTankPanelTests
Assert.Equal(panel.CombatEnabled ? "True" : "False", after);
}
/// <summary>
/// D-5 (round E architecture re-check): RefreshAdvancedOptions' own
/// doc comment already lists every real mutation site that must call
/// it — category toggle, edit/apply, selection change, and profile
/// load — but opening the popup itself (ShowAdvancedOptions) was
/// missing. A setting changed via the Options tab (ToggleCombatEnabled,
/// the same mutator <see cref="AdvancedOptionValueColumnMirrorsTheLiveSettingValue"/>
/// exercises through a "selection change") while the popup was closed
/// must already read correctly the FIRST time it opens — not only
/// after some in-popup interaction happens to refresh it.
/// </summary>
[Fact]
public void ShowAdvancedOptionsRefreshesValuesChangedWhileThePopupWasClosed()
{
var panel = new MossTankPanel(new FakeHost(new FakeAutomation()));
int index = panel.AdvancedOptionNames.ToList().IndexOf("EnableCombat");
Assert.True(index >= 0);
string before = panel.AdvancedOptionValueColumn[index];
Assert.Equal(panel.CombatEnabled ? "True" : "False", before);
panel.ToggleCombatEnabled();
panel.ShowAdvancedOptions();
string after = panel.AdvancedOptionValueColumn[index];
Assert.NotEqual(before, after);
Assert.Equal(panel.CombatEnabled ? "True" : "False", after);
}
[Fact]
public void AdvancedOptionsPopupBindingsDoNotReallocateOnEveryRead()
{