Merge FX-park snapshot guard: refuse to park when the pre-park truth is gone
A snapshot is never taken from a chain a park has touched, on either half -- flags or per-FX offline. Stranded tracks are refused, named once in the console, and left recoverable by hand.
This commit is contained in:
+142
-1
@@ -4,7 +4,8 @@
|
||||
// The properties under test: a mode switch leaves its per-FX offline work here,
|
||||
// so a second switch arriving before the first drained must leave every track in
|
||||
// the state the SECOND switch specifies — never the first's, never both replayed;
|
||||
// and a cancel must not strand the pre-park FX state it was the last record of.
|
||||
// a cancel must not strand the pre-park FX state it was the last record of; and
|
||||
// a pre-park snapshot is never taken from a chain a park has already touched.
|
||||
|
||||
#include "../src/shell/view/view_fx_park.h"
|
||||
|
||||
@@ -233,6 +234,131 @@ static void testRestorePlanOpsRebuildTheSlotKeyedSnapshotVerbatim() {
|
||||
CHECK(rebuilt.states == snap.fxOffline);
|
||||
}
|
||||
|
||||
// -- may this chain be snapshotted? -------------------------------------------
|
||||
//
|
||||
// Rationale: the snapshot-source invariant (view_fx_park.h, this directory's
|
||||
// CLAUDE.md).
|
||||
|
||||
// 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.
|
||||
struct FlagValues {
|
||||
int showInTcp = 0, showInMixer = 0, mainSend = 0, fxEnable = 0;
|
||||
|
||||
int operator()(Flag f) const {
|
||||
switch (f) {
|
||||
case Flag::ShowInTcp: return showInTcp;
|
||||
case Flag::ShowInMixer: return showInMixer;
|
||||
case Flag::MainSend: return mainSend;
|
||||
case Flag::FxEnable: return fxEnable;
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
};
|
||||
|
||||
static void testAChainSittingAtEveryValueTheParkWouldWriteReadsAsParked() {
|
||||
// The four zeros are what a parked track's driven flags actually read; if
|
||||
// makeParkPlan ever writes something else, this is the test that says so.
|
||||
const TrackPlan park = makeParkPlan("{A}", /*fxCount=*/0);
|
||||
|
||||
CHECK(park.flags.size() == 4);
|
||||
CHECK(parkFlagsAlreadyApplied(park.flags, FlagValues{0, 0, 0, 0}));
|
||||
}
|
||||
|
||||
static void testOneFlagStillAtTheUsersValueMeansNoParkReachedTheChain() {
|
||||
const TrackPlan park = makeParkPlan("{A}", /*fxCount=*/0);
|
||||
|
||||
// Each case varies exactly one flag, so together they also prove all four are
|
||||
// in the plan: a missing op makes its case read parked and fail here.
|
||||
CHECK(!parkFlagsAlreadyApplied(park.flags, FlagValues{1, 0, 0, 0}));
|
||||
CHECK(!parkFlagsAlreadyApplied(park.flags, FlagValues{0, 1, 0, 0}));
|
||||
CHECK(!parkFlagsAlreadyApplied(park.flags, FlagValues{0, 0, 1, 0}));
|
||||
CHECK(!parkFlagsAlreadyApplied(park.flags, FlagValues{0, 0, 0, 1}));
|
||||
}
|
||||
|
||||
static void testAnEmptyPlanProvesNothing() {
|
||||
CHECK(!parkFlagsAlreadyApplied(std::vector<TrackFlagOp>{}, FlagValues{0, 0, 0, 0}));
|
||||
}
|
||||
|
||||
static void testEitherHalfOfTheChainReadingParkedIsEnoughToRefuse() {
|
||||
// Disjunction pinned; rationale at chainReadsParked (view_fx_park.h).
|
||||
CHECK(!chainReadsParked(/*parkFlagsRead=*/false, /*anyFxOffline=*/false));
|
||||
CHECK(chainReadsParked(/*parkFlagsRead=*/true, /*anyFxOffline=*/false));
|
||||
CHECK(chainReadsParked(/*parkFlagsRead=*/false, /*anyFxOffline=*/true));
|
||||
CHECK(chainReadsParked(/*parkFlagsRead=*/true, /*anyFxOffline=*/true));
|
||||
}
|
||||
|
||||
static void testACleanChainIsSnapshottedThenParked() {
|
||||
CHECK(decidePark(/*haveSnapshot=*/false, /*chainReadsParked=*/false) ==
|
||||
ParkAction::SnapshotThenPark);
|
||||
}
|
||||
|
||||
static void testAHeldSnapshotIsNeverOverwrittenWhateverTheChainReads() {
|
||||
// The held snapshot IS the pre-park truth, so the chain is not consulted —
|
||||
// which is what lets the shell skip the live flag reads on this path.
|
||||
CHECK(decidePark(/*haveSnapshot=*/true, /*chainReadsParked=*/false) == ParkAction::ParkOnly);
|
||||
CHECK(decidePark(/*haveSnapshot=*/true, /*chainReadsParked=*/true) == ParkAction::ParkOnly);
|
||||
}
|
||||
|
||||
static void testAParkedChainWithNoSnapshotIsRefusedRatherThanResnapshotted() {
|
||||
// The defect this whole section exists for: the truth is gone, so the only
|
||||
// non-destructive act is to leave the track alone. Snapshotting here commits
|
||||
// park state as the user's state and no later restore can undo it.
|
||||
CHECK(decidePark(/*haveSnapshot=*/false, /*chainReadsParked=*/true) == ParkAction::Refuse);
|
||||
}
|
||||
|
||||
static void testRefusalNamesEveryRefusedTrackAndSaysNothingWhenNoneWere() {
|
||||
CHECK(describeRefusedParks({}).empty());
|
||||
|
||||
const std::string one = describeRefusedParks({"Bass"});
|
||||
CHECK(one.find("1 track ") != std::string::npos);
|
||||
CHECK(one.find("Bass") != std::string::npos);
|
||||
|
||||
const std::string two = describeRefusedParks({"Bass", "Drum bus"});
|
||||
CHECK(two.find("2 tracks ") != std::string::npos);
|
||||
CHECK(two.find("Bass") != std::string::npos);
|
||||
CHECK(two.find("Drum bus") != std::string::npos);
|
||||
}
|
||||
|
||||
static void testRefusalRecoveryNamesThePerFxHalfAndAssertsNoCause() {
|
||||
const std::string msg = describeRefusedParks({"Bass"});
|
||||
|
||||
// I_FXEN is the chain bypass: a user who restores only the four flags leaves
|
||||
// every individually offlined FX offline and walks straight back into a
|
||||
// refusal, so the recovery has to spell the per-FX step out.
|
||||
CHECK(msg.find("every FX in its chain online") != std::string::npos);
|
||||
// And it must not name a cause: the pair has several routes, and on a track
|
||||
// the user themselves keeps hidden/bypassed there was no lost state at all.
|
||||
CHECK(msg.find("undo") == 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 -------------------------------------------------------------
|
||||
|
||||
static void testTakeDetachesEverythingAndLeavesTheQueueEmpty() {
|
||||
@@ -297,6 +423,21 @@ int main() {
|
||||
testSlotKeyedRestoreDoesNotBecomeIdentityKeyedWithNoIdentities();
|
||||
testRestorePlanOpsRebuildTheIdentityKeyedSnapshotVerbatim();
|
||||
testRestorePlanOpsRebuildTheSlotKeyedSnapshotVerbatim();
|
||||
testAChainSittingAtEveryValueTheParkWouldWriteReadsAsParked();
|
||||
testOneFlagStillAtTheUsersValueMeansNoParkReachedTheChain();
|
||||
testAnEmptyPlanProvesNothing();
|
||||
testEitherHalfOfTheChainReadingParkedIsEnoughToRefuse();
|
||||
testACleanChainIsSnapshottedThenParked();
|
||||
testAHeldSnapshotIsNeverOverwrittenWhateverTheChainReads();
|
||||
testAParkedChainWithNoSnapshotIsRefusedRatherThanResnapshotted();
|
||||
testRefusalNamesEveryRefusedTrackAndSaysNothingWhenNoneWere();
|
||||
testRefusalRecoveryNamesThePerFxHalfAndAssertsNoCause();
|
||||
|
||||
testUnchangedOwnerAndSetStaysSilent();
|
||||
testADifferentNamedSetReports();
|
||||
testADifferentOwnerReportsEvenWithTheSameNames();
|
||||
testAFreshEmptySetReportsAndResetsTheMemo();
|
||||
|
||||
testTakeDetachesEverythingAndLeavesTheQueueEmpty();
|
||||
testIntentsArrivingDuringADrainSurviveIt();
|
||||
testAReEntrantParkCancelsOnlyWhatIsStillPending();
|
||||
|
||||
@@ -756,9 +756,15 @@ namespace {
|
||||
using LiveFlags = std::map<std::string, int>;
|
||||
|
||||
// Runs one applyMode-equivalent toggle against `vm` + `live`. `guard` selects the
|
||||
// fixed (snapshot-once) behavior vs. the original buggy (snapshot-every-park) one.
|
||||
// fixed behavior vs. the original buggy (snapshot-every-park) one.
|
||||
// Returns nothing; mutates `vm` snapshots/active mode and `live` flags in place,
|
||||
// exactly mirroring view.cpp's park then restore then setActiveMode ordering.
|
||||
//
|
||||
// Guarded, the park mirrors decidePark's three ways: a held snapshot parks
|
||||
// without recapturing, a chain that already reads parked with no snapshot is
|
||||
// REFUSED (left exactly as found), and only a clean chain is snapshotted. The one
|
||||
// `live` flag stands in for the whole park-flag-plus-FX fold the shell reads —
|
||||
// the model half of the defect is identical either way.
|
||||
void simulateApplyMode(ViewModeModel& vm, LiveFlags& live, const FolderTree& tree,
|
||||
const std::string& target, bool guard) {
|
||||
TogglePlan plan = vm.planToggle(tree, target);
|
||||
@@ -767,7 +773,14 @@ void simulateApplyMode(ViewModeModel& vm, LiveFlags& live, const FolderTree& tre
|
||||
for (const auto& tp : plan.park) {
|
||||
if (tp.flags.empty()) continue;
|
||||
const std::string& guid = tp.flags.front().guid;
|
||||
if (!guard || vm.snapshot(guid) == nullptr) {
|
||||
if (guard) {
|
||||
if (vm.snapshot(guid) == nullptr) {
|
||||
if (live[guid] == 0) continue; // reads parked, no snapshot — refuse
|
||||
TrackSnapshot snap;
|
||||
snap.showInTcp = live[guid];
|
||||
vm.storeSnapshot(guid, snap);
|
||||
}
|
||||
} else {
|
||||
TrackSnapshot snap;
|
||||
snap.showInTcp = live[guid]; // capture the LIVE visible flag
|
||||
vm.storeSnapshot(guid, snap);
|
||||
@@ -868,6 +881,74 @@ static void testNestedToggleSnapshotSurvivesRepark() {
|
||||
}
|
||||
}
|
||||
|
||||
// -- 9b. Reload strand: snapshots gone, live flags still parked ---------------
|
||||
//
|
||||
// The pair the park refusal exists for, driven through the same harness. A model
|
||||
// replaced without the project rolling back with it — an unreadable view_state, a
|
||||
// snapshot reconciled away while its track was deleted, a hand or script edit —
|
||||
// leaves tracks sitting at parked values with nothing recording what they were
|
||||
// before. Snapshotting there commits the parked state as the user's, and the very
|
||||
// next toggle writes it back over whatever they have since fixed by hand.
|
||||
|
||||
static void testReloadedModelWithParkedTracksRefusesRatherThanResnapshotting() {
|
||||
const FolderTree tree = nestedTree();
|
||||
|
||||
// GUARDED (fixed shell).
|
||||
{
|
||||
ViewModeModel vm;
|
||||
vm.membership().tag("{L1}", kDesignModeId);
|
||||
LiveFlags live{{"{L1}", 1}, {"{L2}", 1}, {"{L3}", 1}};
|
||||
|
||||
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/true);
|
||||
CHECK(live["{L2}"] == 0 && live["{L3}"] == 0);
|
||||
CHECK(vm.snapshot("{L2}") != nullptr);
|
||||
|
||||
// The reload: the model comes back with no snapshots while the project's
|
||||
// tracks are still parked.
|
||||
vm.clearSnapshot("{L2}");
|
||||
vm.clearSnapshot("{L3}");
|
||||
|
||||
// The load tick reapplies the saved active mode. Both leaves are refused —
|
||||
// nothing written, and nothing false captured.
|
||||
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/true);
|
||||
CHECK(vm.snapshot("{L2}") == nullptr && vm.snapshot("{L3}") == nullptr);
|
||||
CHECK(live["{L2}"] == 0 && live["{L3}"] == 0);
|
||||
|
||||
// The documented hand recovery, on {L2} only, while still in Design.
|
||||
// Toggling back must leave it where the user put it: with no snapshot
|
||||
// there is nothing to restore FROM, so the restore writes nothing.
|
||||
live["{L2}"] = 1;
|
||||
simulateApplyMode(vm, live, tree, kArrangeModeId, /*guard=*/true);
|
||||
CHECK(live["{L2}"] == 1); // the hand fix survives
|
||||
CHECK(live["{L3}"] == 0); // never recovered — stuck, but never falsely committed
|
||||
|
||||
// And {L2} is back under mode control from there: the next park sees a
|
||||
// clean chain, captures the user's value, and restores it.
|
||||
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/true);
|
||||
CHECK(live["{L2}"] == 0 && vm.snapshot("{L2}") != nullptr);
|
||||
simulateApplyMode(vm, live, tree, kArrangeModeId, /*guard=*/true);
|
||||
CHECK(live["{L2}"] == 1);
|
||||
}
|
||||
|
||||
// UNGUARDED (the defect): the reapply after the reload recaptures the parked
|
||||
// zero, so the very next toggle writes it back over the hand recovery.
|
||||
{
|
||||
ViewModeModel vm;
|
||||
vm.membership().tag("{L1}", kDesignModeId);
|
||||
LiveFlags live{{"{L1}", 1}, {"{L2}", 1}, {"{L3}", 1}};
|
||||
|
||||
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/false);
|
||||
vm.clearSnapshot("{L2}");
|
||||
vm.clearSnapshot("{L3}");
|
||||
simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/false);
|
||||
|
||||
live["{L2}"] = 1; // the same hand recovery
|
||||
simulateApplyMode(vm, live, tree, kArrangeModeId, /*guard=*/false);
|
||||
|
||||
CHECK(live["{L2}"] == 0); // wiped — the false snapshot won
|
||||
}
|
||||
}
|
||||
|
||||
// ===========================================================================
|
||||
// D2 two-canvas lane extension tests
|
||||
// ===========================================================================
|
||||
@@ -2125,6 +2206,7 @@ int main() {
|
||||
testUntaggedLeavesManagedByModeSystem();
|
||||
testTaggedLeafBehaviorUnchangedWithUntagged();
|
||||
testNestedToggleSnapshotSurvivesRepark();
|
||||
testReloadedModelWithParkedTracksRefusesRatherThanResnapshotting();
|
||||
testReconcilePrunesOrphanedSnapshots();
|
||||
testReconcileFullLiveSetIsNoOp();
|
||||
testReconcileThenReparkLifecycleIntact();
|
||||
|
||||
Reference in New Issue
Block a user