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

fix(chat): CH6c review fixes — opaque default, opacity-transition register clauses

Browse source 
BLOCKER: ChatSettings.DefaultOpacity shipped retail's base ChatInterface
value (0.5) as ONE shared global default applied to every
RetailWindowManager-registered window, not just the four floating chat
windows retail itself fades. That faded the whole out-of-box registered
UI (radar, vitals, toolbar, main chat, ...) to 50% opacity, including
several windows that can never take keyboard focus and so were stuck at
0.5 permanently. Fixed to gmMainChatUI's 1.0/1.0 override
(0x004CD0F0) instead — retail-identical opaque presentation for the 11
non-chat windows and the main chat window; only the four floating chat
windows now diverge from retail's 0.5-while-idle default, and the
Settings -> Chat transparency slider remains fully user-settable.

AP-190 reworded and gains two new decomp-verified clauses: (3) retail
eases opacity toward its target by 5% of the delta per tick
(ChatInterface::ListenToGlobalMessage @0x004F3840, armed from the focus
element-messages at @0x004F5275) where acdream snaps -- deferred, needs
a UI frame-tick hook the opacity controller doesn't have; (4) retail's
focus predicate is the chat ENTRY FIELD specifically
(ChatInterface::IsTextEntryFocused @0x004F30A0) where acdream uses
any-focusable-descendant. Both findings + the pre-existing UiMenu.cs
PushAlphaAbsolute(1f) popup bypass are folded into the window-shell
research doc's opacity section.

NITs: fixed the stale "text bypasses the alpha" comment in
UiElement.DrawSelfAndChildren (CH6c already routed DrawStringDat/
DrawString through the same ApplyAlpha chokepoint as sprites/rects);
added RetailWindowManager.WindowUnregistered + wired
RetailWindowOpacityController to detach and forget a window unregistered
while it held focus (previously only Dispose detached, leaking any
window unregistered mid-focus for the rest of the session); added
post-Dispose no-op guards to the three Set* opacity mutators; added a
DrawString (BitmapFont path) alpha regression test and a DrawStringDat
outline/background-pass alpha test (the existing tests only ever
exercised the foreground/fill pass).

Also fixes RuntimeSettingsControllerTests.SettingsViewModelSavePreserves
SectionAndTargetOrder's now-stale "target-chat-opacity:0.5:1" expectation
(caught by the full-suite run this fix requires) to match the new 1.0
default.

Campaign ledger CH6c row updated to APPROVE-WITH-FIXES.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-10 14:45:14 +02:00
parent 964af62e25
commit cc58289967
14 changed files with 319 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

2
tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs
View file

@ -312,7 +312,7 @@ public sealed class RuntimeSettingsControllerTests
"save-gameplay",
"target-ui-lock:True",
"save-chat",
"target-chat-opacity:0.5:1",
"target-chat-opacity:1:1",
"save-character:default",
],
events);

59
tests/AcDream.App.Tests/UI/RetailWindowOpacityControllerTests.cs
View file

@ -185,4 +185,63 @@ public sealed class RetailWindowOpacityControllerTests
// focus change after Dispose is not observed anymore.
Assert.Equal(0.5f, handle.Opacity);
}
[Fact]
public void WindowUnregistered_DetachesSubscription_AndForgetsFocusedState()
{
// CH6c review NIT: before this fix, the only detach point for a
// handle's DescendantFocusChanged subscription (and its membership in
// _focused) was the CONTROLLER's own Dispose — a window unregistered
// while it held focus stayed subscribed and pinned in _focused for
// the rest of the session. Prove the RetailWindowManager.WindowUnregistered
// wiring actually detaches: a stray post-unregister notification (the
// kind a lingering external reference to the handle could still fire)
// must not reach the controller anymore.
UiRoot root = NewRoot();
(RetailWindowHandle handle, UiElement child) = RegisterWindow(root, "Chat");
var controller = new RetailWindowOpacityController(
root.WindowManager, defaultOpacity: 0.5f, activeOpacity: 1.0f);
root.SetKeyboardFocus(child);
Assert.Equal(1.0f, handle.Opacity);
root.WindowManager.Unregister("Chat");
// Unregister hides the outer frame, which drops keyboard focus off
// the now-invisible child — the manager's (still-live at that point)
// focus-change plumbing reapplies DefaultOpacity naturally. Expected
// either way; not itself the thing this test pins.
Assert.Equal(0.5f, handle.Opacity);
controller.SetActiveOpacity(0.7f);
// A stray post-unregister focus-gain notification (the kind a
// lingering external reference to the handle could still fire). If
// the controller were STILL subscribed, this would re-add the stale
// handle to _focused and apply the NEW active opacity (0.7).
handle.NotifyDescendantFocusChanged(child);
Assert.Equal(0.5f, handle.Opacity);
}
[Fact]
public void SetMutators_AfterDispose_AreNoOps()
{
UiRoot root = NewRoot();
(RetailWindowHandle handle, _) = RegisterWindow(root, "Chat");
var controller = new RetailWindowOpacityController(
root.WindowManager, defaultOpacity: 0.5f, activeOpacity: 1.0f);
controller.Dispose();
controller.SetDefaultOpacity(0.9f);
controller.SetActiveOpacity(0.9f);
controller.SetOpacity(0.2f, 0.3f);
// None of the three post-Dispose calls changed anything — no
// ObjectDisposedException either, matching Dispose's own idempotent
// shape.
Assert.Equal(0.5f, controller.DefaultOpacity);
Assert.Equal(1.0f, controller.ActiveOpacity);
Assert.Equal(0.5f, handle.Opacity);
}
}

