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

fix(vtank): slice 7 fix round B item 16 — small ones (dedupe, tab-switch cleanup, slider validation)

Browse source 
Three small, unrelated fixes bundled per the fix round's own item list
(the fourth, cropping the three popup screenshots to the panel, belongs to
the live-capture pass, item 17):

- SortedCombatItemNames(): one shared helper for the ordinal-sorted
  registered weapon roster, replacing five separate identical
  `_combatSettings.CombatItemNames.OrderBy(...).ToArray()` call sites
  (RefreshItemEditors, DeleteItemRowAtCore, CycleItemHandsAtCore,
  RemoveSelectedItemCore, CycleMonsterEquipmentAt).
- SelectTab already cleared LootEditorVisible/AdvancedOptionsVisible when
  the user switched tabs (fix round A). The buff picker (S7.4) and the
  Meta rule editor (item 5) are the same "own popup, own window" shape and
  were missing from that guard — switching away from Buffs or Meta while
  either popup was open left it orphaned, open over whatever tab the user
  switched to. Both flags now clear in SelectTab too.
- <slider min max>: max<=min used to silently fall back to a range of 1
  (the old `range == 0f` check, covering only max==min) or produce a
  slider whose drag direction is inverted from its declared range
  (max<min, never caught at all). Both are now a build-time
  FormatException, the same rule every other <slider>/<menu>
  attribute-format check in this file already follows.

New tests: SwitchingTabsClosesTheBuffPickerAndMetaEditorPopups,
Build_SliderWithMaxLessThanOrEqualToMin_Throws (both max<min and
max==min cases). Mutation checks: removing the two SelectTab clears
turned the first red ("Expected: False, Actual: True"); reverting the
slider validation to the old range==0f fallback turned both Theory cases
of the second red ("No exception was thrown"). Both restored to green.

tests/AcDream.Plugins.MossTank.Tests: 676/676 (was 675/675, +1).
tests/AcDream.App.Tests --filter Markup|Plugin|UiMenu|Slider: 285/3 skipped/288 (was 283/3/286, +2: the new Theory's 2 cases).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-07 14:09:58 +02:00
parent 7ca9785665
commit 8d3c6ad7c3
4 changed files with 72 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

14
src/AcDream.App/UI/MarkupDocument.cs
View file

@ -389,9 +389,19 @@ public static class MarkupDocument
// (which never sets min/max) is byte-for-byte unaffected.
float sliderMin = FOr(el, "min", 0f);
float sliderMax = FOr(el, "max", 1f);
// Fix round B item 16: max<=min used to silently fall back to
// a range of 1 (via the old "== 0f" check) rather than being
// caught as an authoring error — a max<min case slipped
// through entirely, producing a slider whose drag direction
// is inverted from its declared range. Both are now a
// build-time author error, same rule as every other
// <slider>/<menu> attribute-format check in this file.
if (sliderMax <= sliderMin)
{
throw new FormatException(
$"<slider min=\"{sliderMin}\" max=\"{sliderMax}\"> must have max > min");
}
float sliderRange = sliderMax - sliderMin;
if (sliderRange == 0f)
sliderRange = 1f;
Func<float?> sliderValueSource = BindFloat(
(string?)el.Attribute("value"),

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

@ -1754,8 +1754,7 @@ internal sealed partial class MossTankPanel
private void RefreshItemEditors()
{
RefreshConsumableCategories();
_itemRows = _combatSettings.CombatItemNames
.OrderBy(static name => name, StringComparer.Ordinal)
_itemRows = SortedCombatItemNames()
.Select(name => _noBuffItemNames.Contains(name)
? name + " [no buffs]"
: name)
@ -1791,9 +1790,7 @@ internal sealed partial class MossTankPanel
private void DeleteItemRowAtCore(int row)
{
string[] names = _combatSettings.CombatItemNames
.OrderBy(static name => name, StringComparer.Ordinal)
.ToArray();
string[] names = SortedCombatItemNames();
if ((uint)row >= (uint)names.Length)
return;
string removed = names[row];
@ -1807,9 +1804,7 @@ internal sealed partial class MossTankPanel
private void CycleItemHandsAtCore(int row)
{
string[] names = _combatSettings.CombatItemNames
.OrderBy(static name => name, StringComparer.Ordinal)
.ToArray();
string[] names = SortedCombatItemNames();
if ((uint)row >= (uint)names.Length)
return;
string name = names[row];
@ -1921,11 +1916,16 @@ internal sealed partial class MossTankPanel
? 0
: Math.Clamp(index, 0, count - 1);
// Fix round B item 16: one shared helper for the ordinal-sorted
// registered weapon roster, replacing several separate identical
// `_combatSettings.CombatItemNames.OrderBy(...).ToArray()` call sites.
private string[] SortedCombatItemNames() => _combatSettings.CombatItemNames
.OrderBy(static name => name, StringComparer.Ordinal)
.ToArray();
private void RemoveSelectedItemCore()
{
string[] names = _combatSettings.CombatItemNames
.OrderBy(static name => name, StringComparer.Ordinal)
.ToArray();
string[] names = SortedCombatItemNames();
if (names.Length == 0)
{
_profileNotice = "The Items profile is empty.";
@ -2507,6 +2507,13 @@ internal sealed partial class MossTankPanel
_activeTab = tab;
_lootEditorVisible = false;
_advancedOptionsVisible = false;
// Fix round B item 16: the buff picker (S7.4) and the Meta rule
// editor (item 5) are the same "own popup, own window" shape as
// Loot Editor/Advanced Options above — a stale open popup from a
// tab the user just left is a real orphaned-window bug, not a
// cosmetic one.
_buffPickerVisible = false;
_metaEditorVisible = false;
}
private void LoadAdvancedOptionDraft()
@ -2738,9 +2745,7 @@ internal sealed partial class MossTankPanel
/// </summary>
private void CycleMonsterEquipmentAt(int row, bool offhand)
{
string[] names = _combatSettings.CombatItemNames
.OrderBy(static n => n, StringComparer.Ordinal)
.ToArray();
string[] names = SortedCombatItemNames();
UpdateMonsterActionsAt(row, actions =>
{
string currentName = offhand ? actions.OffhandName : actions.WeaponName;

19
tests/AcDream.App.Tests/UI/MarkupDocumentTests.cs
View file

@ -377,6 +377,25 @@ public class MarkupDocumentTests
Assert.Equal(120f, binding.Value, 3);
}
[Theory]
[InlineData("100", "0")] // max < min
[InlineData("50", "50")] // max == min
public void Build_SliderWithMaxLessThanOrEqualToMin_Throws(string min, string max)
{
// Fix round B item 16: max<=min used to silently fall back to a
// range of 1 (max==min) or produce an inverted-drag-direction
// slider (max<min) instead of being caught as an authoring error.
string xml =
"<panel x=\"0\" y=\"0\" w=\"160\" h=\"20\">" +
$"<slider x=\"0\" y=\"0\" w=\"150\" h=\"16\" min=\"{min}\" max=\"{max}\" " +
"value=\"{Value}\" onchange=\"{SetValue}\"/>" +
"</panel>";
var binding = new RangeBinding();
Assert.Throws<FormatException>(
() => MarkupDocument.Build(xml, binding, _ => (1u, 16, 16)));
}
// Fix round B item 11: <slider> now defaults to the PLAIN style
// (RetailArt=false), so a markup slider with no style attribute never
// reaches RetailScrollbarChrome.ApplyHorizontal's sprite thumb any more

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

@ -2392,6 +2392,28 @@ public sealed class MossTankPanelTests
Assert.False(panel.MetaEditorVisible);
}
[Fact]
public void SwitchingTabsClosesTheBuffPickerAndMetaEditorPopups()
{
// Fix round B item 16: SelectTab already cleared LootEditorVisible/
// AdvancedOptionsVisible when the user left a tab — the buff
// picker (S7.4) and the Meta rule editor (item 5) are the same
// "own popup" shape and were missing from that same guard, leaving
// a stale open popup from a tab the user just left.
var panel = new MossTankPanel(new FakeHost(new FakeAutomation()));
panel.ShowExtraBuffPicker();
Assert.True(panel.BuffPickerVisible);
panel.ShowOptions(); // switch away from Buffs
Assert.False(panel.BuffPickerVisible);
panel.ShowMeta();
panel.CreateMetaRule();
Assert.True(panel.MetaEditorVisible);
panel.ShowOptions(); // switch away from Meta
Assert.False(panel.MetaEditorVisible);
}
[Fact]
public void MetaCurrentStateMenuForcesTheLiveEngineIntoTheChosenState()
{