fix(ui,runtime): OP4 review fixes — live re-seed, enable-gating, Combat panel re-point, universal timestamps

Both OP4 reviews converged on one headline bug (Character-tab rows never
re-read live server truth after their pre-login constructor-word seed) plus
overlapping MUST-FIXes. All ten converged/consolidated findings land here:

MUST-FIX:
- BoolOptionRow.SaveCurrentValue now re-reads its live binding (retail's
  GetValue()-into-SaveCurrentValue) on every OnShown — panel open, tab
  switch in, initial activation — instead of trusting the pre-login
  constructor word it was built with. Reset/tab-switch can now only
  restore values that were actually live at the last show. LockUI's
  host.Root.UiLocked one-shot mount seed now also converges on every
  PlayerDescription via the existing OnCharacterOptionsChanged hook.
- Apply/Reset are wired to OptionPage.OnOptionChanged in production
  (Ghosted when nothing changed, Normal when dirty, run once at bind so
  both start disabled per retail's PostInit); Defaults stays ungated.
- The Combat panel's three LEDs (Repeat Attacks/Auto Target/Keep in View)
  now read/write the same RuntimeCharacterOptionsState seam the Character
  tab uses instead of a disconnected client-local GameplaySettings copy —
  closes the "two writable copies" divergence. The three now-orphaned
  GameplaySettings fields and RuntimeSettingsController's mirror
  properties/SetCombatGameplay are deleted outright; the headless host's
  hardcoded AutoRepeatAttack/AutoTarget now read the live option bit.
- RuntimeSettingsController.SetUiLocked's convergence guard now compares
  against the last value actually applied to the runtime target instead
  of the persisted GameplaySettings.LockUI snapshot, which could already
  match a server-derived request without ever having been pushed.

SHOULD-FIX:
- DisplayTimeStamps now prefixes every chat producer (ChatLog.Append is
  the one seam all of them funnel through), not just AddText's own
  callers — heard speech, emotes, Turbine channels, and combat text were
  previously missed. The prefix format escapes its colons and forces
  InvariantCulture instead of the culture-dependent TimeSeparator
  placeholder.
- sky.frag now honors uFogParams.w (fog mode) like the mesh/terrain
  shaders, so Disable Distance Fog stops the sky dome's horizon band from
  blending toward fog color too.
- Corrected the "byte-verified" overclaim on the timestamp format string
  doc comment (BN-sourced, wire doc U6) and the AP-194 anchor-column
  class-name typo; the RunAsDefaultMovement doc comments now cite retail's
  actual acclient.h enumerator name.
- Added: DispatcherMovementInputSource's option x modifier truth table
  (incl. || AutoRunActive with the option off), the per-page Apply/Reset
  enable-gate tests, a real checkbox.OnClick/ToggleBehavior-driven click
  test, and hash-pins for the six header string keys.
- Gate script step 8 corrected for the logout-flush false-failure
  (closing the panel before relogging is load-bearing); a new step
  documents the enable-gate sequence and the Combat-panel/Character-tab
  cross-check.

Register: AP-196 (the Group-C default-source change + GameplaySettings
retirement) and AP-197 (the ignored per-character timestamp format
override) filed in this commit.

Full Release suite: 13,044 passed / 4 skipped / 0 failed (was 13,008/4/0;
net +36 tests from new coverage and legitimate assertion updates from the
GameplaySettings retirement).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-11 05:30:26 +02:00
parent 85afaae5fd
commit bc43fb1d1d
36 changed files with 923 additions and 220 deletions

View file

@ -142,6 +142,25 @@ public sealed class CharacterOptionsPageControllerTests
DatStringResolver.ComputeHash("ID_PlayerOption_HearPKDeaths"));
}
[Theory]
[InlineData("ID_CharacterOption_UIBehavior_Section", 0x06489B6Eu)]
[InlineData("ID_CharacterOption_UIDisplay_Section", 0x0A9BC99Eu)]
[InlineData("ID_CharacterOption_Grouping_Section", 0x0CBAAFAEu)]
[InlineData("ID_CharacterOption_OtherPlayers_Section", 0x0872DFFEu)]
[InlineData("ID_CharacterOption_CharacterBehavior_Section", 0x08674D5Eu)]
[InlineData("ID_CharacterOption_Chat_Section", 0x0987FE8Eu)]
public void HeaderKey_RetailNameHash_MatchesByteVerifiedStringId(
string headerKey, uint expectedHash)
{
// SF-2 (OP4 review-fix round, 2026-08-11): structure doc §7's six
// byte-verified header string ids — previously verified only by
// hand in the mechanism review, not pinned by a test. A typo in a
// HeaderKey literal would otherwise produce a silently blank
// header that only the user's eye catches.
Assert.Equal(expectedHash, DatStringResolver.ComputeHash(headerKey));
Assert.Contains(headerKey, CharacterOptionsPageController.Groups.Select(g => g.HeaderKey));
}
[Theory]
[MemberData(nameof(RetailEnumNameCases))]
public void RetailName_MatchesVerbatimAcclientEnumSpelling(
@ -479,6 +498,147 @@ public sealed class CharacterOptionsPageControllerTests
Assert.False(controller.CharacterPage.Changed);
}
// ── MUST-FIX 1 (OP4 review-fix round, 2026-08-11): the panel re-seeds
// from the live binding on OnShown instead of the pre-login
// constructor-default word it was constructed with. ─────────────────
[Fact]
public void OnShown_ReSeedsRow_FromLiveBindingValue_ChangedBehindItsBack()
{
// Simulates a fresh PlayerDescription landing (or simply "the
// character's real server value differs from the word this row
// was constructed with") without ever calling SetCurrentValue.
(OptionsPanelController controller, FakeBindings bindings, _) = BindReal();
CharacterOptionId id = AllRows().First().Id;
var row = Assert.IsType<BoolOptionRow>(controller.CharacterPage.Rows[0]);
bool initial = row.Current;
bindings.Values[id] = !initial;
bindings.Sets.Clear();
controller.CharacterPage.OnShown();
Assert.Equal(!initial, row.Current);
Assert.Equal(!initial, row.Saved);
Assert.False(controller.CharacterPage.Changed);
// The re-read must never round-trip through SetOption — that would
// send the re-seeded value back out over the wire (retail's own
// GetValue()-into-SaveCurrentValue never calls SetPlayerOption).
Assert.Empty(bindings.Sets);
}
[Fact]
public void OnShown_AllFiftyRows_ConvergeToTheLiveBindingSnapshot()
{
// The pre-login case: bind against a constructor-default word (the
// fake's dictionary starts empty -> every row seeds false), THEN
// the "server" state is populated (a PlayerDescription landing),
// THEN the page is shown — every one of the 50 rows must converge,
// matching retail's own InitOptions()+PostInit() / first tab-
// activation schedule (SaveCurrentValue re-reads GetValue() live).
var fakeBindings = new FakeBindings();
var random = new Random(20260811);
foreach (CharacterOptionsPageController.RowSpec spec in AllRows())
fakeBindings.Values[spec.Id] = random.Next(2) == 0;
ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost();
OptionsPanelController controller = OptionsPanelController.Bind(
layout,
new OptionsPanelController.Callbacks(
Toggle: () => { },
RequestExitToCharacterSelection: () => { },
ExitGame: () => { },
UseMouseTurningSettings: () => { },
DisplaySystemMessage: _ => { }))!;
bool bound = CharacterOptionsPageController.Bind(
layout,
controller.CharacterPage,
MakeTemplateResolver(),
(_, _) => null,
fakeBindings.ToBindings());
Assert.True(bound);
controller.CharacterPage.OnShown();
int i = 0;
foreach (CharacterOptionsPageController.RowSpec spec in AllRows())
{
var row = (BoolOptionRow)controller.CharacterPage.Rows[i++];
Assert.Equal(fakeBindings.Values[spec.Id], row.Current);
Assert.Equal(fakeBindings.Values[spec.Id], row.Saved);
}
Assert.False(controller.CharacterPage.Changed);
}
[Fact]
public void Reset_AfterReseed_RestoresTheLiveValue_NotTheStaleConstructionDefault()
{
// The historical bug MF-1 closes: before the fix, Reset/tab-switch
// could only ever restore whatever the row was seeded with AT BIND
// TIME (the pre-login constructor word) — visually-idempotent
// "toggle then cancel" could silently mutate the server bit in the
// wrong direction. After the fix, OnShown re-seeds _saved from the
// live bit first, so Reset can only revert to what was ACTUALLY
// live at the last show.
(OptionsPanelController controller, FakeBindings bindings, _) = BindReal();
CharacterOptionId id = AllRows().First().Id;
var row = Assert.IsType<BoolOptionRow>(controller.CharacterPage.Rows[0]);
bindings.Values[id] = true;
controller.CharacterPage.OnShown(); // re-seed: current == saved == true
bindings.Sets.Clear();
row.SetCurrentValue(false); // user toggles it off, never clicks Apply
controller.CharacterPage.Reset();
Assert.True(row.Current);
Assert.Contains(bindings.Sets, s => s.Id == id && s.Value);
}
[Fact]
public void ClickingTheRealCheckboxWidget_PublishesSetOption_ViaMouseDownUpClick()
{
// SF-2/S6 (OP4 review-fix round, 2026-08-11): every other test in
// this suite drives BoolOptionRow.SetCurrentValue directly, which
// would stay green even if the toggle template's checkbox
// (0x10000219) ever lost its authored DAT property 0x0B
// (UiButton.ToggleBehavior) — the mechanism the whole LED click
// interaction rests on (mechanism review §1.5). This drives the
// REAL MouseDown/MouseUp/Click sequence: MouseUp flips
// UiButton.Selected FIRST (ToggleBehavior), then Click invokes
// checkbox.OnClick, which reads the NEW Selected value.
ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost();
OptionsPanelController controller = OptionsPanelController.Bind(
layout,
new OptionsPanelController.Callbacks(
Toggle: () => { },
RequestExitToCharacterSelection: () => { },
ExitGame: () => { },
UseMouseTurningSettings: () => { },
DisplaySystemMessage: _ => { }))!;
var fakeBindings = new FakeBindings();
bool bound = CharacterOptionsPageController.Bind(
layout, controller.CharacterPage, MakeTemplateResolver(), (_, _) => null,
fakeBindings.ToBindings());
Assert.True(bound);
var listBox = Assert.IsType<UiTemplateListBox>(
layout.FindElement(CharacterOptionsPageController.ListBoxElementId));
var checkbox = Assert.IsType<UiButton>(
UiElement.FindDescendant(listBox, 0x10000219u));
CharacterOptionId id = AllRows().First().Id;
Assert.False(checkbox.Selected);
checkbox.OnEvent(new UiEvent(0, checkbox, UiEventType.MouseDown, Data1: 0, Data2: 0));
checkbox.OnEvent(new UiEvent(0, checkbox, UiEventType.MouseUp, Data1: 0, Data2: 0));
checkbox.OnEvent(new UiEvent(0, checkbox, UiEventType.Click));
Assert.True(checkbox.Selected);
var set = Assert.Single(fakeBindings.Sets);
Assert.Equal(id, set.Id);
Assert.True(set.Value);
}
[Fact]
public void ScrollbarLinkage_ModelPointsAtTheListBoxScroll()
{

View file

@ -2,8 +2,7 @@ using AcDream.App.Combat;
using AcDream.App.UI;
using AcDream.App.UI.Layout;
using AcDream.Core.Combat;
using AcDream.Runtime.Gameplay;
using AcDream.UI.Abstractions.Panels.Settings;
using AcDream.Core.Net.Messages;
namespace AcDream.App.Tests.UI.Layout;
@ -11,6 +10,21 @@ public sealed class CombatUiControllerTests
{
private static (uint, int, int) NoTex(uint _) => (0u, 0, 0);
/// <summary>
/// OP4 review-fix round (2026-08-11, MUST-FIX 3 / blast M2): the three
/// combat LEDs now read/write through the SAME server-bit seam the
/// Character tab's rows use — this fake is the test-local stand-in for
/// the production RuntimeCharacterOptionsState-backed binding.
/// </summary>
private sealed class FakeOptionBindings
{
public Dictionary<CharacterOptionId, bool> Values { get; } = new();
public CombatUiController.Bindings ToBindings() => new(
CurrentValue: id => Values.TryGetValue(id, out bool v) && v,
SetOption: (id, value) => Values[id] = value);
}
[Fact]
public void CombatMode_ShowsPhysicalAndMagicPages_AndSelectsMediumByDefault()
{
@ -19,9 +33,9 @@ public sealed class CombatUiControllerTests
using var attacks = CreateAttacks(combat, () => now, []);
var (layout, basic, spellcasting, power, high, medium, low) = BuildLayout();
var visibility = new List<bool>();
GameplaySettings gameplay = GameplaySettings.Default;
var options = new FakeOptionBindings();
using var controller = CombatUiController.Bind(
layout, combat, attacks, () => gameplay, value => gameplay = value,
layout, combat, attacks, options.ToBindings(),
Labels, visibility.Add)!;
controller.SyncVisibility();
@ -45,9 +59,9 @@ public sealed class CombatUiControllerTests
var combat = new CombatState();
using var attacks = CreateAttacks(combat, () => now, sent);
var (layout, basic, advanced, power, high, _, _) = BuildLayout();
GameplaySettings gameplay = GameplaySettings.Default;
var options = new FakeOptionBindings();
using var controller = CombatUiController.Bind(
layout, combat, attacks, () => gameplay, value => gameplay = value,
layout, combat, attacks, options.ToBindings(),
Labels, _ => { })!;
combat.SetCombatMode(CombatMode.Melee);
@ -69,9 +83,10 @@ public sealed class CombatUiControllerTests
var combat = new CombatState();
using var attacks = CreateAttacks(combat, () => 0d, []);
var (layout, _, _, _, _, _, _) = BuildLayout();
GameplaySettings gameplay = GameplaySettings.Default;
var options = new FakeOptionBindings();
options.Values[CharacterOptionId.AutoTarget] = true;
using var controller = CombatUiController.Bind(
layout, combat, attacks, () => gameplay, value => gameplay = value,
layout, combat, attacks, options.ToBindings(),
Labels, _ => { })!;
var autoTarget = Assert.IsType<UiButton>(layout.FindElement(CombatUiController.AutoTargetId));
@ -79,7 +94,7 @@ public sealed class CombatUiControllerTests
autoTarget.OnEvent(new UiEvent(0, autoTarget, UiEventType.MouseUp, Data1: 3, Data2: 3));
autoTarget.OnEvent(new UiEvent(0, autoTarget, UiEventType.Click, Data1: 3, Data2: 3));
Assert.False(gameplay.AutoTarget);
Assert.False(options.Values[CharacterOptionId.AutoTarget]);
}
[Fact]
@ -88,9 +103,9 @@ public sealed class CombatUiControllerTests
var combat = new CombatState();
using var attacks = CreateAttacks(combat, () => 0d, []);
var (layout, _, _, power, _, _, _) = BuildLayout();
GameplaySettings gameplay = GameplaySettings.Default;
var options = new FakeOptionBindings();
using var controller = CombatUiController.Bind(
layout, combat, attacks, () => gameplay, value => gameplay = value,
layout, combat, attacks, options.ToBindings(),
Labels, _ => { })!;
var speed = Assert.IsType<UiText>(layout.FindElement(CombatUiController.SpeedLabelId));
@ -108,9 +123,9 @@ public sealed class CombatUiControllerTests
ImportedLayout layout = LayoutImporter.Build(info, NoTex, datFont: null);
var combat = new CombatState();
using var attacks = CreateAttacks(combat, () => 0d, []);
GameplaySettings gameplay = GameplaySettings.Default;
var options = new FakeOptionBindings();
using var controller = CombatUiController.Bind(
layout, combat, attacks, () => gameplay, value => gameplay = value,
layout, combat, attacks, options.ToBindings(),
Labels, _ => { })!;
ApplyAnchors(layout.Root);

View file

@ -158,6 +158,113 @@ public sealed class OptionsPanelControllerTests
Assert.Equal(0, flushCount);
}
// ── MUST-FIX 2 (OP4 review-fix round, 2026-08-11): Apply/Reset gating ──────
private static (UiButton Apply, UiButton Reset, UiButton Defaults) GetCharacterPageButtons(
OptionsPanelController controller)
{
UiElement pageRoot = UiElement.FindDescendant(controller.Root, 0x10000211u)!; // Character page slot
var apply = Assert.IsType<UiButton>(UiElement.FindDescendant(pageRoot, 0x100001FCu));
var reset = Assert.IsType<UiButton>(UiElement.FindDescendant(pageRoot, 0x100001FDu));
var defaults = Assert.IsType<UiButton>(UiElement.FindDescendant(pageRoot, 0x100001FEu));
return (apply, reset, defaults);
}
[Fact]
public void CharacterPage_ApplyAndReset_StartDisabled_OnFreshBind()
{
// Retail's PostInit calls InitOptions() then OnOptionChanged(0), so
// the pair starts Ghosted before any row has ever been touched.
ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost();
var calls = new List<string>();
OptionsPanelController controller =
OptionsPanelController.Bind(layout, MakeCallbacks(calls))!;
(UiButton apply, UiButton reset, _) = GetCharacterPageButtons(controller);
Assert.False(apply.Enabled);
Assert.False(reset.Enabled);
}
[Fact]
public void CharacterPage_OneLedClick_EnablesApplyAndReset()
{
ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost();
var calls = new List<string>();
OptionsPanelController controller =
OptionsPanelController.Bind(layout, MakeCallbacks(calls))!;
var row = new BoolOptionRow(initial: false, defaultValue: false);
controller.CharacterPage.Register(row);
(UiButton apply, UiButton reset, _) = GetCharacterPageButtons(controller);
row.SetCurrentValue(true);
Assert.True(apply.Enabled);
Assert.True(reset.Enabled);
}
[Fact]
public void CharacterPage_Apply_DisablesApplyAndResetAgain()
{
ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost();
var calls = new List<string>();
OptionsPanelController controller =
OptionsPanelController.Bind(layout, MakeCallbacks(calls))!;
var row = new BoolOptionRow(initial: false, defaultValue: false);
controller.CharacterPage.Register(row);
(UiButton apply, UiButton reset, _) = GetCharacterPageButtons(controller);
row.SetCurrentValue(true);
controller.CharacterPage.Apply();
Assert.False(apply.Enabled);
Assert.False(reset.Enabled);
}
[Fact]
public void CharacterPage_Defaults_LeavesApplyAndResetEnabled_WhenSomethingActuallyChanged()
{
ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost();
var calls = new List<string>();
OptionsPanelController controller =
OptionsPanelController.Bind(layout, MakeCallbacks(calls))!;
var row = new BoolOptionRow(initial: false, defaultValue: true);
controller.CharacterPage.Register(row);
(UiButton apply, UiButton reset, _) = GetCharacterPageButtons(controller);
controller.CharacterPage.Defaults();
Assert.True(row.Current);
Assert.True(controller.CharacterPage.Changed);
Assert.True(apply.Enabled);
Assert.True(reset.Enabled);
}
[Fact]
public void CharacterPage_Defaults_IsNeverGated()
{
// Retail's Defaults override never fetches its own child id at all
// — it must never grey itself out, before OR after a real change.
ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost();
var calls = new List<string>();
OptionsPanelController controller =
OptionsPanelController.Bind(layout, MakeCallbacks(calls))!;
var row = new BoolOptionRow(initial: false, defaultValue: false);
controller.CharacterPage.Register(row);
(_, _, UiButton defaults) = GetCharacterPageButtons(controller);
Assert.True(defaults.Enabled);
row.SetCurrentValue(true);
Assert.True(defaults.Enabled);
controller.CharacterPage.Apply();
Assert.True(defaults.Enabled);
controller.CharacterPage.Defaults();
Assert.True(defaults.Enabled);
}
// ── Close button ─────────────────────────────────────────────────────────
[Fact]