fix(ui): OP5 re-check residuals R1/R2 (coordinator pass) — OP5 CLOSED
R1: UiRoot now delivers WM_CAPTURECHANGED (0x215 — retail's own Win32 event-id space) to the element losing pointer capture on BOTH release and re-target; UiScrollbar terminates a mid-drag gesture there, completing it (one DragCompleted flush persisting the user's last-seen value) and unlatching IsDragging — a panel-close keybind mid-drag or a second-button re-target can no longer latch the drag flag forever and silently suppress every later settings flush. Normal MouseUp paths no-op (the latch is already clear when capture releases). R2: the scalar latch arms BEFORE the track-click jump applies, so the jump's own ScalarChanged tick defers its flush to the MouseUp's single DragCompleted — one flush per press gesture, never the inline-then-completed double; the DragCompleted doc now states the real contract (fires once per value-capable gesture incl. capture loss) instead of the refuted never-on-jump claim. Tests: capture-loss mid-drag (ends + completes once + stray-MouseUp no-double), no-drag capture-change no-op, bare-track-click single-completion with the latch observed armed during the jump tick. Also reconciles the research doc's U4 row to its closure (the six caption pairs, the BN zero-fold post-mortem) per the OP6 rework's flag. Full Release suite: 13,128 passed / 4 skipped / 0 failed (one documented #250-class allocation flake on first run, green in isolation and on full-suite rerun). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
472525b99e
commit
67b0815c79
5 changed files with 142 additions and 10 deletions
|
|
@ -910,7 +910,7 @@ wire**. The user's own DAT dir currently holds `acclient.keymap` and `test.keyma
|
|||
| U1 | **What `m_default` is for the Character tab's 49 checkboxes.** `InitOptions` never calls `SetDefaultValue`, so "Defaults" behaviour on the biggest tab is unestablished. | Read `UIOption_Checkbox`'s ctor and `UIOption::InqDefaultGameplayOptionProperty` / `InqGameplayOptionNameAndTooltip`. **Blocking for the Defaults button.** |
|
||||
| U2 | **Which options are "auto-save"** (immediate `0x0005`) vs batched into the `0x01A1` blob. | `CPlayerModule::IsAutoSaveOption` — enumerate it. This is exactly handoff Q4's discriminator. |
|
||||
| U3 | Whether acdream ships the **50th Character row** ("Listen to PK death messages", `ID_PlayerOption_HearPKDeaths`, string `0x0D16E9A3` present in the DAT, absent from the 2013 code). | Design call. The string and ACE support exist; only the 2013 wiring is missing. |
|
||||
| U4 | The exact **`SetSliderLabel` operand pairs** for the Config tab's six labelled sliders (which of `Dark/Bright`, `Speed/Detail`, `Close/Far`, `Narrow/Wide`, `Slow/Fast`, `Soft/Hard` goes with which). | Same push-imm decode technique as `AddHeader`, applied at the six `SetSliderLabel` sites. Cosmetic; not blocking. |
|
||||
| U4 | ~~The exact **`SetSliderLabel` operand pairs** for the Config tab's six labelled sliders.~~ **CLOSED 2026-08-11 (OP6 review + rework).** An intermediate OP6 claim that retail ships ZERO captions was the SAME BN zero-fold artifact as `AddHeader` (the review byte-decoded the `mov ecx/edx,[disp32]` string-id loads at `0x0049E4C6` etc.; the rework independently re-read 2 of 6 sites + the PDB global sequence `0x0083E768`–`0x0083E794`). The six pairs, in declaration order: Stiffness `Soft/Hard`, Adjustment Speed `Slow/Fast`, FOV `Narrow/Wide`, Screen Brightness `Dark/Bright`, Graphics Performance `Speed/Detail`, Degrade Distance `Close/Far`. Implemented in the OP6 rework (`472525b9`). | Evidence: `docs/research/2026-08-11-op6-review.md` M1; the rework commit's own byte log. |
|
||||
| U5 | Semantics of layout property **`0x57`** (page/window registration) and **`0x58`** (`enum 1` on every options root). | Find the `GetAttribute_Enum(this, 0x57 …)` read. Not blocking — the toggle action can be driven from `0x12` on the buttons. |
|
||||
| U6 | Consumer of input actions **`0x10000027`** ("Exit Game") and **`0x1000001F`** ("Configure Keyboard"). | Neither has a default keybind. `0x1000001F` is well corroborated by the keyboard screen's OK/Cancel; `0x10000027` rests on the label alone. |
|
||||
| U7 | The two **`support.turbine.com` URLs** behind Urgent Assistance / Report Abuse. | Dead endpoints. Recommend a **register row**: acdream shows a "not available" notice rather than launching a browser at a dead Turbine host. Extract the exact strings only if the register row needs to quote them. |
|
||||
|
|
|
|||
|
|
@ -68,6 +68,13 @@ public static class UiEventType
|
|||
public const int MouseMove = 0x200;
|
||||
public const int MouseDown = 0x201; // left button down
|
||||
public const int MouseUp = 0x202; // left button up
|
||||
/// <summary>WM_CAPTURECHANGED (0x215) — delivered by <see cref="UiRoot"/>
|
||||
/// to the element LOSING pointer capture when capture is released or
|
||||
/// re-targeted WITHOUT a MouseUp reaching that element (panel hidden by a
|
||||
/// keybind mid-drag; a second button press re-targeting capture). A widget
|
||||
/// holding gesture state keyed to capture (the scrollbar's drag latch)
|
||||
/// must terminate the gesture here (OP5 re-check R1, 2026-08-11).</summary>
|
||||
public const int CaptureChanged = 0x215;
|
||||
public const int DoubleClickLeft = 0x203;
|
||||
public const int RightDown = 0x204;
|
||||
public const int RightUp = 0x205;
|
||||
|
|
|
|||
|
|
@ -787,6 +787,7 @@ public sealed class UiRoot : UiElement
|
|||
if (ReferenceEquals(Captured, e)) return;
|
||||
UiElement? previous = Captured;
|
||||
Captured = e;
|
||||
NotifyCaptureLost(previous);
|
||||
PointerCaptureChanged?.Invoke(previous, e);
|
||||
}
|
||||
|
||||
|
|
@ -797,10 +798,27 @@ public sealed class UiRoot : UiElement
|
|||
// Retail restarts the tooltip idle deadline when capture is released.
|
||||
_hoverStartedMs = _nowMs;
|
||||
_tooltipFired = false;
|
||||
NotifyCaptureLost(previous);
|
||||
if (previous is not null)
|
||||
PointerCaptureChanged?.Invoke(previous, null);
|
||||
}
|
||||
|
||||
/// <summary>OP5 re-check R1 (2026-08-11): WM_CAPTURECHANGED to the element
|
||||
/// losing capture — a capture drop WITHOUT a MouseUp (panel hidden by a
|
||||
/// keybind mid-drag; a second button re-targeting capture) must let the
|
||||
/// element terminate any capture-keyed gesture (the scrollbar's drag
|
||||
/// latch, which otherwise reads IsDragging=true forever and silently
|
||||
/// suppresses every later settings flush). A normal MouseUp path is
|
||||
/// unaffected: the gesture state is already cleared by the time capture
|
||||
/// releases, so the handler no-ops.</summary>
|
||||
private static void NotifyCaptureLost(UiElement? previous)
|
||||
{
|
||||
if (previous is null) return;
|
||||
var lost = new UiEvent(
|
||||
previous.EventId, previous, UiEventType.CaptureChanged);
|
||||
previous.OnEvent(in lost);
|
||||
}
|
||||
|
||||
// ── Window manager (named top-level windows: Show / Hide / Toggle) ───
|
||||
|
||||
// Registry state lives in RetailWindowManager; methods below are compatibility forwarders.
|
||||
|
|
|
|||
|
|
@ -44,14 +44,19 @@ public sealed class UiScrollbar : UiElement
|
|||
public bool IsDragging => _draggingThumb;
|
||||
|
||||
/// <summary>
|
||||
/// Fires once, at the <c>MouseUp</c> that ends a thumb drag — never on a
|
||||
/// <c>MouseUp</c> that was not preceded by an actual drag (a bare click that
|
||||
/// only page-scrolled or jumped, or a stray <c>MouseUp</c> with no prior
|
||||
/// <c>MouseDown</c>). OP5 review fix S1: the drag-end seam neither
|
||||
/// <see cref="ScalarChanged"/> (fires on every tick) nor <see cref="Model"/>
|
||||
/// scrolling provided — the Chat tab's opacity sliders use this to flush a
|
||||
/// batched settings write exactly once per drag gesture instead of once per
|
||||
/// <c>MouseMove</c>.
|
||||
/// Fires once at the end of a press gesture that could have changed the
|
||||
/// value: the <c>MouseUp</c> ending a MODEL-mode thumb drag, the
|
||||
/// <c>MouseUp</c> ending ANY scalar-mode press (thumb drag OR bare
|
||||
/// track-click jump — OP5 re-check R2: the scalar latch arms on
|
||||
/// <c>MouseDown</c> before the jump applies, so the jump's flush defers
|
||||
/// here rather than double-flushing), or a <c>WM_CAPTURECHANGED</c>
|
||||
/// capture loss mid-drag (OP5 re-check R1 — the gesture completes with
|
||||
/// the user's last-seen value). Never fires on a stray <c>MouseUp</c>
|
||||
/// with no prior press, nor on model-mode button/page clicks. OP5 review
|
||||
/// fix S1: the drag-end seam neither <see cref="ScalarChanged"/> (fires
|
||||
/// per tick) nor <see cref="Model"/> scrolling provided — the Chat tab's
|
||||
/// opacity sliders flush a batched settings write exactly once per
|
||||
/// gesture instead of once per <c>MouseMove</c>.
|
||||
/// </summary>
|
||||
public Action? DragCompleted { get; set; }
|
||||
|
||||
|
|
@ -372,6 +377,20 @@ public sealed class UiScrollbar : UiElement
|
|||
|
||||
public override bool OnEvent(in UiEvent e)
|
||||
{
|
||||
// OP5 re-check R1: a capture drop without a MouseUp (panel hidden by
|
||||
// a keybind mid-drag; a second button re-targeting capture) ends the
|
||||
// drag HERE — completing the gesture (flush via DragCompleted) so the
|
||||
// user's last-seen value persists and IsDragging cannot latch true
|
||||
// forever. A normal MouseUp already cleared the latch, so this no-ops.
|
||||
if (e.Type == UiEventType.CaptureChanged)
|
||||
{
|
||||
bool wasDragging = _draggingThumb;
|
||||
_draggingThumb = false;
|
||||
_pressedButton = EndButton.None;
|
||||
if (wasDragging) DragCompleted?.Invoke();
|
||||
return false; // informational — never consumes
|
||||
}
|
||||
|
||||
if (IsModelDisabled)
|
||||
{
|
||||
_draggingThumb = false;
|
||||
|
|
@ -532,6 +551,12 @@ public sealed class UiScrollbar : UiElement
|
|||
float travel = MathF.Max(1f, Width - thumbWidth);
|
||||
float thumbX = travel * ScalarPosition;
|
||||
float x = e.Data1;
|
||||
// OP5 re-check R2: the latch is set BEFORE the track-click
|
||||
// jump below, so the jump's own ScalarChanged tick reads
|
||||
// IsDragging=true and DEFERS its flush to the MouseUp's
|
||||
// DragCompleted — one flush per press gesture, never the
|
||||
// inline-then-DragCompleted double the previous order caused.
|
||||
_draggingThumb = true;
|
||||
if (x >= thumbX && x <= thumbX + thumbWidth)
|
||||
{
|
||||
_dragOffsetX = x - thumbX;
|
||||
|
|
@ -541,7 +566,6 @@ public sealed class UiScrollbar : UiElement
|
|||
_dragOffsetX = thumbWidth * 0.5f;
|
||||
ChangeScalarPosition((x - _dragOffsetX) / travel);
|
||||
}
|
||||
_draggingThumb = true;
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -178,6 +178,89 @@ public class UiScrollbarTests
|
|||
Assert.Equal(1, completedCount);
|
||||
}
|
||||
|
||||
// ── OP5 re-check R1/R2 (2026-08-11, coordinator pass) ───────────────────
|
||||
|
||||
[Fact]
|
||||
public void CaptureLossMidDrag_EndsTheGesture_CompletesOnce_AndUnlatchesIsDragging()
|
||||
{
|
||||
// R1: UiRoot can drop capture WITHOUT a MouseUp (panel hidden by a
|
||||
// keybind mid-drag; a second button re-targeting capture). The
|
||||
// WM_CAPTURECHANGED delivery must end the drag, fire ONE completion
|
||||
// (persisting the user's last-seen value), and unlatch IsDragging —
|
||||
// otherwise every later Reset/Defaults flush is silently suppressed.
|
||||
int completedCount = 0;
|
||||
var bar = new UiScrollbar
|
||||
{
|
||||
Width = 90f,
|
||||
Height = 14f,
|
||||
Horizontal = true,
|
||||
ScalarChanged = _ => { },
|
||||
DragCompleted = () => completedCount++,
|
||||
};
|
||||
bar.SetScalarPosition(0f);
|
||||
|
||||
Assert.True(bar.OnEvent(new UiEvent(0u, bar, UiEventType.MouseDown, Data1: 5)));
|
||||
Assert.True(bar.OnEvent(new UiEvent(0u, bar, UiEventType.MouseMove, Data1: 30)));
|
||||
Assert.True(bar.IsDragging);
|
||||
|
||||
bar.OnEvent(new UiEvent(0u, bar, UiEventType.CaptureChanged));
|
||||
|
||||
Assert.False(bar.IsDragging);
|
||||
Assert.Equal(1, completedCount);
|
||||
|
||||
// A later stray MouseUp (capture already gone) must not double-fire.
|
||||
bar.OnEvent(new UiEvent(0u, bar, UiEventType.MouseUp, Data1: 30));
|
||||
Assert.Equal(1, completedCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CaptureChange_WithNoActiveDrag_IsANoOp()
|
||||
{
|
||||
int completedCount = 0;
|
||||
var bar = new UiScrollbar
|
||||
{
|
||||
Width = 90f,
|
||||
Height = 14f,
|
||||
Horizontal = true,
|
||||
DragCompleted = () => completedCount++,
|
||||
};
|
||||
|
||||
bar.OnEvent(new UiEvent(0u, bar, UiEventType.CaptureChanged));
|
||||
|
||||
Assert.False(bar.IsDragging);
|
||||
Assert.Equal(0, completedCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HorizontalScalar_BareTrackClickJump_DefersItsTickAndCompletesExactlyOnce()
|
||||
{
|
||||
// R2: a bare track click (outside the thumb) jumps the scalar. The
|
||||
// latch now arms BEFORE the jump applies, so the jump's own
|
||||
// ScalarChanged tick observes IsDragging=true (a consumer defers its
|
||||
// flush) and the MouseUp's single DragCompleted carries the gesture's
|
||||
// one flush — never the inline-then-completed double.
|
||||
int completedCount = 0;
|
||||
bool draggingDuringTick = false;
|
||||
UiScrollbar bar = null!;
|
||||
bar = new UiScrollbar
|
||||
{
|
||||
Width = 90f,
|
||||
Height = 14f,
|
||||
Horizontal = true,
|
||||
ScalarChanged = _ => draggingDuringTick = bar.IsDragging,
|
||||
DragCompleted = () => completedCount++,
|
||||
};
|
||||
bar.SetScalarPosition(0f); // thumb spans [0, 16]
|
||||
|
||||
// Click far outside the thumb — the jump branch.
|
||||
Assert.True(bar.OnEvent(new UiEvent(0u, bar, UiEventType.MouseDown, Data1: 70)));
|
||||
Assert.True(draggingDuringTick); // the jump tick saw the latch armed
|
||||
Assert.Equal(0, completedCount);
|
||||
|
||||
Assert.True(bar.OnEvent(new UiEvent(0u, bar, UiEventType.MouseUp, Data1: 70)));
|
||||
Assert.Equal(1, completedCount); // one gesture, one completion
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HorizontalModel_DragCompleted_FiresOnceAtMouseUp()
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue