From bcfac09165ac89fc08313f556d184c373ecca8df Mon Sep 17 00:00:00 2001 From: revtex Date: Sat, 29 Aug 2026 10:00:03 -0400 Subject: [PATCH 1/2] Ask each Advanced setting once, instead of spreading it over switches Two settings on that page were spread across several controls that could not all be true at once, so the page spent its height greying things out to explain hierarchies that were never visible in the first place. Both collapse to a dropdown, by the same arithmetic each time: count the combinations the controls allowed, count the outcomes they produced, and the difference is what the user had to keep rediscovering. Mute advertisements / Record everything Spotify plays / Include advertisements: three switches, four combinations, three outcomes. Two of the switches did nothing unless a third allowed it. It is now recording.recordSelection, with the values the booleans could actually express between them - recognisable tracks only, everything except advertisements, everything including them. Mute advertisements never muted anything, which is why this was worth looking at rather than relabelling. AudioSessions.SetMute had no callers, so the switch shipped on by default and its one real effect was to veto record-everything silently: turn that on and nothing happened, with nothing on the page to say why. The predecessor never hit this because its form unchecked mute-ads the moment record-everything was checked; Offstream had inherited the rule and not the interlock. Restoring it was considered and rejected: the predecessor's mute-ads muted every other application on the machine and held Spotify at full volume, to catch video adverts on a separate audio session, so it never silenced the advert either. A recorder reaching out to mute unrelated applications is a side effect nobody asked for. Existing-file policy plus "also tell Spotify to move on": three values and a boolean, six combinations, four outcomes. Moving on does nothing under Replace it or Save the new one alongside it, since both write the file again and leave nothing to move past. The switch was the second half of one answer, and it is now the second of four: ExistingFilePolicy.SkipAndMoveOn. Adding a member to that enum is the one dangerous edit here. Four sites compared against ExistingFilePolicy.Skip, and one of them - the check in TrackRecorder.AlreadyOnDisk - compared negated. A member missing from a comparison like that fails no build and no test; it quietly records over a file the user asked to keep. All four now ask RecordingSettings.KeepsTheExistingFile, and the enriched-template test became a Theory over both keep-it policies to pin it. Skip keeps its name and its zero value. Member names are what settings.json stores and JsonStringEnumConverter throws on a value it does not recognise, which SettingsStore.Load turns into a hard "fix or delete the file to start from defaults" - so renaming it to Keep would fail every existing install to gain a tidier identifier. Session_UnderAnOverwritePolicy_NeverAsksSpotifyToMoveOn is gone rather than converted. It existed to prove the core read the policy and the boolean together, because a hand-edited settings file could disagree with the greyed-out page. That state cannot be expressed now. What replaced it asserts the live question instead: under Overwrite the track is recorded, not kept - the negative side of KeepsTheExistingFile. Both collapses drop settings keys. recording.recordSelection replaces three, output.existingFilePolicy absorbs one, and there is no importer, so a settings.json tuned around the old behaviour comes back at the defaults for those settings. Deliberate: the keys never shipped in a release, and a permanent migration path is a poor trade for them. The rest of the page follows from having room again: Descriptions are tooltips. A line of prose under every switch cost height on a page that has none - the bottom card has now been clipped three times - and said the same thing whether or not anyone needed it. The row carries a ToolTip so hovering anywhere finds it, and the control carries the same sentence as AutomationProperties.HelpText, because a tooltip is the one thing on the page a screen reader cannot see. Text unchanged, still in Strings.resx and .fr.resx, still translated. Recorded as a standing convention in CLAUDE.md rather than a one-off. The existing-file policy and the move-on setting moved out of File names. What to do about a file already on disk is a rule about recording, not about what recordings are called. They now sit in Recording, which is what the Detection card is called: it had long since stopped being only about detection. Giving the policy dropdown its own row also fixes the switch label, which was clipped at "Also tell Spotify to mov". A test that fails the build on the page's height was attempted and cannot be kept. It measures correctly - 495 units for Advanced against the Settings page's 536, at the shell's content width of 976 - but StaticResource resolves at load time, so the dictionaries must exist before the page's constructor runs, which needs an Application. Application.Current is a process-wide singleton bound to its creating thread, and that thread ends with the test, leaving every later test in the assembly marshalling onto a dead dispatcher: nine RecordViewModelTests failed on the first run. Measuring the shell instead does not work either, since an unshown window arranges to nothing. Settings has never been reported as clipped, so Advanced now fits wherever Settings does - which is the closest thing to a proof available without a shown window. Both the measurement and the two dead ends are in the plan, with the assembly-scoped STA fixture that would actually work. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 55 ++- CLAUDE.md | 1 + README.md | 9 +- docs/MODERNIZATION-PLAN.md | 22 +- src/Offstream.App/Resources/Strings.fr.resx | 31 +- src/Offstream.App/Resources/Strings.resx | 48 +- .../ViewModels/AdvancedViewModel.cs | 64 +-- .../Views/Pages/AdvancedPage.xaml | 434 +++++++----------- .../Recording/RecordingEnums.cs | 46 ++ .../Recording/RecordingPolicy.cs | 16 +- .../Recording/RecordingSession.cs | 6 +- src/Offstream.Core/Recording/TrackRecorder.cs | 2 +- .../Settings/OffstreamSettings.cs | 27 +- .../Settings/RecordingSettings.cs | 40 +- .../Recording/RecordingPolicyTests.cs | 67 ++- .../Recording/RecordingSessionTests.cs | 41 +- .../Recording/TrackRecorderTests.cs | 13 +- .../Settings/SettingsMappingTests.cs | 29 +- .../Settings/SettingsSchemaDefaultsTests.cs | 11 +- .../Settings/SettingsStoreTests.cs | 4 +- .../AdvancedViewModelTests.cs | 66 ++- tests/Offstream.UI.Tests/AppServicesTests.cs | 9 + 22 files changed, 540 insertions(+), 501 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b614c6f..1a7f50b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,55 @@ phase plan these entries follow. ## [Unreleased] +### Changed + +- **The three advertisement switches are one dropdown, because between them they only ever had + three answers.** "Mute advertisements", "Record everything Spotify plays" and "Include + advertisements" were three on/off settings over four combinations — two of which did exactly the + same thing, while two of the switches did nothing at all unless a third allowed it. The card had + to grey controls out to explain a hierarchy that was never visible in the first place, and it + still read as three contradictory answers to one question. It is now one question with one + answer: **Record — recognisable tracks only** (the default), **everything except + advertisements**, or **everything, advertisements included**. Nothing about which files get + recorded has changed; there is simply no longer a way to ask for a combination that means + something other than what it says. +- **"Mute advertisements" never muted anything.** The code behind it had no callers, so the switch + shipped on by default and its one real effect was to silently veto "Record everything Spotify + plays" — turn that on and nothing happened, with nothing on the page to say why. The predecessor + never hit this because its form unchecked mute-ads the moment record-everything was checked; + Offstream had inherited the rule and not the interlock. The setting is stored as + `recording.recordSelection` now and the three keys it replaces are gone, so a settings file + hand-tuned around the old behaviour comes back at the default. +- **The Advanced page's settings sit in the sections they belong to.** What to do about a file that + already exists — and whether to tell Spotify to move on from it — are rules about recording, not + about what recordings are called, and both had been living under **File names**. They have moved + into **Recording**, which is what the **Detection** card is now called: it had long since stopped + being only about detection. +- **Telling Spotify to move on is a fourth answer to "when that file already exists", not a switch + beside it.** It was a separate on/off setting that did nothing under two of the three policies — + overwriting the file and saving a second copy both record the track again, leaving nothing to + move past — so it greyed itself out half the time and needed a sentence explaining why. Six + combinations, four outcomes: the same arithmetic as the advertisement switches above, and the + same fix. The choice is now **keep the one on disk**, **keep it and skip to the next track**, + **replace it**, or **save the new one alongside it**. Its `output.skipAlreadyRecordedTracks` key + is gone and its value lives in `output.existingFilePolicy`, so anyone who had asked Offstream to + move on will need to say so again. Giving the dropdown the row to itself also fixes the switch + label, which was clipped at "Also tell Spotify to mov". +- **A setting's description is a tooltip now.** The sentence under a switch cost a line of height + on a page that has none to spare — the card at the bottom has been clipped three times — and it + said the same thing every time whether or not anyone needed it. Hovering the row shows it + instead. The text is unchanged and still translated, and each control carries it as accessible + help text as well, because a tooltip is the one thing on the page a screen reader cannot see. + +### Removed + +- **Muting advertisements is off the roadmap rather than unfinished.** It sat in the parity list as + something still owed, but there is no coherent behaviour to owe: the predecessor's version muted + *every other application* on the machine and left Spotify at full volume, to catch video adverts + playing through a separate audio session — so it never silenced the advert either. A recorder + reaching out to mute unrelated applications is a side effect nobody asked for, and with the + detection options now saying plainly that adverts are discarded, silencing them buys nothing. + ### Added - **Offstream can tell Spotify to move on from a track it already has.** Keeping the file on disk @@ -20,8 +69,10 @@ phase plan these entries follow. through the parts that already are. "Also tell Spotify to move on", beside the existing-file setting on the Advanced page, sends a skip through the Windows media transport controls — the same command as the keyboard's next-track key, so it needs no account, no scope and no Premium - subscription, and it reaches Spotify while it is minimised to the tray. Off by default, greyed - out under the two policies that record the file again, and asked exactly once per track, because + subscription, and it reaches Spotify while it is minimised to the tray. It is the second of the + four answers to **When that file already exists** on the Advanced page — *keep it, and skip to + the next track* — so it cannot be asked for under a policy that records the file again anyway. + Off unless chosen, and asked exactly once per track, because Spotify goes on reporting the outgoing song for a moment after it takes the command and a second skip would land on a song nobody had recorded. It is asked twice per track, though: once immediately, and again when the metadata lookup lands, since a template built on `{album}`, diff --git a/CLAUDE.md b/CLAUDE.md index 4d5055d..746f919 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -93,4 +93,5 @@ Beyond that: ffmpeg argv golden tests, encode-integration tests asserted with ff - Nullable reference types enabled. - MVVM via CommunityToolkit.Mvvm source generators; no code-behind logic beyond wiring. - Inline validation (`INotifyDataErrorInfo`), not modal dialogs. +- **A setting's description is a tooltip, not a line under its label** (decided 2026-08-29). The Advanced page has no `ScrollViewer` and the shell's `MinHeight` promises no setting is ever out of reach, so a line of prose per row is height the page does not have — it has been clipped off the bottom three times. Put the sentence in `ToolTip` on the whole row so hovering anywhere finds it, and in `AutomationProperties.HelpText` on the control, because a tooltip is invisible to a screen reader. The text still lives in `Strings.resx` + `.fr.resx` and is still translated. Labels stay inline. - User-facing strings live in `Offstream.App/Resources/Strings.resx` (+ `.fr.resx`) with an en/fr key-parity test. Resource **keys are re-keyed for Offstream**; do not carry the predecessor's key names across. diff --git a/README.md b/README.md index 7e5744b..4a96ea9 100644 --- a/README.md +++ b/README.md @@ -150,9 +150,12 @@ and sorts where you expect it to. The rest of the page, briefly: -- **If a file already exists** — skip it, overwrite it, or keep both. -- **Mute advertisements** — on by default; adverts are silenced and never saved. -- **Record everything Spotify plays** — include podcasts and anything else without an artist. +- **Record** — how much of what Spotify plays is worth keeping. *Recognisable tracks only* is the + default: Offstream saves what Spotify reports as an artist and a title and discards the rest. + Widen it to *everything except advertisements* to keep podcasts too, or to *everything, + advertisements included* if you want the lot. +- **When that file already exists** — keep the one on disk, keep it and have Spotify skip to the + next track, replace it, or save the new one alongside it. - **Stop recording after** — a timer, for recording overnight or for exactly an hour. - **Number the tracks** — writes the counter into the track-number tag, so players sort recordings in the order they were made. diff --git a/docs/MODERNIZATION-PLAN.md b/docs/MODERNIZATION-PLAN.md index bd21a83..90a1790 100644 --- a/docs/MODERNIZATION-PLAN.md +++ b/docs/MODERNIZATION-PLAN.md @@ -230,7 +230,7 @@ Two rules the schema must honour: The acceptance checklist. Everything the app does today. ### Recording -WASAPI loopback capture · device selection · device volume · per-track splitting · minimum recorded length · recording timer (hhmmss) · silence trim start/end · mute ads · record everything (podcast) · record ads · skip/overwrite/duplicate existing · force Spotify to skip recorded track · listen to playback on default device · prevent sleep while recording. +WASAPI loopback capture · device selection · device volume · per-track splitting · minimum recorded length · recording timer (hhmmss) · silence trim start/end · ~~mute ads~~ (**dropped 2026-08-28** — see below) · record everything (podcast) · record ads · skip/overwrite/duplicate existing · force Spotify to skip recorded track · listen to playback on default device · prevent sleep while recording. ### Output MP3 128/160/256/320 · WAV · Opus · **FLAC (new)** · **AAC (new)** · filename template with tokens and folder support · counter with padding · output path · 260-char budgeting (**plus long-path support, new**). @@ -243,6 +243,24 @@ Tabs (**Record / Settings / Advanced**, and **Logs** from 2026-08-14) · console > The tab *structure* carries over because it works. The **tab labels do not** — the predecessor's "Spy" tab is Offstream's **Record** tab, and its "spy options" are **detection options**. §0 applies to user-visible strings as much as to code. > + +### Finding: "mute ads" is dropped, not deferred (2026-08-28) + +The setting shipped as **Mute advertisements**, on by default, and never muted anything: `AudioSessions.SetMute` had no callers. Its only effect was one `!` in `RecordingPolicy.IsRecordUnknownActive`, where it silently vetoed "record everything" — so the option could be turned on and would do nothing, with no indication why. The predecessor guarded against exactly that by unchecking mute-ads whenever record-everything was checked (`frmEspionSpotify.cs:732`); Offstream ported the policy expression and not the interlock. + +Restoring it was considered and rejected. There is no single behaviour to restore: the predecessor's mute-ads muted **every other application** and held Spotify at full volume (`MainAudioSession.cs:185-195`), to catch video adverts playing through a separate audio session. It never silenced the advert itself, so its own label was wrong too. Reimplementing that would reach outside Spotify to mute unrelated applications — a side effect no one asked a recorder for. + +So the three switches collapse into one choice, `recording.recordSelection`, with the three values they could actually express between them: `KnownTracksOnly` (the default), `EverythingExceptAds`, `Everything`. Of the four combinations the booleans allowed, two were behaviourally identical — which is the arithmetic behind the confusion, and the reason relabelling them was not enough. Session mute stays proven (§ Phase 0) and `AudioSessions.SetMute` stays in Core; nothing in the product depends on it. + +The same reduction then applied a second time, on the user's prompt, to the existing-file policy. `output.skipAlreadyRecordedTracks` was a boolean beside a three-valued dropdown that did nothing under two of its three values — six combinations, four outcomes — so it is now a fourth policy, `ExistingFilePolicy.SkipAndMoveOn`. Two notes for anyone extending that enum. `Skip` keeps its name and its zero value because the member names are what `settings.json` stores and `JsonStringEnumConverter` throws on a value it does not recognise, which `SettingsStore.Load` turns into a hard "fix or delete the file"; a rename would fail every existing install to gain a tidier identifier. And the four call sites that asked `== Skip` — one of them negated, in `TrackRecorder.AlreadyOnDisk` — now ask `RecordingSettings.KeepsTheExistingFile` instead: a policy forgotten at a comparison like that does not fail the build or a test, it silently records over a file the user asked to keep. + +### Finding: the Advanced page cannot be measured by a test (2026-08-29) + +The bottom of that page has now been clipped three times, each caught by a screenshot rather than by the build, because nothing above it scrolls and nothing in the build knows how tall it is. A measuring test was attempted: construct `AdvancedPage` on an STA thread with the three dictionaries `App.xaml` merges, `Measure` at the shell's content width, assert `DesiredSize.Height`. It measures correctly, and the numbers are worth keeping even though the test is not: at the shell's content width of 976, the Advanced page needs **495** units against the Settings page's **536**. Those were measured on 2026-08-29 and are carried forward, not re-taken — the existing-file rework later that day turned a row holding a dropdown and a switch into the same row holding only the dropdown, so the card's row count and heights are unchanged and 495 still stands. Any edit that adds or removes a row invalidates the figure, and there is no test to say so. Settings has never been reported as clipped, so Advanced now fits wherever Settings does — which is the closest thing to a proof available without a shown window. The test itself **cannot be kept**, and the reason is not the STA thread. + +`StaticResource` resolves at load time, so the dictionaries must be in place before the page's constructor runs, which means an `Application`. `Application.Current` is a process-wide singleton bound to the thread that created it, and the test's STA thread ends with the test — leaving every later test in the assembly marshalling onto a dead dispatcher. Nine `RecordViewModelTests` failed on the first run for exactly that reason. Measuring the shell instead does not work either: an unshown window arranges to nothing, so the host `ContentControl` reports zero. + +This is the empirical form of the note already on `AppServicesTests` ("asserts what is registered, never what resolves"). The page's height stays a hand-checked property until either the suite gains an assembly-scoped STA fixture that owns the `Application` for its lifetime, or the page gains a `ScrollViewer` and stops needing to be measured at all. > The log pane became a fourth tab on 2026-08-14; see the Phase 7 findings. The predecessor's structure is a starting point, not a ceiling. ### Filename template — behaviour preserved exactly @@ -465,7 +483,7 @@ Delivered in four PRs: **PR 1** (shell scaffold, DI, navigation, design tokens), - ✅ App host, DI, navigation, Fluent theme, dark mode. - ✅ **Record tab:** status, now-playing, elapsed, console log (with filter + copy), start/stop — **plus a live waveform**, which the plan did not ask for; see below. - ✅ **Settings tab:** output path, device, quality, min length, format (incl. FLAC/AAC), metadata provider (*and, from 2026-08-12, the Last.fm API key and the Spotify sign-in that make the provider choice mean something*). -- ✅ **Advanced tab:** tray *(setting; the icon itself is below)*, timer, counter, filename template with a token reference **and live preview**, existing-file policy, detection options, tag options. +- ✅ **Advanced tab:** tray *(setting; the icon itself is below)*, timer, counter, filename template with a token reference **and live preview**, existing-file policy, recording options *(the "detection options" card, renamed on 2026-08-29 once it had stopped being only about detection)*, tag options. - ✅ Inline validation via `INotifyDataErrorInfo` instead of modal dialogs. - ✅ i18n from `Offstream.App/Resources/Strings.resx` with an en/fr key-parity test (the reference tree's translation test, re-namespaced). **Resource keys are re-keyed for Offstream** — this is the file where inherited naming would otherwise survive longest. - ✅ Tray icon, minimise behaviour, single-instance guard. diff --git a/src/Offstream.App/Resources/Strings.fr.resx b/src/Offstream.App/Resources/Strings.fr.resx index 9163102..71d6308 100644 --- a/src/Offstream.App/Resources/Strings.fr.resx +++ b/src/Offstream.App/Resources/Strings.fr.resx @@ -319,32 +319,35 @@ Garder celui sur le disque + + Le garder et passer au morceau suivant + Le remplacer Enregistrer le nouveau à côté - - Demander aussi à Spotify de passer au suivant + + Passer au morceau suivant change ce que vous écoutez : cette option n'est active que si vous la choisissez. - - Uniquement lorsque le réglage ci-dessus conserve le fichier sur le disque. + + Enregistrement - - Détection + + Enregistrer - - Couper le son des publicités + + Offstream conserve ce que Spotify annonce comme un artiste et un titre. Élargissez ce réglage pour garder les podcasts et tout ce qui n'a pas d'artiste. - - Enregistrer tout ce que joue Spotify + + Morceaux reconnus uniquement - - Y compris les podcasts et tout ce qui n'a pas d'artiste. + + Tout sauf les publicités - - Inclure les publicités + + Tout, publicités comprises Arrêter l'enregistrement après diff --git a/src/Offstream.App/Resources/Strings.resx b/src/Offstream.App/Resources/Strings.resx index a24d99c..ede9a6f 100644 --- a/src/Offstream.App/Resources/Strings.resx +++ b/src/Offstream.App/Resources/Strings.resx @@ -407,7 +407,11 @@ Keep the one on disk - Existing-file policy: do not record over it. + Existing-file policy: do not record over it, and say nothing to Spotify. + + + Keep it, and skip to the next track + Existing-file policy: keep the file and send Spotify a skip-next command. Was a separate switch beside this dropdown. Replace it @@ -417,33 +421,33 @@ Save the new one alongside it Existing-file policy: keep both, numbering the newcomer. - - Also tell Spotify to move on - Checkbox beside the existing-file policy: send a skip-next command when the track playing is already in the library. + + Skipping to the next track changes what you are listening to, so it is off unless you pick it. + Tooltip on the existing-file policy row. Flags the one option that reaches out and drives Spotify. - - Only while the setting above keeps the file on disk. - Explains why the skip checkbox is greyed out under the Overwrite and Duplicate policies. + + Recording + Advanced page: heading over what gets recorded and what happens to the file. Was "Detection", which named only the first of the three settings under it. The predecessor called these "spy options"; plan §0 renames them. - - Detection - Advanced page: heading over what counts as a track worth recording. The predecessor called these "spy options"; plan §0 renames them. + + Record + Label over the dropdown choosing how much of what Spotify plays is saved. - - Mute advertisements - Detection option: silence Spotify's adverts instead of hearing them. + + Offstream saves what Spotify reports as an artist and a title. Widen this to keep podcasts and anything else that has no artist. + Tooltip on the record-selection dropdown. - - Record everything Spotify plays - Detection option: do not require an artist - title window title. + + Recognisable tracks only + Record-selection option, the default and the strictest: artist - title and nothing else. - - Includes podcasts and anything else without an artist. - Explains the record-everything option. + + Everything except advertisements + Record-selection option: podcasts and untitled content too, but no adverts. - - Include advertisements - Detection option, only meaningful while everything is being recorded. + + Everything, advertisements included + Record-selection option, the widest. Stop recording after diff --git a/src/Offstream.App/ViewModels/AdvancedViewModel.cs b/src/Offstream.App/ViewModels/AdvancedViewModel.cs index f703446..97d1025 100644 --- a/src/Offstream.App/ViewModels/AdvancedViewModel.cs +++ b/src/Offstream.App/ViewModels/AdvancedViewModel.cs @@ -64,12 +64,8 @@ public sealed partial class AdvancedViewModel : ObservableValidator private string _fileCounter = "1"; [ObservableProperty] - [NotifyPropertyChangedFor(nameof(CanSkipAlreadyRecorded))] private ExistingFilePolicy _existingFilePolicy; - [ObservableProperty] - private bool _skipAlreadyRecordedTracks; - [ObservableProperty] [NotifyPropertyChangedFor(nameof(TemplatePreview))] private bool _isTimerEnabled; @@ -80,14 +76,7 @@ public sealed partial class AdvancedViewModel : ObservableValidator private string _timer = "01:00:00"; [ObservableProperty] - private bool _muteAds; - - [ObservableProperty] - [NotifyPropertyChangedFor(nameof(CanRecordAds))] - private bool _recordEverything; - - [ObservableProperty] - private bool _recordAds; + private RecordSelection _recordSelection; [ObservableProperty] private bool _writeCounterToTrackNumber; @@ -120,13 +109,25 @@ public AdvancedViewModel(SettingsDocument document, IFileSystem fileSystem) _document = document; _fileSystem = fileSystem; + // The two keep-it answers adjacent, so the choice between them reads as the detail it + // is — what to tell Spotify — rather than as a fourth unrelated policy. Policies = [ new(ExistingFilePolicy.Skip, Strings.AdvancedExistingSkip), + new(ExistingFilePolicy.SkipAndMoveOn, Strings.AdvancedExistingSkipAndMoveOn), new(ExistingFilePolicy.Overwrite, Strings.AdvancedExistingOverwrite), new(ExistingFilePolicy.Duplicate, Strings.AdvancedExistingDuplicate), ]; + // Widest last. The list reads as one scale from strictest to loosest, so the effect of + // moving down it is the shape of the list rather than something to be read off it. + Selections = + [ + new(RecordSelection.KnownTracksOnly, Strings.AdvancedSelectionKnownTracksOnly), + new(RecordSelection.EverythingExceptAds, Strings.AdvancedSelectionEverythingExceptAds), + new(RecordSelection.Everything, Strings.AdvancedSelectionEverything), + ]; + // Language names are written in the language they name. "Français" translated into // English would be no help at all to the person looking for it. Languages = @@ -146,15 +147,14 @@ public AdvancedViewModel(SettingsDocument document, IFileSystem fileSystem) /// What to do when the destination file already exists. public IReadOnlyList> Policies { get; } - /// - /// Whether skipping past a recorded track can do anything under the chosen policy. - /// + /// How much of what Spotify plays is worth saving. /// - /// Overwrite and Duplicate both write the file again, so there is nothing to skip past. The - /// checkbox greys out rather than disappearing, so the setting is still findable — and the - /// core reads the two together anyway, since a hand-edited settings file can disagree. + /// One list where there were three switches. The switches encoded these same three + /// outcomes, but two of them did nothing unless the third allowed it, so the card had to + /// grey controls out to explain itself and still read as three contradictory answers to + /// one question. A question with one answer is a list. /// - public bool CanSkipAlreadyRecorded => ExistingFilePolicy == ExistingFilePolicy.Skip; + public IReadOnlyList> Selections { get; } /// UI languages, plus following Windows. public IReadOnlyList> Languages { get; } @@ -162,14 +162,6 @@ public AdvancedViewModel(SettingsDocument document, IFileSystem fileSystem) /// The tokens a template may use, with what each renders to. public IReadOnlyList Tokens { get; } - /// Whether the advertisement toggle applies. - /// - /// Advertisements are only recordable at all when everything is being recorded — with - /// track detection on, an advert has no artist and is never a file. The dependent control - /// is disabled rather than hidden, so the relationship between the two is visible. - /// - public bool CanRecordAds => RecordEverything; - /// Whether the template uses {count}, and so whether the counter matters. public bool UsesCounter => FileNameTemplate.UsesCounter(Template); @@ -237,10 +229,7 @@ private void Load() Template = settings.Output.Template; FileCounter = settings.Output.CurrentFileCounter.ToString(CultureInfo.CurrentCulture); ExistingFilePolicy = settings.Output.ExistingFilePolicy; - SkipAlreadyRecordedTracks = settings.Output.SkipAlreadyRecordedTracks; - MuteAds = settings.Recording.MuteAds; - RecordEverything = settings.Recording.RecordEverything; - RecordAds = settings.Recording.RecordAds; + RecordSelection = settings.Recording.RecordSelection; WriteCounterToTrackNumber = settings.Metadata.WriteCounterToTrackNumber; MinimizeToTray = settings.App.MinimizeToTray; FfmpegPath = settings.App.FfmpegPath ?? string.Empty; @@ -333,17 +322,11 @@ .. FileNameTemplate.KnownTokens.Select(token => partial void OnExistingFilePolicyChanged(ExistingFilePolicy value) => Persist(); - partial void OnSkipAlreadyRecordedTracksChanged(bool value) => Persist(); - partial void OnIsTimerEnabledChanged(bool value) => Persist(); partial void OnTimerChanged(string value) => Persist(); - partial void OnMuteAdsChanged(bool value) => Persist(); - - partial void OnRecordEverythingChanged(bool value) => Persist(); - - partial void OnRecordAdsChanged(bool value) => Persist(); + partial void OnRecordSelectionChanged(RecordSelection value) => Persist(); partial void OnWriteCounterToTrackNumberChanged(bool value) => Persist(); @@ -372,14 +355,11 @@ private void Persist() { Template = Template.Trim(), ExistingFilePolicy = ExistingFilePolicy, - SkipAlreadyRecordedTracks = SkipAlreadyRecordedTracks, CurrentFileCounter = int.Parse(FileCounter, CultureInfo.CurrentCulture), }, Recording = settings.Recording with { - MuteAds = MuteAds, - RecordEverything = RecordEverything, - RecordAds = RecordAds, + RecordSelection = RecordSelection, Timer = IsTimerEnabled ? ToStoredTimer(Timer) : null, }, Metadata = settings.Metadata with { WriteCounterToTrackNumber = WriteCounterToTrackNumber }, diff --git a/src/Offstream.App/Views/Pages/AdvancedPage.xaml b/src/Offstream.App/Views/Pages/AdvancedPage.xaml index 807a7d7..1656fa9 100644 --- a/src/Offstream.App/Views/Pages/AdvancedPage.xaml +++ b/src/Offstream.App/Views/Pages/AdvancedPage.xaml @@ -61,14 +61,6 @@ - -