fix(CT): CT3 fix round — port Refresh's unconditional selection clear, drop unresolvable-id rows
Opus dual-lens review of CT3 (03e073b7) found 1 BLOCKER + 2 SHOULD-FIX + notes.
BLOCKER: CharacterTitlesController never ported Refresh @0x0049abc0's own
SetSelectedItem(nullptr, 1) (@0x0049ac5a) — retail clears the current
title selection UNCONDITIONALLY on every Refresh() call, regardless of
whether the previously-selected id is still earned. OnTableReplaced
(0x0029) and OnDisplayTitleChanged (the display half of 0x002B) are
retail's two Refresh() call sites, so both now clear _selectedTitleId
before rebuilding/re-highlighting. OnTitleAdded (0x002B's add half) is a
DIFFERENT retail method — RecvNotice_AddCharacterTitle @0x0049a990 splices
one row without ever touching m_pSelectedItem — so it deliberately still
preserves selection. Net effect: after the user sets a display title and
ACE echoes 0x002B, the previously-highlighted row now goes dark and the
Set-as-Display button re-ghosts, matching retail; earning a new title
while a row is selected still leaves that selection alone.
SHOULD-FIX: ported AddTitleToList @0x0049A840's early-outs
(@0x0049a873/@0x0049a914) — an id of 0, or an id CharacterTitleResolver
fails to resolve, now produces NO row at all. The "Unknown" fallback
literal belongs only to the display-title text (Refresh @0x0049abc0's
other half), never a row — this was previously ported backwards.
SHOULD-FIX: rows and the display text now use their UiText's own authored
DefaultColor instead of a hardcoded Vector4.One, and each LinesProvider
now returns a cached UiText.Line[] built once per text change instead of
allocating a fresh array literal every draw call (pattern:
CharacterCreationSkillsPage.cs:829).
Notes (all ruled in): corrected two CharacterStatController comments that
falsely claimed the Titles page authors its own copies of the raise
buttons (verified against the fixture — it does not; the hide loop that
comment guarded is a defensive no-op given Visible's draw/click cascade,
kept only for the contentPage-not-found fallback); switched the row sort
from List.Sort to a stable OrderBy/ThenBy (ties broken by title id) so
equal-text rows keep retail's insert-after-equals order; wrapped the
title-resolver delegate in RetailUiRuntime.MountCharacter with the same
DatLock the row-template resolver already takes (DatCollection is
documented not thread-safe); set the list box's authored 24px row height
so wheel/line scroll lands row-aligned; kept the bind-time display-text
refresh with a comment explaining why the pre-notice "Unknown" frame is
unreachable in live play (ACE always sends 0x0029 before this panel can
open).
Tests: inverted TableReplaced_SelectedTitleStillEarned_KeepsSelectionHighlighted
into TableReplaced_ClearsSelection_EvenWhenTheSelectedIdIsStillEarned (cites
@0x0049ac5a), added its DisplayTitleChanged twin, and added
TitleAdded_PreservesSelection (the case most at risk from the blocker fix).
Inverted Rows_UnresolvedTitle_ShowsRetailUnknownLiteral into
Rows_UnresolvedTitle_ProducesNoRow (cites @0x0049a873/@0x0049a914) and added
Rows_TitleIdZero_ProducesNoRow for the other early-out. Extended
ClickingSetDisplay_..._AndMutatesNothingLocally to assert the row set and
selection are untouched by the click. Added
Fixture_PageCaptions_ResolveToNonEmptyText, which rebuilds the committed
character_2100002E.json fixture with a stub string resolver to pin this
class's own claim that the two page captions (0x1000052E/0x10000531) carry
a resolvable authored StringInfo.
Verified pre-existing/unrelated: the full hermetic suite run surfaced 2
failures in AcDream.App.Tests (LiveEntityNetworkBranchRoutingTests IL-shape
assertion, GameWindowRenderLeafCompositionTests IL-shape assertion) that
also fail with these five files stashed back to their pre-fix-round state —
confirmed unrelated to this change.
Build green. CharacterTitlesControllerTests: 24/24 (was 21, +3 net after
one invert-and-split and two new facts). Full hermetic solution suite
(Lane!=InstalledDat/PreparedPackage/Live/Manual/Timing/Windows/Linux/
SystemFont, Purpose!=Diagnostic, Status!=KnownFailure): only the two
pre-existing IL-shape failures above; every other project green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
4ea946257d
commit
4cc9448b0a
5 changed files with 287 additions and 33 deletions
|
|
@ -233,12 +233,40 @@ not yet acted on):
|
||||||
to a wire arrival; the reverse still holds (every genuine new row has
|
to a wire arrival; the reverse still holds (every genuine new row has
|
||||||
a `TitleAdded` firing).
|
a `TitleAdded` firing).
|
||||||
|
|
||||||
**CT3 — Titles page UI.** Bind the authored page through the standard
|
**CT3 — Titles page UI. REVIEW-CLOSED 2026-08-24: landed `03e073b7`,
|
||||||
|
Opus dual-lens review (1 BLOCKER + 2 should-fix + notes, all applied
|
||||||
|
in the fix round below).** Bind the authored page through the standard
|
||||||
GUI classes (`UiTemplateListBox`/`UiScrollbar`/`UiButton` — zero
|
GUI classes (`UiTemplateListBox`/`UiScrollbar`/`UiButton` — zero
|
||||||
bespoke widgets): sorted rows via the ported title-table lookup,
|
bespoke widgets): sorted rows via the ported title-table lookup,
|
||||||
selection, ghost-when-current logic (state 0xd contract), display-title
|
selection, ghost-when-current logic (state 0xd contract), display-title
|
||||||
text, Set-as-Display round trip, scrollbar. Retires half of AP-109.
|
text, Set-as-Display round trip, scrollbar. Retires half of AP-109.
|
||||||
|
|
||||||
|
**CT3 fix round (Opus dual-lens review, 2026-08-24).** BLOCKER: ported
|
||||||
|
`Refresh @0x0049abc0`'s unconditional `SetSelectedItem(nullptr, 1)`
|
||||||
|
(`@0x0049ac5a`) — selection now clears on BOTH `TableReplaced` and
|
||||||
|
`DisplayTitleChanged`, regardless of whether the previously-selected id
|
||||||
|
is still earned in the new table, but deliberately survives
|
||||||
|
`TitleAdded` (`RecvNotice_AddCharacterTitle @0x0049a990` splices one
|
||||||
|
row without ever touching `m_pSelectedItem` — a genuinely different
|
||||||
|
retail method from `Refresh`). SHOULD-FIX: `AddTitleToList @0x0049A840`'s
|
||||||
|
early-outs (`@0x0049a873`/`@0x0049a914`) ported — an id of 0, or an id
|
||||||
|
`CharacterTitleResolver.Resolve` fails to resolve, now produces NO row
|
||||||
|
at all (the `"Unknown"` fallback literal belongs only to the
|
||||||
|
display-title text, never a row — this was previously ported
|
||||||
|
backwards); rows use the row template's own authored `DefaultColor`
|
||||||
|
instead of a hardcoded white, and each row/display-text `UiText.Line[]`
|
||||||
|
is built once per text change and cached instead of reallocated every
|
||||||
|
draw call. Notes also applied: corrected two comments that falsely
|
||||||
|
claimed the Titles page authors its own copies of the raise buttons
|
||||||
|
(verified against the fixture — it does not; the hide loop that
|
||||||
|
comment guarded is a defensive no-op, kept only for the
|
||||||
|
contentPage-not-found fallback path), switched the row sort from
|
||||||
|
`List.Sort` to a stable `OrderBy`/`ThenBy` (ties broken by title id),
|
||||||
|
wrapped the title-resolver delegate in the same `DatLock` the
|
||||||
|
row-template resolver already takes (`RetailUiRuntime.MountCharacter`),
|
||||||
|
and set the list box's authored 24px row height so wheel/line scroll
|
||||||
|
lands row-aligned.
|
||||||
|
|
||||||
**CT4 — Header identity block.** Retail composition: name; "<Gender>
|
**CT4 — Header identity block.** Retail composition: name; "<Gender>
|
||||||
<Heritage> <DisplayTitle>"; PK status line — authored fonts/colors
|
<Heritage> <DisplayTitle>"; PK status line — authored fonts/colors
|
||||||
(pure white per probe), live refresh on display-title change and PK
|
(pure white per probe), live refresh on display-title change and PK
|
||||||
|
|
|
||||||
|
|
@ -374,8 +374,12 @@ public static class CharacterStatController
|
||||||
// Mutable selected-index box: -1 = nothing selected.
|
// Mutable selected-index box: -1 = nothing selected.
|
||||||
|
|
||||||
// Gather EVERY copy of the raise buttons in the tree. The raise button ids
|
// Gather EVERY copy of the raise buttons in the tree. The raise button ids
|
||||||
// (0x10000246, 0x100005EB) appear once per tab page (Attributes/Skills/Titles)
|
// (0x10000246, 0x100005EB) each appear TWICE under BOTH the Attributes page
|
||||||
// in the dat inheritance structure; ImportedLayout._byId keeps only the LAST
|
// (0x1000022B) and the Skills page (0x1000022C) — once per footer-state group
|
||||||
|
// (0x10000247/0x10000241) — four copies total. Verified against the committed
|
||||||
|
// fixture (CT3 fix round): Titles (0x10000539) authors NO copies of its own —
|
||||||
|
// correcting this comment's earlier, false "once per tab page
|
||||||
|
// (Attributes/Skills/Titles)" claim. ImportedLayout._byId keeps only the LAST
|
||||||
// mounted copy. We collect all copies so we can hide them all initially and
|
// mounted copy. We collect all copies so we can hide them all initially and
|
||||||
// show/hide the correct set when a row is selected.
|
// show/hide the correct set when a row is selected.
|
||||||
//
|
//
|
||||||
|
|
@ -526,10 +530,19 @@ public static class CharacterStatController
|
||||||
|
|
||||||
if (showTitles)
|
if (showTitles)
|
||||||
{
|
{
|
||||||
// Titles authors its own (unused) copies of the raise buttons
|
// CT3 fix round: the prior comment here ("Titles authors its
|
||||||
// (CT1 ground truth); nothing on this page ever selects a stat
|
// own copies of the raise buttons") was FALSE — verified
|
||||||
// row, so keep them hidden rather than rebuilding a list this
|
// against the fixture, Titles (0x10000539) has none; see the
|
||||||
// tab does not show.
|
// corrected collection comment above. contentPage.Visible =
|
||||||
|
// false (just above) already suppresses the Attributes
|
||||||
|
// page's real raise-button copies for both draw and click
|
||||||
|
// routing (UiElement early-returns on an invisible node
|
||||||
|
// before descending to children), so this loop is a no-op
|
||||||
|
// in the common case. It is kept only as a defensive
|
||||||
|
// fallback for the case where contentPage was not found at
|
||||||
|
// bind time (contentPage is null, line ~525) but allRaise1/
|
||||||
|
// allRaise10 were still populated via the tree-walk/FindElement
|
||||||
|
// fallback above.
|
||||||
foreach (var b in allRaise1) b.Visible = false;
|
foreach (var b in allRaise1) b.Visible = false;
|
||||||
foreach (var b in allRaise10) b.Visible = false;
|
foreach (var b in allRaise10) b.Visible = false;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
using System;
|
using System;
|
||||||
using System.Collections.Generic;
|
using System.Collections.Generic;
|
||||||
using System.Numerics;
|
using System.Linq;
|
||||||
using AcDream.Runtime;
|
using AcDream.Runtime;
|
||||||
|
|
||||||
namespace AcDream.App.UI.Layout;
|
namespace AcDream.App.UI.Layout;
|
||||||
|
|
@ -29,7 +29,13 @@ namespace AcDream.App.UI.Layout;
|
||||||
/// codebase needs one either (Friends/Squelch/Fellowship/Allegiance/chargen
|
/// codebase needs one either (Friends/Squelch/Fellowship/Allegiance/chargen
|
||||||
/// skills/the Options tabs all rebuild-on-change the same way) — the
|
/// skills/the Options tabs all rebuild-on-change the same way) — the
|
||||||
/// resulting VISIBLE order is retail-exact even though the underlying
|
/// resulting VISIBLE order is retail-exact even though the underlying
|
||||||
/// mechanism is "rebuild," not "splice."
|
/// mechanism is "rebuild," not "splice." <c>AddTitleToList</c> itself
|
||||||
|
/// early-outs (<c>@0x0049a873</c>/<c>@0x0049a914</c>) before ever reaching
|
||||||
|
/// the insert — retail NEVER creates a row for id 0 or for an id
|
||||||
|
/// <c>GetCharacterTitleFromID</c> fails to resolve, so <see cref="RebuildRows"/>
|
||||||
|
/// skips those ids entirely rather than falling back to a placeholder row
|
||||||
|
/// text (CT3 fix round — the "Unknown" literal belongs ONLY to the
|
||||||
|
/// display-title text below, never to a row).
|
||||||
/// </para>
|
/// </para>
|
||||||
/// <para>
|
/// <para>
|
||||||
/// <b>Selection + highlight.</b> Retail's <c>InfoRegion::SetState</c>
|
/// <b>Selection + highlight.</b> Retail's <c>InfoRegion::SetState</c>
|
||||||
|
|
@ -43,6 +49,27 @@ namespace AcDream.App.UI.Layout;
|
||||||
/// swap, unlike pages whose row template lacks a state-based highlight.
|
/// swap, unlike pages whose row template lacks a state-based highlight.
|
||||||
/// </para>
|
/// </para>
|
||||||
/// <para>
|
/// <para>
|
||||||
|
/// <b>Selection lifetime (CT3 fix round — BLOCKER).</b> <c>Refresh
|
||||||
|
/// @0x0049abc0</c> itself calls <c>SetSelectedItem(nullptr, 1)</c>
|
||||||
|
/// (<c>@0x0049ac5a</c>) UNCONDITIONALLY, before it repopulates the list —
|
||||||
|
/// every code path that reaches <c>Refresh()</c> drops the current
|
||||||
|
/// selection outright, regardless of whether the previously-selected title
|
||||||
|
/// id is still earned. <c>Refresh()</c> runs on BOTH
|
||||||
|
/// <see cref="RuntimeCharacterTitleState.TableReplaced"/> (<c>0x0029</c>)
|
||||||
|
/// and <see cref="RuntimeCharacterTitleState.DisplayTitleChanged"/> (the
|
||||||
|
/// display-title half of <c>0x002B</c>), so <see cref="OnTableReplaced"/>
|
||||||
|
/// and <see cref="OnDisplayTitleChanged"/> both clear
|
||||||
|
/// <see cref="_selectedTitleId"/> before rebuilding/re-highlighting.
|
||||||
|
/// <c>RecvNotice_AddCharacterTitle @0x0049a990</c> (the add half of
|
||||||
|
/// <c>0x002B</c>, <see cref="OnTitleAdded"/>) is a DIFFERENT retail method
|
||||||
|
/// that splices one row into <c>mTitleList</c> without ever touching
|
||||||
|
/// <c>m_pSelectedItem</c> — selection SURVIVES a title add. Concretely:
|
||||||
|
/// after the user sets a display title and ACE echoes <c>0x002B</c>, the
|
||||||
|
/// previously-highlighted row goes dark and the Set-as-Display button
|
||||||
|
/// re-ghosts, exactly like retail — but earning a brand-new title while a
|
||||||
|
/// row is selected leaves that selection alone.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
/// <b>The "Set as Display Title" button (<c>0x10000535</c>).</b>
|
/// <b>The "Set as Display Title" button (<c>0x10000535</c>).</b>
|
||||||
/// <c>UpdateButtons @0x0049A500</c> (CORRECTED per the campaign plan's CT1
|
/// <c>UpdateButtons @0x0049A500</c> (CORRECTED per the campaign plan's CT1
|
||||||
/// fix round): Ghosted (state 0xD) UNLESS a row is SELECTED whose title id
|
/// fix round): Ghosted (state 0xD) UNLESS a row is SELECTED whose title id
|
||||||
|
|
@ -149,6 +176,12 @@ public sealed class CharacterTitlesController : IDisposable
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
listBox.TemplateResolver = templateResolver;
|
listBox.TemplateResolver = templateResolver;
|
||||||
|
// The row template (0x10000536) authors a 270x24 box (CT1 ground
|
||||||
|
// truth §3); UiTemplateListBox's own DefaultLineHeight is 16, which
|
||||||
|
// would desync wheel/line scroll from the actual row pitch
|
||||||
|
// (CharacterManagementUiController.cs:463 sets its own row height
|
||||||
|
// the same way for the same reason).
|
||||||
|
listBox.LineHeight = 24;
|
||||||
|
|
||||||
uint scrollbarElementId = listBox.ScrollbarElementId;
|
uint scrollbarElementId = listBox.ScrollbarElementId;
|
||||||
UiElement? scrollbarElement = scrollbarElementId == 0
|
UiElement? scrollbarElement = scrollbarElementId == 0
|
||||||
|
|
@ -175,6 +208,14 @@ public sealed class CharacterTitlesController : IDisposable
|
||||||
titles.DisplayTitleChanged += controller.OnDisplayTitleChanged;
|
titles.DisplayTitleChanged += controller.OnDisplayTitleChanged;
|
||||||
|
|
||||||
controller.RebuildRows();
|
controller.RebuildRows();
|
||||||
|
// Bind-time refresh (CT3 fix round NOTE 6): retail itself only
|
||||||
|
// shows "Unknown" until the first notice arrives (nothing runs
|
||||||
|
// Refresh() before Refresh() is first called), but ACE always sends
|
||||||
|
// 0x0029 at SendSelf before this panel can even open, so the
|
||||||
|
// pre-notice "Unknown" frame is unreachable in live play. Refreshing
|
||||||
|
// at bind time instead keeps a window RE-mount (tab re-open, panel
|
||||||
|
// rebuild) consistent with whatever the table already holds, rather
|
||||||
|
// than flashing "Unknown" for one frame before the next notice.
|
||||||
controller.RefreshDisplayText();
|
controller.RefreshDisplayText();
|
||||||
controller.RefreshButtonGhost();
|
controller.RefreshButtonGhost();
|
||||||
return controller;
|
return controller;
|
||||||
|
|
@ -201,12 +242,15 @@ public sealed class CharacterTitlesController : IDisposable
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// <c>0x0029 CharacterTitle</c> — retail's own <c>Refresh()</c> is
|
/// <c>0x0029 CharacterTitle</c> — retail's own <c>Refresh()</c> is
|
||||||
/// unconditional here (CT2 review anchor 1), and <c>UnPack</c> always
|
/// unconditional here (CT2 review anchor 1), and <c>UnPack</c> always
|
||||||
/// rebuilds <c>mTitleList</c> from scratch. <see cref="RebuildRows"/>
|
/// rebuilds <c>mTitleList</c> from scratch. BLOCKER fix (CT3 fix round):
|
||||||
/// itself decides whether the current selection survives (it does when
|
/// <c>Refresh</c> also calls <c>SetSelectedItem(nullptr, 1)</c>
|
||||||
/// the selected id is still earned in the new table).
|
/// (<c>@0x0049ac5a</c>) unconditionally, BEFORE it repopulates — so the
|
||||||
|
/// selection is cleared here regardless of whether the previously
|
||||||
|
/// selected id is still earned, not merely dropped when it disappears.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private void OnTableReplaced()
|
private void OnTableReplaced()
|
||||||
{
|
{
|
||||||
|
ClearSelection();
|
||||||
RebuildRows();
|
RebuildRows();
|
||||||
RefreshDisplayText();
|
RefreshDisplayText();
|
||||||
RefreshButtonGhost();
|
RefreshButtonGhost();
|
||||||
|
|
@ -216,6 +260,10 @@ public sealed class CharacterTitlesController : IDisposable
|
||||||
/// <c>0x002B UpdateTitle</c>, add half — CT2's F1 fix already dedupes
|
/// <c>0x002B UpdateTitle</c>, add half — CT2's F1 fix already dedupes
|
||||||
/// this event to genuine new memberships only (a repeat add fires no
|
/// this event to genuine new memberships only (a repeat add fires no
|
||||||
/// event at all), so every firing here is a real new row.
|
/// event at all), so every firing here is a real new row.
|
||||||
|
/// <c>RecvNotice_AddCharacterTitle @0x0049a990</c> splices the one new
|
||||||
|
/// row into <c>mTitleList</c> without ever touching
|
||||||
|
/// <c>m_pSelectedItem</c> (unlike <c>Refresh</c>'s unconditional
|
||||||
|
/// clear) — selection deliberately survives a title add.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private void OnTitleAdded(uint titleId)
|
private void OnTitleAdded(uint titleId)
|
||||||
{
|
{
|
||||||
|
|
@ -223,36 +271,68 @@ public sealed class CharacterTitlesController : IDisposable
|
||||||
RefreshButtonGhost();
|
RefreshButtonGhost();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// <c>0x002B UpdateTitle</c>, display half — this is the other trigger
|
||||||
|
/// for retail's <c>Refresh()</c> (CT2 review anchor 1), so it carries
|
||||||
|
/// the same unconditional <c>SetSelectedItem(nullptr, 1)</c>
|
||||||
|
/// (<c>@0x0049ac5a</c>) as <see cref="OnTableReplaced"/>. This handler
|
||||||
|
/// does not call <see cref="RebuildRows"/> (the row SET is unchanged —
|
||||||
|
/// only the display title moved), so it re-applies highlights directly
|
||||||
|
/// to actually dark out the previously-selected row.
|
||||||
|
/// </summary>
|
||||||
private void OnDisplayTitleChanged(uint titleId)
|
private void OnDisplayTitleChanged(uint titleId)
|
||||||
{
|
{
|
||||||
|
ClearSelection();
|
||||||
|
ApplyRowHighlights();
|
||||||
RefreshDisplayText();
|
RefreshDisplayText();
|
||||||
RefreshButtonGhost();
|
RefreshButtonGhost();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary><c>Refresh @0x0049abc0</c>'s <c>SetSelectedItem(nullptr,
|
||||||
|
/// 1)</c> (<c>@0x0049ac5a</c>) — clears the tracked selection only; the
|
||||||
|
/// caller is responsible for re-applying row highlights and the button
|
||||||
|
/// ghost state afterward.</summary>
|
||||||
|
private void ClearSelection() => _selectedTitleId = null;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Full sorted rebuild — see the class remarks for why this port
|
/// Full sorted rebuild — see the class remarks for why this port
|
||||||
/// rebuilds rather than performing retail's literal single-row
|
/// rebuilds rather than performing retail's literal single-row
|
||||||
/// positional insert. Preserves scroll position
|
/// positional insert. Preserves scroll position
|
||||||
/// (<see cref="UiTemplateListBox.FlushPreservingScroll"/>). The current
|
/// (<see cref="UiTemplateListBox.FlushPreservingScroll"/>). Callers that
|
||||||
/// selection survives when the selected id is still present in the
|
/// mirror retail's unconditional <c>Refresh()</c> selection clear
|
||||||
/// rebuilt row set; otherwise it is cleared here so the Set-as-Display
|
/// (<see cref="OnTableReplaced"/>) call <see cref="ClearSelection"/>
|
||||||
/// button's ghost state can never desync from what is actually
|
/// themselves before this runs; the check below is a defensive
|
||||||
/// highlighted (a selection pointing at a no-longer-visible row would
|
/// fallback for any other caller (<see cref="OnTitleAdded"/> included)
|
||||||
/// leave the button enabled with nothing shown selected).
|
/// so a selection can never point at a row that no longer exists.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private void RebuildRows()
|
private void RebuildRows()
|
||||||
{
|
{
|
||||||
_listBox.FlushPreservingScroll();
|
_listBox.FlushPreservingScroll();
|
||||||
_rows.Clear();
|
_rows.Clear();
|
||||||
|
|
||||||
|
// AddTitleToList @0x0049A840 early-outs (@0x0049a873/@0x0049a914):
|
||||||
|
// retail never creates a row for id 0 or for an id
|
||||||
|
// GetCharacterTitleFromID fails to resolve — "Unknown" is the
|
||||||
|
// display-title text's OWN fallback (RefreshDisplayText), never a
|
||||||
|
// row's (CT3 fix round — was previously ported backwards).
|
||||||
// A3/CT2 doc warning: EarnedTitleIds allocates a fresh array per
|
// A3/CT2 doc warning: EarnedTitleIds allocates a fresh array per
|
||||||
// read — safe here (a UI refresh call site, not a per-frame poll).
|
// read — safe here (a UI refresh call site, not a per-frame poll).
|
||||||
var sorted = new List<(uint Id, string Text)>();
|
var candidates = new List<(uint Id, string Text)>();
|
||||||
foreach (uint id in _titles.EarnedTitleIds)
|
foreach (uint id in _titles.EarnedTitleIds)
|
||||||
sorted.Add((id, _resolveTitle(id) ?? UnknownTitleText));
|
{
|
||||||
|
if (id == 0) continue;
|
||||||
|
string? text = _resolveTitle(id);
|
||||||
|
if (text is null) continue;
|
||||||
|
candidates.Add((id, text));
|
||||||
|
}
|
||||||
// FindSortedInsertPosition @0x0049A760: ordinal string sort on the
|
// FindSortedInsertPosition @0x0049A760: ordinal string sort on the
|
||||||
// resolved display text.
|
// resolved display text. OrderBy is a STABLE sort (unlike
|
||||||
sorted.Sort(static (a, b) => string.CompareOrdinal(a.Text, b.Text));
|
// List<T>.Sort) so equal-text rows keep retail's insert-after-
|
||||||
|
// equals order; ties are broken by title id for full determinism.
|
||||||
|
List<(uint Id, string Text)> sorted = candidates
|
||||||
|
.OrderBy(static c => c.Text, StringComparer.Ordinal)
|
||||||
|
.ThenBy(static c => c.Id)
|
||||||
|
.ToList();
|
||||||
|
|
||||||
foreach ((uint id, string text) in sorted)
|
foreach ((uint id, string text) in sorted)
|
||||||
{
|
{
|
||||||
|
|
@ -272,8 +352,14 @@ public sealed class CharacterTitlesController : IDisposable
|
||||||
|
|
||||||
if (UiElement.FindDescendant(row, RowTextId) is UiText rowText)
|
if (UiElement.FindDescendant(row, RowTextId) is UiText rowText)
|
||||||
{
|
{
|
||||||
string capturedText = text;
|
// Build the line array once per text change and capture it
|
||||||
rowText.LinesProvider = () => [new UiText.Line(capturedText, Vector4.One)];
|
// — LinesProvider runs every draw, so a `=> [new Line(...)]`
|
||||||
|
// literal would allocate a fresh array every frame
|
||||||
|
// (pattern: CharacterCreationSkillsPage.cs:829). DefaultColor
|
||||||
|
// is the row template's own authored font color, not a
|
||||||
|
// hardcoded white.
|
||||||
|
UiText.Line[] lines = [new UiText.Line(text, rowText.DefaultColor)];
|
||||||
|
rowText.LinesProvider = () => lines;
|
||||||
}
|
}
|
||||||
|
|
||||||
_rows.Add(new Row(row, id));
|
_rows.Add(new Row(row, id));
|
||||||
|
|
@ -314,7 +400,10 @@ public sealed class CharacterTitlesController : IDisposable
|
||||||
{
|
{
|
||||||
if (_displayText is null) return;
|
if (_displayText is null) return;
|
||||||
string text = _resolveTitle(_titles.DisplayTitleId) ?? UnknownTitleText;
|
string text = _resolveTitle(_titles.DisplayTitleId) ?? UnknownTitleText;
|
||||||
_displayText.LinesProvider = () => [new UiText.Line(text, Vector4.One)];
|
// Cached array, authored color — same reasoning as the row text
|
||||||
|
// above (CT3 fix round).
|
||||||
|
UiText.Line[] lines = [new UiText.Line(text, _displayText.DefaultColor)];
|
||||||
|
_displayText.LinesProvider = () => lines;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|
|
||||||
|
|
@ -4080,10 +4080,20 @@ public sealed class RetailUiRuntime : IDisposable
|
||||||
lock (_bindings.Assets.DatLock)
|
lock (_bindings.Assets.DatLock)
|
||||||
return titleRowTemplates.Resolve(templateLayoutId, templateElementId);
|
return titleRowTemplates.Resolve(templateLayoutId, templateElementId);
|
||||||
}
|
}
|
||||||
|
// CT3 fix round: CharacterTitleResolver.Resolve reads the SAME
|
||||||
|
// IDatReaderWriter (EnumMapper + StringTable lookups) as the row
|
||||||
|
// template resolver just above — DatCollection is documented not
|
||||||
|
// thread-safe, so this delegate needs the identical DatLock scope,
|
||||||
|
// not just the template resolver.
|
||||||
|
string? TitleResolver(uint titleId)
|
||||||
|
{
|
||||||
|
lock (_bindings.Assets.DatLock)
|
||||||
|
return _bindings.Character.TitleResolver.Resolve(titleId);
|
||||||
|
}
|
||||||
_characterTitlesController = Layout.CharacterTitlesController.Bind(
|
_characterTitlesController = Layout.CharacterTitlesController.Bind(
|
||||||
layout.Root,
|
layout.Root,
|
||||||
_bindings.Character.Titles,
|
_bindings.Character.Titles,
|
||||||
_bindings.Character.TitleResolver.Resolve,
|
TitleResolver,
|
||||||
TitleTemplateResolver,
|
TitleTemplateResolver,
|
||||||
_bindings.Character.SendSetTitle);
|
_bindings.Character.SendSetTitle);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -95,6 +95,37 @@ public sealed class CharacterTitlesControllerTests
|
||||||
Assert.Same(h.ListBox.Scroll, scrollbar.Model);
|
Assert.Same(h.ListBox.Scroll, scrollbar.Model);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Fixture_PageCaptions_ResolveToNonEmptyText()
|
||||||
|
{
|
||||||
|
// Pins this class's own remarks claim (CT3 fix round item 5b):
|
||||||
|
// LayoutImporter.BuildText already resolves every element's
|
||||||
|
// authored StringInfo caption at import time, so the Titles page's
|
||||||
|
// two static captions (0x1000052E/0x10000531) must actually carry a
|
||||||
|
// resolvable authored StringInfo -- not silently come through as
|
||||||
|
// empty/missing text -- even though this controller never touches
|
||||||
|
// either element itself. FixtureLoader.LoadCharacter() passes NO
|
||||||
|
// string resolver (it needs no live DAT for structural conformance
|
||||||
|
// checks elsewhere), so this test rebuilds the SAME committed
|
||||||
|
// fixture with a stub resolver that stands in for
|
||||||
|
// DatStringResolver.Resolve -- exercising the real
|
||||||
|
// ResolveAuthoredString → stringResolve pipeline
|
||||||
|
// (DatWidgetFactory.cs) without needing a live StringTable.
|
||||||
|
static string? StubResolve(UiStringInfoValue info) =>
|
||||||
|
info.TableId != 0u && info.StringId != 0u ? "<resolved>" : null;
|
||||||
|
ImportedLayout layout = LayoutImporter.Build(
|
||||||
|
FixtureLoader.LoadCharacterInfos(),
|
||||||
|
static _ => (0u, 0, 0),
|
||||||
|
null,
|
||||||
|
stringResolve: StubResolve);
|
||||||
|
|
||||||
|
var currentTitleCaption = Assert.IsType<UiText>(layout.FindElement(0x1000052Eu));
|
||||||
|
var titlesEarnedCaption = Assert.IsType<UiText>(layout.FindElement(0x10000531u));
|
||||||
|
|
||||||
|
Assert.Equal("<resolved>", currentTitleCaption.LinesProvider()[0].Text);
|
||||||
|
Assert.Equal("<resolved>", titlesEarnedCaption.LinesProvider()[0].Text);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void Bind_MissingListBox_ReturnsNullWithoutThrowing()
|
public void Bind_MissingListBox_ReturnsNullWithoutThrowing()
|
||||||
{
|
{
|
||||||
|
|
@ -206,15 +237,34 @@ public sealed class CharacterTitlesControllerTests
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void Rows_UnresolvedTitle_ShowsRetailUnknownLiteral()
|
public void Rows_UnresolvedTitle_ProducesNoRow()
|
||||||
{
|
{
|
||||||
|
// AddTitleToList @0x0049A840 early-outs (@0x0049a873/@0x0049a914):
|
||||||
|
// retail never creates a row for an id GetCharacterTitleFromID
|
||||||
|
// fails to resolve -- "Unknown" is exclusively the display-title
|
||||||
|
// text's own Refresh fallback literal (below), never a row's
|
||||||
|
// (CT3 fix round -- this was previously ported backwards).
|
||||||
Harness h = BindWithEarnedTitles(
|
Harness h = BindWithEarnedTitles(
|
||||||
[99u],
|
[99u],
|
||||||
displayTitleId: 0u,
|
displayTitleId: 0u,
|
||||||
names: []);
|
names: []);
|
||||||
|
|
||||||
|
Assert.Empty(h.Rows);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Rows_TitleIdZero_ProducesNoRow()
|
||||||
|
{
|
||||||
|
// Same early-out (@0x0049a873), the OTHER guarded case: retail
|
||||||
|
// never creates a row for id 0 even if a resolver were somehow
|
||||||
|
// willing to answer for it.
|
||||||
|
Harness h = BindWithEarnedTitles(
|
||||||
|
[0u, 1u],
|
||||||
|
displayTitleId: 0u,
|
||||||
|
names: new() { [0u] = "Should Never Appear", [1u] = "Adventurer" });
|
||||||
|
|
||||||
UiElement row = Assert.Single(h.Rows);
|
UiElement row = Assert.Single(h.Rows);
|
||||||
Assert.Equal("Unknown", h.RowText(row));
|
Assert.Equal("Adventurer", h.RowText(row));
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Selection + highlight ─────────────────────────────────────────────
|
// ── Selection + highlight ─────────────────────────────────────────────
|
||||||
|
|
@ -288,6 +338,7 @@ public sealed class CharacterTitlesControllerTests
|
||||||
names: new() { [1u] = "Adventurer", [13u] = "War Mage" });
|
names: new() { [1u] = "Adventurer", [13u] = "War Mage" });
|
||||||
UiElement warMageRow = h.Rows.Single(r => h.RowText(r) == "War Mage");
|
UiElement warMageRow = h.Rows.Single(r => h.RowText(r) == "War Mage");
|
||||||
((UiDatElement)warMageRow).OnClick!();
|
((UiDatElement)warMageRow).OnClick!();
|
||||||
|
List<UiElement> rowsBeforeClick = h.Rows.ToList();
|
||||||
|
|
||||||
h.SetDisplayButton.OnClick!();
|
h.SetDisplayButton.OnClick!();
|
||||||
|
|
||||||
|
|
@ -297,6 +348,12 @@ public sealed class CharacterTitlesControllerTests
|
||||||
// ever changes from a DisplayTitleChanged event).
|
// ever changes from a DisplayTitleChanged event).
|
||||||
Assert.Equal(1u, h.Titles.DisplayTitleId);
|
Assert.Equal(1u, h.Titles.DisplayTitleId);
|
||||||
Assert.Equal("Adventurer", h.DisplayText.LinesProvider().Single().Text);
|
Assert.Equal("Adventurer", h.DisplayText.LinesProvider().Single().Text);
|
||||||
|
// The click is wire-only: no RebuildRows, no selection change. The
|
||||||
|
// row set is the SAME instances in the same order, and War Mage
|
||||||
|
// stays selected/highlighted/enabled exactly as before the click.
|
||||||
|
Assert.Equal(rowsBeforeClick, h.Rows);
|
||||||
|
Assert.Equal(RowHighlightSprite, h.RowMedia(warMageRow));
|
||||||
|
Assert.True(h.SetDisplayButton.Enabled);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|
@ -331,8 +388,14 @@ public sealed class CharacterTitlesControllerTests
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void TableReplaced_SelectedTitleStillEarned_KeepsSelectionHighlighted()
|
public void TableReplaced_ClearsSelection_EvenWhenTheSelectedIdIsStillEarned()
|
||||||
{
|
{
|
||||||
|
// BLOCKER fix (CT3 fix round): Refresh @0x0049abc0 calls
|
||||||
|
// SetSelectedItem(nullptr, 1) (@0x0049ac5a) UNCONDITIONALLY, before
|
||||||
|
// it repopulates the list -- a still-earned selected id is no
|
||||||
|
// defense. A byte-identical resend must still dark out the row and
|
||||||
|
// re-ghost the button (this test used to assert the OPPOSITE and
|
||||||
|
// was wrong).
|
||||||
Harness h = BindWithEarnedTitles(
|
Harness h = BindWithEarnedTitles(
|
||||||
[1u, 5u], displayTitleId: 0u,
|
[1u, 5u], displayTitleId: 0u,
|
||||||
names: new() { [1u] = "Adventurer", [5u] = "Life Mage" });
|
names: new() { [1u] = "Adventurer", [5u] = "Life Mage" });
|
||||||
|
|
@ -340,13 +403,64 @@ public sealed class CharacterTitlesControllerTests
|
||||||
((UiDatElement)lifeMageRow).OnClick!();
|
((UiDatElement)lifeMageRow).OnClick!();
|
||||||
Assert.True(h.SetDisplayButton.Enabled);
|
Assert.True(h.SetDisplayButton.Enabled);
|
||||||
|
|
||||||
// A resend of the SAME table (retail's own Refresh() is
|
// Same table resent (retail's own Refresh() is unconditional —
|
||||||
// unconditional — CT2 review anchor 1) must not silently desync the
|
// CT2 review anchor 1) — the id (5) is STILL earned afterward, yet
|
||||||
// ghost state from the still-valid selection.
|
// the selection must still be dropped.
|
||||||
h.Titles.ReplaceTable(0u, [1u, 5u]);
|
h.Titles.ReplaceTable(0u, [1u, 5u]);
|
||||||
|
|
||||||
UiElement rebuiltLifeMageRow = h.Rows.Single(r => h.RowText(r) == "Life Mage");
|
UiElement rebuiltLifeMageRow = h.Rows.Single(r => h.RowText(r) == "Life Mage");
|
||||||
Assert.Equal(RowHighlightSprite, h.RowMedia(rebuiltLifeMageRow));
|
Assert.Equal(RowNormalSprite, h.RowMedia(rebuiltLifeMageRow));
|
||||||
|
Assert.False(h.SetDisplayButton.Enabled);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void DisplayTitleChanged_ClearsSelection_EvenWhenTheSelectedIdIsStillEarned()
|
||||||
|
{
|
||||||
|
// The display-change twin of the TableReplaced test above: 0x002B's
|
||||||
|
// display half is retail's OTHER Refresh() call site, so it carries
|
||||||
|
// the same unconditional SetSelectedItem(nullptr, 1) (@0x0049ac5a).
|
||||||
|
// This handler never calls RebuildRows (the row SET does not
|
||||||
|
// change), so it specifically proves the highlight is re-applied
|
||||||
|
// via ApplyRowHighlights even without a rebuild. Selecting a
|
||||||
|
// DIFFERENT id than the one becoming the new display title isolates
|
||||||
|
// this from the already-covered "selection == new display title"
|
||||||
|
// ghost case (DisplayTitleChanged_UpdatesTextAndReevaluatesGhost):
|
||||||
|
// id 1 remains earned and still differs from the new display id 5,
|
||||||
|
// yet selection must still clear.
|
||||||
|
Harness h = BindWithEarnedTitles(
|
||||||
|
[1u, 5u], displayTitleId: 0u,
|
||||||
|
names: new() { [1u] = "Adventurer", [5u] = "Life Mage" });
|
||||||
|
UiElement adventurerRow = h.Rows.Single(r => h.RowText(r) == "Adventurer");
|
||||||
|
((UiDatElement)adventurerRow).OnClick!();
|
||||||
|
Assert.Equal(RowHighlightSprite, h.RowMedia(adventurerRow));
|
||||||
|
Assert.True(h.SetDisplayButton.Enabled); // selected(1) != display(0)
|
||||||
|
|
||||||
|
h.Titles.ApplyUpdateTitle(5u, setAsDisplay: true);
|
||||||
|
|
||||||
|
Assert.Equal(RowNormalSprite, h.RowMedia(adventurerRow));
|
||||||
|
Assert.False(h.SetDisplayButton.Enabled);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void TitleAdded_PreservesSelection()
|
||||||
|
{
|
||||||
|
// RecvNotice_AddCharacterTitle @0x0049a990 splices the new row into
|
||||||
|
// mTitleList without ever touching m_pSelectedItem -- a DIFFERENT
|
||||||
|
// retail method from Refresh, and the one case that must NOT clear
|
||||||
|
// selection. This is the case most at risk from the blocker fix
|
||||||
|
// above (it would be trivial to over-clear on every wire event).
|
||||||
|
Harness h = BindWithEarnedTitles(
|
||||||
|
[1u], displayTitleId: 0u,
|
||||||
|
names: new() { [1u] = "Adventurer", [13u] = "War Mage" });
|
||||||
|
UiElement adventurerRow = Assert.Single(h.Rows);
|
||||||
|
((UiDatElement)adventurerRow).OnClick!();
|
||||||
|
Assert.Equal(RowHighlightSprite, h.RowMedia(adventurerRow));
|
||||||
|
Assert.True(h.SetDisplayButton.Enabled);
|
||||||
|
|
||||||
|
h.Titles.ApplyUpdateTitle(13u, setAsDisplay: false);
|
||||||
|
|
||||||
|
UiElement rebuiltAdventurerRow = h.Rows.Single(r => h.RowText(r) == "Adventurer");
|
||||||
|
Assert.Equal(RowHighlightSprite, h.RowMedia(rebuiltAdventurerRow));
|
||||||
Assert.True(h.SetDisplayButton.Enabled);
|
Assert.True(h.SetDisplayButton.Enabled);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue