diff --git a/src/AcDream.Plugins.MossTank/MossTankPanel.cs b/src/AcDream.Plugins.MossTank/MossTankPanel.cs index 02f329c69..f585567dd 100644 --- a/src/AcDream.Plugins.MossTank/MossTankPanel.cs +++ b/src/AcDream.Plugins.MossTank/MossTankPanel.cs @@ -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 _advancedOptionNames = Array.Empty(); + private IReadOnlyList _advancedOptionValueColumn = Array.Empty(); + 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 AdvancedOptionNames => FilteredAdvancedOptionNames(); - public IReadOnlyList AdvancedOptionValueColumn => FilteredAdvancedOptionNames() - .Select(name => GetMetaOption(name).ToDisplayString()) - .ToArray(); + public IReadOnlyList AdvancedOptionNames => _advancedOptionNames; + public IReadOnlyList AdvancedOptionValueColumn => _advancedOptionValueColumn; public IReadOnlyList AdvancedOptionCategoryNames { get; } = VtankOptionCatalog.CategoryBits.Select(bit => $"0x{bit:X}").ToArray(); public IReadOnlyList 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 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 FilteredAdvancedOptionNames() @@ -520,6 +520,37 @@ internal sealed partial class MossTankPanel || mask == 0 || (mask & enabledMask) != 0).ToArray(); } + + /// + /// 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. + /// + 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 SelectAdvancedOption => index => { - _selectedAdvancedOption = ClampRow(index, FilteredAdvancedOptionNames().Count); + _selectedAdvancedOption = ClampRow(index, _advancedOptionNames.Count); + RefreshAdvancedOptions(); LoadAdvancedOptionDraft(); }; public Action 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() diff --git a/tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs b/tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs index 93c568bd1..40968c0f9 100644 --- a/tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs +++ b/tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs @@ -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 namesAfterSelect = panel.AdvancedOptionNames; + IReadOnlyList valuesAfterSelect = panel.AdvancedOptionValueColumn; + // ...but once settled, repeated reads must again share one instance. + Assert.Same(namesAfterSelect, panel.AdvancedOptionNames); + Assert.Same(valuesAfterSelect, panel.AdvancedOptionValueColumn); + } + /// /// 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