erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions

fix(vtank): slice 7 fix round C item D1 — materialize Advanced Options bindings once

Browse source 
AdvancedOptionNames, AdvancedOptionValueColumn, AdvancedOptionName, and
AdvancedOptionDescription each re-ran FilteredAdvancedOptionNames() (a
Where over the full 163-entry VtankOptionCatalog) on EVERY retained-UI
draw of the popup, and the value column additionally ran
GetMetaOption(name).ToDisplayString() per row (Trim()/ToLowerInvariant()
allocations) every frame. All four are now materialized once into fields
by a new RefreshAdvancedOptions(), called from every real mutation point:
category toggle, edit/apply, selection change, and profile
load/create/clear/delete (via ResetProfileConsumers), plus construction.
Same pattern fix round B item 12 used for the Monsters/Meta/Route grids.

AdvancedOptionValueColumnMirrorsTheLiveSettingValue exercised a settings
change made OUTSIDE the popup's own mutators (ToggleCombatEnabled, a
main Options-tab checkbox) and expected the value column to reflect it
on the very next read — a real behavior this caching model intentionally
narrows (mirrored settings now catch up at the next real popup mutator,
not on every read). Updated the test to re-select the row afterward,
exercising the "selection change" refresh point, and documented why.

Mutation named: reverted the two property getters to their old live-
recomputing form and confirmed the new
AdvancedOptionsPopupBindingsDoNotReallocateOnEveryRead pin fails
(Assert.Same throws — different array instances per read) before
restoring the fix.

MossTank suite 676 -> 677 (one new pin). Full solution build green.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-07 15:11:26 +02:00
parent 8d3c6ad7c3
commit 466fac426a
2 changed files with 101 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

85
src/AcDream.Plugins.MossTank/MossTankPanel.cs
View file

