From 2536b4bedb05b89e4f288181f1e57802a8cd76ce Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 23 Jul 2026 05:56:25 -0400 Subject: [PATCH] fix: insert onto selected track(s) at edit cursor, not a new track Wire both insert actions to CurrentTrack (InsertMedia base 0). Per selected track: SetOnlyTrackSelected, reset cursor to snapshot, InsertMedia; then restore original selection + cursor. No-op when no track selected. Corrected stale extraflags / new-track / preservePitch comments. --- src/insert.cpp | 133 ++++++++++++++++++++++++++----------- src/insert.h | 23 ++++--- src/insert_plan.h | 10 +-- src/main.cpp | 14 ++-- tests/test_insert_plan.cpp | 28 ++++++-- 5 files changed, 142 insertions(+), 66 deletions(-) diff --git a/src/insert.cpp b/src/insert.cpp index 34b2843..32153a3 100644 --- a/src/insert.cpp +++ b/src/insert.cpp @@ -10,21 +10,25 @@ // // FLAGGED RUNTIME ASSUMPTIONS (semantics the header does not fully specify — must // be DAW-verified by Daniel post-merge; see the handoff): -// A. InsertMedia base modes 0/1 insert AT THE EDIT CURSOR. The header names the -// base targets ("add to current track" / "add new track") but does not spell -// out an explicit "at edit cursor" bit — placement at the edit cursor is -// REAPER's documented convention for these modes, relied on here. -// B. InsertMedia ADVANCES the edit cursor to the end of the inserted media. This -// is the behavior that makes sequential multi-insert lay items end-to-end. It -// is REAPER's long-standing behavior but is not stated in the header — flagged. -// We do NOT re-read/patch the cursor between inserts (we trust B); if B proved -// false in the DAW, the fix is to advance the cursor ourselves by the inserted -// item length. Not done now (no evidence it is needed, and item length is not -// returned by InsertMedia). -// C. New-track insert (mode base 1) creates the track and leaves it selected; -// current-track insert (base 0) targets the current/last-selected track. We do -// not force a track selection — the user's current selection is the target for -// base 0, matching REAPER's drag-to-track semantics. +// A. InsertMedia base mode 0 ("add to current track") targets the track that is +// currently the ONLY selected track. The header names the base target but does +// not spell out how "current track" resolves at runtime. We force exactly one +// selected track via SetOnlyTrackSelected before each InsertMedia call, which +// is the most defensible interpretation; if REAPER uses a different notion of +// "current" (e.g. last-focused, not last-selected), DAW-verify and adjust. +// B. InsertMedia mode 0 inserts AT THE EDIT CURSOR. Placement at the edit cursor +// is REAPER's documented convention for base modes 0/1 (the header does not +// spell out an explicit "at edit cursor" bit). Flagged for DAW-verification. +// C. InsertMedia ADVANCES the edit cursor to the end of the inserted media. We +// reset the cursor to the snapshot position before EACH track's insert, so +// assumption C's truth or falsity is irrelevant: we own the cursor reset. +// D. SetEditCurPos(time, false, false) moves the cursor without scrolling the view +// and without seeking the transport. The header lists the args as +// (time, moveview, seekplay) — moveview=false and seekplay=false are the +// non-disruptive choice; flagged in case the DAW shows otherwise. +// E. SetOnlyTrackSelected deselects all tracks and selects exactly one. The header +// doc-comment says "Set exactly one track selected, deselect all others" — +// this is the strongest confirmation we have; flagged for DAW-verification. #include "insert.h" @@ -38,9 +42,15 @@ #include "persist.h" #define REAPERAPI_MINIMAL +#define REAPERAPI_WANT_CountSelectedTracks #define REAPERAPI_WANT_EnumProjects #define REAPERAPI_WANT_GetCursorPosition +#define REAPERAPI_WANT_GetSelectedTrack #define REAPERAPI_WANT_InsertMedia +#define REAPERAPI_WANT_SetEditCurPos +#define REAPERAPI_WANT_SetOnlyTrackSelected +#define REAPERAPI_WANT_SetTrackSelected +#define REAPERAPI_WANT_ShowConsoleMsg #define REAPERAPI_WANT_Undo_BeginBlock2 #define REAPERAPI_WANT_Undo_EndBlock2 #include "reaper_plugin_functions.h" @@ -64,15 +74,51 @@ std::string currentProjectDir() { return normalizeSlashes(fs::path(rpp).parent_path().string()); } +// Snapshot the user's currently-selected track set (ignores master, matches +// CountSelectedTracks / GetSelectedTrack which both skip master). Returns the +// tracks in selection order so we can restore the original state afterward. +std::vector snapshotSelectedTracks() { + const int n = CountSelectedTracks(nullptr); // nullptr = active project + std::vector tracks; + tracks.reserve(static_cast(n)); + for (int i = 0; i < n; ++i) + tracks.push_back(GetSelectedTrack(nullptr, i)); + return tracks; +} + +// Restore a previously-snapshotted track selection: deselect all (by setting the +// first track alone) then re-select the full set. If the snapshot is empty we +// leave all tracks deselected; no-op guard handles a completely empty project. +void restoreSelectedTracks(const std::vector& tracks) { + if (tracks.empty()) return; + // Deselect all via the first track, then re-add the rest. + SetOnlyTrackSelected(tracks[0]); + for (size_t i = 1; i < tracks.size(); ++i) + SetTrackSelected(tracks[i], true); +} + } // namespace InsertResult runInsert(ReaSamplerSession* session, const InsertRequest& request) { InsertResult result; if (!session) { result.status = InsertStatus::NoSelection; return result; } - // WHAT to place: the panel's current selection (ids, in bank order). + // WHO to target: the user's currently-selected track set. No-op (with a clear + // console message) when nothing is selected — inserting without a target track + // would create an unintended new track or behave unpredictably. + const std::vector selectedTracks = snapshotSelectedTracks(); + if (selectedTracks.empty()) { + ShowConsoleMsg("ReaSampler insert: select a track first.\n"); + result.status = InsertStatus::NoSelection; + return result; + } + + // WHAT to place: the single focused sample from the panel. Multi-select is + // deprioritized; take the first (or only) selected id. An empty panel selection + // is a no-op — nothing to place. const std::vector ids = bankPanelSelectedSampleIds(); if (ids.empty()) { result.status = InsertStatus::NoSelection; return result; } + const std::string& id = ids.front(); // focused / first selected — single sample // WHERE the bank lives on disk. An unsaved project has no resolvable bank dir; // insert is a no-op rather than resolving against CWD (CLAUDE.md invariant). @@ -80,40 +126,53 @@ InsertResult runInsert(ReaSamplerSession* session, const InsertRequest& request) if (projectDir.empty()) { result.status = InsertStatus::NoProject; return result; } const BankIndex& bank = session->bank(); + const Sample* sample = bank.query(id); + if (!sample) { result.status = InsertStatus::NothingResolved; return result; } + + const std::string abs = resolveBankFile(projectDir, sample->relativePath); + if (abs.empty() || !fs::exists(fs::path(abs))) { + result.status = InsertStatus::NothingResolved; + return result; + } + const int mode = computeInsertMode(request.options); - // Wrap the whole placement in ONE undo block so a single undo removes every - // item inserted by this action (proj=nullptr -> active project). Opened before - // the first InsertMedia and closed after the last, unconditionally, so the block - // is always balanced even if nothing resolves (an empty block is harmless). + // Snapshot the edit cursor position up front so we can restore it to the same + // position for each track insert (and after the whole operation). + const double cursorPos = GetCursorPosition(); + + // Wrap the whole placement (all tracks + selection/cursor save-restore) in ONE + // undo block so a single undo removes every item and restores the state before + // the action. Opened before the first InsertMedia, closed after the restore, + // unconditionally — the block is always balanced. Undo_BeginBlock2(nullptr); - for (const std::string& id : ids) { - const Sample* sample = bank.query(id); - if (!sample) { ++result.skipped; continue; } // id no longer in the bank - - const std::string abs = resolveBankFile(projectDir, sample->relativePath); - if (abs.empty()) { ++result.skipped; continue; } // unresolvable relative path - if (!fs::exists(fs::path(abs))) { ++result.skipped; continue; } // file missing - - // Insert AT THE EDIT CURSOR (assumption A). InsertMedia advances the cursor - // to the end of the inserted media (assumption B), so the next iteration - // lands contiguously — no manual cursor math needed. Non-destructive to the - // bank: this references abs, it does not modify the file or the index. + // Insert onto EACH selected track at the SAME edit-cursor position (assumption B). + // For each track: isolate it as the only selection so InsertMedia mode 0 targets + // it unambiguously (assumption A + E), reset the cursor to the snapshot position + // (assumption C cursor advance is irrelevant — we own the reset), then insert. + for (MediaTrack* track : selectedTracks) { + SetOnlyTrackSelected(track); // assumption A + E + SetEditCurPos(cursorPos, false, false); // assumption D InsertMedia(abs.c_str(), mode); ++result.inserted; } + // Restore the user's original track selection and cursor position so the action + // is non-destructive to their DAW state (non-negotiable per the brief). + restoreSelectedTracks(selectedTracks); + SetEditCurPos(cursorPos, false, false); + // Label reflects the count and the conform choice so the undo history reads - // clearly ("ReaSampler: insert 2 samples" etc.). extraflags 0 = default scope. + // clearly ("ReaSampler: insert on 2 tracks" etc.). extraflags -1 = UNDO_STATE_ALL + // (superset: tracks, items, envelope points, project state). const std::string label = - "ReaSampler: insert " + std::to_string(result.inserted) + - (result.inserted == 1 ? " sample" : " samples") + + "ReaSampler: insert on " + std::to_string(result.inserted) + + (result.inserted == 1 ? " track" : " tracks") + (request.options.conform == TempoConform::None ? "" : " (conform)"); Undo_EndBlock2(nullptr, label.c_str(), -1); - if (result.inserted == 0) result.status = InsertStatus::NothingResolved; - else result.status = InsertStatus::Ok; + result.status = InsertStatus::Ok; return result; } diff --git a/src/insert.h b/src/insert.h index a9e2c7d..b61aee9 100644 --- a/src/insert.h +++ b/src/insert.h @@ -27,7 +27,7 @@ class ReaSamplerSession; // tempo-conform choice) so the two action variants (native-length vs // conform-to-tempo) differ only by this struct — no divergent code paths. struct InsertRequest { - InsertOptions options; // defaults: new track, no conform, native length + InsertOptions options; // defaults: current track, no conform, native length }; // The outcome of an insert action, for the caller to log to the console. @@ -44,16 +44,19 @@ struct InsertResult { int skipped = 0; // selected-but-unresolvable/unreadable samples skipped }; -// Runs the insert: reads bank_panel's selection, resolves each sample against the -// current project dir, and inserts them AT THE EDIT CURSOR in bank order, advancing -// the cursor so multiple samples lay end-to-end. The whole placement is wrapped in -// a single Undo_BeginBlock2 / Undo_EndBlock2 so one undo removes the entire insert. -// `session` supplies the live bank the selected ids resolve against. +// Runs the insert: reads the bank panel's single focused sample and the user's +// currently-selected track set, then inserts the sample onto EACH selected track +// at the SAME edit-cursor position. Snapshot/restore ensures the user's track +// selection and cursor position are unchanged after the action. The whole operation +// is wrapped in a single Undo_BeginBlock2 / Undo_EndBlock2. // -// Multi-select behavior: each selected sample is inserted sequentially at the -// then-current edit cursor; InsertMedia advances the cursor to the end of the -// inserted media, so N samples lay contiguously left-to-right. Single-select is the -// N==1 case of the same path. +// No-op cases (with console messages): +// - No track selected: prints "select a track first." +// - No sample selected in the panel: NoSelection status. +// - Unsaved project (no resolvable bank dir): NoProject status. +// - Sample id not in bank / file missing: NothingResolved status. +// +// `session` supplies the live bank the selected id resolves against. InsertResult runInsert(ReaSamplerSession* session, const InsertRequest& request); } // namespace reasampler diff --git a/src/insert_plan.h b/src/insert_plan.h index 8f589e6..7a6e2cb 100644 --- a/src/insert_plan.h +++ b/src/insert_plan.h @@ -46,15 +46,15 @@ enum class TempoConform { RatioDouble,// &32: try to match tempo 2x }; -// Options that shape one InsertMedia call. Defaults encode the safe path: -// new track, no conform, pitch preserved. +// Options that shape one InsertMedia call. Defaults encode the intended path: +// current track (user's selection), no conform, pitch preserved. struct InsertOptions { - InsertTarget target = InsertTarget::NewTrack; + InsertTarget target = InsertTarget::CurrentTrack; TempoConform conform = TempoConform::None; - // Only meaningful when conform != None. When true, adds &64 ("don't preserve + // Only meaningful when conform != None. When false, adds &64 ("don't preserve // pitch when matching tempo") so a tempo match also shifts pitch (classic - // varispeed). Default false = preserve pitch across the tempo match. Ignored + // varispeed). Default true = preserve pitch across the tempo match. Ignored // when conform == None (no tempo bits set, so pitch is moot). bool preservePitch = true; }; diff --git a/src/main.cpp b/src/main.cpp index 3096aee..046bded 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -142,7 +142,8 @@ static void RunCaptureMasterSpike() static void RunInsertSelected(bool conform) { reasampler::InsertRequest req; - req.options.target = reasampler::InsertTarget::NewTrack; // sensible default: own track + // target defaults to CurrentTrack (InsertOptions::target) — inserts onto the + // user's currently-selected track(s) at the edit cursor. req.options.conform = conform ? reasampler::TempoConform::Ratio1x : reasampler::TempoConform::None; // preservePitch stays true: a tempo conform matches tempo without varispeeding @@ -154,14 +155,13 @@ static void RunInsertSelected(bool conform) switch (res.status) { case reasampler::InsertStatus::Ok: - msg = "ReaSampler: inserted " + std::to_string(res.inserted) + - (res.inserted == 1 ? " sample" : " samples") + - (conform ? " (conformed to tempo)" : " (native length)"); - if (res.skipped > 0) - msg += ", skipped " + std::to_string(res.skipped) + " unresolvable"; - msg += "\n"; + msg = "ReaSampler: inserted onto " + std::to_string(res.inserted) + + (res.inserted == 1 ? " track" : " tracks") + + (conform ? " (conformed to tempo)" : " (native length)") + "\n"; break; case reasampler::InsertStatus::NoSelection: + // "select a track first" is printed by runInsert when no track is + // selected; this branch covers the no-panel-selection case. msg = "ReaSampler insert: nothing selected in the bank panel.\n"; break; case reasampler::InsertStatus::NoProject: diff --git a/tests/test_insert_plan.cpp b/tests/test_insert_plan.cpp index 8f883c2..1339ddb 100644 --- a/tests/test_insert_plan.cpp +++ b/tests/test_insert_plan.cpp @@ -25,11 +25,11 @@ constexpr int MATCH_HALF = 16; constexpr int MATCH_DBL = 32; constexpr int NO_PITCH = 64; -static void testDefaultIsNewTrackNativeLength() { - // Defaults: new track (base 1), no conform, pitch preserved. +static void testDefaultIsCurrentTrackNativeLength() { + // Defaults: current track (base 0), no conform, pitch preserved. InsertOptions opts; const int mode = computeInsertMode(opts); - CHECK(mode == 1); // base 1 only, no other bits + CHECK(mode == 0); // base 0 only, no other bits CHECK((mode & STRETCH_FIT) == 0); // never stretch-to-time-sel CHECK((mode & MATCH_1X) == 0); // no tempo bits at native length CHECK((mode & MATCH_HALF) == 0); @@ -47,11 +47,11 @@ static void testCurrentTrackBaseIsZero() { } static void testConform1xSetsOnlyMatchBit() { - InsertOptions opts; // new track base 1 + InsertOptions opts; // current track base 0 (default) opts.conform = TempoConform::Ratio1x; const int mode = computeInsertMode(opts); CHECK((mode & MATCH_1X) == MATCH_1X); // the 1x match bit is set - CHECK((mode & 3) == 1); // base target unchanged + CHECK((mode & 3) == 0); // base target: current track (0) CHECK((mode & STRETCH_FIT) == 0); // still never the stretch bit CHECK((mode & (MATCH_HALF | MATCH_DBL)) == 0); // no other ratio bits CHECK((mode & NO_PITCH) == 0); // pitch preserved by default @@ -87,7 +87,20 @@ static void testPreservePitchGatesTheNoPitchBit() { noConformNoPitch.conform = TempoConform::None; noConformNoPitch.preservePitch = false; CHECK((computeInsertMode(noConformNoPitch) & NO_PITCH) == 0); - CHECK(computeInsertMode(noConformNoPitch) == 1); // just base 1, nothing else + CHECK(computeInsertMode(noConformNoPitch) == 0); // base 0 (current track), nothing else +} + +static void testDefaultActionIsCurrentTrackBase0() { + // The default InsertOptions must compute base 0 (current track) so that both + // insert actions (default + conform) target the user's selected track(s), not + // a new track. This is the wired M6 insert-target change. + InsertOptions defaultOpts; + CHECK(defaultOpts.target == InsertTarget::CurrentTrack); + CHECK((computeInsertMode(defaultOpts) & 3) == 0); // base 0 + + InsertOptions conformOpts; + conformOpts.conform = TempoConform::Ratio1x; + CHECK((computeInsertMode(conformOpts) & 3) == 0); // conform variant also base 0 } static void testStretchBitNeverSetAcrossAllOptions() { @@ -107,11 +120,12 @@ static void testStretchBitNeverSetAcrossAllOptions() { } int main() { - testDefaultIsNewTrackNativeLength(); + testDefaultIsCurrentTrackNativeLength(); testCurrentTrackBaseIsZero(); testConform1xSetsOnlyMatchBit(); testConformHalfAndDoubleRatios(); testPreservePitchGatesTheNoPitchBit(); + testDefaultActionIsCurrentTrackBase0(); testStretchBitNeverSetAcrossAllOptions(); if (g_fail == 0) std::printf("insert_plan: all tests passed\n");