102
tests/AcDream.App.Tests/UI/UiRenderContextAlphaTests.cs
View file

@ -5,6 +5,7 @@ using AcDream.App.Rendering.Gpu;
using AcDream.App.Tests.Rendering.Gpu;
using AcDream.App.UI;
using DatReaderWriter.Types;
using Xunit.Sdk;
namespace AcDream.App.Tests.UI;
@ -62,6 +63,43 @@ public sealed class UiRenderContextAlphaTests
},
});
/// <summary>Same shape as <see cref="BuildFont"/> but with a non-zero
/// background (outline) atlas, so <see cref="UiRenderContext.DrawStringDat"/>'s
/// <c>outline: true</c> branch actually emits its background sprite pass.</summary>
private static UiDatFont BuildOutlinedFont() => new(
fgTex: 1, fgW: 64, fgH: 64,
bgTex: 2, bgW: 64, bgH: 64,
lineHeight: 16f, baselineOffset: 12f,
glyphs: new Dictionary<char, FontCharDesc>
{
['A'] = new FontCharDesc
{
Unicode = 'A',
Width = 8,
Height = 16,
OffsetX = 0,
OffsetY = 0,
HorizontalOffsetBefore = 0,
HorizontalOffsetAfter = 0,
VerticalOffsetBefore = 0,
},
});
/// <summary>Bakes a real <see cref="BitmapFont"/> from a system TTF so
/// <see cref="UiRenderContext.DrawString"/> (the BitmapFont path, distinct
/// from the dat-font <see cref="UiRenderContext.DrawStringDat"/> above) can
/// be exercised end-to-end. Skips rather than fails on a machine with none
/// of <see cref="BitmapFont.TryLoadSystemMonospaceFont"/>'s well-known
/// paths — matches the skip pattern other environment-dependent tests in
/// this suite already use (e.g. <c>RetailSelectionAssetTests</c>).</summary>
private static BitmapFont BuildBitmapFontOrSkip(IGpuDevice device)
{
byte[]? ttf = BitmapFont.TryLoadSystemMonospaceFont();
if (ttf is null)
throw SkipException.ForSkip("No system TTF font found for BitmapFont construction.");
return new BitmapFont(device, ttf, pixelHeight: 16f);
}
// -- DrawSprite: full-opacity identity ---------------------------------
[Fact]
@ -172,4 +210,68 @@ public sealed class UiRenderContextAlphaTests
var seg = Assert.Single(renderer.DebugSpriteSegments);
Assert.Equal(0.5f, seg.Alpha);
}
[Fact]
public void HalfOpacityWindow_MultipliesDatFontOutlineAndForegroundPassAlpha()
{
// CH6c review NIT: BuildFont() above has bgTex == 0, so every existing
// DrawStringDat alpha test only ever exercised the foreground (fill)
// sprite pass. The background (outline) pass is a SEPARATE
// DrawSpriteAbsolute(applyAlpha: true) call site — this pins it too,
// with a font whose background atlas is actually present (bgTex != 0)
// and outline: true so both passes fire.
(TextRenderer renderer, UiRenderContext ctx) = Build();
UiDatFont font = BuildOutlinedFont();
ctx.PushAlpha(0.5f);
ctx.DrawStringDat(font, "A", 0, 0, new Vector4(1f, 1f, 1f, 1f), outline: true);
ctx.PopAlpha();
// Background pass (texture 2) submitted first, then foreground (texture 1) —
// both routes guarded by the same ApplyAlpha chokepoint.
Assert.Equal(2, renderer.DebugSpriteSegments.Count);
foreach (var seg in renderer.DebugSpriteSegments)
Assert.Equal(0.5f, seg.Alpha);
}
// -- DrawString (BitmapFont path): the same alpha chokepoint, guarded ---
[Fact]
public void FullOpacity_DrawString_BitmapFontPath_MatchesRequestedAlpha_Identity()
{
// CH6c review NIT: DrawStringDat (retail dat-font glyphs) had its own
// alpha regression tests above; UiRenderContext.DrawString — the
// BitmapFont path used for D.6 world-space HUD text — had none. Both
// route through the SAME private ApplyAlpha, but nothing pinned it for
// this path specifically.
var device = new RecordingGpuDevice();
using BitmapFont font = BuildBitmapFontOrSkip(device);
var renderer = new TextRenderer(device, new NullGpuFrameSource(), "unused");
renderer.Begin(new Vector2(800f, 600f));
var ctx = new UiRenderContext(renderer, new Vector2(800f, 600f));
ctx.DrawString("A", 0, 0, new Vector4(1f, 1f, 1f, 1f), font);
(int vertexCount, float alpha) = renderer.DebugTextBuffer;
Assert.True(vertexCount > 0);
Assert.Equal(1f, alpha);
}
[Fact]
public void HalfOpacityWindow_MultipliesBitmapFontGlyphAlpha()
{
var device = new RecordingGpuDevice();
using BitmapFont font = BuildBitmapFontOrSkip(device);
var renderer = new TextRenderer(device, new NullGpuFrameSource(), "unused");
renderer.Begin(new Vector2(800f, 600f));
var ctx = new UiRenderContext(renderer, new Vector2(800f, 600f));
ctx.PushAlpha(0.5f);
ctx.DrawString("A", 0, 0, new Vector4(1f, 1f, 1f, 1f), font);
ctx.PopAlpha();
(int vertexCount, float alpha) = renderer.DebugTextBuffer;
Assert.True(vertexCount > 0);
Assert.Equal(0.5f, alpha);
}
}

