diff --git a/src/shell/view/view.cpp b/src/shell/view/view.cpp index 651476d..132bc3e 100644 --- a/src/shell/view/view.cpp +++ b/src/shell/view/view.cpp @@ -114,6 +114,7 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject bool laneModeChanged = false; bool wrote = false; // anything at all this apply changed — see applyMintsUndoPoint + std::vector exposedNames; // named while handles are fresh, reported after the block Undo_BeginBlock2(project); { @@ -133,7 +134,11 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject const std::string& guid = tp.flags.front().guid; MediaTrack* tr = resolve(trackByGuid, guid); if (!tr) continue; // stale/deleted GUID — prune - if (parkTrack(model, guid, tr, tp.flags)) wrote = true; + const TrackParkResult r = parkTrack(model, guid, tr, tp.flags); + if (r.wrote) wrote = true; + // Named here, not after the block: TrackList_AdjustWindows below can + // invalidate the handle. + if (r.exposed) exposedNames.push_back(trackDisplayName(tr)); } for (const TrackPlan& tp : plan.restore) { @@ -189,6 +194,10 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject const bool mint = applyMintsUndoPoint(realSwitch, wrote); Undo_EndBlock2(project, mint ? undoLabel.c_str() : "", mint ? kViewUndoMask : 0); + // After the undo block: an EXPOSE report is a console message, not a project + // write, and must not join what a Ctrl-Z rolls back. + reportExposedTracks(project, exposedNames); + // The tool's own account of what it just left driven — the only one that survives // an undo rolling the model back underneath it. noteAppliedState(project, model); diff --git a/src/shell/view/view_park.cpp b/src/shell/view/view_park.cpp index 4b0c95d..ef74936 100644 --- a/src/shell/view/view_park.cpp +++ b/src/shell/view/view_park.cpp @@ -3,12 +3,15 @@ #include "shell/view/view_park.h" +#include // strnlen — bounded read of GetTrackName's buffer #include #include #define REAPERAPI_MINIMAL #define REAPERAPI_WANT_GetMediaTrackInfo_Value +#define REAPERAPI_WANT_GetTrackName #define REAPERAPI_WANT_SetMediaTrackInfo_Value +#define REAPERAPI_WANT_ShowConsoleMsg #define REAPERAPI_WANT_TrackFX_GetCount #define REAPERAPI_WANT_TrackFX_GetFXGUID #define REAPERAPI_WANT_TrackFX_GetOffline @@ -73,6 +76,22 @@ bool applyFlags(MediaTrack* tr, const std::vector& flags) { return wrote; } +// The visible/active mirror of a park write: same four ops, value 1 instead of +// whatever the plan says (always 0 for a park plan — see makeParkPlan). Reusing +// the incoming park ops rather than minting a separate "expose plan" keeps the +// two writes provably addressing the same four parameters. +bool exposeFlags(MediaTrack* tr, const std::vector& parkFlags) { + bool wrote = false; + for (const TrackFlagOp& op : parkFlags) + if (writeIfChanged(tr, trackFlagParm(op.flag), 1.0)) wrote = true; + return wrote; +} + +// The last EXPOSE report, so an unchanged set is not reprinted on every reapply +// the track hasn't yet self-healed from. +ReaProject* g_reportedOwner = nullptr; +std::vector g_reportedExposed; + } // namespace const int kViewUndoMask = UNDO_STATE_TRACKCFG | UNDO_STATE_ITEMS | UNDO_STATE_MISCCFG; @@ -83,11 +102,28 @@ bool writeIfChanged(MediaTrack* tr, const char* parm, double value) { return true; } -bool parkTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, - const std::vector& flags) { - bool wrote = false; +TrackParkResult parkTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, + const std::vector& flags) { + TrackParkResult result; if (!model.holdsSnapshotFlags(guid)) { + const TrackSnapshot* prior = model.snapshot(guid); + + // No record of ANY kind (see parkTrack's header doc for why prior == nullptr + // is the right gate): the ambiguous case. A live chain already reading + // park-shaped is EXPOSEd — handed back visible, no snapshot stored — rather + // than adopted as the user's intent. + if (!prior) { + auto read = [tr](Flag f) { + return static_cast(GetMediaTrackInfo_Value(tr, trackFlagParm(f))); + }; + if (chainReadsParked(flags, read)) { + if (exposeFlags(tr, flags)) result.wrote = true; + result.exposed = true; + return result; // this cycle hands the track back; it does not park it + } + } + TrackSnapshot snap = snapshotTrack(tr); // A restore hands the flags back immediately but leaves the chain offline @@ -100,7 +136,6 @@ bool parkTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, // — so the false arm is unreached. Kept anyway: it is the direct // statement of what this branch actually needs, and cheaper to keep true // than to re-derive if that guarantee ever loosens. - const TrackSnapshot* prior = model.snapshot(guid); if (prior && prior->fxHeld && prior->fxKeying == FxKeying::Identity) { snap.fxOffline = unionFxOfflineOnRepark(prior->fxOffline, snap.fxOffline); snap.fxKeying = FxKeying::Identity; @@ -112,11 +147,11 @@ bool parkTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, snap.fxKeying = prior->fxKeying; } model.storeSnapshot(guid, snap); - wrote = true; + result.wrote = true; } - if (applyFlags(tr, flags)) wrote = true; - return wrote; + if (applyFlags(tr, flags)) result.wrote = true; + return result; } bool restoreTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, @@ -126,4 +161,22 @@ bool restoreTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, return true; } +std::string trackDisplayName(MediaTrack* tr) { + std::vector buf(1024, '\0'); + if (!GetTrackName || !GetTrackName(tr, buf.data(), static_cast(buf.size()))) + return "(name unavailable)"; + return std::string(buf.data(), strnlen(buf.data(), buf.size())); +} + +void reportExposedTracks(ReaProject* proj, const std::vector& trackNames) { + if (!shouldReport(proj == g_reportedOwner, g_reportedExposed, trackNames)) return; + g_reportedOwner = proj; + g_reportedExposed = trackNames; + + const std::string msg = describeExposedTracks(trackNames); + // "!SHOW:" so an unattended project-load reapply cannot force the console open + // (reaper_plugin_functions.h:6536). + if (!msg.empty()) ShowConsoleMsg(("!SHOW:" + msg).c_str()); +} + } // namespace reasampler diff --git a/src/shell/view/view_park.h b/src/shell/view/view_park.h index 650f0fc..4b99706 100644 --- a/src/shell/view/view_park.h +++ b/src/shell/view/view_park.h @@ -12,6 +12,7 @@ // Forward-declared to keep this header SDK-free; the .cpp includes the real SDK header. class MediaTrack; +class ReaProject; namespace reasampler { @@ -50,6 +51,21 @@ inline std::vector unionFxOfflineOnRepark( return merged; } +// Does the track already sit at every value `parkFlags` would write? Templated on +// the read so live code compares as it reads (one call per Flag, no live-value +// vector) while a test drives the same fold from a table. FX-offline is +// deliberately NOT folded in: Wave 3 moved every per-FX write off this path onto +// the reclaimer, so a chain with these four flags fine but an FX hand-offlined is +// not evidence THIS path ever touched the track — only the flags it would itself +// have driven are. An empty plan is evidence of nothing and answers false. +template +bool chainReadsParked(const std::vector& parkFlags, ReadFlag readFlag) { + if (parkFlags.empty()) return false; + for (const TrackFlagOp& op : parkFlags) + if (readFlag(op.flag) != op.value) return false; + return true; +} + // The undo-state domains both the mode-switch apply (view.cpp) and lane minting // (view_lanes.cpp) write: track config/routing, item lane assignment, and the // ext-state block (UNDO_STATE_MISCCFG covers extension state). NOT UNDO_STATE_ALL, @@ -71,21 +87,81 @@ extern const int kViewUndoMask; // Returns whether it wrote. bool writeIfChanged(MediaTrack* tr, const char* parm, double value); +// What one parkTrack call did. `wrote` is any project write at all — a flag write +// or a record write — and feeds the apply's undo-point fold. `exposed` is true iff +// this call handed the track back visible instead of parking or adopting it; the +// caller (view.cpp) collects exposed tracks across the whole apply for the one +// batched console report. +struct TrackParkResult { + bool wrote = false; + bool exposed = false; +}; + // Parks one inactive leaf: capture the pre-park truth when the model does not -// already hold it, then drive the flags. Returns whether anything moved — a project -// write or the record. +// already hold ANY record for it (holdsSnapshotFlags false AND model.snapshot == +// nullptr), then drive the flags. // -// The record is the tool's own account of an act it performed, and it is the ONLY -// account — nothing infers a park from a live value any more. That is what retired -// the old refusal: a track that merely LOOKS parked while no record names it is a -// track the tool did not park, so its values are the user's own and capturing them -// is right. So the capture is owed exactly when the flag half is not held. +// A record with a held FX half but no held flag half (a re-park landing right +// after a restore, before the reclaimer catches up) is NOT the no-record case — +// model.snapshot(guid) is non-null there, so it takes the existing union path +// below, never the branch this comment is about. That is how EXPOSE is known not +// to fire on a track the tool legitimately still holds a record for: EXPOSE is +// gated on snapshot(guid) == nullptr, and a legitimately-held record is by +// definition non-null (eraseIfSpent only drops a record once BOTH halves are +// spent). // -// A still-held FX half is the one thing not re-read: it outlives the flag half the -// restore spends, the chain stays offline until the reclaimer onlines it, and -// re-reading it there would file the park's own state as the user's. -bool parkTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, - const std::vector& flags); +// The true no-record case splits on whether the live chain already reads +// park-shaped (chainReadsParked, flags only — see its own comment for why FX is +// excluded). Not park-shaped: the chain is the track's own honest state, captured +// and parked exactly as ever. Park-shaped: parking now would adopt a park with no +// account behind it — an undo/redo or reload that dropped the snapshot, a hand or +// script edit, or simply a track the user keeps hidden — as the user's intent, +// which either wipes a hand repair on the very next restore or hides an untouched +// track for good. EXPOSE hands it back instead: no snapshot stored, the four +// flags driven to visible/active, nothing reported here (the caller batches it). +// The next apply sees an honest chain again and captures/parks it normally — +// self-healing, no user action required. +TrackParkResult parkTrack(ViewModeModel& model, const std::string& guid, MediaTrack* tr, + const std::vector& flags); + +// GetTrackName, matching capture::trackName's reason for the same choice: "Track +// N" for an unnamed track, matching REAPER's own convention. Exported so a caller +// holding the handle only briefly (view.cpp's park loop, before +// Undo_EndBlock2/TrackList_AdjustWindows can invalidate it) captures the name at +// resolve time rather than the handle. +std::string trackDisplayName(MediaTrack* tr); + +// One console message for the tracks EXPOSEd this apply, empty when none. NAMES +// them: an exposed track was hidden from both the TCP and the mixer moments ago, +// so a bare count points at nothing the user can go and find. Says the tool lost +// its own record and handed the track back — never "refused". +inline std::string describeExposedTracks(const std::vector& trackNames) { + if (trackNames.empty()) return {}; + const bool one = trackNames.size() == 1; + std::string msg = "ReaSampler: " + std::to_string(trackNames.size()) + " track" + + (one ? "" : "s") + " handed back visible -- the tool's record of " + + (one ? "its" : "their") + + " pre-park state is gone (an undo/redo or a project reload dropped " + "it), and parking now would have recorded the parked state as " + "yours instead.\n"; + for (const std::string& name : trackNames) msg += " " + name + "\n"; + msg += "The next mode switch will park it correctly from what you see now.\n"; + return msg; +} + +// Pure gate behind reportExposedTracks' one-per-changed-set memo: true on any +// change of owner or of the named set, false on an exact repeat. +inline bool shouldReport(bool sameOwnerAsLast, const std::vector& last, + const std::vector& now) { + return !(sameOwnerAsLast && last == now); +} + +// Prints describeExposedTracks for `trackNames` (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 exposure recurs on every reapply until the track +// self-heals, and repeating it verbatim is noise the user cannot act on any +// better the second time. `proj` is used only as a memo key, never dereferenced. +void reportExposedTracks(ReaProject* proj, const std::vector& trackNames); // Restores one returning leaf from its record and spends the record's flag half. // Always true: the record is consumed here, so this apply changed state and is not diff --git a/tests/test_view_mode_model.cpp b/tests/test_view_mode_model.cpp index 93345d9..3bbf76a 100644 --- a/tests/test_view_mode_model.cpp +++ b/tests/test_view_mode_model.cpp @@ -821,6 +821,13 @@ using LiveFlags = std::map; // The FX half of the two-half record is never exercised here (nothing sets fxHeld // false), so it stays held throughout and never blocks a re-capture the way // flagsHeld does — the one place this harness is narrower than the real record. +// +// `live[guid] == 0` doubles as this harness's single-flag stand-in for +// parkTrack's real chainReadsParked (all four driven flags at their parked +// value) — the harness tracks one representative flag, not four, so "already at +// its parked value" is the whole predicate here. `vm.snapshot(guid) == nullptr` +// mirrors parkTrack's own gate for "no record of any kind" — see that function's +// header doc for why an FX-held-only record must not reach EXPOSE. void simulateApplyMode(ViewModeModel& vm, LiveFlags& live, const FolderTree& tree, const std::string& target, bool guard) { TogglePlan plan = vm.planToggle(tree, target); @@ -831,6 +838,13 @@ void simulateApplyMode(ViewModeModel& vm, LiveFlags& live, const FolderTree& tre const std::string& guid = tp.flags.front().guid; if (guard) { if (!vm.holdsSnapshotFlags(guid)) { + if (!vm.snapshot(guid) && live[guid] == 0) { + // EXPOSE: hand it back visible rather than adopt the + // park-shaped read as intent. No snapshot stored; this cycle + // does not park the track. + live[guid] = 1; + continue; + } TrackSnapshot snap; snap.showInTcp = live[guid]; vm.storeSnapshot(guid, snap); @@ -941,17 +955,14 @@ static void testNestedToggleSnapshotSurvivesRepark() { // 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. The park refusal that once existed for this pair (view_fx_park's -// decidePark) was retired by the Wave-2 shell restructure: view_park.h now states -// plainly that nothing infers a park from a live value any more, so a parked-looking -// chain with no record is treated as the user's own state and captured, not refused. -// This test documents the CURRENT consequence — it is not asserting the consequence -// is desirable: a hand recovery made before the next reapply survives, but one made -// after it (the reapply has already re-captured the still-hidden value as "the -// user's") is overwritten by the next restore, converging with the always-recapture -// defect test 9's unguarded block pins. Guarded and unguarded no longer differ here. +// they were before. Daniel's ruling: hand the track back rather than either +// adopting the parked read as intent (silently wipes a hand repair on the very +// next restore, or hides an untouched track forever) or refusing outright (the +// retired `decidePark`). EXPOSE does neither: no snapshot stored, the flags +// driven visible, and the very next reapply — no user action needed — finds an +// honest chain and captures/parks it normally. -static void testReloadedModelWithParkedTracksIsCapturedNotRefused() { +static void testReloadedModelWithParkedTracksIsExposedNotAdopted() { const FolderTree tree = nestedTree(); ViewModeModel vm; @@ -963,22 +974,29 @@ static void testReloadedModelWithParkedTracksIsCapturedNotRefused() { CHECK(vm.snapshot("{L2}") != nullptr); // The reload: the model comes back with no snapshots while the project's - // tracks are still parked. + // tracks are still parked -- park-shaped, no record. vm.clearSnapshot("{L2}"); vm.clearSnapshot("{L3}"); - // The load tick reapplies the saved active mode. Both leaves are captured - // fresh from their still-hidden live value — not refused. + // The load-tick reapply hits the no-record/park-shaped pair on both leaves. + // EXPOSE fires for each: handed back visible, no snapshot stored -- neither + // leaf needed a hand recovery first, and {L3} (never touched by hand in the + // old test) comes back on its own exactly like {L2}. simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/true); - CHECK(vm.snapshot("{L2}") != nullptr && vm.snapshot("{L3}") != nullptr); - CHECK(live["{L2}"] == 0 && live["{L3}"] == 0); + CHECK(live["{L2}"] == 1 && live["{L3}"] == 1); + CHECK(vm.snapshot("{L2}") == nullptr && vm.snapshot("{L3}") == nullptr); - // A hand recovery made NOW, after that recapture, is not what the next restore - // reads back — the fresh (hidden) capture just above is, so it is lost. - live["{L2}"] = 1; + // Self-healing: the chain no longer reads park-shaped, so the very next apply + // of the same target takes the normal path -- captures the (now-visible) + // truth and parks it correctly. + simulateApplyMode(vm, live, tree, kDesignModeId, /*guard=*/true); + CHECK(live["{L2}"] == 0 && live["{L3}"] == 0); + CHECK(vm.snapshot("{L2}") != nullptr && vm.snapshot("{L3}") != nullptr); + + // A normal round trip back to Arrange restores the captured (visible) truth -- + // the wipe the old adopt-based behavior produced is gone. simulateApplyMode(vm, live, tree, kArrangeModeId, /*guard=*/true); - CHECK(live["{L2}"] == 0); // wiped by the restore - CHECK(live["{L3}"] == 0); // never recovered + CHECK(live["{L2}"] == 1 && live["{L3}"] == 1); } // =========================================================================== @@ -2166,7 +2184,7 @@ int main() { testUntaggedLeavesManagedByModeSystem(); testTaggedLeafBehaviorUnchangedWithUntagged(); testNestedToggleSnapshotSurvivesRepark(); - testReloadedModelWithParkedTracksIsCapturedNotRefused(); + testReloadedModelWithParkedTracksIsExposedNotAdopted(); testReconcilePrunesOrphanedSnapshots(); testReconcileFullLiveSetIsNoOp(); testReconcileThenReparkLifecycleIntact(); diff --git a/tests/test_view_park.cpp b/tests/test_view_park.cpp index 538231e..c8ef44a 100644 --- a/tests/test_view_park.cpp +++ b/tests/test_view_park.cpp @@ -132,6 +132,85 @@ static void testAPluginRemovedWhileRestoredStaysInTheUnionForResolveFxRestoreToD CHECK(foundRemoved); } +// -- chainReadsParked: the EXPOSE trigger, flags only ------------------------- +// +// 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 testAChainSittingAtEveryParkedValueReadsAsParkShaped() { + const TrackPlan park = makeParkPlan("{A}"); + CHECK(chainReadsParked(park.flags, FlagValues{0, 0, 0, 0})); +} + +static void testOneFlagStillAtTheUsersValueMeansNotParkShaped() { + const TrackPlan park = makeParkPlan("{A}"); + + // 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(!chainReadsParked(park.flags, FlagValues{1, 0, 0, 0})); + CHECK(!chainReadsParked(park.flags, FlagValues{0, 1, 0, 0})); + CHECK(!chainReadsParked(park.flags, FlagValues{0, 0, 1, 0})); + CHECK(!chainReadsParked(park.flags, FlagValues{0, 0, 0, 1})); +} + +static void testAnEmptyPlanProvesNothing() { + CHECK(!chainReadsParked(std::vector{}, FlagValues{0, 0, 0, 0})); +} + +static void testFxOfflineAloneIsNotFoldedIntoTheParkedRead() { + // chainReadsParked takes no FX argument at all: an FX-only-offline chain + // cannot make it read parked, by construction -- there is nothing here it + // could read. That is the resolved design question (see view_park.h): + // flags-fine/FX-offline stays on the ordinary capture path, never EXPOSE. + const TrackPlan park = makeParkPlan("{A}"); + CHECK(!chainReadsParked(park.flags, FlagValues{1, 1, 1, 1})); // flags all fine +} + +// -- the EXPOSE console report ------------------------------------------------- + +static void testExposedTracksAreNamedAndEmptyListSaysNothing() { + CHECK(describeExposedTracks({}).empty()); + + const std::string one = describeExposedTracks({"Bass"}); + CHECK(one.find("1 track ") != std::string::npos); + CHECK(one.find("Bass") != std::string::npos); + + const std::string two = describeExposedTracks({"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 testExposedReportNeverUsesRefusalWording() { + // Daniel's ruling was EXPOSE, not refuse -- the wording must never suggest the + // tool declined to act. + const std::string msg = describeExposedTracks({"Bass"}); + CHECK(msg.find("refus") == std::string::npos); +} + +static void testShouldReportSkipsAnExactRepeatOnly() { + const std::vector a{"Bass"}; + const std::vector b{"Bass", "Drum bus"}; + + CHECK(!shouldReport(/*sameOwnerAsLast=*/true, a, a)); // exact repeat: silent + CHECK(shouldReport(/*sameOwnerAsLast=*/true, a, b)); // set changed: reprint + CHECK(shouldReport(/*sameOwnerAsLast=*/false, a, a)); // owner (project) changed: reprint + CHECK(shouldReport(/*sameOwnerAsLast=*/true, a, {})); // healed to none: reprint (resets memo) +} + int main() { testEachDrivenFlagAddressesItsOwnReaperParameter(); testNoDrivenFlagCanEverAddressMuteOrSolo(); @@ -144,6 +223,15 @@ int main() { testAPluginAddedWhileRestoredJoinsTheUnionRatherThanBeingDropped(); testAPluginRemovedWhileRestoredStaysInTheUnionForResolveFxRestoreToDrop(); + testAChainSittingAtEveryParkedValueReadsAsParkShaped(); + testOneFlagStillAtTheUsersValueMeansNotParkShaped(); + testAnEmptyPlanProvesNothing(); + testFxOfflineAloneIsNotFoldedIntoTheParkedRead(); + + testExposedTracksAreNamedAndEmptyListSaysNothing(); + testExposedReportNeverUsesRefusalWording(); + testShouldReportSkipsAnExactRepeatOnly(); + if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; }