fix(chargen): Campaign CC CC7 review fix round — F1-F9 — REVIEW-CLOSED

Both dual-lens reviewers of `9cf6c522`+`ddcbf1fb` returned PASS-with-items.
This round closes all nine findings:

F1 files AP-229 for the screen-layering divergence (retail destroys/
reconstructs the current UI framework via UIFlow::UseNewMode; acdream
keeps both CharacterManagementUiController and CharacterCreationUiController
mounted for the whole lifetime and reveals/occludes) plus its narrow
residual risk (the shared RetailDialogFactory can hand UiRoot.Modal to a
dialog opened by the still-ticking, occluded management screen on an
inbound CharacterError) and what already matches retail (selection/
world-name persistence, click-through isolation, one coherent Modal
stack).

F2 rewrites the connected-gate script's roster-full step with the exact
`@modifylong max_chars_per_account` recipe and the pending-delete-counts
note. F3 adds AP-221's console-diagnostic lines to the known-gaps
paragraph. F4 adds an empty-name/AP-227 step. F9 notes that a uniform
Random pick over 13 heritages can repeat.

F5 adds an App-layer source-text pin
(GameWindowLiveSessionOwnershipTests.LiveSessionRuntimeFactoryBinds
CharacterCreatedAndCreationFailedToTheStatusWriter) for the delegate
wiring the reviewer proved was deletable without breaking any test — no
practical seam exists to construct LiveSessionRuntimeFactory without a
GameWindow, so this follows the file's own established source-text-pin
pattern; the payload shape is already pinned separately at
SessionStatusWriterTests.

F6 corrects the CC7 ledger's checksum-assertion wording (it is a
round-trip purity check, not an independent golden — the golden is
CharacterCreateTests.ComputeChecksum_ExactRetailAccumulationSet) and
cross-references it from the test's own doc comment.

F7 corrects the CC7 ledger's fixture-ordering claim (it had chargen
constructing first, backwards from RetailUiRuntime.Tick's real
management-then-chargen order) and reorders CharacterScreensFixedCanvas
ArbiterTests to match production, adding ClickThrough/ZOrder assertions
that pin the occlusion the reviewer previously verified only by hand.

F8 records a known flake (RuntimeCollisionReportingStateTests.
WarmedSteadyContactRefreshDoesNotAllocate, allocation-assertion load
sensitivity, pre-existing) seen under full-solution parallel load on
both reviewer runs.

Campaign status: all seven slices (CC1-CC7) are REVIEW-CLOSED; the
campaign is CODE-COMPLETE pending the user's own connected gate.

Runtime 1735/0 (unchanged), App 5257/3 skips (+1: the new F5 pin).
Full Release build: 0 warnings, 0 errors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-16 03:13:02 +02:00
parent ddcbf1fbf2
commit 2176ba768e
6 changed files with 175 additions and 19 deletions

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View file