15
tests/AcDream.UI.Abstractions.Tests/Panels/Settings/ChatSettingsTests.cs
View file

@ -28,9 +28,18 @@ public sealed class ChatSettingsTests
// Campaign CH slice CH6c: retail's base ChatInterface constructor
// (0x004F4550) sets DefaultOpacity=0.5/ActiveOpacity=1.0 — the value
// every gmFloatyChatUI (the four floating windows) keeps unmodified.
// acdream ships that pair as ONE shared global default (register row
// AP-190), rather than gmMainChatUI's own 1.0/1.0 override.
Assert.Equal(0.5f, d.DefaultOpacity);
// CH6c review fix (2026-08-10): shipping the base pair as acdream's
// ONE shared global default faded every registered window (radar,
// vitals, toolbar, main chat, ...) to 50% opacity out of the box,
// including several that can never take keyboard focus and so were
// permanently half-transparent. acdream now ships gmMainChatUI's own
// 1.0/1.0 override (0x004CD0F0) as the shared global default instead
// — retail-identical opaque presentation for the 11 non-chat windows
// and the main chat window; only the four floating chat windows
// diverge from retail's 0.5-while-idle fade, and the Settings → Chat
// transparency slider remains fully user-settable (register row
// AP-190).
Assert.Equal(1.0f, d.DefaultOpacity);
Assert.Equal(1.0f, d.ActiveOpacity);
}

3
tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsPanelTests.cs
View file

@ -523,7 +523,8 @@ public sealed class SettingsPanelTests
// LAST-rendered opacity slider ("Active", rendered after "Background")
// determines the final draft. That exercises ChatOpacityLink.SetActive's
// drag-background-down path end-to-end through the real panel code,
// starting from ChatSettings.Default (DefaultOpacity=0.5, ActiveOpacity=1.0).
// starting from ChatSettings.Default (DefaultOpacity=1.0, ActiveOpacity=1.0
// as of the CH6c review fix).
var (panel, vm, _, _) = Build();
var r = new FakePanelRenderer
{

9
tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsStoreTests.cs
View file

@ -333,12 +333,17 @@ public sealed class SettingsStoreTests : System.IDisposable
// -- Campaign CH slice CH6c: window opacity round-trip -----------------
[Fact]
public void LoadChat_returns_retail_ChatInterface_opacity_defaults_when_file_is_missing()
public void LoadChat_returns_acdream_opaque_opacity_defaults_when_file_is_missing()
{
// CH6c review fix (2026-08-10): the pre-fix default was retail's base
// ChatInterface value (0.5/1.0), which faded every registered window
// to 50% opacity out of the box. acdream now ships gmMainChatUI's own
// 1.0/1.0 override (0x004CD0F0) as the shared global default — see
// ChatSettings.DefaultOpacity and register row AP-190.
var store = new SettingsStore(_tempPath);
ChatSettings loaded = store.LoadChat();
Assert.Equal(0.5f, loaded.DefaultOpacity);
Assert.Equal(1.0f, loaded.DefaultOpacity);
Assert.Equal(1.0f, loaded.ActiveOpacity);
}