fix(app,runtime,headless): Campaign CC slice CC4 review-fix round — F1-F12
Dual-lens review of CC4 (0e71d3b8) returned architectural FAIL (F1, F6)
and retail-fidelity PASS-with-reservations (F2, F3, F4), plus LOW
findings F5, F7-F12. F13 (TS-82's merge collision with campaign-cc6a) is
merge mechanics for the orchestrator, not addressed here.
F1 (HIGH, blocking): CharacterCreationUiController never released
UiRoot.FixedCanvasSize, on a FALSE premise that CharacterManagementUi-
Controller does a per-tick set (it does not — it sets once on activation
and nulls on Deactivate/Dispose). Root cause: RuntimeCharacterCreation-
State had no CompleteEnter() analogue to RuntimeCharacterSelectionState's,
so the creation view reported IsActive=true for an entire in-world
session. Added CompleteEnter(), wired at both LiveSessionController
in-world edges (StartCore, EnterHighlightedCore); made Open/Close/
Deactivate/Dispose set/null the canvas symmetrically; corrected the false
comment and ledger claim; added FixedCanvasSize test coverage.
F2 (MEDIUM-HIGH, blocking): the attribute-slider scalar mapping was not
retail's. Fixed display to value/100f (UpdateAttributeValues @
0x0048251d) and the drag inverse to truncate+clamp-low-only, no rescale
(ListenToElementMessage @ 0x004829c0, independently re-verified against
the decomp). Added tests at scalar 0.5/0.0 plus a display-direction test.
F3 (MEDIUM, blocking): ported the unported heritage-button tab-restore
arm (ListenToElementMessage @ 0x004e9450) — SHOW/HIDE id sets independently
re-derived from the decomp, including the genuine Lugian (0x100005f1)
no-restore quirk, reproduced faithfully. Wired via a new HeritagePage
click callback; added restore + quirk tests.
F4 (MEDIUM): ported SetTown's (@ 0x0047c360) separate per-town page-root
state literal (Holtburg->0x10000034 etc.), independently re-derived from
the decomp's tail-merged branches; wired via the existing
IUiDatStateful.TrySetRetailState seam; added a test.
F5 (MEDIUM): softened AD-103's unmeasured pixel-equivalence claim.
F6 (MEDIUM, blocking): DECISION — install ChargenOptions in the headless
content path (chosen over marking headless creation out-of-scope).
HeadlessSessionHost now calls InstallOptions off the shared content
lease's Dats, beside the existing InstallSpellMetadata call.
F7: AP-213 already named the label format and click/double-click
substitution explicitly on inspection — no edit needed.
F8: AP-212 now names all six DoRandom primitives with a known landing site.
F9: AD-101 retirement corrected to precede CC5's Finish un-ghosting.
F10: merged ItemAppraisalTextFormatter's duplicate <summary> block.
F11: fixed TS-82's wrong AP-211 cross-reference.
F12: cached the chargen DatStringResolver once per composition instead of
per ResolveText call.
Runtime 1713/0, App 5125/13 skips (+8 new tests), Headless 165/0, full
solution Release build green. Live-DAT probes 7/7 under
ACDREAM_PROBE_LIVE_MOUNT=1.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
0e71d3b829
commit
ec854db045
12 changed files with 419 additions and 30 deletions
|
|
@ -643,6 +643,19 @@ internal sealed class RetailInteractionRetainedUiCompositionFactory
|
|||
d.DebugFont,
|
||||
controls,
|
||||
iconComposer);
|
||||
// Review fix round F12 (2026-08-15): constructed ONCE per
|
||||
// composition and captured by the ResolveText closure below,
|
||||
// rather than a fresh DatStringResolver per lookup. The
|
||||
// Heritage/Town pages' description composers each call
|
||||
// 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
|
||||
// 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
|
||||
// elsewhere (construct once, lock only around Resolve calls).
|
||||
var characterCreationStrings = new DatStringResolver(d.Dats);
|
||||
var bindings = new RetailUiRuntimeBindings(
|
||||
Host: host,
|
||||
Assets: assets,
|
||||
|
|
@ -987,7 +1000,7 @@ internal sealed class RetailInteractionRetainedUiCompositionFactory
|
|||
{
|
||||
lock (d.DatLock)
|
||||
{
|
||||
return new DatStringResolver(d.Dats).Resolve(
|
||||
return characterCreationStrings.Resolve(
|
||||
0x23000002u,
|
||||
DatStringResolver.ComputeHash(key));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -73,21 +73,35 @@ internal sealed class CharacterCreationHeritagePage : IDisposable
|
|||
};
|
||||
|
||||
private readonly CharacterCreationRuntimeBindings _bindings;
|
||||
private readonly Action<uint> _onButtonClicked;
|
||||
private readonly Dictionary<UiButton, uint> _buttons = [];
|
||||
private readonly UiText? _description;
|
||||
private bool _disposed;
|
||||
|
||||
/// <param name="onButtonClicked">Review fix round F3 (2026-08-15):
|
||||
/// invoked with the RAW button element id (not the resolved heritage
|
||||
/// id) on every heritage-button click, before <see cref="Select"/>
|
||||
/// runs — mirrors retail's message bubbling from
|
||||
/// <c>gmCGHeritagePage::ListenToElementMessage</c> up to
|
||||
/// <c>gmCharGenMainUI::ListenToElementMessage</c>'s own tab-restore
|
||||
/// arm, which is keyed on the same raw id.</param>
|
||||
internal CharacterCreationHeritagePage(
|
||||
UiElement pageRoot,
|
||||
CharacterCreationRuntimeBindings bindings)
|
||||
CharacterCreationRuntimeBindings bindings,
|
||||
Action<uint> onButtonClicked)
|
||||
{
|
||||
_bindings = bindings;
|
||||
_onButtonClicked = onButtonClicked;
|
||||
foreach ((uint buttonId, uint heritageId) in HeritageByButtonId)
|
||||
{
|
||||
if (UiElement.FindDescendant(pageRoot, buttonId) is not UiButton button)
|
||||
continue;
|
||||
_buttons[button] = heritageId;
|
||||
button.OnClick = () => Select(heritageId);
|
||||
button.OnClick = () =>
|
||||
{
|
||||
_onButtonClicked(buttonId);
|
||||
Select(heritageId);
|
||||
};
|
||||
}
|
||||
|
||||
_description = UiElement.FindDescendant(pageRoot, 0x100003C4u) as UiText;
|
||||
|
|
|
|||
|
|
@ -147,8 +147,12 @@ internal sealed class CharacterCreationProfessionPage : IDisposable
|
|||
foreach ((ChargenAttributeId attribute, SliderWidgets widgets) in _sliders)
|
||||
{
|
||||
int value = GetAttribute(snapshot.Attributes, attribute);
|
||||
float scalar = (value - ChargenAttributeMath.AttributeMin)
|
||||
/ (float)(ChargenAttributeMath.AttributeMax - ChargenAttributeMath.AttributeMin);
|
||||
// gmCGProfessionPage::UpdateAttributeValues @ 0x0048251d:
|
||||
// SetAttribute_Float(pSlider, 0x86, value * 0.00999999978f) —
|
||||
// scalar = value/100, NOT (value-AttributeMin)/(AttributeMax-
|
||||
// AttributeMin). Review fix round F2 (2026-08-15): the earlier
|
||||
// [10,100]<->[0,1] normalization here did not match retail.
|
||||
float scalar = value / 100f;
|
||||
widgets.Slider?.SetScalarPosition(scalar);
|
||||
widgets.Value?.SetText(value.ToString(CultureInfo.InvariantCulture));
|
||||
if (widgets.Lock is { } lockButton)
|
||||
|
|
@ -211,14 +215,21 @@ internal sealed class CharacterCreationProfessionPage : IDisposable
|
|||
_bindings.SelectTemplate(templateIndex);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// gmCGProfessionPage::ListenToElementMessage @ 0x004829c0, the
|
||||
/// scrollbar-drag case (relative id 0x100002ee, idMessage 0xa):
|
||||
/// <c>ebx = _ftol2(param*100); if (ebx < 0xa) ebx = 0xa;
|
||||
/// SetAttribValue(this, parent, ebx)</c> — truncate (not round) the
|
||||
/// scalar times 100, clamp LOW only to 10, with NO upper clamp/rescale.
|
||||
/// Review fix round F2 (2026-08-15): the earlier
|
||||
/// AttributeMin+Round(scalar*(Max-Min)) formula here did not match
|
||||
/// retail (it only happened to agree with retail at scalar=1).
|
||||
/// </summary>
|
||||
private void SetAttributeFromScalar(ChargenAttributeId attribute, float scalar)
|
||||
{
|
||||
if (_disposed)
|
||||
return;
|
||||
int value = ChargenAttributeMath.AttributeMin
|
||||
+ (int)MathF.Round(
|
||||
scalar * (ChargenAttributeMath.AttributeMax - ChargenAttributeMath.AttributeMin),
|
||||
MidpointRounding.AwayFromZero);
|
||||
int value = Math.Max(ChargenAttributeMath.AttributeMin, (int)(scalar * 100f));
|
||||
_bindings.SetAttribute(attribute, value);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -40,7 +40,32 @@ internal sealed class CharacterCreationTownPage : IDisposable
|
|||
[3] = "ID_CharGen_SanamarText",
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// Start-area index -> the page's OWN retail state literal — a
|
||||
/// SEPARATE state machine from <c>CharacterCreationUiController</c>'s
|
||||
/// master-page per-page-index cycling
|
||||
/// (<c>0x10000025 + (page - 1)</c>). <c>gmCGTownPage::SetTown @
|
||||
/// 0x0047c360</c> calls <c>this->vtable->SetState(...)</c> (the
|
||||
/// gmCGTownPage/page-root object itself) with these four literals
|
||||
/// verbatim, alongside the per-button highlight state — note these do
|
||||
/// NOT sit in button/startArea numeric order: Holtburg->0x10000034,
|
||||
/// Shoushi->0x10000037, Yaraq->0x10000036, Sanamar->0x10000035.
|
||||
/// Re-asserted directly (inlined, bypassing SetTown) at the Sanamar
|
||||
/// click site @0x0047c518. Review fix round F4 (2026-08-15): only the
|
||||
/// master page's state cycling was ported — this page's own state was
|
||||
/// missed entirely.
|
||||
/// </summary>
|
||||
private static readonly IReadOnlyDictionary<int, uint> PageStateByStartArea =
|
||||
new Dictionary<int, uint>
|
||||
{
|
||||
[0] = 0x10000034u, // Holtburg
|
||||
[1] = 0x10000037u, // Shoushi
|
||||
[2] = 0x10000036u, // Yaraq
|
||||
[3] = 0x10000035u, // Sanamar
|
||||
};
|
||||
|
||||
private readonly CharacterCreationRuntimeBindings _bindings;
|
||||
private readonly UiElement _pageRoot;
|
||||
private readonly Dictionary<UiButton, int> _buttons = [];
|
||||
private readonly UiText? _description;
|
||||
private bool _disposed;
|
||||
|
|
@ -50,6 +75,7 @@ internal sealed class CharacterCreationTownPage : IDisposable
|
|||
CharacterCreationRuntimeBindings bindings)
|
||||
{
|
||||
_bindings = bindings;
|
||||
_pageRoot = pageRoot;
|
||||
foreach ((uint buttonId, int startArea) in StartAreaByButtonId)
|
||||
{
|
||||
if (UiElement.FindDescendant(pageRoot, buttonId) is not UiButton button)
|
||||
|
|
@ -68,6 +94,12 @@ internal sealed class CharacterCreationTownPage : IDisposable
|
|||
foreach ((UiButton button, int startArea) in _buttons)
|
||||
button.Selected = startArea == snapshot.StartArea;
|
||||
|
||||
if (PageStateByStartArea.TryGetValue(snapshot.StartArea, out uint pageStateId)
|
||||
&& _pageRoot is IUiDatStateful stateful)
|
||||
{
|
||||
stateful.TrySetRetailState(pageStateId);
|
||||
}
|
||||
|
||||
if (_description is null)
|
||||
return;
|
||||
|
||||
|
|
|
|||
|
|
@ -201,18 +201,21 @@ 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 — see that controller's own comment.
|
||||
// Both screens author the identical extent, so it is safe for both
|
||||
// controllers to independently (idempotently) push the SAME value
|
||||
// to the shared UiRoot.FixedCanvasSize; this controller therefore
|
||||
// never NULLS it back out on close (see Deactivate/Close), leaving
|
||||
// char-management's own per-tick set as the surviving owner once
|
||||
// this screen is not the active one.
|
||||
// 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.
|
||||
_authoredCanvas = new Vector2(
|
||||
Root.Width > 0f ? Root.Width : 800f,
|
||||
Root.Height > 0f ? Root.Height : 600f);
|
||||
|
||||
_heritagePage = new CharacterCreationHeritagePage(heritagePageRoot, bindings);
|
||||
_heritagePage = new CharacterCreationHeritagePage(heritagePageRoot, bindings, ApplyHeritageTabRestore);
|
||||
_professionPage = new CharacterCreationProfessionPage(professionPageRoot, bindings);
|
||||
_skillsPage = new CharacterCreationSkillsPage(skillsPageRoot, bindings, templateResolver);
|
||||
_townPage = new CharacterCreationTownPage(townPageRoot, bindings);
|
||||
|
|
@ -354,7 +357,6 @@ internal sealed class CharacterCreationUiController : IDisposable
|
|||
if (_isOpen)
|
||||
{
|
||||
Root.Visible = true;
|
||||
_host.FixedCanvasSize = _authoredCanvas;
|
||||
_host.BringToFront(Root);
|
||||
}
|
||||
else
|
||||
|
|
@ -378,19 +380,27 @@ 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>).</summary>
|
||||
/// <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>
|
||||
internal void Open()
|
||||
{
|
||||
if (_disposed)
|
||||
return;
|
||||
_isOpen = true;
|
||||
_host.FixedCanvasSize = _authoredCanvas;
|
||||
ApplyProgressState(Page.Heritage);
|
||||
}
|
||||
|
||||
private void Close()
|
||||
{
|
||||
if (!_isOpen)
|
||||
return;
|
||||
_isOpen = false;
|
||||
Root.Visible = false;
|
||||
_host.FixedCanvasSize = null;
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
|
|
@ -404,6 +414,10 @@ 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;
|
||||
_back.OnClick = null;
|
||||
_next.OnClick = null;
|
||||
_finish.OnClick = null;
|
||||
|
|
@ -609,6 +623,60 @@ internal sealed class CharacterCreationUiController : IDisposable
|
|||
stateful.TrySetRetailState(stateId);
|
||||
}
|
||||
|
||||
// ── Heritage tab-restore (gmCharGenMainUI::ListenToElementMessage @ ────
|
||||
// ── 0x004e9450, the heritage-button bubble arm) ─────────────────────
|
||||
|
||||
/// <summary>SHOW ids (label_4e9673, three <c>SetVisible(1)</c> calls) —
|
||||
/// verbatim off the decompiled switch's case list at
|
||||
/// <c>0x004e9450</c>.</summary>
|
||||
private static readonly IReadOnlySet<uint> HeritageTabShowButtonIds = new HashSet<uint>
|
||||
{
|
||||
0x100003BFu, 0x100003C1u, 0x100003C2u, 0x100003C3u,
|
||||
0x10000590u, 0x10000591u, 0x100005A9u, 0x100005BFu,
|
||||
0x100005C4u, 0x100005E8u,
|
||||
};
|
||||
|
||||
/// <summary>HIDE ids (@0x004e96b9, three <c>SetVisible(0)</c> calls) —
|
||||
/// the Olthoi/OlthoiAcid heritage buttons.</summary>
|
||||
private static readonly IReadOnlySet<uint> HeritageTabHideButtonIds = new HashSet<uint>
|
||||
{
|
||||
0x100005C7u, 0x100005C8u,
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// Ports <c>gmCharGenMainUI::ListenToElementMessage @ 0x004e9450</c>'s
|
||||
/// heritage-button tab-restore arm: heritage-button clicks bubble to
|
||||
/// the master shell and SYNCHRONOUSLY show/hide the Profession/Skills/
|
||||
/// Town tabs, independent of <see cref="ApplyProgressState"/>'s own
|
||||
/// tab-visibility recompute at page-switch time (that recompute only
|
||||
/// runs when Back/Next/a tab is clicked — not on every heritage pick).
|
||||
/// Retail quirk reproduced faithfully: Lugian's button id
|
||||
/// (<c>0x100005f1</c>) sits OUTSIDE both the SHOW and HIDE case lists
|
||||
/// in the decompiled switch, so clicking Lugian neither restores nor
|
||||
/// hides the tabs — a genuine retail bug (the tabs stay in whatever
|
||||
/// state the PREVIOUS heritage selection left them), not an acdream
|
||||
/// omission. Review fix round F3 (2026-08-15): this arm was entirely
|
||||
/// unported — before this fix, selecting a human heritage right after
|
||||
/// Olthoi/OlthoiAcid left the tabs hidden until the next Back/Next/tab
|
||||
/// click recomputed them.
|
||||
/// </summary>
|
||||
private void ApplyHeritageTabRestore(uint buttonElementId)
|
||||
{
|
||||
if (HeritageTabShowButtonIds.Contains(buttonElementId))
|
||||
{
|
||||
_professionTab.Visible = true;
|
||||
_skillsTab.Visible = true;
|
||||
_townTab.Visible = true;
|
||||
}
|
||||
else if (HeritageTabHideButtonIds.Contains(buttonElementId))
|
||||
{
|
||||
_professionTab.Visible = false;
|
||||
_skillsTab.Visible = false;
|
||||
_townTab.Visible = false;
|
||||
}
|
||||
// Else (including Lugian, 0x100005f1): no-op, matching retail.
|
||||
}
|
||||
|
||||
private void ReconcileDialogs(RuntimeCharacterCreationSnapshot snapshot)
|
||||
{
|
||||
// Local-refusal / rejection surfacing is CC5's Summary-page job
|
||||
|
|
@ -622,9 +690,8 @@ internal sealed class CharacterCreationUiController : IDisposable
|
|||
if (_active)
|
||||
{
|
||||
_active = false;
|
||||
_isOpen = false;
|
||||
_openOnStartConsumed = false;
|
||||
Root.Visible = false;
|
||||
Close();
|
||||
}
|
||||
CloseAllDialogs(suppressCallbacks: true);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1715,10 +1715,10 @@ public static class ItemAppraisalTextFormatter
|
|||
_ => string.Empty,
|
||||
};
|
||||
|
||||
/// <summary><c>AppraisalSystem::SkillToString @ 0x005B4A30</c>.</summary>
|
||||
/// <summary>Retail skill-id -> display-name table. Made <c>internal</c>
|
||||
/// (Campaign CC slice CC4) so the chargen Skills page can reuse the
|
||||
/// same names instead of duplicating this table.</summary>
|
||||
/// <summary><c>AppraisalSystem::SkillToString @ 0x005B4A30</c> — retail
|
||||
/// skill-id -> display-name table. Made <c>internal</c> (Campaign CC
|
||||
/// slice CC4) so the chargen Skills page can reuse the same names
|
||||
/// instead of duplicating this table.</summary>
|
||||
internal static string SkillName(int skill) => skill switch
|
||||
{
|
||||
1 => "Axe",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue