fix(ui): OP6 rework — six range captions, un-invert Sound enabled flags, five font faces
Fixes all three MUST-FIX findings from the OP6 REJECT review (docs/research/2026-08-11-op6-review.md) plus its SHOULD-FIXes and NOTEs. M1 — the "retail ships zero range captions" claim was a Binary Ninja constant-folding artifact (the same class the header-string globals a few lines above already worked around). The six SetSliderLabel call sites byte-decode to reads of runtime-filled ID_Graphics_Value_* globals, not immediate zeros (PE-byte-verified against the PDB-paired acclient.exe, independently re-derived in this session, not just re-asserted from the review). ConfigOptionsPageController.BuildSliderRow gained optional rangeLowKey/rangeHighKey parameters wired for all six idx6 sliders (Camera Stiffness Soft/Hard, Adjustment Speed Slow/Fast, FOV Narrow/Wide, Screen Brightness Dark/Bright, Graphics Performance Speed/Detail, Degrade Distance Close/Far) via the same SetRangeLabel mechanism OP5's Chat opacity sliders already established. Mouse Look Sensitivity (idx3) correctly stays uncaptioned — the one genuine SetSliderLabel omission. Class doc corrected; gate-script lines 535/653-equivalent corrected in place. M2 — the three Sound "Disabled" toggles were semantically inverted: SoundManager::effect_sounds_enabled/ambient_sounds_enabled/ interface_sounds_enabled are all compiled = 1 in .data, and UserPreferences::RegisterPreference binds the checkbox's boolean value DIRECTLY onto those enabled-sense statics — checked-by-default means enabled-by-default, not disabled. AudioSettings.SfxDisabled/AmbientDisabled/ InterfaceDisabled renamed to SfxEnabled/AmbientEnabled/InterfaceEnabled (fresh JSON keys — the rejected slice's keys never shipped in an accepted build); RuntimeSettingsStartupTargets.ApplyAudio now computes effective volume through the extracted, independently-unit-tested pure function ComputeEffectiveCategoryVolumes (enabled ? slider : 0f). This closes the blast radius the review flagged: a missing key in an EXISTING settings.json now falls back to AudioSettings.Default, which is enabled=true, so a fresh launch is audible, not muted. AP-199's wording and gate-script step 6 corrected; the enshrined-inversion test rewritten to assert the correct default and a new SettingsStore test pins the legacy-file fallback path. M3 — UI_ChatFontFace now ships all five of retail's authored choices (Arial, CourierNew, PalatinoLinotype, Tahoma, TimesNewRoman — a fixed compile-time array at gmClient::InitUIPreferences, PE-byte-verified present verbatim in .rdata, not a per-machine runtime enumeration as the rejected slice's comment claimed). Default index 2 (PalatinoLinotype) now indexes a real entry. S1 — Bind() now emits the sixth trailing AddSeperator retail's own InitOptions ends with (0x0049E80D), matching retail's 39-item ListBox (6 headers + 6 separators + 27 option-widget-rows) instead of 38. S2 — Screen Brightness gets its own DisplaySettings.ScreenBrightness field ([-1,1], default 0) instead of overloading Gamma, which has a different unit system (default 1.0, legacy [0.5,2.0] slider) and its own live Settings-panel consumer. S3 — UiScrollbar and UiMenu gained a settable TooltipText surfaced through GetTooltipText (UiButton's existing pattern). Every slider and menu row's own interactive widget (not just toggle/trio rows) now carries retail's "<label>_Help" tooltip, verified as a universal suffix convention across every AttachPreference site touched by this tab. S4 — "800x600" added to DisplaySettings.AvailableResolutions: a genuine retail display mode (Device::ForceDisplayResolution(1,0x320,0x258) at startup) and the Config tab's own byte-verified Resolution row default, not an invented preset. Defaults now lands on a highlighted, re-selectable dropdown entry instead of an orphaned value. S5 — four new/extended tests: ComputeEffectiveCategoryVolumes gets a dedicated pure-function value assertion (Theory + a default-profile-is- audible Fact) in RuntimeSettingsControllerTests, closing the "only event order was asserted" gap that let M2 ship; a label/choice-key conformance table in ConfigOptionsPageControllerTests enumerates every key this tab queries (traced directly from the fixed code paths, not guessed) and fails on an invented OR a dropped key; a per-row DefaultValue pin asserts every row's default against the retail literal directly, independent of the underlying settings-record defaults; and the S1 separator fix gets its own 39-item stacked-ListBox count pin. NOTEs — AP-198's row count was always ten (its own enumeration never said nine); the commit-message inconsistency N1 flagged is reconciled in both the row and the section-summary line, and its Screen Brightness sub-clause now matches S2. N2: Bind() now reads the scrollbar id from UiTemplateListBox.ScrollbarElementId (dat property 0x72) instead of a hardcoded constant. N3 (batch Defaults writes) and N4 (AfterApply on Config-tab entry, needs no action) are left as recorded — out of this rework's scope per the review's own disposition. Full Release suite: 13,125 passed / 4 skipped / 0 failed (baseline 13,117/4/0 — net +8 tests added, 0 regressions, 0 removed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
4996974cfd
commit
472525b99e
11 changed files with 774 additions and 168 deletions
|
|
@ -531,9 +531,17 @@ rows total, byte-verified directly from `gmConfigUI::InitOptions
|
|||
@0x0049E400` and `gmClient::InitUIPreferences @0x004035b0` (the retail
|
||||
registration table that supplies every row's label/tooltip/range/enum-
|
||||
choice — a stronger source than this campaign's own research docs, which
|
||||
had left the slider-caption pairing explicitly UNVERIFIED; OP6 resolved it:
|
||||
**retail ships zero range captions on this tab** — do not expect
|
||||
"Dark/Bright"-style low/high labels on any slider).
|
||||
had left the slider-caption pairing explicitly UNVERIFIED). **CORRECTED at
|
||||
the OP6 rework round (2026-08-11, review M1):** the "zero range captions"
|
||||
reading was a Binary Ninja constant-folding artifact — the raw bytes at
|
||||
each `SetSliderLabel` call site are reads of runtime-filled string-id
|
||||
globals, not immediate zeros. **Retail DOES ship six low/high range
|
||||
captions**, one per labelled slider: Camera Stiffness (Soft/Hard), Camera
|
||||
Adjustment Speed (Slow/Fast), Field Of View (Narrow/Wide), Screen
|
||||
Brightness (Dark/Bright), Graphics Performance (Speed/Detail), Degrade
|
||||
Distance (Close/Far). Mouse Look Sensitivity is the one genuine exception —
|
||||
retail's own `InitOptions` never calls `SetSliderLabel` for it, so it alone
|
||||
should show no caption.
|
||||
|
||||
### Opening the tab and reading the rows
|
||||
|
||||
|
|
@ -556,20 +564,27 @@ had left the slider-caption pairing explicitly UNVERIFIED; OP6 resolved it:
|
|||
ambient sound source, or trigger a combat/UI sound repeatedly). The
|
||||
volume should change LIVE, continuously, as you drag — not just on
|
||||
release.
|
||||
4. **Check the "Disable Sound Effects" checkbox** (the SAME row's toggle
|
||||
half). Sound effects should go SILENT immediately, regardless of where
|
||||
the slider is set. Uncheck it — sound effects resume at the slider's
|
||||
current level.
|
||||
5. **Repeat steps 3-4 for the "Ambient Sound" trio** (the second row) using
|
||||
4. **CORRECTED at the OP6 rework round (2026-08-11, review M2) — the
|
||||
checkbox's stored value is ENABLED-sense, not "Disabled" as the
|
||||
rejected slice's own gate script previously described.** With the
|
||||
"Disable Sound Effects" checkbox CHECKED (its default state — see step
|
||||
6), sound effects are AUDIBLE. **Uncheck it** — sound effects go SILENT
|
||||
immediately, regardless of where the slider is set. Check it again —
|
||||
sound effects resume at the slider's current level. (The checkbox's own
|
||||
retail LABEL reads as if checked should mean muted; byte evidence says
|
||||
the opposite — see `AudioSettings`' class doc for the full citation.
|
||||
This gate is about the OBSERVED behaviour, not the label's English.)
|
||||
5. **Repeat step 4 for the "Ambient Sound" trio** (the second row) using
|
||||
an ambient loop (wind, water, torches) as your audible test — same live
|
||||
drag + mute-on-check behaviour.
|
||||
6. **Note the retail-faithful surprise:** on a FRESH character (never
|
||||
drag + mute-on-uncheck behaviour.
|
||||
6. **Note the retail-faithful default:** on a FRESH character (never
|
||||
touched these settings), BOTH "Disable Sound Effects" and "Disable
|
||||
Ambient Sound" checkboxes are CHECKED by default — i.e., sound starts
|
||||
MUTED out of the box. This is retail's own byte-verified 2013 EoR
|
||||
default (`gmConfigUI::InitOptions`'s `SetDefaultValue(1, ...)` on both
|
||||
trios), reproduced faithfully — **not a bug**, however counterintuitive.
|
||||
Uncheck both to hear anything.
|
||||
Ambient Sound" checkboxes are CHECKED by default
|
||||
(`gmConfigUI::InitOptions`'s `SetDefaultValue(1, ...)` on both trios) —
|
||||
and CHECKED means AUDIBLE, matching retail's own byte-verified
|
||||
`SoundManager::effect_sounds_enabled`/`ambient_sounds_enabled` statics
|
||||
(both compiled `= 1`). **Sound starts ON out of the box, not muted.**
|
||||
If you hear silence on a fresh character, that IS a defect — report it.
|
||||
7. **Drag the third slider / check its toggle ("Disable Interface Sound",
|
||||
third trio).** Expect NO audible change either way — this is retail's
|
||||
OWN dead knob (register row AP-174/AP-199): retail registers this
|
||||
|
|
@ -626,10 +641,13 @@ had left the slider-caption pairing explicitly UNVERIFIED; OP6 resolved it:
|
|||
ACTUAL effect too (sound un-mutes or re-mutes, resolution changes
|
||||
back), not just the checkbox art.
|
||||
15. **Click Defaults.** Every row jumps LIVE to retail's byte-verified
|
||||
default (Sound/Ambient trios back to muted+100% volume, Resolution to
|
||||
800×600, Full Screen on, Camera Stiffness to 0.45, etc.) — Apply/Reset
|
||||
light up afterward if anything actually changed (Defaults never
|
||||
commits by itself).
|
||||
default (Sound/Ambient trios back to AUDIBLE+100% volume — CORRECTED
|
||||
at the OP6 rework round, review M2; the checked-by-default checkbox
|
||||
means enabled, not muted — Resolution to 800×600, now a highlighted
|
||||
dropdown entry since it is one of `DisplaySettings.AvailableResolutions`
|
||||
(review S4), Full Screen on, Camera Stiffness to 0.45, etc.) —
|
||||
Apply/Reset light up afterward if anything actually changed (Defaults
|
||||
never commits by itself).
|
||||
16. **Switch to another tab (e.g. Gameplay) without clicking Apply after
|
||||
an edit.** Uncommitted edits silently revert — switching tabs is a
|
||||
Reset, not a save.
|
||||
|
|
@ -649,11 +667,18 @@ had left the slider-caption pairing explicitly UNVERIFIED; OP6 resolved it:
|
|||
|
||||
### What to report
|
||||
|
||||
- Any section/row/label missing, or a slider showing a low/high range
|
||||
caption (retail ships none on this tab — see this section's own intro).
|
||||
- Any section/row/label missing, OR (CORRECTED at the OP6 rework round,
|
||||
review M1) any of the SIX labelled sliders (Camera Stiffness/Adjustment
|
||||
Speed, Field Of View, Screen Brightness, Graphics Performance, Degrade
|
||||
Distance) MISSING its low/high range caption — retail ships one on each
|
||||
of those six (see this section's own intro). Mouse Look Sensitivity is
|
||||
the one slider that should show none.
|
||||
- A live row (Sound/Ambient volume+mute, Resolution, Full Screen) that
|
||||
does not change live, or a store-only row that unexpectedly DOES change
|
||||
something (a sign its "no consumer" premise is stale).
|
||||
- Sound effects or ambient audio SILENT on a freshly-created character
|
||||
before you touch any Config-tab control (see step 6) — that is the M2
|
||||
regression, not expected behaviour.
|
||||
- Reset/Defaults not behaving as described in 14-15.
|
||||
- Any row that fails to persist across a full relaunch (step 18),
|
||||
including the two next-launch-only rows (Sync To Refresh, Field of
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue