fix(app,headless): Campaign CC slice CC4 re-review round — R1 arbiter + R2-R4
CC4 re-review returned NOT CLOSED: R1 (MEDIUM, blocking) is a new residual the F1 fix itself introduced, plus three LOW riders (R2, R3, R4). R1 — nulling UiRoot.FixedCanvasSize on chargen Close() stripped it from character-management, which stays active underneath and only sets the canvas on its own activation edge. Root cause (reviewer-named): two controllers writing one host-global with no owner. Fixed with the root-cause shape (reviewer's option (c)): UiRoot.DeclareFixedCanvas(owner, size)/RevokeFixedCanvas(owner), an owner-scoped arbiter — every declarer must agree on the canvas size (a mismatch throws instead of silently last-writer-wins), and the canvas nulls only once EVERY declarer has revoked. Both CharacterCreationUiController and CharacterManagementUi- Controller now declare/revoke instead of writing FixedCanvasSize directly; grepped for stragglers, none remain in production code (the raw setter stays public only for UiRootFixedCanvasTests' isolated scale-math coverage). New test (reviewer-specified): CharacterScreensFixedCanvasArbiterTests — two controllers sharing one UiRoot, proving the canvas stays set through chargen's Exit-confirm Close while char-management is still active, nulling only once char-management also deactivates, plus the original F1 defect's own covering case (both revoke together at world entry). R3 — HeadlessSessionHostTests.ContentLease_InstallsRealChargenOptions_ SelectHeritageIsAccepted proves F6's install actually opens the gate: a content lease carrying a real hand-built DatCharGen heritage (not ChargenOptions.Empty) is installed, and TrySelectHeritage for it succeeds. R2 — filed docs/ISSUES.md #402 for the pre-existing Streaming.LandblockBuildFactoryTests.Build_UsesTheSuppliedSharedReaderGate full-suite flake (unrelated to Campaign CC). R4 — fixed "unchached" -> "uncached" typo in InteractionRetainedUiComposition.cs. Runtime 1713/0, App 5127/13 skips (+2), Headless 166/0 (+1), full solution Release build green. Live-DAT probes 7/7 under ACDREAM_PROBE_LIVE_MOUNT=1. The known #402 flake did not fire across 3 consecutive full-suite runs this session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
ec854db045
commit
8add0667a5
9 changed files with 780 additions and 29 deletions
|
|
@ -650,7 +650,7 @@ internal sealed class RetailInteractionRetainedUiCompositionFactory
|
|||
// ResolveText several times per Refresh, and CharacterCreation-
|
||||
// UiController.ApplyProgressState forces a full refresh on
|
||||
// every page switch (`_lastRevision = long.MinValue`) — so an
|
||||
// unchached resolver meant several fresh allocations + DatLock
|
||||
// uncached resolver meant several fresh allocations + DatLock
|
||||
// acquisitions per click. DatStringResolver's own constructor
|
||||
// does no DAT I/O (only .Resolve reads), so building it here
|
||||
// outside the lock matches this file's existing pattern
|
||||
|
|
|
|||
|
|
@ -201,16 +201,15 @@ internal sealed class CharacterCreationUiController : IDisposable
|
|||
Root.ClickThrough = false;
|
||||
Root.Visible = false;
|
||||
// AD-98: the same authored 800x600 fixed-canvas treatment as the
|
||||
// character-management screen. CharacterManagementUiController sets
|
||||
// UiRoot.FixedCanvasSize ONCE on its own activation edge
|
||||
// (Tick's `if (!_active)` arm) and NULLS it in both Deactivate AND
|
||||
// Dispose — it is NOT a per-tick set, and this controller must be
|
||||
// symmetric with that exact shape (review fix round F1, 2026-08-15
|
||||
// — the earlier claim here that it was safe to leave the canvas
|
||||
// pinned forever was FALSE and left an 800x600-scaled canvas
|
||||
// covering the in-world UI whenever this screen had been opened).
|
||||
// See Open/Close/Deactivate/Dispose below for the matching set/null
|
||||
// pair.
|
||||
// character-management screen. Both controllers DECLARE/REVOKE
|
||||
// through UiRoot's owner-scoped arbiter (review fix round R1,
|
||||
// 2026-08-15) rather than writing UiRoot.FixedCanvasSize directly —
|
||||
// char-management can be simultaneously active underneath this
|
||||
// screen, and a raw write from either controller is a last-writer-
|
||||
// wins race with no owner (the F1 fix's own Close() null wiped
|
||||
// char-management's still-active canvas out from under it). See
|
||||
// Open/Close/Deactivate/Dispose below for the matching declare/
|
||||
// revoke pair.
|
||||
_authoredCanvas = new Vector2(
|
||||
Root.Width > 0f ? Root.Width : 800f,
|
||||
Root.Height > 0f ? Root.Height : 600f);
|
||||
|
|
@ -380,17 +379,19 @@ internal sealed class CharacterCreationUiController : IDisposable
|
|||
|
||||
/// <summary>Opens the screen at retail's authored default page
|
||||
/// (<c>gmCharGenMainUI::gmCharGenMainUI</c>'s trailing
|
||||
/// <c>SetProgressState(this, ECG_HERTAGE)</c>). Sets the fixed canvas
|
||||
/// on this exact activation edge — matching
|
||||
/// <see cref="CharacterManagementUiController"/>'s own one-shot set —
|
||||
/// not per-tick; <see cref="Close"/>/<see cref="Deactivate"/>/
|
||||
/// <see cref="Dispose"/> null it back out symmetrically.</summary>
|
||||
/// <c>SetProgressState(this, ECG_HERTAGE)</c>). Declares the fixed
|
||||
/// canvas on this exact activation edge through <see cref="UiRoot"/>'s
|
||||
/// arbiter — matching <see cref="CharacterManagementUiController"/>'s
|
||||
/// own one-shot declare — not per-tick; <see cref="Close"/>/
|
||||
/// <see cref="Deactivate"/>/<see cref="Dispose"/> revoke it back out
|
||||
/// symmetrically, and the canvas stays set for as long as ANY other
|
||||
/// declarer (e.g. character-management underneath) remains active.</summary>
|
||||
internal void Open()
|
||||
{
|
||||
if (_disposed)
|
||||
return;
|
||||
_isOpen = true;
|
||||
_host.FixedCanvasSize = _authoredCanvas;
|
||||
_host.DeclareFixedCanvas(this, _authoredCanvas);
|
||||
ApplyProgressState(Page.Heritage);
|
||||
}
|
||||
|
||||
|
|
@ -400,7 +401,7 @@ internal sealed class CharacterCreationUiController : IDisposable
|
|||
return;
|
||||
_isOpen = false;
|
||||
Root.Visible = false;
|
||||
_host.FixedCanvasSize = null;
|
||||
_host.RevokeFixedCanvas(this);
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
|
|
@ -415,9 +416,11 @@ internal sealed class CharacterCreationUiController : IDisposable
|
|||
finally
|
||||
{
|
||||
// Matches CharacterManagementUiController.Dispose's own
|
||||
// unconditional null — defends against disposing while _isOpen
|
||||
// (Close() is not otherwise called on this path).
|
||||
_host.FixedCanvasSize = null;
|
||||
// unconditional revoke — defends against disposing while
|
||||
// _isOpen (Close() is not otherwise called on this path). Idle
|
||||
// if Close() already revoked (RevokeFixedCanvas is a no-op for
|
||||
// an owner that already revoked).
|
||||
_host.RevokeFixedCanvas(this);
|
||||
_back.OnClick = null;
|
||||
_next.OnClick = null;
|
||||
_finish.OnClick = null;
|
||||
|
|
|
|||
|
|
@ -114,9 +114,13 @@ internal sealed class CharacterManagementUiController : IDisposable
|
|||
// while this screen is active, the host stretches the ENTIRE canvas —
|
||||
// widgets, glyphs, and the painted background (which carries the
|
||||
// "World"/"Characters" captions as art) — as one unit via
|
||||
// UiRoot.FixedCanvasSize. Resizing the root here instead is exactly the
|
||||
// half-substitution that misaligned the widgets against the stretched
|
||||
// art at the 2026-08-15 user gate.
|
||||
// UiRoot.FixedCanvasSize, declared/revoked through the owner-scoped
|
||||
// arbiter (review fix round R1, 2026-08-15) rather than written
|
||||
// directly — character-creation can be simultaneously active on top
|
||||
// of this screen, and a raw write from either controller is a last-
|
||||
// writer-wins race with no owner. Resizing the root here instead of
|
||||
// using the canvas is exactly the half-substitution that misaligned
|
||||
// the widgets against the stretched art at the 2026-08-15 user gate.
|
||||
_authoredCanvas = new Vector2(
|
||||
Root.Width > 0f ? Root.Width : 800f,
|
||||
Root.Height > 0f ? Root.Height : 600f);
|
||||
|
|
@ -298,7 +302,7 @@ internal sealed class CharacterManagementUiController : IDisposable
|
|||
{
|
||||
_active = true;
|
||||
Root.Visible = true;
|
||||
_host.FixedCanvasSize = _authoredCanvas;
|
||||
_host.DeclareFixedCanvas(this, _authoredCanvas);
|
||||
_host.BringToFront(Root);
|
||||
}
|
||||
|
||||
|
|
@ -368,7 +372,7 @@ internal sealed class CharacterManagementUiController : IDisposable
|
|||
}
|
||||
finally
|
||||
{
|
||||
_host.FixedCanvasSize = null;
|
||||
_host.RevokeFixedCanvas(this);
|
||||
_enter.OnClick = null;
|
||||
_delete.OnClick = null;
|
||||
_restore.OnClick = null;
|
||||
|
|
@ -752,7 +756,7 @@ internal sealed class CharacterManagementUiController : IDisposable
|
|||
{
|
||||
_active = false;
|
||||
Root.Visible = false;
|
||||
_host.FixedCanvasSize = null;
|
||||
_host.RevokeFixedCanvas(this);
|
||||
}
|
||||
foreach (UiButton row in _rows)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Numerics;
|
||||
|
||||
namespace AcDream.App.UI;
|
||||
|
|
@ -41,9 +42,94 @@ public sealed class UiRoot : UiElement
|
|||
/// renderer's quad chokepoint; the mouse entry points apply the inverse,
|
||||
/// so <see cref="MouseX"/>/<see cref="MouseY"/> and every hit test live
|
||||
/// in canvas space. Null (the in-world default) is native 1:1.
|
||||
///
|
||||
/// <para>
|
||||
/// Campaign CC slice CC4 review-fix round R1 (2026-08-15): this raw
|
||||
/// setter remains public for tests that exercise the scale/mouse-
|
||||
/// mapping math in isolation (<c>UiRootFixedCanvasTests</c>), but
|
||||
/// PRODUCTION code must go through <see cref="DeclareFixedCanvas"/>/
|
||||
/// <see cref="RevokeFixedCanvas"/> instead of writing this property
|
||||
/// directly. Two fixed-canvas screens can be active at once
|
||||
/// (character-management underneath character-creation) and a raw
|
||||
/// write from either one is a last-writer-wins race with no owner —
|
||||
/// the F1 fix's own <c>Close()</c> null wiped the OTHER screen's still-
|
||||
/// active canvas out from under it (see AD-98).
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public Vector2? FixedCanvasSize { get; set; }
|
||||
|
||||
/// <summary>Screens currently declaring a fixed canvas, keyed by owner
|
||||
/// (see <see cref="DeclareFixedCanvas"/>).</summary>
|
||||
private readonly Dictionary<object, Vector2> _fixedCanvasDeclarations = new();
|
||||
|
||||
/// <summary>
|
||||
/// Declares that <paramref name="owner"/> wants the retained tree laid
|
||||
/// out in <paramref name="size"/> while it is active. This is the single
|
||||
/// arbiter for <see cref="FixedCanvasSize"/>: multiple owners may declare
|
||||
/// concurrently (character-management stays declared while character-
|
||||
/// creation is also open on top of it), and the effective
|
||||
/// <see cref="FixedCanvasSize"/> is the shared declaration set's value.
|
||||
/// Every current declarer must agree on the size — a mismatched second
|
||||
/// declaration throws rather than silently overwriting the first
|
||||
/// (Campaign CC CC4 review-fix round R1, 2026-08-15; see
|
||||
/// <c>docs/architecture/retail-divergence-register.md</c> AD-98). Pair
|
||||
/// every call with <see cref="RevokeFixedCanvas"/> on the SAME owner at
|
||||
/// deactivate/close/dispose.
|
||||
/// </summary>
|
||||
public void DeclareFixedCanvas(object owner, Vector2 size)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(owner);
|
||||
if (_fixedCanvasDeclarations.TryGetValue(owner, out Vector2 existing))
|
||||
{
|
||||
if (existing == size)
|
||||
return; // idempotent re-declare (e.g. a re-ticked activation edge)
|
||||
throw new InvalidOperationException(
|
||||
$"UiRoot.DeclareFixedCanvas: owner {owner} re-declared a different " +
|
||||
$"canvas ({existing} -> {size}) without revoking first.");
|
||||
}
|
||||
|
||||
foreach (Vector2 declared in _fixedCanvasDeclarations.Values)
|
||||
{
|
||||
if (declared != size)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
$"UiRoot.DeclareFixedCanvas: owner {owner} declared {size} but " +
|
||||
$"another active owner already declared {declared} — every " +
|
||||
"concurrently-active fixed-canvas screen must author the SAME " +
|
||||
"canvas size (see AD-98).");
|
||||
}
|
||||
}
|
||||
|
||||
_fixedCanvasDeclarations[owner] = size;
|
||||
FixedCanvasSize = size;
|
||||
}
|
||||
|
||||
/// <summary>Revokes <paramref name="owner"/>'s declaration from
|
||||
/// <see cref="DeclareFixedCanvas"/>. <see cref="FixedCanvasSize"/>
|
||||
/// becomes null only once EVERY declarer has revoked; while another
|
||||
/// owner is still declared, it stays set to that shared value. A
|
||||
/// revoke from an owner that never declared (or already revoked) is a
|
||||
/// no-op, matching the idempotent shutdown paths (<c>Deactivate</c>
|
||||
/// AND <c>Dispose</c> can both revoke the same owner).</summary>
|
||||
public void RevokeFixedCanvas(object owner)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(owner);
|
||||
if (!_fixedCanvasDeclarations.Remove(owner))
|
||||
return;
|
||||
|
||||
if (_fixedCanvasDeclarations.Count == 0)
|
||||
{
|
||||
FixedCanvasSize = null;
|
||||
return;
|
||||
}
|
||||
|
||||
foreach (Vector2 declared in _fixedCanvasDeclarations.Values)
|
||||
{
|
||||
FixedCanvasSize = declared;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The coordinate space the retained tree currently lays out in: the fixed
|
||||
/// authored canvas while one is active, else the window itself. Anything
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue