fix #FA4-mechanism-MUST-FIX-1,4: truncate (not round) the XP-share percentage; port the world->panel fellow-selection sync
MUST-FIX 1 -- D5's percentage conversion rounds where retail truncates. gmFellowshipUI::UpdateFellowStats @0x0048ECC9 forms pct*100.0f on the x87 stack then calls _ftol2 (MSVC's round-to-truncate helper), never MathF.Round. The stored floats for 6 and 8 fellows are 0.44999998807907104 and 0.3499999940395355 (byte-read from the PDB-paired binary), so retail's own products truncate to 44/34, not 45/35 -- and (int)(pct*100f) alone does not fix it, since 0.45f*100f already rounds UP to exactly 45.0f in single precision. Fixed as (int)((double)pct * 100.0), forming the product the same wider-than-single-precision way retail's x87 does. Pinned with new [InlineData] cases for both sizes. MUST-FIX 4 -- gmFellowshipUI::UpdateFellowSelection @0x0048F0F0 (the world->panel arm of retail's two-directional selection coupling) was never ported; only the panel->world arm (SelectFellow) shipped. Selecting a fellow in the WORLD left Dismiss/Assign-Leader disabled and showed no row highlight. SyncSelectionFromWorld/SetSelectedFellow reproduce the observable contract (button-enable + a row tint) against this controller's own guid-keyed row dictionary instead of porting retail's generic ListBox SetAttribute_InstanceID/SetSelectedItem primitive (scoped disposition recorded at register row AD-82). Also in this pass over the controller: - SF-1: cache the fellowship-name LinesProvider; only reassign on an actual name change (was allocating once per Tick, even while hidden). - SF-2/SF-3: track true membership in _memberGuids, independent of which rows finished building. Fixes an unbounded DAT-locked rebuild retry when a row template permanently fails to build, and fixes Recruit's "already a fellow" check reading render rows instead of membership. - N-0: the Open/Close caption now flips optimistically on click, matching retail's pre-toggle-before-server-echo (lane B feature 11). - N-1/N-2/N-3: doc-only notes on the meter-child-text gap, the max>0 guard, and Tick's two-read non-atomicity. - ResetPageVisibleLatch: the fellowship-controller half of MUST-FIX 3 (see the SocialPanelController commit for the panel-level half). Per docs/research/2026-08-12-fa4-review-mechanism.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
290f9b584a
commit
5499f0581f
2 changed files with 424 additions and 25 deletions
|
|
@ -126,6 +126,7 @@ public sealed class SocialFellowshipPageControllerTests
|
|||
public SelectionState Selection = new();
|
||||
public uint LocalPlayerGuid;
|
||||
public Func<uint, uint, UiElement?> TemplateResolver = FakeRowResolver;
|
||||
public Func<uint, uint, string?> ResolveString = static (_, _) => null;
|
||||
|
||||
public SocialFellowshipPageController.Bindings Build() => new(
|
||||
Snapshot: () => Snapshot,
|
||||
|
|
@ -142,7 +143,7 @@ public sealed class SocialFellowshipPageControllerTests
|
|||
LocalPlayerGuid: () => LocalPlayerGuid,
|
||||
CurrentCharacterOption: id => Options.TryGetValue(id, out bool v) && v,
|
||||
SetCharacterOption: (id, value) => { Options[id] = value; Calls.Add($"set-option:{id}:{value}"); },
|
||||
ResolveString: (_, _) => null);
|
||||
ResolveString: ResolveString);
|
||||
}
|
||||
|
||||
// ── Empty/full frame swap (still correct after the FA4 rewrite) ────────
|
||||
|
|
@ -240,8 +241,21 @@ public sealed class SocialFellowshipPageControllerTests
|
|||
Assert.Equal(0.10f, health.Fill()!.Value, 3);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Fix-round SF-5: RENAMED from
|
||||
/// <c>..._ButPreservesScrollPosition</c> — with only one short row this
|
||||
/// test cannot observe a NONZERO preserved offset (its own prior
|
||||
/// comment admitted as much), so the "preserves scroll position" half
|
||||
/// of the old name was aspirational, not verified here. The actual
|
||||
/// shrink/clamp math IS covered, correctly, at the widget level
|
||||
/// (<c>UiTemplateListBoxFlushPreservingScrollTests.FlushPreservingScroll_ClampsToTheNewShorterContent</c>).
|
||||
/// This test's real job — proven below — is that a genuine roster
|
||||
/// CHANGE (Bob joins) goes through <see cref="UiTemplateListBox.FlushPreservingScroll"/>
|
||||
/// (not the scroll-resetting <see cref="UiTemplateListBox.Flush"/>) and
|
||||
/// produces the right row count.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Tick_MembershipChange_RebuildsRoster_ButPreservesScrollPosition()
|
||||
public void Tick_MembershipChange_RebuildsRoster()
|
||||
{
|
||||
UiElement root = BuildPageRoot(out UiTemplateListBox listBox, out _);
|
||||
var alice = new RuntimeFellowMemberSnapshot(0x50000001u, "Alice", 12, 100, 80, 60, 100, 80, 60, false);
|
||||
|
|
@ -256,13 +270,6 @@ public sealed class SocialFellowshipPageControllerTests
|
|||
listBox.ViewportForTest!.LayoutScrollableChildren();
|
||||
listBox.Scroll.SetScrollY(0); // only one short row — nothing to scroll, but exercise the path
|
||||
|
||||
// Carry-forward 1: a genuine roster change (Bob joins) must not
|
||||
// silently desync — verified via row count — AND must go through
|
||||
// the scroll-preserving flush, not the scroll-resetting one. We
|
||||
// can't observe a NONZERO preserved offset with only one short
|
||||
// row, so this test's decisive assertion is closer to the wiring:
|
||||
// FlushPreservingScroll leaves Scroll.ScrollY untouched immediately
|
||||
// after the clear, unlike Flush.
|
||||
var bob = new RuntimeFellowMemberSnapshot(0x50000002u, "Bob", 10, 90, 70, 50, 90, 70, 50, false);
|
||||
b.Snapshot = b.Snapshot with { Revision = 2, MemberCount = 2 };
|
||||
b.Members = [alice, bob];
|
||||
|
|
@ -271,6 +278,196 @@ public sealed class SocialFellowshipPageControllerTests
|
|||
Assert.Equal(2, listBox.ViewportForTest!.Children.Count);
|
||||
}
|
||||
|
||||
/// <summary>Fix-round SF-5: the shrink counterpart of the join test
|
||||
/// above — a member LEAVING must also rebuild (not silently desync),
|
||||
/// and a departed fellow's panel selection must clear rather than stay
|
||||
/// stale (so a subsequent Dismiss/Leader click can't re-target a guid
|
||||
/// that is no longer on the roster).</summary>
|
||||
[Fact]
|
||||
public void Tick_MemberLeaves_ShrinksRoster_AndClearsSelectionIfTheyWereSelected()
|
||||
{
|
||||
UiElement root = BuildPageRoot(out UiTemplateListBox listBox, out _);
|
||||
var alice = new RuntimeFellowMemberSnapshot(0x50000001u, "Alice", 12, 100, 80, 60, 100, 80, 60, false);
|
||||
var bob = new RuntimeFellowMemberSnapshot(0x50000002u, "Bob", 10, 90, 70, 50, 90, 70, 50, false);
|
||||
var b = new FellowshipBindingsBuilder
|
||||
{
|
||||
Snapshot = new RuntimeFellowshipSnapshot
|
||||
{
|
||||
IsInFellowship = true, Revision = 1, MemberCount = 2, LeaderGuid = 0x50000001u,
|
||||
},
|
||||
Members = [alice, bob],
|
||||
LocalPlayerGuid = 0x50000001u, // Alice, the leader
|
||||
};
|
||||
|
||||
SocialFellowshipPageController controller = SocialFellowshipPageController.Bind(root, b.Build())!;
|
||||
Assert.Equal(2, listBox.ViewportForTest!.Children.Count);
|
||||
|
||||
UiElement bobRow = listBox.ViewportForTest!.Children[1];
|
||||
UiText bobName = Assert.IsType<UiText>(UiElement.FindDescendant(bobRow, RowNameTextId));
|
||||
bobName.OnClick!(); // selects Bob
|
||||
((UiButton)UiElement.FindDescendant(root, DismissButtonId)!).OnClick!();
|
||||
Assert.Contains("dismiss:50000002", b.Calls);
|
||||
b.Calls.Clear();
|
||||
|
||||
// Bob leaves (independently of the click above — e.g. he quit).
|
||||
b.Snapshot = b.Snapshot with { Revision = 2, MemberCount = 1 };
|
||||
b.Members = [alice];
|
||||
controller.Tick();
|
||||
|
||||
Assert.Single(listBox.ViewportForTest!.Children);
|
||||
|
||||
// The stale selection must be gone — Dismiss must not re-send
|
||||
// Bob's guid.
|
||||
((UiButton)UiElement.FindDescendant(root, DismissButtonId)!).OnClick!();
|
||||
Assert.DoesNotContain(b.Calls, c => c.StartsWith("dismiss:"));
|
||||
}
|
||||
|
||||
/// <summary>Fix-round SF-2: a permanently-unbuildable row template must
|
||||
/// be attempted once, at the real membership change, not re-attempted
|
||||
/// (a full DAT-locked <see cref="UiTemplateListBox.FlushPreservingScroll"/>
|
||||
/// rebuild) on every subsequent pure-vitals revision bump. Without the
|
||||
/// fix, comparing against <c>_rows.Count</c> (which stays 0 forever
|
||||
/// here) makes every Tick see a "membership change" that never actually
|
||||
/// happened.</summary>
|
||||
[Fact]
|
||||
public void Tick_RowTemplatePermanentlyFailsToBuild_DoesNotRetryOnEveryVitalsTick()
|
||||
{
|
||||
UiElement root = BuildPageRoot(out UiTemplateListBox listBox, out _);
|
||||
var member = new RuntimeFellowMemberSnapshot(0x50000001u, "Alice", 12, 100, 80, 60, 100, 80, 60, false);
|
||||
int resolverCalls = 0;
|
||||
var b = new FellowshipBindingsBuilder
|
||||
{
|
||||
Snapshot = new RuntimeFellowshipSnapshot { IsInFellowship = true, Revision = 1, MemberCount = 1 },
|
||||
Members = [member],
|
||||
TemplateResolver = (_, _) => { resolverCalls++; return null; }, // permanently unbuildable
|
||||
};
|
||||
|
||||
SocialFellowshipPageController controller = SocialFellowshipPageController.Bind(root, b.Build())!;
|
||||
Assert.Empty(listBox.ViewportForTest?.Children ?? []);
|
||||
int callsAfterFirstAttempt = resolverCalls;
|
||||
Assert.True(callsAfterFirstAttempt >= 1);
|
||||
|
||||
// Two pure vitals ticks: SAME member set, revision bumps (the exact
|
||||
// shape a 0x02C0 vitals refresh produces).
|
||||
b.Snapshot = b.Snapshot with { Revision = 2 };
|
||||
controller.Tick();
|
||||
b.Snapshot = b.Snapshot with { Revision = 3 };
|
||||
controller.Tick();
|
||||
|
||||
Assert.Equal(callsAfterFirstAttempt, resolverCalls);
|
||||
}
|
||||
|
||||
/// <summary>Fix-round SF-3: Recruit's "already a fellow" check must read
|
||||
/// TRUE membership, not which rows happen to have finished building —
|
||||
/// otherwise a permanently-unbuildable row lets Recruit light up for
|
||||
/// someone who IS already a fellow.</summary>
|
||||
[Fact]
|
||||
public void RecruitButton_TargetIsAnExistingFellowWhoseRowFailedToBuild_StaysDisabled()
|
||||
{
|
||||
UiElement root = BuildPageRoot(out _, out _);
|
||||
var member = new RuntimeFellowMemberSnapshot(0x50000001u, "Alice", 12, 100, 80, 60, 100, 80, 60, false);
|
||||
var selection = new SelectionState();
|
||||
selection.Select(0x50000001u, SelectionChangeSource.World); // targeting the (already-a-fellow) guid
|
||||
var b = new FellowshipBindingsBuilder
|
||||
{
|
||||
Snapshot = new RuntimeFellowshipSnapshot { IsInFellowship = true, Revision = 1, MemberCount = 1, LeaderGuid = 1u },
|
||||
Members = [member],
|
||||
LocalPlayerGuid = 1u,
|
||||
Selection = selection,
|
||||
TemplateResolver = (_, _) => null, // Alice's row never builds -- but she IS still a fellow
|
||||
};
|
||||
|
||||
SocialFellowshipPageController.Bind(root, b.Build());
|
||||
|
||||
Assert.False(((UiButton)UiElement.FindDescendant(root, RecruitButtonId)!).Enabled);
|
||||
}
|
||||
|
||||
// ── MUST-FIX 4: world→panel selection sync ──────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void WorldSelectionOfAFellow_EnablesDismissAndLeader_WithoutClickingTheirRow()
|
||||
{
|
||||
UiElement root = BuildPageRoot(out _, out _);
|
||||
var member = new RuntimeFellowMemberSnapshot(0x50000001u, "Alice", 12, 100, 80, 60, 100, 80, 60, false);
|
||||
var selection = new SelectionState();
|
||||
var b = new FellowshipBindingsBuilder
|
||||
{
|
||||
Snapshot = new RuntimeFellowshipSnapshot { IsInFellowship = true, Revision = 1, MemberCount = 1, LeaderGuid = 0xFFu },
|
||||
Members = [member],
|
||||
LocalPlayerGuid = 0xFFu, // leader
|
||||
Selection = selection,
|
||||
};
|
||||
SocialFellowshipPageController controller = SocialFellowshipPageController.Bind(root, b.Build())!;
|
||||
Assert.False(((UiButton)UiElement.FindDescendant(root, DismissButtonId)!).Enabled);
|
||||
Assert.False(((UiButton)UiElement.FindDescendant(root, LeaderButtonId)!).Enabled);
|
||||
|
||||
// MUST-FIX 4 — selecting Alice in the WORLD (not clicking her panel
|
||||
// row) must still enable Dismiss/Leader, matching retail's
|
||||
// gmFellowshipUI::UpdateFellowSelection reverse arm.
|
||||
selection.Select(0x50000001u, SelectionChangeSource.World);
|
||||
controller.Tick();
|
||||
|
||||
Assert.True(((UiButton)UiElement.FindDescendant(root, DismissButtonId)!).Enabled);
|
||||
Assert.True(((UiButton)UiElement.FindDescendant(root, LeaderButtonId)!).Enabled);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void WorldSelectionOfANonFellow_DoesNotClearAnExistingPanelSelection()
|
||||
{
|
||||
UiElement root = BuildPageRoot(out UiTemplateListBox listBox, out _);
|
||||
var member = new RuntimeFellowMemberSnapshot(0x50000001u, "Alice", 12, 100, 80, 60, 100, 80, 60, false);
|
||||
var selection = new SelectionState();
|
||||
var b = new FellowshipBindingsBuilder
|
||||
{
|
||||
Snapshot = new RuntimeFellowshipSnapshot { IsInFellowship = true, Revision = 1, MemberCount = 1, LeaderGuid = 0xFFu },
|
||||
Members = [member],
|
||||
LocalPlayerGuid = 0xFFu,
|
||||
Selection = selection,
|
||||
};
|
||||
SocialFellowshipPageController controller = SocialFellowshipPageController.Bind(root, b.Build())!;
|
||||
|
||||
UiElement row = Assert.Single(listBox.ViewportForTest!.Children);
|
||||
UiText name = Assert.IsType<UiText>(UiElement.FindDescendant(row, RowNameTextId));
|
||||
name.OnClick!(); // selects Alice via her row
|
||||
controller.Tick(); // Enabled is refreshed by RefreshButtonStates, which only runs in Tick
|
||||
Assert.True(((UiButton)UiElement.FindDescendant(root, DismissButtonId)!).Enabled);
|
||||
|
||||
// Retail's fallback arm: a world selection that is NOT a fellow
|
||||
// must not clear the panel's own selection while that fellow is
|
||||
// still on the roster.
|
||||
selection.Select(0x99999999u, SelectionChangeSource.World);
|
||||
controller.Tick();
|
||||
|
||||
Assert.True(((UiButton)UiElement.FindDescendant(root, DismissButtonId)!).Enabled);
|
||||
}
|
||||
|
||||
// ── N-0: optimistic Open/Close caption ──────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void OpenButton_Click_FlipsCaptionImmediately_NotWaitingForTheNextTick()
|
||||
{
|
||||
UiElement root = BuildPageRoot(out _, out _);
|
||||
var b = new FellowshipBindingsBuilder
|
||||
{
|
||||
Snapshot = new RuntimeFellowshipSnapshot { IsInFellowship = true, IsOpen = false, LeaderGuid = 1u },
|
||||
LocalPlayerGuid = 1u,
|
||||
ResolveString = (_, hash) =>
|
||||
hash == DatStringResolver.ComputeHash("ID_Fellowship_OpenFellowshipButtonText") ? "Open"
|
||||
: hash == DatStringResolver.ComputeHash("ID_Fellowship_CloseFellowshipButtonText") ? "Close"
|
||||
: null,
|
||||
};
|
||||
SocialFellowshipPageController.Bind(root, b.Build());
|
||||
var openButton = (UiButton)UiElement.FindDescendant(root, OpenButtonId)!;
|
||||
Assert.Equal("Open", openButton.Label);
|
||||
|
||||
openButton.OnClick!();
|
||||
|
||||
// N-0 (fix round): retail's Open button handler pre-toggles its own
|
||||
// state before the server echo (0x02BE) lands (lane B feature 11)
|
||||
// -- the caption flips on click, not on the next Tick.
|
||||
Assert.Equal("Close", openButton.Label);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Tick_NotInFellowship_ClearsAnyStaleRoster()
|
||||
{
|
||||
|
|
@ -294,8 +491,17 @@ public sealed class SocialFellowshipPageControllerTests
|
|||
|
||||
[Theory]
|
||||
[InlineData(false, true, 1, "12 0%")] // ShareXp off -> retail's literal 0.0
|
||||
[InlineData(true, true, 9, "12 31%")] // even split, 9 fellows -> 0.3111111 -> round(31.11) = 31
|
||||
[InlineData(true, true, 9, "12 31%")] // even split, 9 fellows -> 0.3111111 -> truncate(31.11) = 31
|
||||
[InlineData(true, false, 3, "12")] // proportional -> no acdream XP table -> level only, no invented %
|
||||
// MUST-FIX 1 (fix round): retail TRUNCATES (_ftol2 @0x0048ECC9), it
|
||||
// does not round. The stored float constants for 6 and 8 fellows are
|
||||
// 0.44999998807907104 and 0.3499999940395355 (byte-read from the
|
||||
// PDB-paired binary) -- ×100 = 44.999998.../34.999999..., which
|
||||
// truncate to 44/34, NOT 45/35 (what MathF.Round -- or even a naive
|
||||
// single-precision (int)(pct*100f) cast, since 0.45f*100f already
|
||||
// rounds UP to exactly 45.0f in single precision -- would produce).
|
||||
[InlineData(true, true, 6, "12 44%")]
|
||||
[InlineData(true, true, 8, "12 34%")]
|
||||
public void FormatStatsText_MatchesD5Rules(bool shareXp, bool evenSplit, int memberCount, string expected)
|
||||
{
|
||||
UiElement root = BuildPageRoot(out UiTemplateListBox listBox, out _);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue