From ebdb4eae66ba62ca4826c1dfc89db9bd45f69d79 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 23 Jul 2026 17:51:55 -0400 Subject: [PATCH] fix(D2-W2): correct manual-lane exemption, drop read-only item C_LANEPLAYS write isOnManualLane predicate now correctly treats unnamed fixed-lane lanes as manual; item-side C_LANEPLAYS loop removed (SDK read-only); explicit #include in view_mode_model.cpp; undo-block omission in detectNewContent documented as intentional. --- src/bank_panel.cpp | 43 +++++++++++++++++++++++++++++++--------- src/lane_keys.cpp | 10 ++++++++++ src/lane_keys.h | 16 +++++++++++++++ src/view.cpp | 30 ++++++++++------------------ src/view_mode_model.cpp | 1 + tests/test_lane_keys.cpp | 24 ++++++++++++++++++++++ 6 files changed, 96 insertions(+), 28 deletions(-) diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 29f80c6..eee2e2c 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -566,13 +566,22 @@ bool refreshFingerprint() { // enumeration is READ-ONLY on the project; the only mutation is to the in-memory // membership index (persisted by persist on the next save, same as an action-driven tag). -// The durable name of item `it`'s fixed lane, or empty if the item's track is not in -// fixed-lane mode (⇒ not a lane at all, treated as non-manual normal content). Mirrors -// view.cpp's laneName but item-side: read the item's I_FIXEDLANE ordinal, then that -// lane's P_LANENAME:n off the owning track. +// True iff `tr` has I_FREEMODE==2 (fixed lanes enabled). The SDK value is verified +// in view.cpp (kFreeModeFixedLanes=2); reproduced here as a local constant so +// bank_panel.cpp stays self-contained without pulling in view.cpp's private namespace. +constexpr int kFreeModeFixedLanes = 2; + +bool isFixedLaneTrack(MediaTrack* tr) { + return static_cast(GetMediaTrackInfo_Value(tr, "I_FREEMODE")) == kFreeModeFixedLanes; +} + +// Reads the durable P_LANENAME for the lane that item `it` sits on. Returns empty if +// the lane is unnamed or P_LANENAME is unavailable. Callers must already know the track +// is a fixed-lane track (I_FREEMODE==2) before calling this — the manual/non-manual +// distinction only applies there. On a non-fixed-lane track `I_FIXEDLANE` is +// meaningless; the isOnManualLane predicate handles that case via its isFixedLaneTrack +// argument, so callers should not call this at all for non-fixed-lane tracks. std::string itemLaneName(MediaTrack* tr, MediaItem* it) { - if (static_cast(GetMediaTrackInfo_Value(tr, "I_FREEMODE")) != 2) - return {}; // not a fixed-lane track ⇒ no lane name (normal timeline content) const int laneIdx = static_cast(GetMediaItemInfo_Value(it, "I_FIXEDLANE")); char parm[32]; std::snprintf(parm, sizeof(parm), "P_LANENAME:%d", laneIdx); @@ -592,6 +601,10 @@ std::string itemGuid(MediaItem* it) { // Enumerates the live project's track + item GUIDs. Fills `allGuids` (the full live set, // baseline input) and, for each item, records whether it sits on a manual lane so a // newly-detected item can be exempted from auto-tag without a second project walk. +// +// Manual-lane classification uses the single pure predicate isOnManualLane(isFixedLaneTrack, +// laneName) from lane_keys — the same predicate the apply path consults — so the exemption +// rule is defined in exactly one place and is unit-tested there. void enumerateLiveGuids(ReaProject* proj, std::set& allGuids, std::map& itemOnManualLane) { const int trackCount = CountTracks(proj); @@ -601,6 +614,10 @@ void enumerateLiveGuids(ReaProject* proj, std::set& allGuids, std::string tg = guidString(tr); if (!tg.empty()) allGuids.insert(tg); + // Compute the fixed-lane status once per track (not per item) — I_FREEMODE is a + // track-level attribute and is the same for every item on the track. + const bool fixedLane = isFixedLaneTrack(tr); + const int itemCount = CountTrackMediaItems(tr); for (int i = 0; i < itemCount; ++i) { MediaItem* it = GetTrackMediaItem(tr, i); @@ -608,9 +625,11 @@ void enumerateLiveGuids(ReaProject* proj, std::set& allGuids, std::string ig = itemGuid(it); if (ig.empty()) continue; allGuids.insert(ig); - // Manual iff the item is on a fixed lane whose name is NOT tool-managed. - const std::string ln = itemLaneName(tr, it); - itemOnManualLane[ig] = (!ln.empty() && !isManagedLaneName(ln)); + // Classify via the single shared predicate. For a fixed-lane track we read + // the item's lane name; for a normal track we pass "" (isOnManualLane returns + // false immediately for non-fixed-lane tracks regardless of name). + const std::string ln = fixedLane ? itemLaneName(tr, it) : std::string{}; + itemOnManualLane[ig] = isOnManualLane(fixedLane, ln); } } } @@ -619,6 +638,12 @@ void enumerateLiveGuids(ReaProject* proj, std::set& allGuids, // into the active mode. Runs every timer tick regardless of panel open/close (content // is created in the arrange). READ-ONLY on the project; mutates only the in-memory // membership index. +// +// INTENTIONAL: membership mutation happens OUTSIDE any Undo block. Auto-tag is a +// background metadata update (like setting a label), not a destructive project edit. +// persist.cpp writes it on the next project save alongside the bank and view state, the +// same way an action-driven tag is persisted. Wrapping this in an Undo block would flood +// the REAPER undo history with a new entry for every timer tick that sees new content. void detectNewContent() { if (!g_panel.session) return; diff --git a/src/lane_keys.cpp b/src/lane_keys.cpp index 923edbc..83b4c7a 100644 --- a/src/lane_keys.cpp +++ b/src/lane_keys.cpp @@ -31,4 +31,14 @@ std::string laneNameForMode(const std::string& modeId) { return std::string(kManagedLanePrefix) + modeId; } +bool isOnManualLane(bool isFixedLaneTrack, const std::string& laneName) { + // On a normal (non-fixed-lane) track there is no concept of a manual lane; the + // item follows the normal auto-tag rule. + if (!isFixedLaneTrack) return false; + // On a fixed-lane track: a managed lane (prefixed) is NOT manual; everything else + // — including the empty/unnamed lane that REAPER creates by default — IS manual + // (user-minted, off-limits to auto-tag and to the lane-drive path). + return !hasManagedPrefix(laneName); +} + } // namespace reasampler diff --git a/src/lane_keys.h b/src/lane_keys.h index 537972d..29ffee1 100644 --- a/src/lane_keys.h +++ b/src/lane_keys.h @@ -58,4 +58,20 @@ std::optional managedLaneKey(const std::string& laneName); // contract is asserted here so minting and reading cannot drift. std::string laneNameForMode(const std::string& modeId); +// True iff an item on a fixed-lane track with the given lane name is on a MANUAL lane +// (i.e. exempt from auto-tag). The two inputs are: +// isFixedLaneTrack — whether the item's track has I_FREEMODE==2. On a normal +// (non-fixed-lane) track the concept of a "manual lane" does not +// apply; the item follows the normal auto-tag rule (return false). +// laneName — the durable P_LANENAME of the lane the item sits on. A lane +// that carries kManagedLanePrefix is a tool-minted managed lane +// (not manual); any other name — including empty (unnamed) — is +// a user-minted manual lane (exempt from auto-tag). +// +// This is the SINGLE predicate that governs BOTH the apply path (which lanes may be +// driven) and the auto-tag exemption path (which items are exempt). It is unit-tested +// here so both paths share exactly one definition; the shell supplies the two REAPER +// inputs (I_FREEMODE result, P_LANENAME string) and never re-derives this logic. +bool isOnManualLane(bool isFixedLaneTrack, const std::string& laneName); + } // namespace reasampler diff --git a/src/view.cpp b/src/view.cpp index b0e2571..73d90aa 100644 --- a/src/view.cpp +++ b/src/view.cpp @@ -31,10 +31,6 @@ #define REAPERAPI_WANT_TrackFX_GetCount #define REAPERAPI_WANT_TrackFX_GetOffline #define REAPERAPI_WANT_TrackFX_SetOffline -#define REAPERAPI_WANT_CountTrackMediaItems -#define REAPERAPI_WANT_GetTrackMediaItem -#define REAPERAPI_WANT_GetMediaItemInfo_Value -#define REAPERAPI_WANT_SetMediaItemInfo_Value #define REAPERAPI_WANT_Undo_BeginBlock2 #define REAPERAPI_WANT_Undo_EndBlock2 #define REAPERAPI_WANT_TrackList_AdjustWindows @@ -180,25 +176,21 @@ std::map managedLaneOrdinals(MediaTrack* tr) { return byKey; } -// Drives one managed lane on `tr` to `lanePlays` (C_LANEPLAYS value): the track-side -// C_LANEPLAYS:N for lane ordinal `laneIdx`, plus every ITEM on that lane (item-side -// C_LANEPLAYS). Items are matched to the lane by their read-only I_FIXEDLANE ordinal. -// B_FIXEDLANE_HIDDEN is READ-ONLY (SDK) — hide/show follows from C_LANEPLAYS=0/1, never -// written directly. Non-destructive: only reversible play/show flags; no item is moved -// or deleted. +// Drives one managed lane on `tr` to `lanePlays` (C_LANEPLAYS value) via the +// TRACK-SIDE C_LANEPLAYS:N write. Track-side C_LANEPLAYS:N alone produces the +// hide+silence effect for all items on lane N — no per-item write is needed or +// possible (item-side C_LANEPLAYS is marked read-only in the SDK). +// B_FIXEDLANE_HIDDEN is READ-ONLY (SDK) — hide/show follows from C_LANEPLAYS=0/1, +// never written directly. Non-destructive: only reversible play/show flags; no item +// is moved or deleted. +// +// DAW-VERIFY: confirm that track-side C_LANEPLAYS:N alone hides+silences all items +// on lane N without a per-item write. (SDK marks item-side C_LANEPLAYS as read-only; +// the track-side write is the documented mechanism.) void applyLanePlays(MediaTrack* tr, int laneIdx, int lanePlays) { char parm[32]; std::snprintf(parm, sizeof(parm), "C_LANEPLAYS:%d", laneIdx); SetMediaTrackInfo_Value(tr, parm, static_cast(lanePlays)); - - const int itemCount = CountTrackMediaItems(tr); - for (int i = 0; i < itemCount; ++i) { - MediaItem* it = GetTrackMediaItem(tr, i); - if (!it) continue; - const int itemLane = static_cast(GetMediaItemInfo_Value(it, "I_FIXEDLANE")); - if (itemLane != laneIdx) continue; // item is on a different lane - SetMediaItemInfo_Value(it, "C_LANEPLAYS", static_cast(lanePlays)); - } } // Applies the plan's managed-lane ops. Groups ops by track, resolves each op's durable diff --git a/src/view_mode_model.cpp b/src/view_mode_model.cpp index 95f6fc8..409c0a5 100644 --- a/src/view_mode_model.cpp +++ b/src/view_mode_model.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include // view_mode_model implementation. diff --git a/tests/test_lane_keys.cpp b/tests/test_lane_keys.cpp index f9bd58d..5f33e6f 100644 --- a/tests/test_lane_keys.cpp +++ b/tests/test_lane_keys.cpp @@ -46,6 +46,29 @@ static void testManagedLaneKey() { CHECK(!managedLaneKey("guitar-double").has_value()); } +static void testIsOnManualLane() { + // Non-fixed-lane track: concept does not apply regardless of name. + CHECK(!isOnManualLane(false, "")); // normal track, unnamed ⇒ not manual + CHECK(!isOnManualLane(false, "Comp 1")); // normal track, user name ⇒ not manual + CHECK(!isOnManualLane(false, "reasampler:design")); // normal track, managed name ⇒ not manual + + // Fixed-lane track: managed lane (tool-prefixed) ⇒ NOT manual (tool drives it). + CHECK(!isOnManualLane(true, "reasampler:design")); + CHECK(!isOnManualLane(true, "reasampler:arrange")); + CHECK(!isOnManualLane(true, "reasampler:")); // prefix-only: still managed + + // Fixed-lane track: unnamed lane (empty P_LANENAME) ⇒ manual. + // REAPER starts fixed lanes unnamed; an item on an unnamed fixed lane is a user + // comp lane and must be exempt from auto-tag. + CHECK(isOnManualLane(true, "")); + + // Fixed-lane track: user-named but non-managed ⇒ manual. + CHECK(isOnManualLane(true, "Comp 1")); + CHECK(isOnManualLane(true, "Lead vocal")); + CHECK(isOnManualLane(true, "reasample")); // near-miss, no colon ⇒ manual + CHECK(isOnManualLane(true, "Reasampler:x")); // wrong case ⇒ manual +} + static void testRoundTrip() { // Minting then reading must agree: managedLaneKey(laneNameForMode(m)) recovers the // prefixed name for every mode id. @@ -62,6 +85,7 @@ static void testRoundTrip() { int main() { testIsManagedLaneName(); testManagedLaneKey(); + testIsOnManualLane(); testRoundTrip(); if (g_fail == 0) std::printf("All tests passed.\n");