@ -230,6 +230,19 @@ internal sealed partial class MossTankPanel
// unchecks something).
private readonly bool[] _advancedOptionCategoryEnabled =
Enumerable.Repeat(true, VtankOptionCatalog.CategoryBits.Length).ToArray();
// Fix round C item D1: AdvancedOptionNames/AdvancedOptionValueColumn/
// AdvancedOptionName/AdvancedOptionDescription used to recompute
// FilteredAdvancedOptionNames() (a Where over the full 163-entry
// VtankOptionCatalog) AND, for the value column, run
// GetMetaOption(name).ToDisplayString() per row on EVERY retained-UI
// draw of the popup. Materialized once in RefreshAdvancedOptions,
// called from every real mutation point (category toggle, edit/apply,
// selection change, profile load) — the same pattern item 12 used for
// the grids.
private IReadOnlyList<string> _advancedOptionNames = Array.Empty<string>();
private IReadOnlyList<string> _advancedOptionValueColumn = Array.Empty<string>();
private string _advancedOptionSelectedName = string.Empty;
private string _advancedOptionSelectedDescription = string.Empty;
private string _advancedOptionValueDraft = string.Empty;
private string _advancedOptionNotice =
"All VTank settings are available here.";
@ -379,6 +392,7 @@ internal sealed partial class MossTankPanel
RefreshLootEditor();
RefreshRouteEditor();
RefreshMetaEditor();
RefreshAdvancedOptions();
_automationWasAvailable = host.Automation.IsAvailable;
}
@ -464,10 +478,8 @@ internal sealed partial class MossTankPanel
// their raw bit value ("0x04" etc.) instead of a guessed name; a
// setting with no recorded bitmask always shows regardless of filter
// state (never silently hidden by a filter that doesn't apply to it).
public IReadOnlyList<string> AdvancedOptionNames => FilteredAdvancedOptionNames();
public IReadOnlyList<string> AdvancedOptionValueColumn => FilteredAdvancedOptionNames()
.Select(name => GetMetaOption(name).ToDisplayString())
.ToArray();
public IReadOnlyList<string> AdvancedOptionNames => _advancedOptionNames;
public IReadOnlyList<string> AdvancedOptionValueColumn => _advancedOptionValueColumn;
public IReadOnlyList<string> AdvancedOptionCategoryNames { get; } =
VtankOptionCatalog.CategoryBits.Select(bit => $"0x{bit:X}").ToArray();
public IReadOnlyList<bool> AdvancedOptionCategoryEnabled => _advancedOptionCategoryEnabled;
@ -484,28 +496,16 @@ internal sealed partial class MossTankPanel
if ((uint)index >= (uint)_advancedOptionCategoryEnabled.Length)
return;
_advancedOptionCategoryEnabled[index] = !_advancedOptionCategoryEnabled[index];
_selectedAdvancedOption = ClampRow(_selectedAdvancedOption, AdvancedOptionNames.Count);
RefreshAdvancedOptions();
LoadAdvancedOptionDraft();
};
public int SelectedAdvancedOptionIndex => _selectedAdvancedOption;
public string AdvancedOptionName
{
get
{
IReadOnlyList<string> names = FilteredAdvancedOptionNames();
return names.Count == 0
? string.Empty
: names[Math.Clamp(_selectedAdvancedOption, 0, names.Count - 1)];
}
}
public string AdvancedOptionName => _advancedOptionSelectedName;
public string AdvancedOptionValueDraft => _advancedOptionValueDraft;
// Fix round B item 9: VTank's own Settings.Description column (93 of
// 137 rows non-empty, real retail help text) — fills the 384x80
// txtInfo readout.
public string AdvancedOptionDescription =>
VtankDefaultSettingsDatabase.SettingDescriptions.TryGetValue(AdvancedOptionName, out string? text)
? $"{AdvancedOptionName}: {text}"
: AdvancedOptionName;
public string AdvancedOptionDescription => _advancedOptionSelectedDescription;
public string AdvancedOptionNotice => _advancedOptionNotice;
private IReadOnlyList<string> FilteredAdvancedOptionNames()
@ -520,6 +520,37 @@ internal sealed partial class MossTankPanel
|| mask == 0
|| (mask & enabledMask) != 0).ToArray();
}
/// <summary>
/// Fix round C item D1: the single materialization point for every
/// Advanced Options popup binding — the filtered name list, the
/// per-row value column (each entry costs a GetMetaOption switch plus
/// ToDisplayString's Trim()/ToLowerInvariant() allocations), the
/// clamped selected name, and its description lookup. Called from
/// every real mutation: category toggle, edit/apply, selection
/// change, and profile load/create/clear/delete (via
/// ResetProfileConsumers) plus initial construction — never from a
/// property getter, which is what let this recompute on every single
/// retained-UI draw of the popup before this fix.
/// </summary>
private void RefreshAdvancedOptions()
{
_advancedOptionNames = FilteredAdvancedOptionNames();
_selectedAdvancedOption = ClampRow(_selectedAdvancedOption, _advancedOptionNames.Count);
_advancedOptionSelectedName = _advancedOptionNames.Count == 0
? string.Empty
: _advancedOptionNames[
Math.Clamp(_selectedAdvancedOption, 0, _advancedOptionNames.Count - 1)];
_advancedOptionValueColumn = _advancedOptionNames
.Select(name => GetMetaOption(name).ToDisplayString())
.ToArray();
_advancedOptionSelectedDescription =
VtankDefaultSettingsDatabase.SettingDescriptions.TryGetValue(
_advancedOptionSelectedName, out string? text)
? $"{_advancedOptionSelectedName}: {text}"
: _advancedOptionSelectedName;
}
public Action ShowAdvancedOptions => () =>
{
_advancedOptionsVisible = true;
@ -542,7 +573,8 @@ internal sealed partial class MossTankPanel
};
public Action<int> SelectAdvancedOption => index =>
{
_selectedAdvancedOption = ClampRow(index, FilteredAdvancedOptionNames().Count);
_selectedAdvancedOption = ClampRow(index, _advancedOptionNames.Count);
RefreshAdvancedOptions();
LoadAdvancedOptionDraft();
};
public Action<string> SetAdvancedOptionValueDraft => value =>
@ -2532,15 +2564,21 @@ internal sealed partial class MossTankPanel
_advancedOptionNotice = "Enter a value first.";
return;
}
string name = AdvancedOptionName;
try
{
if (!SetMetaOption(AdvancedOptionName, value))
if (!SetMetaOption(name, value))
{
_advancedOptionNotice = $"{AdvancedOptionName} is unavailable.";
_advancedOptionNotice = $"{name} is unavailable.";
return;
}
// Fix round C item D1: the option's value just changed, so the
// cached value column (and, in principle, the description if a
// future setting's help text ever depended on its own value)
// must be re-materialized before the next draw.
RefreshAdvancedOptions();
LoadAdvancedOptionDraft();
_advancedOptionNotice = $"Applied {AdvancedOptionName}.";
_advancedOptionNotice = $"Applied {name}.";
}
catch (Exception exception) when (exception is FormatException
or OverflowException)
@ -4283,6 +4321,7 @@ internal sealed partial class MossTankPanel
RefreshItemEditors();
RefreshLootEditor();
RefreshRouteEditor();
RefreshAdvancedOptions();
}
private void ClearMossTankActionLocks()

