From a33e8974003aebd39220d2ff2b8399c619fff09a Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 18 Jun 2026 16:57:04 +0200 Subject: [PATCH] perf(D.5.4): toolbar re-binds only on shortcut-guid object changes; clear on remove Now that the object table holds ALL entities (creatures, NPCs, world objects), filtering ObjectAdded/Updated/Removed to the 18 shortcut guids prevents the bar from thrashing on every creature spawn in a busy zone. Also subscribes to ObjectRemoved so a despawned/traded-away item clears its slot (matching retail gmToolbarUI::SetDelayedShortcutNum's deferred-bind contract). Four new unit tests (iconIds spy pattern) verify: non-shortcut ObjectAdded/Removed do NOT invoke Populate; shortcut ObjectAdded deferred-binds; shortcut ObjectRemoved clears the slot. 2671 tests, 4 skipped, 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../UI/Layout/ToolbarController.cs | 20 +++- .../UI/Layout/ToolbarControllerTests.cs | 107 ++++++++++++++++++ 2 files changed, 124 insertions(+), 3 deletions(-) diff --git a/src/AcDream.App/UI/Layout/ToolbarController.cs b/src/AcDream.App/UI/Layout/ToolbarController.cs index 12b4f77b..0cfd9d4c 100644 --- a/src/AcDream.App/UI/Layout/ToolbarController.cs +++ b/src/AcDream.App/UI/Layout/ToolbarController.cs @@ -109,9 +109,23 @@ public sealed class ToolbarController if (combatState is not null) combatState.CombatModeChanged += SetCombatMode; - // Re-bind any deferred slot whenever the repo learns about a new/updated item. - repo.ObjectAdded += _ => Populate(); - repo.ObjectUpdated += _ => Populate(); + // D.5.4: the table now holds ALL objects (creatures, NPCs, etc.), so filter + // to our 18 shortcut guids — else every creature spawn in a busy zone + // needlessly re-populates the bar (gmToolbarUI::SetDelayedShortcutNum pattern). + repo.ObjectAdded += o => { if (IsShortcutGuid(o.ObjectId)) Populate(); }; + repo.ObjectUpdated += o => { if (IsShortcutGuid(o.ObjectId)) Populate(); }; + repo.ObjectRemoved += o => { if (IsShortcutGuid(o.ObjectId)) Populate(); }; + } + + /// + /// Returns true if is one of the currently-active shortcut guids. + /// Used to gate repo-event subscriptions so we don't re-populate on every creature spawn. + /// + private bool IsShortcutGuid(uint guid) + { + foreach (var sc in _shortcuts()) + if (sc.ObjectGuid == guid) return true; + return false; } /// diff --git a/tests/AcDream.App.Tests/UI/Layout/ToolbarControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/ToolbarControllerTests.cs index 9bd13b00..9668a586 100644 --- a/tests/AcDream.App.Tests/UI/Layout/ToolbarControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/ToolbarControllerTests.cs @@ -314,4 +314,111 @@ public class ToolbarControllerTests foreach (var id in Row1) Assert.Null(slots[id].Cell.EmptyDigits); } + + // ── E1: Guid filter + ObjectRemoved tests (D.5.4) ─────────────────────── + + /// + /// ObjectAdded for a guid NOT in the shortcut list does NOT call iconIds again + /// (no spurious Populate on creature/NPC spawns in a busy zone). + /// D.5.4: ToolbarController filters to shortcut guids only. + /// The iconIds spy lets us count how many times Populate actually ran. + /// + [Fact] + public void ObjectAdded_nonShortcutGuid_doesNotCallIconIds() + { + var (layout, _, _) = FakeToolbar(); + var repo = new ClientObjectTable(); + repo.AddOrUpdate(new ClientObject { ObjectId = 0x5001u, WeenieClassId = 1u, IconId = 0x06001234u }); + var shortcuts = new List + { new(Index: 0, ObjectGuid: 0x5001u, SpellId: 0, Layer: 0) }; + + int iconCallCount = 0; + ToolbarController.Bind(layout, repo, () => shortcuts, + iconIds: (_,_,_,_,_) => { iconCallCount++; return 0x77u; }, useItem: _ => { }); + + int callsAfterBind = iconCallCount; // 1 call from initial Populate + + // Fire ObjectAdded with a completely unrelated guid (a creature, NOT a shortcut). + repo.AddOrUpdate(new ClientObject { ObjectId = 0xDEADBEEFu, WeenieClassId = 42u, IconId = 0u }); + + // iconIds must NOT have been called again — the filter blocked Populate. + Assert.Equal(callsAfterBind, iconCallCount); + } + + /// + /// ObjectAdded for a guid that IS in the shortcut list calls iconIds again (deferred bind). + /// This is the filtered-path counterpart of DeferredRebind_whenItemArrivesLate. + /// + [Fact] + public void ObjectAdded_shortcutGuid_callsIconIds() + { + var (layout, slots, _) = FakeToolbar(); + var repo = new ClientObjectTable(); // item NOT present yet + var shortcuts = new List + { new(Index: 1, ObjectGuid: 0x5003u, SpellId: 0, Layer: 0) }; + + int iconCallCount = 0; + ToolbarController.Bind(layout, repo, () => shortcuts, + iconIds: (_,_,_,_,_) => { iconCallCount++; return 0x99u; }, useItem: _ => { }); + + Assert.Equal(0, iconCallCount); // not called — item absent during initial Populate + Assert.Equal(0u, slots[Row1[1]].Cell.ItemId); + + // Now the shortcut item arrives — filter must PASS and Populate re-run. + repo.AddOrUpdate(new ClientObject { ObjectId = 0x5003u, WeenieClassId = 1u, IconId = 0x06005678u }); + + Assert.Equal(1, iconCallCount); // iconIds called exactly once for the deferred bind + Assert.Equal(0x5003u, slots[Row1[1]].Cell.ItemId); + } + + /// + /// ObjectRemoved for a guid that IS in the shortcut list clears the slot. + /// D.5.4: subscribes to ObjectRemoved so a removed item evicts its icon. + /// + [Fact] + public void ObjectRemoved_shortcutGuid_clearsSlot() + { + var (layout, slots, _) = FakeToolbar(); + var repo = new ClientObjectTable(); + repo.AddOrUpdate(new ClientObject { ObjectId = 0x5004u, WeenieClassId = 1u, IconId = 0x06001234u }); + var shortcuts = new List + { new(Index: 3, ObjectGuid: 0x5004u, SpellId: 0, Layer: 0) }; + + ToolbarController.Bind(layout, repo, () => shortcuts, + iconIds: (_,_,_,_,_) => 0xAAu, useItem: _ => { }); + + Assert.Equal(0x5004u, slots[Row1[3]].Cell.ItemId); // bound + + // Remove the item from the session (server despawn / trade away). + // Populate re-runs: item is gone from repo → slot clears. + repo.Remove(0x5004u); + + Assert.Equal(0u, slots[Row1[3]].Cell.ItemId); + } + + /// + /// ObjectRemoved for a guid NOT in the shortcut list does NOT call iconIds again. + /// D.5.4: the ObjectRemoved subscription also filters to shortcut guids. + /// + [Fact] + public void ObjectRemoved_nonShortcutGuid_doesNotCallIconIds() + { + var (layout, _, _) = FakeToolbar(); + var repo = new ClientObjectTable(); + repo.AddOrUpdate(new ClientObject { ObjectId = 0x5005u, WeenieClassId = 1u, IconId = 0x06001234u }); + repo.AddOrUpdate(new ClientObject { ObjectId = 0xCAFEBABEu, WeenieClassId = 99u, IconId = 0u }); + var shortcuts = new List + { new(Index: 4, ObjectGuid: 0x5005u, SpellId: 0, Layer: 0) }; + + int iconCallCount = 0; + ToolbarController.Bind(layout, repo, () => shortcuts, + iconIds: (_,_,_,_,_) => { iconCallCount++; return 0xBBu; }, useItem: _ => { }); + + int callsAfterBind = iconCallCount; // 1 call for the shortcut item + + // Remove an unrelated object — filter must block Populate. + repo.Remove(0xCAFEBABEu); + + Assert.Equal(callsAfterBind, iconCallCount); // unchanged + } }