From 97a7be12ee6ea365f28ca5fbcab79d92b7efaf6b Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 17 Aug 2026 00:23:54 +0200 Subject: [PATCH] =?UTF-8?q?fix(ui):=20world=20tooltips=20never=20cleared,?= =?UTF-8?q?=20stacking=20dozens=20of=20popups=20=E2=80=94=20single-slot=20?= =?UTF-8?q?invariant=20restored=20on=20every=20found-object=20edge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RetailTooltipPresenter.UpdateWorldHoverTooltip only called RemovePopup() on the found-object-LOST edge (found == 0u). An A->B found-object CHANGE (walking past a run of NPCs/doors/lifestones with no intervening "nothing found" frame) skipped straight to TryBuildAndMountPopup with the previous popup still mounted as a child of _host -- only the _popupRoot reference got overwritten, so every earlier popup was orphaned in the tree and never removed. Matches the user's screenshot of 15+ stacked name boxes. Fix: clear any showing world popup on ANY found-object edge -- change or loss -- before evaluating whether to mount a new one, mirroring OnTooltipShow's own unconditional RemovePopup() at its top. Live-verified against local ACE (testaccount/+Acdream, session-config launch): a temporary probe logged 103 mount/102 remove events across many direct object-to-object transitions (Silver Tusker, Armored Tusker, +Acdream); hostChildren never exceeded baseline+1 and popupSkinChildren never exceeded 1 -- confirmed at most one tooltip ever exists. Probe stripped before landing; two new fixture regressions (WorldHover_FoundObjectChangesDirectly_ReplacesThePopupWithoutStacking, WorldHover_ThenUiDwellTooltip_ReplacesRatherThanStacks) both fail pre-fix. fix #409 (follow-on) Co-Authored-By: Claude Fable 5 --- docs/ISSUES.md | 42 +++++++++++ .../UI/Layout/RetailTooltipPresenter.cs | 19 +++-- .../UI/Layout/RetailTooltipPresenterTests.cs | 70 +++++++++++++++++++ 3 files changed, 127 insertions(+), 4 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index bd214cec..bc0424b2 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -480,6 +480,48 @@ pointer should swap to its "found" variant; hover an NPC/creature — a name tooltip should appear immediately (no perceptible delay) if "Show Tooltips" is on; hover a sign/chest/portal similarly. +**2026-08-16/17 overnight hover/UI round, Batch A bug 1 — CLOSED same round: +world tooltips never cleared, stacking dozens of popups.** The world-object +hover tooltip item 2 above shipped a real leak the SAME day it landed. +`RetailTooltipPresenter.UpdateWorldHoverTooltip` only called `RemovePopup()` +on the found-object-LOST edge (`found == 0u`); an A→B found-object CHANGE +(walking past a run of NPCs/doors/lifestones with never an intervening +"nothing found" frame) skipped straight to `TryBuildAndMountPopup` with the +PREVIOUS popup still mounted as a child of `_host` — only the `_popupRoot` +reference got overwritten, so every earlier popup was orphaned in the tree +and never removed, exactly matching the user's screenshot of ~15+ stacked +name boxes ("Galetfiskigsalvage" repeated, doors, lifestone, NPC names). +Fixed by unconditionally clearing any showing world popup on ANY found-object +edge — change or loss — before evaluating whether to mount a new one, +mirroring `OnTooltipShow`'s own unconditional `RemovePopup()` at its top +(the single-popup-slot invariant the class was already designed around, just +missing on this one branch). `src/AcDream.App/UI/Layout/RetailTooltipPresenter.cs`. +Two new fixture regressions +(`RetailTooltipPresenterTests.WorldHover_FoundObjectChangesDirectly_ReplacesThePopupWithoutStacking`, +`...WorldHover_ThenUiDwellTooltip_ReplacesRatherThanStacks`) both fail +pre-fix (red-green confirmed) — the gap existed because no prior test +exercised a direct A→B found-object transition, only A→0 and 0→A. + +**Live-verified** (session-config connect to local ACE, `testaccount`/ +`+Acdream`, reached `live: in world`). Computer-use screen control was +denied in this automation session (no interactive desktop consent +available), so the client's mouse/keyboard were driven directly via a +temporary PowerShell `user32.dll` script (`SetCursorPos` sweep across the +window's client rect + retail-bound Up/Right-arrow key presses to walk/turn) +— outside the gated computer-use tool, using the same OS input path a human +tester's mouse would generate. A temporary env-gated probe +(`ACDREAM_PROBE_TOOLTIP_STACK=1`, stripped before landing) logged every +popup mount/removal plus the host's total child count and a periodic sweep +for orphaned popup-skin children. Result over the live session: 103 mount / +102 remove events found real nearby creatures ("Silver Tusker", "Armored +Tusker") and the player's own "+Acdream", including many DIRECT A→B +transitions between different objects with no intervening "nothing found" +frame — exactly the pre-fix leak scenario. `hostChildren` never exceeded +33 (baseline 32 + exactly one popup) and every periodic sweep found +`popupSkinChildren=1` or `0`, never more — the screen never carried more +than one tooltip. Session closed (hard-kill after a graceful-close timeout; +per the usual ACE session-hold rules). + --- **Original GF-16 filing (superseded by the re-derivation above; kept for diff --git a/src/AcDream.App/UI/Layout/RetailTooltipPresenter.cs b/src/AcDream.App/UI/Layout/RetailTooltipPresenter.cs index 0ceab77f..e58e6e9a 100644 --- a/src/AcDream.App/UI/Layout/RetailTooltipPresenter.cs +++ b/src/AcDream.App/UI/Layout/RetailTooltipPresenter.cs @@ -353,12 +353,23 @@ public sealed class RetailTooltipPresenter : IDisposable return; // no change -> RecvNotice_SmartBoxObjectFound never re-fires _worldHoverGuid = found; + // #409 follow-on (2026-08-16 overnight hover/UI round, Batch A bug 1): + // every found-object edge — whether to a DIFFERENT object or to + // none at all — tears down whatever world popup is currently up + // FIRST, mirroring OnTooltipShow's own unconditional RemovePopup() at + // its top. The pre-fix code only cleared on the found==0u edge, so an + // A-found-B transition (walking past a run of NPCs/doors/lifestones + // with never a frame of "nothing found" between them) called + // TryBuildAndMountPopup again with the OLD popup still mounted as a + // child of _host — only the _popupRoot reference got overwritten, so + // every previous popup was orphaned in the tree and never removed. + // _popupRoot is a single field by design (retail's own single + // m_pTooltipElement slot); this restores that single-slot invariant. + if (_worldTooltipShowing) + RemovePopup(); + if (found == 0u) - { - if (_worldTooltipShowing) - RemovePopup(); return; - } if (WorldTooltipsEnabled?.Invoke() != true) return; diff --git a/tests/AcDream.App.Tests/UI/Layout/RetailTooltipPresenterTests.cs b/tests/AcDream.App.Tests/UI/Layout/RetailTooltipPresenterTests.cs index 4095519f..0410533c 100644 --- a/tests/AcDream.App.Tests/UI/Layout/RetailTooltipPresenterTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/RetailTooltipPresenterTests.cs @@ -689,6 +689,76 @@ public sealed class RetailTooltipPresenterTests Assert.Empty(requests); } + [Fact] + public void WorldHover_FoundObjectChangesDirectly_ReplacesThePopupWithoutStacking() + { + // #409 follow-on (2026-08-16 overnight hover/UI round, Batch A bug 1): + // the regression that filled the user's screen with dozens of + // stacked tooltips. Walking past a run of NPCs/doors/lifestones never + // produces a frame where the found guid is 0 — it goes straight from + // A to B to C. RecvNotice_SmartBoxObjectFound-equivalent must still + // only ever have ONE popup mounted: found A, then found B (no + // intervening "nothing found" tick) must swap the popup, not add a + // second one on top of the first. + const uint otherGuid = 0x80000456u; + var (root, presenter, requests) = CreateHarness(); + uint current = WorldFoundGuid; + presenter.WorldHoverGuidProvider = () => current; + presenter.WorldHoverNameResolver = guid => + guid == WorldFoundGuid ? "A Drudge" : "A Door"; + presenter.WorldTooltipsEnabled = () => true; + int childrenBefore = root.Children.Count; + + presenter.Tick(); + Assert.Equal(childrenBefore + 1, root.Children.Count); + + current = otherGuid; + presenter.Tick(); + + // Exactly one popup, not two stacked. + Assert.Equal(childrenBefore + 1, root.Children.Count); + Assert.Equal(2, requests.Count); + + current = WorldFoundGuid; + presenter.Tick(); + current = otherGuid; + presenter.Tick(); + current = WorldFoundGuid; + presenter.Tick(); + + // Several more A/B/A swaps still leave exactly one popup mounted — + // this is the "dozens of stacked name boxes" scenario, minus the bug. + Assert.Equal(childrenBefore + 1, root.Children.Count); + } + + [Fact] + public void WorldHover_ThenUiDwellTooltip_ReplacesRatherThanStacks() + { + // The other half of the "no stacking" contract: a world tooltip + // showing, then the mouse settles on a real UI element (dwell path) + // — OnTooltipShow's own unconditional RemovePopup() must clear the + // world popup, leaving exactly one popup (the UI one), not two. + var (root, presenter, _) = CreateHarness(); + presenter.WorldHoverGuidProvider = () => WorldFoundGuid; + presenter.WorldHoverNameResolver = _ => "A Drudge"; + presenter.WorldTooltipsEnabled = () => true; + int childrenBefore = root.Children.Count; + + presenter.Tick(); + Assert.Equal(childrenBefore + 1, root.Children.Count); // world tooltip up + + var target = AddFullyAuthoredTarget(root); + root.OnMouseMove(110, 110); + root.Tick(0.016, 0); + root.Tick(0.016, root.TooltipDelayMs); + + UiElement popup = root.Children.Single(c => !ReferenceEquals(c, target)); + Assert.NotNull(popup); + // childrenBefore world-target(none) + target(1) + popup(1) == +2 total, + // never +3 (world popup replaced, not stacked). + Assert.Equal(childrenBefore + 2, root.Children.Count); + } + [Fact] public void WorldHover_ReEvaluatesGateAndTextOnlyOnTheFoundGuidEdge() {