39
tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs
View file

@ -1295,6 +1295,14 @@ public sealed class MossTankPanelTests
{
// Fix round B item 9: lOptionList gained a real clVal value column
// (AdvancedOptionValueColumn) alongside the name column.
// Fix round C item D1: the column is now materialized once by
// RefreshAdvancedOptions (category toggle / edit-apply / selection
// change / profile load), not recomputed on every read — so a
// setting changed OUTSIDE those four mutators (ToggleCombatEnabled
// is a main Options-tab checkbox, not a popup action) is picked up
// the next time a real popup mutator runs rather than on the very
// next property read. Re-selecting the same row exercises exactly
// that "selection change" mutator.
var panel = new MossTankPanel(new FakeHost(new FakeAutomation()));
int index = panel.AdvancedOptionNames.ToList().IndexOf("EnableCombat");
Assert.True(index >= 0);
@ -1302,12 +1310,43 @@ public sealed class MossTankPanelTests
Assert.Equal(panel.CombatEnabled ? "True" : "False", before);
panel.ToggleCombatEnabled();
panel.SelectAdvancedOption(index);
string after = panel.AdvancedOptionValueColumn[index];
Assert.NotEqual(before, after);
Assert.Equal(panel.CombatEnabled ? "True" : "False", after);
}
[Fact]
public void AdvancedOptionsPopupBindingsDoNotReallocateOnEveryRead()
{
// Fix round C item D1: AdvancedOptionNames/AdvancedOptionValueColumn
// used to re-run FilteredAdvancedOptionNames() (a Where over the
// full 163-entry VtankOptionCatalog) — and, for the value column, a
// fresh GetMetaOption(name).ToDisplayString() per row — on every
// single retained-UI draw of the popup. Both are now materialized
// once in RefreshAdvancedOptions: repeated reads with no mutation
// in between must return the SAME array instance, matching the
// precedent MetaAndRouteGridColumnsDoNotReallocateOnEveryRead set
// for item 12's grids.
var panel = new MossTankPanel(new FakeHost(new FakeAutomation()));
Assert.Same(panel.AdvancedOptionNames, panel.AdvancedOptionNames);
Assert.Same(panel.AdvancedOptionValueColumn, panel.AdvancedOptionValueColumn);
int index = panel.AdvancedOptionNames.ToList().IndexOf("EnableCombat");
Assert.True(index >= 0);
panel.SelectAdvancedOption(index);
// A real mutator (selection change) is allowed to swap the cached
// instance out for a fresh one...
IReadOnlyList<string> namesAfterSelect = panel.AdvancedOptionNames;
IReadOnlyList<string> valuesAfterSelect = panel.AdvancedOptionValueColumn;
// ...but once settled, repeated reads must again share one instance.
Assert.Same(namesAfterSelect, panel.AdvancedOptionNames);
Assert.Same(valuesAfterSelect, panel.AdvancedOptionValueColumn);
}
/// <summary>
/// Campaign VT slice 7 S7.2: VTank's cOn (Options tab, "Run Macro") is a
/// Checkbox with a STATIC caption whose checked state reflects whether