@ -71,6 +71,26 @@ and should be exercised at least once per gate.
and does nothing when clicked. This is retail's own gate and does nothing when clicked. This is retail's own gate
(`gmCharacterManagementUI::UpdateButtons`) — a full roster ghosts (`gmCharacterManagementUI::UpdateButtons`) — a full roster ghosts
Create exactly like Enter/Delete grey out for an unselected row. Create exactly like Enter/Delete grey out for an unselected row.
**A fresh test account is unlikely to be full on its own** (ACE's
default `max_chars_per_account` is 11) — force this state instead of
waiting for it:
1. From the ACE server console (or a GM-privileged in-game `@` command),
run `@modifylong max_chars_per_account 2` to lower the ceiling below
your current roster count.
2. Reconnect (a fresh `CharacterList` only arrives on a new connection —
the client does not re-fetch it live) and confirm Create is now
GREYED OUT.
3. Restore the default afterward: `@modifylong max_chars_per_account 11`,
then reconnect again and confirm Create is enabled once more.
4. **The count includes pending-delete (greyed) characters** — a
character mid-deletion still occupies a roster slot both in ACE's
`GameMessageCharacterList` and in acdream's own gate
(`RuntimeCharacterSelectionState.BuildButtons`'s
`_entries.Length < _slotCount`, which counts every roster entry
regardless of pending-delete state) — matching retail's own
`RebuildCharacterList`, which walks the same full set. If you have a
pending-delete character sitting around, it still counts toward the
ceiling above.
### Leaving the screen (Back-at-Heritage and Exit) ### Leaving the screen (Back-at-Heritage and Exit)
@ -121,7 +141,10 @@ bug — do not report it.
uniformly-picked one of the 13 — this is AP-212's documented uniformly-picked one of the 13 — this is AP-212's documented
approximation (retail's own Heritage-page Random rolls with retail's own approximation (retail's own Heritage-page Random rolls with retail's own
distribution; acdream picks uniformly over every installed heritage). distribution; acdream picks uniformly over every installed heritage).
Not a bug to report unless the button does nothing or crashes. A uniform pick over 13 can land back on the heritage you already have —
click a few times if the first click looks like a no-op; occasional
repeats are expected, not a bug. Not a bug to report unless the button
does nothing or crashes across several clicks.
3. Select **Olthoi** or **OlthoiAcid**. Confirm the Profession, Skills, and 3. Select **Olthoi** or **OlthoiAcid**. Confirm the Profession, Skills, and
Town tabs are hidden (Olthoi variants skip straight to a fixed Custom Town tabs are hidden (Olthoi variants skip straight to a fixed Custom
template with no attribute/skill/town choices) and the screen template with no attribute/skill/town choices) and the screen
@ -194,6 +217,19 @@ other eight (AP-222 — this is a MEASURED gap in acdream's own art, not yet
attributed to a specific missing asset; report clearly if you can visually attributed to a specific missing asset; report clearly if you can visually
compare with retail here). compare with retail here).
**Known session-permanent gap (AP-221) — check the console before
reporting a dead preview.** On an unlucky frame where the DAT/GPU resource
read backing the 3D preview isn't ready at the client's single composition
pass, the Appearance page's zoom/rotate controls can go dead for the rest
of the session (or the Summary page's preview can simply never render),
with no on-screen error — the only evidence is a console line:
`[UI] chargen preview viewport unavailable at composition time...` (or the
Summary-page sibling, `[UI] summary preview viewport unavailable at
composition time...`). If zoom/rotate stop responding or a preview stays
blank, check the console for one of these lines FIRST. If it's there,
restart the client and retry before reporting a bug — this is a known,
already-registered gap, not a new one.
### Town page ### Town page
1. Confirm four town buttons: Holtburg, Shoushi, Yaraq, Sanamar (not id 1. Confirm four town buttons: Holtburg, Shoushi, Yaraq, Sanamar (not id
@ -293,6 +329,23 @@ Already covered in §CC1's "Leaving the screen" section above.
your typing at 32 characters as you go; the rejection only fires on your typing at 32 characters as you go; the rejection only fires on
commit. That is retail-correct, not a bug. commit. That is retail-correct, not a bug.
### Empty name (AP-227, an acdream/retail divergence — expected)
1. On the Summary page, select all the text in the name field and delete
it entirely, then blur the field (click elsewhere) without typing a
replacement.
2. Click **Finish**. Confirm the `NoName`/`ID_CharGen_NoNameWarning` dialog
appears — acdream clears its internal name state the instant the field
is emptied, so Finish sees an empty name and refuses. **This is NOT what
retail does**: retail's own commit handler only acts when the field's
length is greater than 1 (NUL-inclusive, so an empty field's length is
exactly 1) — an emptied-then-blurred field is a silent no-op in retail,
and the character's internal name stays whatever it was BEFORE you
cleared the field, even though the field visually shows empty. A real
retail client would create the character under that old, uncleared name
here instead of showing a dialog. Expect acdream's dialog, not retail's
silent keep-old-name behavior — register AP-227.
### What to report for §CC3 ### What to report for §CC3
- Whether the happy path truly lands you in-world with no intermediate - Whether the happy path truly lands you in-world with no intermediate

View file

@ -135,6 +135,70 @@ public sealed class GameWindowLiveSessionOwnershipTests
Assert.Null(typeof(GameWindow).GetMethod(methodName, PrivateInstance)); Assert.Null(typeof(GameWindow).GetMethod(methodName, PrivateInstance));
} }
/// <summary>
/// Campaign CC CC7 review-fix round, F5 (2026-08-16): the reviewer
/// found that deleting the <c>CharacterCreated</c>/<c>CreationFailed</c>
/// delegate assignments from <see cref="AcDream.App.Net.LiveSessionRuntimeFactory"/>
/// (the App-layer wiring that forwards those two Runtime events to
/// <c>SessionStatusWriter</c>, feeding the launcher's status-payload
/// cycle) leaves every test suite green. <c>LiveSessionRuntimeFactory</c>
/// has exactly one production construction site
/// (<c>SessionPlayerComposition.cs</c>), buried inside the full
/// <c>GameWindow</c> composition graph, and no test in this repository
/// constructs it directly — there is no practical seam to exercise the
/// wiring behaviorally without a <see cref="GameWindow"/>. This test
/// follows the SAME source-text-pin pattern the rest of this file
/// already uses for wiring that can't otherwise be unit-tested
/// (<see cref="ProductionWindowConstructsOnlyTheCanonicalRuntimeRoot"/>,
/// <see cref="DisplacedLifecycleBodiesAreAbsent"/>): it fails if either
/// delegate assignment is removed or its argument mapping changes. The
/// exact PAYLOAD shape these calls must produce is pinned separately,
/// at <c>SessionStatusWriterTests.CharacterCreatedAndCreationFailed_WriteThePinnedShape</c>
/// (<c>tests/AcDream.Runtime.Tests/Session/SessionStatusWriterTests.cs</c>)
/// — together the two tests cover "the delegates are bound" (here) and
/// "they produce §LA1's exact payload" (there).
/// </summary>
[Fact]
public void LiveSessionRuntimeFactoryBindsCharacterCreatedAndCreationFailedToTheStatusWriter()
{
string root = FindRepositoryRoot();
string source = File.ReadAllText(Path.Combine(
root,
"src",
"AcDream.App",
"Net",
"LiveSessionRuntimeFactory.cs"));
Assert.Contains(
"CharacterCreated: identity => _statusWriter.CharacterCreated(",
source,
StringComparison.Ordinal);
Assert.Contains(
"identity.Guid,",
source,
StringComparison.Ordinal);
Assert.Contains(
"identity.Name),",
source,
StringComparison.Ordinal);
Assert.Contains(
"CreationFailed: rejection => _statusWriter.CreationFailed(",
source,
StringComparison.Ordinal);
Assert.Contains(
"rejection.RawCode,",
source,
StringComparison.Ordinal);
Assert.Contains(
"rejection.Reason,",
source,
StringComparison.Ordinal);
Assert.Contains(
"rejection.AttemptedName)),",
source,
StringComparison.Ordinal);
}
private static int CountOccurrences(string source, string value) private static int CountOccurrences(string source, string value)
{ {
int count = 0; int count = 0;

View file

@ -109,6 +109,16 @@ public sealed class CharacterScreensFixedCanvasArbiterTests
// Character-management stays active/visible underneath -- chargen // Character-management stays active/visible underneath -- chargen
// opening on top never deactivates or hides it. // opening on top never deactivates or hides it.
Assert.True(environment.Management.Controller.Root.Visible); Assert.True(environment.Management.Controller.Root.Visible);
// F7 (2026-08-16): pin the occlusion the reviewer verified only by
// hand. Chargen's root is click-opaque over the full authored
// canvas, so no input reaches management underneath it...
Assert.False(environment.Chargen.Controller.Root.ClickThrough);
// ...and BringToFront (called every tick chargen is open, see
// CharacterCreationUiController.Tick) keeps chargen's ZOrder
// strictly above management's, so it paints on top too.
Assert.True(
environment.Chargen.Controller.Root.ZOrder
> environment.Management.Controller.Root.ZOrder);
environment.Chargen.Button(CharacterCreationUiController.ExitElementId) environment.Chargen.Button(CharacterCreationUiController.ExitElementId)
.OnClick!(); .OnClick!();
@ -127,15 +137,26 @@ public sealed class CharacterScreensFixedCanvasArbiterTests
public TwoControllerHarness() public TwoControllerHarness()
{ {
Host = new UiRoot { Width = 800f, Height = 600f }; Host = new UiRoot { Width = 800f, Height = 600f };
// Campaign CC slice CC7: chargen must exist FIRST so // Campaign CC slice CC7 review-fix round, F7 (2026-08-16):
// ManagementHarness can wire its Create button straight to the // construct in PRODUCTION order — management, then chargen —
// real CharacterCreationUiController.Open() — the same shape // matching RetailUiRuntime.Tick's real sequence
// RetailUiRuntime.ConfigureCharacterManagement() uses in // (`_characterManagementMount?.Tick(); ...
// production (a lazily-resolved lambda closing over the OTHER // _characterCreationMount?.Tick(); CharacterCreationController
// controller, since bindings are always built before both // ?.Tick();`). An earlier version of this fixture built chargen
// controllers exist). // FIRST and claimed that matched production; it did not — it
// was the opposite order. Management still needs a RequestCreate
// callback before Chargen's Controller exists, so this closure
// resolves Chargen lazily per-call, the SAME trick production's
// own RetailUiRuntime.ConfigureCharacterManagement uses
// (`bindings with { RequestCreate = () =>
// CharacterCreationController?.Open() }`, closing over `this`
// rather than capturing a not-yet-built controller).
// Chargen is assigned below, before this closure can ever be
// invoked (Management's own ctor only stores the callback, it
// does not call it) -- the null-forgiving operator documents
// that ordering guarantee for the nullable analyzer.
Management = new ManagementHarness(Host, () => Chargen!.Controller.Open());
Chargen = new ChargenHarness(Host); Chargen = new ChargenHarness(Host);
Management = new ManagementHarness(Host, Chargen.Controller.Open);
} }
public UiRoot Host { get; } public UiRoot Host { get; }

View file

@ -430,10 +430,22 @@ public sealed class LiveSessionControllerCharacterCreationTests
/// test above never checked — against exactly the shape /// test above never checked — against exactly the shape
/// <c>CharacterCreateInfo.Unpack</c>/<c>Appearance.Unpack</c> parse (see /// <c>CharacterCreateInfo.Unpack</c>/<c>Appearance.Unpack</c> parse (see
/// <see cref="CharacterCreate"/>'s own doc comment for the ACE /// <see cref="CharacterCreate"/>'s own doc comment for the ACE
/// cross-reference). The checksum is recomputed via the SAME production /// cross-reference).
/// <see cref="CharacterCreate.ComputeChecksum"/> formula rather than
/// re-deriving the sum a second time by hand in the test.
/// </summary> /// </summary>
/// <remarks>
/// Campaign CC CC7 review-fix round, F6 (2026-08-16): the checksum
/// assertion below (<c>Assert.Equal(CharacterCreate.ComputeChecksum(r),
/// decoded.Checksum)</c>) is a ROUND-TRIP/PURITY check, not an
/// independent golden — it recomputes the SAME production
/// <see cref="CharacterCreate.ComputeChecksum"/> formula the encode side
/// already used, rather than re-deriving the sum a second time by hand,
/// so it proves the wire-encode/decode round trip is lossless but
/// cannot by itself catch a bug shared by both the encoder and this
/// formula. The checksum's actual golden value (the 19-term retail
/// accumulation set, hand-summed to <c>205u</c>) is pinned separately
/// at <c>CharacterCreateTests.ComputeChecksum_ExactRetailAccumulationSet</c>
/// (<c>tests/AcDream.Core.Net.Tests/Messages/CharacterCreateTests.cs</c>).
/// </remarks>
[Fact] [Fact]
public void Finish_SendsEveryWireFieldByteExactAgainstACEsUnpackShape() public void Finish_SendsEveryWireFieldByteExactAgainstACEsUnpackShape()
{ {