From a330d50df9e59d914241936c7c9bdb29609802d7 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 21 Aug 2026 10:33:26 +0200 Subject: [PATCH] fix(chat): the unseen-text indicator follows its authored per-state visibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression from cbab79d7, which I introduced: the indicator stopped showing at all. Switching it from Visible to state-driven was half a correction — right about retail's mechanism, wrong about what makes this element appear. Measured, rather than reasoned about (LayoutDump gained --props for it): 0x1000048C state 13 Ghosted 0x3B = True -> hidden state 1 Normal 0x3B = False -> shown state 3 pressed 0x3B = False Dat property 0x3B is "Invisible", authored PER STATE, and it is what puts this element on screen. UiDatElement applies 0x3B on a state change; UiButton does not, and this element builds as a button — so driving the state alone left it hidden forever. The original Visible toggle was, by coincidence, exactly what the authored data prescribes. So the property is applied here rather than left unhonoured. That is the authored data, not a visibility hack layered over the state machinery. The state is still set, for the media it selects, but only on the way IN: TrySetRetailState(Ghosted) means Enabled = false, and disabling the button would also refuse the click that scrolls to the newest text — a second bug waiting behind the first. The test now pins VISIBILITY across the transitions instead of ActiveState. The previous test passed while the feature was broken because the fixture element carried no 0x3B, so the assertion could never see the property that actually decides this. It fails now if the state is driven without the visibility. Proper fix noted for later: UiButton should honour per-state 0x3B the way UiDatElement already does. That is a wider change than this regression wants. Solution builds clean; full hermetic gate green. Co-Authored-By: Claude Opus 5 --- .../UI/Layout/ChatWindowController.cs | 26 +++++++++++---- .../UI/Layout/ChatWindowControllerTests.cs | 33 ++++++++----------- tools/LayoutDump/Program.cs | 25 ++++++++++++++ 3 files changed, 58 insertions(+), 26 deletions(-) diff --git a/src/AcDream.App/UI/Layout/ChatWindowController.cs b/src/AcDream.App/UI/Layout/ChatWindowController.cs index 73ef8c5f..27fef4c8 100644 --- a/src/AcDream.App/UI/Layout/ChatWindowController.cs +++ b/src/AcDream.App/UI/Layout/ChatWindowController.cs @@ -940,14 +940,28 @@ public sealed class ChatWindowController : IRetainedWindowStateController, IReta /// private void SetUnreadIndicatorState(bool unread) { - if (_unreadIndicator is not IUiDatStateful stateful) + if (_unreadIndicator is null) return; - stateful.TrySetRetailState( - unread ? UiButtonStateMachine.Normal : GhostedStateId); - } - /// Retail state 0xD, the id its own click handler sets. - private const uint GhostedStateId = 13u; + // Apply the element's OWN authored per-state visibility (dat property + // 0x3B, "Invisible"), measured on 0x1000048C as: + // + // state 13 Ghosted 0x3B = True -> hidden + // state 1 Normal 0x3B = False -> shown + // + // UiDatElement applies 0x3B on a state change; UiButton does not, and + // this element builds as a button. So the property is applied here + // rather than left unhonoured — this is the authored data, not a + // visibility hack layered over it. + _unreadIndicator.Visible = unread; + + // Set the state too, for the media it selects. Deliberately only on + // the way IN: TrySetRetailState(Ghosted) means Enabled = false, and + // disabling the button would also refuse the click that scrolls to + // the newest text. + if (unread && _unreadIndicator is IUiDatStateful stateful) + stateful.TrySetRetailState(UiButtonStateMachine.Normal); + } /// Aims the chat entry at and focuses it. internal void StartTell(string name) diff --git a/tests/AcDream.App.Tests/UI/Layout/ChatWindowControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/ChatWindowControllerTests.cs index c6c9286e..d1223134 100644 --- a/tests/AcDream.App.Tests/UI/Layout/ChatWindowControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/ChatWindowControllerTests.cs @@ -200,39 +200,32 @@ public class ChatWindowControllerTests } [Fact] - public void TheIndicatorIsDrivenByAuthoredStateNotVisibility() + public void TheIndicatorFollowsItsAuthoredPerStateVisibility() { - // Retail's own click handler ends in SetState(0xD) — Ghosted — which - // is also the element's authored default; the unread look is state 1 - // (Normal), whose media list carries SIX frames and is where the - // flashing comes from. Hiding the element instead would look almost - // right and could never blink. + // The element authors dat property 0x3B ("Invisible") per state, and + // it is what decides whether the thing is on screen: + // state 13 Ghosted 0x3B = True -> hidden + // state 1 Normal 0x3B = False -> shown + // (measured with LayoutDump --props on 0x2100006F). + // + // UiDatElement applies 0x3B on a state change; UiButton does not, and + // this element builds as a button — so driving the state ALONE leaves + // it hidden forever, which is exactly the regression this pins. ChatWindowController ctrl = BindController(); - UiElement indicator = Assert.IsAssignableFrom( ctrl.UnreadIndicatorForTest); - // Visibility is NOT the mechanism: the element stays visible and - // changes STATE. Ghosted authors no media on the real element, so it - // draws nothing without being hidden. - Assert.True(indicator.Visible); + Assert.False(indicator.Visible); // Ghosted at rest - var stateful = Assert.IsAssignableFrom(indicator); - Assert.Equal("Ghosted", ((UiButton)indicator).ActiveState); - - // A line arriving while scrolled up flips it to Normal — the state - // whose authored media carries the six flash frames. ctrl.Transcript.Scroll.SetExtents(contentHeight: 500, viewHeight: 100); ctrl.Transcript.Scroll.SetScrollY(0); ctrl.SetUnreadForTest(true); ctrl.UpdateUnreadIndicator(); - Assert.Equal("Normal", ((UiButton)indicator).ActiveState); + Assert.True(indicator.Visible); // Normal once text is unseen - // ...and returning to the bottom puts it back. ctrl.Transcript.Scroll.ScrollToEnd(); ctrl.UpdateUnreadIndicator(); - Assert.Equal("Ghosted", ((UiButton)indicator).ActiveState); - _ = stateful; + Assert.False(indicator.Visible); // back to Ghosted } [Fact] diff --git a/tools/LayoutDump/Program.cs b/tools/LayoutDump/Program.cs index 3bdd0615..255917ff 100644 --- a/tools/LayoutDump/Program.cs +++ b/tools/LayoutDump/Program.cs @@ -23,6 +23,7 @@ if (args.Length == 0) bool showStates = args.Contains("--states"); bool showColors = args.Contains("--colors"); +bool showProps = args.Contains("--props"); uint[] ids = args.Where(a => !a.StartsWith("--")) .Select(a => Convert.ToUInt32(a, a.StartsWith("0x") ? 16 : 10)) .ToArray(); @@ -141,6 +142,30 @@ void Print(ElementInfo e, int depth) } } + // Raw property ids per state — ToggleBehavior (0x0B) and RolloverEnabled + // (0x13) change how a button interprets a state change, so "which state did + // I set" is not the whole story. + if (showProps) + { + foreach (var (stateId, state) in e.States) + { + if (state.Properties.Values.Count == 0) + continue; + string ids = string.Join(", ", state.Properties.Values + .OrderBy(kv => kv.Key) + .Select(kv => $"0x{kv.Key:X2}={Describe(kv.Value)}")); + + static string Describe(UiPropertyValue v) => v.Kind switch + { + UiPropertyKind.Bool => v.BoolValue.ToString(), + UiPropertyKind.Integer => v.IntegerValue.ToString(), + UiPropertyKind.Enum => $"0x{v.UnsignedValue:X}", + _ => v.Kind.ToString(), + }; + Console.WriteLine($"{pad} state {stateId}: props {ids}"); + } + } + if (showStates && e.States.Count != 0) { string names = string.Join(", ", e.States