Name refused tracks before Undo_EndBlock2, not after; tighten the route-list doc; pin the refusal memo's gate

Fixes D (handle lifetime), C (ASCII dashes), G (comment compression), I (shouldReport extracted + tested); CLAUDE.md route list corrected per A/B/H. E pushed back -- cross-module GUID plumbing is architectural, not a targeted fix.
This commit is contained in:
2026-08-05 17:34:49 -04:00
parent 5376ab085c
commit 4b306dd436
5 changed files with 100 additions and 54 deletions
+22 -10
View File
@@ -162,16 +162,28 @@ applies the resulting lane state to live tracks.
never falsely committed — and never finished either, since the refusal skips the never falsely committed — and never finished either, since the refusal skips the
FX enqueue too. Full contract at `discardDeferredFxParks`. FX enqueue too. Full contract at `discardDeferredFxParks`.
- **Which routes actually reach that no-snapshot-plus-parked-chain pair.** An undo - **Which routes actually reach that no-snapshot-plus-parked-chain pair.** An undo
of a mode switch is NOT one of them: per the observed undo semantics above, the of a mode switch (Ctrl-Z) is NOT one of them: per the observed undo semantics
flags, the FX and the ext-state snapshots all come back at the same step, and above, the flags and the FX come back at the same step. Whether the ext-state
stopping short of it leaves the snapshot in place (so the next park is a snapshot comes back with them is INFERENCE, not observed, and untested in the
`ParkOnly`, not a refusal). What remains: a model replaced without the project REDO direction: `[verify — DAW]` does a redo that re-parks a track's flags/FX
rolling back with it (an unreadable or absent `view_state` against an `.rpp` also restore a `view_state` that is snapshot-free for it (matching a fresh
saved parked), a snapshot `reconcile` pruned while its track was out of the live park), or can it re-park the live track while the model still holds a stale
enumeration and then undo-restored with the track, and a hand or script edit snapshot? What is confirmed to reach the pair: a `view_state` that PARSED but
that leaves a chain park-shaped. A track the USER keeps hidden from both panels, carries no snapshot for the track (a snapshot `reconcile` pruned while its
out of the mix and FX-bypassed reads identically to all of them and is refused track was out of the live enumeration, then undo/redo-restored with the track
too — no lost state there at all, which is why the report asserts no cause. but not the snapshot), and a hand or script edit that leaves a chain
park-shaped — including the single likeliest real case, a track with untouched
flags and just ONE hand-offlined plugin (the FX half of the disjunction
refuses alone; the flags never have to read parked too). A track the USER
keeps hidden from both panels, out of the mix and FX-bypassed reads
identically to all of them and is refused too — no lost state there at all,
which is why the report asserts no cause.
A DIFFERENT strand entirely, which the refusal report does NOT cover: an
absent or malformed `view_state` (`loadViewModel`, `ext_state_io.cpp`) is not
a parked-chain reading at all — it falls back to a DEFAULT model (active mode
Arrange, empty membership), so nothing is planned to park the track,
`decidePark` never runs, and a track saved parked stays stuck with no console
line whatsoever.
- `fx_offline`'s identity keying (`TrackFX_GetFXGUID`) assumes the GUID stays - `fx_offline`'s identity keying (`TrackFX_GetFXGUID`) assumes the GUID stays
attached to its plugin across a chain mutation while parked. That is attached to its plugin across a chain mutation while parked. That is
`[verify — DAW]` (see `fxGuidString` in `view_fx_park.cpp`) and SWS issue #802 is a `[verify — DAW]` (see `fxGuidString` in `view_fx_park.cpp`) and SWS issue #802 is a
+4 -4
View File
@@ -442,7 +442,7 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
int undoMask = kApplyUndoMask; int undoMask = kApplyUndoMask;
bool laneModeChanged = false; bool laneModeChanged = false;
std::vector<MediaTrack*> refusedParks; std::vector<std::string> refusedParkNames;
Undo_BeginBlock2(proj); Undo_BeginBlock2(proj);
{ {
@@ -464,10 +464,10 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
MediaTrack* tr = resolve(trackByGuid, guid); MediaTrack* tr = resolve(trackByGuid, guid);
if (!tr) continue; // stale GUID — prune if (!tr) continue; // stale GUID — prune
// Full contract at decidePark; view_fx_park owns the live reads. // decidePark's contract; view_fx_park owns the reads. A refusal is named HERE, not held past Undo_EndBlock2.
const ParkAction action = const ParkAction action =
decideParkForTrack(tr, model.snapshot(guid) != nullptr, tp.flags); decideParkForTrack(tr, model.snapshot(guid) != nullptr, tp.flags);
if (action == ParkAction::Refuse) { refusedParks.push_back(tr); continue; } if (action == ParkAction::Refuse) { refusedParkNames.push_back(trackDisplayName(tr)); continue; }
// Enqueued FIRST: a park landing on this track's own pending restore // Enqueued FIRST: a park landing on this track's own pending restore
// cancels it, and those ops are then the only surviving record of the // cancels it, and those ops are then the only surviving record of the
@@ -539,7 +539,7 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
// After the undo block and the UI hold: a refusal is a report, not a project // After the undo block and the UI hold: a refusal is a report, not a project
// write, and it must not join what a Ctrl-Z rolls back. // write, and it must not join what a Ctrl-Z rolls back.
reportRefusedParks(proj, refusedParks); reportRefusedParks(proj, refusedParkNames);
return true; return true;
} }
+12 -20
View File
@@ -104,11 +104,8 @@ std::string fxGuidString(MediaTrack* tr, int fx) {
return std::string(buf); return std::string(buf);
} }
// ANY offline FX, not every one: a park offlines the whole chain, but a partial // ANY offline FX, not every — see the snapshot-source invariant (view_fx_park.h,
// undo or a half-finished hand recovery leaves some back online, and there is no // src/shell/view/CLAUDE.md) for why.
// read that tells a park's leftover offline from one the user set. So the test
// refuses in the direction that commits nothing false — the cost is a chain
// carrying a hand-offlined plugin that the tool then declines to park.
bool anyFxOffline(MediaTrack* tr) { bool anyFxOffline(MediaTrack* tr) {
const int fxCount = TrackFX_GetCount(tr); const int fxCount = TrackFX_GetCount(tr);
for (int fx = 0; fx < fxCount; ++fx) for (int fx = 0; fx < fxCount; ++fx)
@@ -116,11 +113,12 @@ bool anyFxOffline(MediaTrack* tr) {
return false; return false;
} }
// "Track N" for an unnamed track, matching REAPER's own convention (GetTrackName, } // namespace
// not P_NAME, for the same reason capture::trackName picks it). The buffer is
// read bounded — the SDK documents no NUL within bufOut_sz. A track the report // GetTrackName, not P_NAME, matching capture::trackName's reason for the same
// cannot name still has to appear in it, so the read failing yields a placeholder // choice. The buffer is read bounded — the SDK documents no NUL within
// rather than dropping the entry. // bufOut_sz. A track the report cannot name still has to appear in it, so the
// read failing yields a placeholder rather than dropping the entry.
std::string trackDisplayName(MediaTrack* tr) { std::string trackDisplayName(MediaTrack* tr) {
std::vector<char> buf(1024, '\0'); std::vector<char> buf(1024, '\0');
if (!GetTrackName || !GetTrackName(tr, buf.data(), static_cast<int>(buf.size()))) if (!GetTrackName || !GetTrackName(tr, buf.data(), static_cast<int>(buf.size())))
@@ -128,8 +126,6 @@ std::string trackDisplayName(MediaTrack* tr) {
return std::string(buf.data(), strnlen(buf.data(), buf.size())); return std::string(buf.data(), strnlen(buf.data(), buf.size()));
} }
} // namespace
ParkAction decideParkForTrack(MediaTrack* tr, bool haveSnapshot, ParkAction decideParkForTrack(MediaTrack* tr, bool haveSnapshot,
const std::vector<TrackFlagOp>& parkFlags) { const std::vector<TrackFlagOp>& parkFlags) {
if (haveSnapshot) return decidePark(true, false); // second term unread — skip the reads if (haveSnapshot) return decidePark(true, false); // second term unread — skip the reads
@@ -139,18 +135,14 @@ ParkAction decideParkForTrack(MediaTrack* tr, bool haveSnapshot,
return decidePark(false, chainReadsParked(flagsRead, anyFxOffline(tr))); return decidePark(false, chainReadsParked(flagsRead, anyFxOffline(tr)));
} }
void reportRefusedParks(ReaProject* proj, const std::vector<MediaTrack*>& refused) { void reportRefusedParks(ReaProject* proj, const std::vector<std::string>& refusedNames) {
ReaProject* const owner = proj ? proj : currentProject(); ReaProject* const owner = proj ? proj : currentProject();
std::vector<std::string> names; if (!shouldReport(owner == g_reportedOwner, g_reportedRefusals, refusedNames)) return;
names.reserve(refused.size());
for (MediaTrack* tr : refused) names.push_back(trackDisplayName(tr));
if (owner == g_reportedOwner && names == g_reportedRefusals) return;
g_reportedOwner = owner; g_reportedOwner = owner;
g_reportedRefusals = names; g_reportedRefusals = refusedNames;
const std::string msg = describeRefusedParks(names); const std::string msg = describeRefusedParks(refusedNames);
// "!SHOW:" so an unattended project-load reapply cannot force the console open // "!SHOW:" so an unattended project-load reapply cannot force the console open
// (reaper_plugin_functions.h:6536), same as the drain's drop report. // (reaper_plugin_functions.h:6536), same as the drain's drop report.
if (!msg.empty()) ShowConsoleMsg(("!SHOW:" + msg).c_str()); if (!msg.empty()) ShowConsoleMsg(("!SHOW:" + msg).c_str());
+27 -12
View File
@@ -147,11 +147,8 @@ bool parkFlagsAlreadyApplied(const std::vector<TrackFlagOp>& parkFlags, ReadFlag
return true; return true;
} }
// The two halves folded. DISJUNCTIVE, and that is the whole point: each half is // DISJUNCTIVE fold — rationale is the snapshot-source invariant above (also
// the sole source of truth for its own half of the snapshot, so one of them // src/shell/view/CLAUDE.md).
// reading parked poisons the snapshot whatever the other says. An AND would let a
// flags-clean/FX-offline chain — what the documented hand recovery produces after
// its first step — bake the parked FX states in as the user's.
inline bool chainReadsParked(bool parkFlagsRead, bool anyFxOffline) { inline bool chainReadsParked(bool parkFlagsRead, bool anyFxOffline) {
return parkFlagsRead || anyFxOffline; return parkFlagsRead || anyFxOffline;
} }
@@ -178,6 +175,12 @@ inline ParkAction decidePark(bool haveSnapshot, bool chainReadsParked) {
ParkAction decideParkForTrack(MediaTrack* tr, bool haveSnapshot, ParkAction decideParkForTrack(MediaTrack* tr, bool haveSnapshot,
const std::vector<TrackFlagOp>& parkFlags); const std::vector<TrackFlagOp>& parkFlags);
// "Track N" for an unnamed track (GetTrackName, matching capture::trackName's
// choice of API for the same reason). Exported so a caller holding the handle
// only briefly (view.cpp's park loop, before Undo_EndBlock2/TrackList_AdjustWindows
// invalidate it) can capture the name at resolve time rather than the handle.
std::string trackDisplayName(MediaTrack* tr);
// The console report for the tracks one apply refused, empty when none. NAMES // The console report for the tracks one apply refused, empty when none. NAMES
// them: a refused track is hidden from both the TCP and the mixer, so a count // them: a refused track is hidden from both the TCP and the mixer, so a count
// alone points at nothing the user can go and find. It asserts no cause — the // alone points at nothing the user can go and find. It asserts no cause — the
@@ -187,22 +190,34 @@ inline std::string describeRefusedParks(const std::vector<std::string>& trackNam
if (trackNames.empty()) return {}; if (trackNames.empty()) return {};
std::string msg = "ReaSampler: " + std::to_string(trackNames.size()) + " track" + std::string msg = "ReaSampler: " + std::to_string(trackNames.size()) + " track" +
(trackNames.size() == 1 ? "" : "s") + (trackNames.size() == 1 ? "" : "s") +
" left unparked no stored pre-park state, and a live state that " " left unparked -- no stored pre-park state, and a live state that "
"already reads as parked; parking now would record that as your own " "already reads as parked; parking now would record that as your own "
"state.\n"; "state.\n";
for (const std::string& name : trackNames) msg += " " + name + "\n"; for (const std::string& name : trackNames) msg += " " + name + "\n";
msg += "To bring a track back under mode control: restore its TCP and mixer visibility, " msg += "To bring a track back under mode control: restore its TCP and mixer visibility, "
"its main send and its FX enable, AND set every FX in its chain online FX enable " "its main send and its FX enable, AND set every FX in its chain online -- FX enable "
"is the chain bypass and does not bring an individually offlined FX back. The next " "is the chain bypass and does not bring an individually offlined FX back. The next "
"mode switch will then park it correctly.\n"; "mode switch will then park it correctly.\n";
return msg; return msg;
} }
// Prints describeRefusedParks for `refused` (track handles, named here), ONCE per // Pure gate behind reportRefusedParks' one-per-changed-set memo: prints (i.e.
// changed set per project: the same refusal recurs on every reapply — every // updates the memo) on any change of owner or of the named set, stays silent on
// project open, every tag/untag — and repeating it verbatim is noise the user // an exact repeat. An empty `now` still counts as a change from a non-empty
// cannot act on any better the second time. // `last` — that update itself prints nothing (describeRefusedParks({}) is
void reportRefusedParks(ReaProject* proj, const std::vector<MediaTrack*>& refused); // empty) but resets the memo so a later refusal is never mistaken for a repeat
// of one that already healed.
inline bool shouldReport(bool sameOwnerAsLast, const std::vector<std::string>& last,
const std::vector<std::string>& now) {
return !(sameOwnerAsLast && last == now);
}
// Prints describeRefusedParks for `refusedNames` (captured by the caller while its
// track handles are still fresh — see view.cpp's park loop), ONCE per changed set
// per project: the same refusal recurs on every reapply — every project open,
// every tag/untag — and repeating it verbatim is noise the user cannot act on any
// better the second time. Gated by shouldReport.
void reportRefusedParks(ReaProject* proj, const std::vector<std::string>& refusedNames);
// The FX half of a snapshot, with the keying it must be read back under. // The FX half of a snapshot, with the keying it must be read back under.
struct PreParkFx { struct PreParkFx {
+35 -8
View File
@@ -236,11 +236,8 @@ static void testRestorePlanOpsRebuildTheSlotKeyedSnapshotVerbatim() {
// -- may this chain be snapshotted? ------------------------------------------- // -- may this chain be snapshotted? -------------------------------------------
// //
// The pre-park snapshot is restore's only source of truth, so one taken from an // Rationale: the snapshot-source invariant (view_fx_park.h, this directory's
// already-parked chain makes every later restore write hidden/out-of-mix/ // CLAUDE.md).
// FX-disabled back, permanently. The park site cannot infer a clean chain from
// an absent snapshot — discardDeferredFxParks drops intents whose flag writes
// already landed — so the chain itself has to be asked.
// The live values the fold reads, keyed by Flag — so an assertion names the flag // The live values the fold reads, keyed by Flag — so an assertion names the flag
// it varies rather than a position in makeParkPlan's op order. // it varies rather than a position in makeParkPlan's op order.
@@ -283,9 +280,7 @@ static void testAnEmptyPlanProvesNothing() {
} }
static void testEitherHalfOfTheChainReadingParkedIsEnoughToRefuse() { static void testEitherHalfOfTheChainReadingParkedIsEnoughToRefuse() {
// The disjunction, pinned: a snapshot has a flag half and an FX half, and each // Disjunction pinned; rationale at chainReadsParked (view_fx_park.h).
// is the only source of truth for its own. An AND here would re-open the
// defect one layer down — flags clean, FX still offline, snapshotted as truth.
CHECK(!chainReadsParked(/*parkFlagsRead=*/false, /*anyFxOffline=*/false)); CHECK(!chainReadsParked(/*parkFlagsRead=*/false, /*anyFxOffline=*/false));
CHECK(chainReadsParked(/*parkFlagsRead=*/true, /*anyFxOffline=*/false)); CHECK(chainReadsParked(/*parkFlagsRead=*/true, /*anyFxOffline=*/false));
CHECK(chainReadsParked(/*parkFlagsRead=*/false, /*anyFxOffline=*/true)); CHECK(chainReadsParked(/*parkFlagsRead=*/false, /*anyFxOffline=*/true));
@@ -337,6 +332,33 @@ static void testRefusalRecoveryNamesThePerFxHalfAndAssertsNoCause() {
CHECK(msg.find("lost") == std::string::npos); CHECK(msg.find("lost") == std::string::npos);
} }
// -- the refusal memo's print-suppression gate --------------------------------
//
// reportRefusedParks' own memo (owner + last-reported names, both REAPER-side
// statics) can't be driven from here, but the pure gate behind it can: prints
// (i.e. updates the memo) on any change, stays silent on an exact repeat, and
// treats a fresh empty set as a change too, so a later real refusal is never
// mistaken for a repeat of one that already healed.
static void testUnchangedOwnerAndSetStaysSilent() {
CHECK(!shouldReport(/*sameOwnerAsLast=*/true, {"Bass"}, {"Bass"}));
}
static void testADifferentNamedSetReports() {
CHECK(shouldReport(/*sameOwnerAsLast=*/true, {"Bass"}, {"Bass", "Drum bus"}));
}
static void testADifferentOwnerReportsEvenWithTheSameNames() {
// Two alternating project tabs must not suppress each other's first refusal.
CHECK(shouldReport(/*sameOwnerAsLast=*/false, {"Bass"}, {"Bass"}));
}
static void testAFreshEmptySetReportsAndResetsTheMemo() {
CHECK(shouldReport(/*sameOwnerAsLast=*/true, {"Bass"}, {}));
// Already empty, same owner: nothing changed, stays silent.
CHECK(!shouldReport(/*sameOwnerAsLast=*/true, {}, {}));
}
// -- re-entrancy ------------------------------------------------------------- // -- re-entrancy -------------------------------------------------------------
static void testTakeDetachesEverythingAndLeavesTheQueueEmpty() { static void testTakeDetachesEverythingAndLeavesTheQueueEmpty() {
@@ -411,6 +433,11 @@ int main() {
testRefusalNamesEveryRefusedTrackAndSaysNothingWhenNoneWere(); testRefusalNamesEveryRefusedTrackAndSaysNothingWhenNoneWere();
testRefusalRecoveryNamesThePerFxHalfAndAssertsNoCause(); testRefusalRecoveryNamesThePerFxHalfAndAssertsNoCause();
testUnchangedOwnerAndSetStaysSilent();
testADifferentNamedSetReports();
testADifferentOwnerReportsEvenWithTheSameNames();
testAFreshEmptySetReportsAndResetsTheMemo();
testTakeDetachesEverythingAndLeavesTheQueueEmpty(); testTakeDetachesEverythingAndLeavesTheQueueEmpty();
testIntentsArrivingDuringADrainSurviveIt(); testIntentsArrivingDuringADrainSurviveIt();
testAReEntrantParkCancelsOnlyWhatIsStillPending(); testAReEntrantParkCancelsOnlyWhatIsStillPending();