erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(settings): OP9 review residuals — restore SaveAudio live-apply pin, delete dead residues

Browse source 
Closes the OP9 combined review's findings (docs/research/2026-08-11-op9-review.md,
APPROVE-WITH-FIXES):

- MUST-FIX 1: SaveAudio -> ApplyAudio (OP6's Config-tab live-apply) lost
  its ONLY assertion when the retired SettingsVM save-order test was
  deleted. Restored directly on the now-public seam:
  SaveAudioPersistsThenPushesLiveApplyAudioWithTheSavedSnapshot pins
  persist-then-push order + the pushed snapshot;
  SaveAudioSkipsTheLivePushWhenPersistenceFails pins the failure ordering
  (a failed persist pushes nothing and commits nothing). Also closes
  SF-5: the OP6 effective-volume comment's 'target-audio assertion above'
  reference is real again and now names the restored test.
- SF-3: dead residues deleted — RuntimeSettingsController's private
  SaveCharacter (zero callers post-371197a3), ISettingsStorage.SaveCharacter
  + its JsonRuntimeSettingsStorage/FakeStorage implementations (the deleted
  private method was the only caller), and IngressShutdownRoots.Settings
  (zero readers since the view-model shutdown stage died). SettingsStore's
  PUBLIC SaveCharacter stays: it is the tested storage-API seam, and
  per-toon entries in existing settings.json files still load through the
  live LoadCharacter path.
- SF-2: code-structure.md's presentation-seam list no longer routes the
  settings preview through 'optional SettingsVM'.
- NIT 6: AP-196's retirement note now attributes LockUI (/lockui +
  PlayerDescription SetUiLocked convergence) and UseMouseTurning
  (Gameplay-tab macro + Config-tab row) to their real channels instead of
  folding all 13 members into the Character tab.

Full Release suite: 13,077 passed / 4 skipped / 0 failed (13,075 + the two
restored tests). One unnamed App-assembly failure appeared on the first
post-fix full run and did not reproduce on the isolated assembly rerun nor
a second full run — consistent with the known #250-class parallel-load
flake, recorded here for honesty rather than silently rerun.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-11 13:42:39 +02:00
parent 289bf5bc6e
commit 07f2b3f72e
6 changed files with 57 additions and 47 deletions
Download patch file Download diff file Expand all files Collapse all files

66
tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs
View file

@ -233,12 +233,58 @@ public sealed class RuntimeSettingsControllerTests
Assert.False(cmd.Value);
}
[Fact]
public void SaveAudioPersistsThenPushesLiveApplyAudioWithTheSavedSnapshot()
{
// OP9 review MUST-FIX 1: SaveAudio's live push into
// IRuntimeSettingsTargets.ApplyAudio (OP6's Config-tab live-apply —
// what makes a Sound-slider drag audible immediately) lost its only
// assertion when the retired SettingsVM save-order test was deleted.
// Re-pinned directly on the now-public SaveAudio seam: persist
// FIRST, then exactly one live push carrying the saved snapshot.
var events = new List<string>();
var controller = CreateController(events: events);
var targets = new FakeRuntimeTargets(events);
controller.BindRuntimeTargets(targets);
events.Clear();
AudioSettings updated = AudioSettings.Default with { Sfx = 0.35f };
controller.SaveAudio(updated);
Assert.Equal(["save-audio", "target-audio"], events);
Assert.Equal(updated, Assert.Single(targets.AudioCalls));
Assert.Equal(updated, controller.Audio);
}
[Fact]
public void SaveAudioSkipsTheLivePushWhenPersistenceFails()
{
// Companion ordering pin: a failed persist must not push a snapshot
// the store never accepted (SaveAudio's try body runs storage →
// committed property → live target, so the throw stops all three).
var events = new List<string>();
var storage = new FakeStorage(events) { ThrowOnAudioSave = true };
var controller = CreateController(storage);
var targets = new FakeRuntimeTargets(events);
controller.BindRuntimeTargets(targets);
events.Clear();
AudioSettings before = controller.Audio;
controller.SaveAudio(AudioSettings.Default with { Sfx = 0.35f });
Assert.Equal(["save-audio"], events);
Assert.Empty(targets.AudioCalls);
Assert.Equal(before, controller.Audio);
}
// OP6 rework (2026-08-11, review S5 / M2): pins the EFFECTIVE volume
// ApplyAudio actually computes, not just that some target was called.
// The FakeRuntimeTargets-based "target-audio" assertion above (and its
// predecessor before this rework) only ever checked that ApplyAudio
// fired, never what it fired WITH — the exact gap that let M2's
// Enabled/Disabled inversion mute every default profile unnoticed.
// The FakeRuntimeTargets-based "target-audio" assertion above
// (SaveAudioPersistsThenPushesLiveApplyAudioWithTheSavedSnapshot —
// restored at the OP9 review round after the retired SettingsVM
// save-order test took the original with it) only ever checked that
// ApplyAudio fired, never what it fired WITH — the exact gap that let
// M2's Enabled/Disabled inversion mute every default profile unnoticed.
[Theory]
[InlineData(true, true, 0.6f, 0.9f, 0.6f, 0.9f)] // enabled: slider value passes through
[InlineData(false, false, 0.6f, 0.9f, 0f, 0f)] // disabled: forced to zero regardless of slider
@ -1002,8 +1048,6 @@ public sealed class RuntimeSettingsControllerTests
public bool ThrowOnChatSave { get; init; }
public bool ThrowOnCharacterSave { get; init; }
public DisplaySettings LoadDisplay()
{
DisplayLoads++;
@ -1057,16 +1101,6 @@ public sealed class RuntimeSettingsControllerTests
ChatValue = chat;
}
public void SaveCharacter(string toonKey, CharacterSettings character)
{
_events.Add($"save-character:{toonKey}");
if (ThrowOnCharacterSave)
throw new IOException("character persistence failed");
Characters[toonKey] = character;
if (string.Equals(toonKey, "default", StringComparison.OrdinalIgnoreCase))
DefaultCharacterValue = character;
}
public CameraTurningSettings LoadCameraTurning() => CameraTurningValue;
public void SaveCameraTurning(CameraTurningSettings cameraTurning)