From 8d3c6ad7c3a0a51ab91db6da2e83ce03a71330f6 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 7 Sep 2026 14:09:58 +0200 Subject: [PATCH] =?UTF-8?q?fix(vtank):=20slice=207=20fix=20round=20B=20ite?= =?UTF-8?q?m=2016=20=E2=80=94=20small=20ones=20(dedupe,=20tab-switch=20cle?= =?UTF-8?q?anup,=20slider=20validation)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. - : 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/ attribute-format check in this file already follows. New tests: SwitchingTabsClosesTheBuffPickerAndMetaEditorPopups, Build_SliderWithMaxLessThanOrEqualToMin_Throws (both max --- src/AcDream.App/UI/MarkupDocument.cs | 14 ++++++-- src/AcDream.Plugins.MossTank/MossTankPanel.cs | 33 +++++++++++-------- .../UI/MarkupDocumentTests.cs | 19 +++++++++++ .../MossTankPanelTests.cs | 22 +++++++++++++ 4 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/AcDream.App/UI/MarkupDocument.cs b/src/AcDream.App/UI/MarkupDocument.cs index 60fe928f2..12e9a06de 100644 --- a/src/AcDream.App/UI/MarkupDocument.cs +++ b/src/AcDream.App/UI/MarkupDocument.cs @@ -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/ attribute-format check in this file. + if (sliderMax <= sliderMin) + { + throw new FormatException( + $" must have max > min"); + } float sliderRange = sliderMax - sliderMin; - if (sliderRange == 0f) - sliderRange = 1f; Func sliderValueSource = BindFloat( (string?)el.Attribute("value"), diff --git a/src/AcDream.Plugins.MossTank/MossTankPanel.cs b/src/AcDream.Plugins.MossTank/MossTankPanel.cs index 1da42204f..02f329c69 100644 --- a/src/AcDream.Plugins.MossTank/MossTankPanel.cs +++ b/src/AcDream.Plugins.MossTank/MossTankPanel.cs @@ -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 /// 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; diff --git a/tests/AcDream.App.Tests/UI/MarkupDocumentTests.cs b/tests/AcDream.App.Tests/UI/MarkupDocumentTests.cs index 7113d250d..3f713612d 100644 --- a/tests/AcDream.App.Tests/UI/MarkupDocumentTests.cs +++ b/tests/AcDream.App.Tests/UI/MarkupDocumentTests.cs @@ -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" + + $"" + + ""; + var binding = new RangeBinding(); + + Assert.Throws( + () => MarkupDocument.Build(xml, binding, _ => (1u, 16, 16))); + } + // Fix round B item 11: 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 diff --git a/tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs b/tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs index 825ffbd72..93c568bd1 100644 --- a/tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs +++ b/tests/AcDream.Plugins.MossTank.Tests/MossTankPanelTests.cs @@ -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() {