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:
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
|
|
@ -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
|
||||||
|
|
|
||||||
|
|
@ -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;
|
||||||
|
|
|
||||||
|
|
@ -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; }
|
||||||
|
|
|
||||||
|
|
@ -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()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue