diff --git a/src/actions.cpp b/src/actions.cpp index f639bf5..1beb0d1 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -476,22 +476,6 @@ bool persistBook() { return g_session->saveToActiveProject(); } // rejected op (duplicate name, un-deletable pool, etc.) returns before reaching here, // so no dangling/empty undo point is ever opened for a rejected op. // -// UNSAVED-PROJECT GUARDRAIL: on an unsaved / no-active project persistBook() no-ops -// (nothing is written to ext state). We must still CLOSE the block we opened, but with -// an EMPTY label and a zero flag so REAPER DISCARDS the point instead of recording a -// no-effect undo entry — mirroring view.cpp's empty-plan close. The in-session model -// change stands and persists on the user's next save; it just earns no undo point until -// there is a project to persist into (undo of an unsaved bank op has nothing to roll -// back to anyway). The Begin/End must still be balanced, hence the close-either-way. -void persistBankOp(const char* label) { - Undo_BeginBlock2(nullptr); - const bool persisted = persistBook(); - if (persisted) - Undo_EndBlock2(nullptr, label, UNDO_STATE_MISCCFG); - else - Undo_EndBlock2(nullptr, "", 0); // no ext-state write -> discard the empty point -} - // Prompts the user for a single line of text via REAPER's stock input dialog. // GetUserInputs(title, num_inputs=1, captions_csv, retvals_csv, sz) -> false on // cancel (SDK ~3808). `initial` pre-fills the field. Returns false (leaving `out` @@ -827,6 +811,39 @@ void doBankRemoveSelected() { } // namespace +// Persists a completed bank-index verb as a SINGLE batched REAPER undo point (R-B) — +// one bank op = one Ctrl-Z. Declared in actions.h so bank_panel.cpp can call it +// without duplicating the undo logic. +// +// WHY THIS WRAPS AND persistBook() DOES NOT: a bank verb mutates ONLY our project +// ext-state (SetProjExtState under "reasampler"), which REAPER's undo system captures +// iff UNDO_STATE_MISCCFG is set in the Undo_EndBlock2 flags — the SDK documents +// MISCCFG as covering "extensions!" project ext-state (reaper_plugin.h ~1544, ~1199). +// We pass exactly UNDO_STATE_MISCCFG (not -1 / UNDO_STATE_ALL as the item-move family +// does): a bank verb touches no tracks, FX, items, or envelopes, so snapshotting them +// would be both heavier and semantically wrong. persistBook() (= SetProjExtState) runs +// INSIDE the block so the post-mutation ext-state is the block's "after" image. +// +// NO-OP GUARDRAIL: callers invoke this ONLY after the model mutation succeeded — a +// rejected op (duplicate name, un-deletable pool, etc.) returns before reaching here, +// so no dangling/empty undo point is ever opened for a rejected op. +// +// UNSAVED-PROJECT GUARDRAIL: on an unsaved / no-active project persistBook() no-ops +// (nothing is written to ext state). We must still CLOSE the block we opened, but with +// an EMPTY label and a zero flag so REAPER DISCARDS the point instead of recording a +// no-effect undo entry — mirroring view.cpp's empty-plan close. The in-session model +// change stands and persists on the user's next save; it just earns no undo point until +// there is a project to persist into (undo of an unsaved bank op has nothing to roll +// back to anyway). The Begin/End must still be balanced, hence the close-either-way. +void persistBankOp(const char* label) { + Undo_BeginBlock2(nullptr); + const bool persisted = persistBook(); + if (persisted) + Undo_EndBlock2(nullptr, label, UNDO_STATE_MISCCFG); + else + Undo_EndBlock2(nullptr, "", 0); // no ext-state write -> discard the empty point +} + void bankRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session) { g_session = session; // shared with the Design View family; same live session diff --git a/src/actions.h b/src/actions.h index 44bf442..ebe0eab 100644 --- a/src/actions.h +++ b/src/actions.h @@ -66,4 +66,13 @@ bool bankHandleCommand(int command); // rec==nullptr (before g_session is torn down). void bankUnregisterActions(reaper_plugin_info_t* rec); +// Persists a completed bank-index verb as a single REAPER undo point (R-B). +// Wraps persistBook() (= SetProjExtState) in a Begin/End block with UNDO_STATE_MISCCFG +// so the bank op is one Ctrl-Z. On an unsaved / no-active project persistBook() no-ops +// and the block is closed with an empty label + zero flag (REAPER discards it). Callers +// must invoke this ONLY after a successful/effective mutation — rejected ops (duplicate +// name, un-deletable pool, etc.) must return before reaching here so no empty undo +// point is ever opened for a no-op. Defined in actions.cpp alongside persistBook(). +void persistBankOp(const char* label); + } // namespace reasampler diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 73f4761..c7327c8 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -46,6 +46,7 @@ #include #include +#include "actions.h" // persistBankOp — shared undo-block wrapper (R-B panel path) #include "bank_book.h" #include "bank_grid.h" #include "bank_model.h" @@ -1224,7 +1225,7 @@ void doCreateBank() { } g_panel.shownBankId = id; // show the freshly-created bank g_panel.focusedRegion = Region::Banks; - persistBook(); + persistBankOp("ReaSampler: create bank"); invalidatePanel(); } @@ -1240,7 +1241,7 @@ void doRenameBank(const std::string& bankId) { "ReaSampler: rename bank", 0); return; } - persistBook(); + persistBankOp("ReaSampler: rename bank"); invalidatePanel(); } @@ -1273,7 +1274,7 @@ void doDeleteBank(const std::string& bankId) { // r == 6 (Yes) falls through to a plain delete (drops members). } if (!book()->deleteBank(bankId)) return; - persistBook(); + persistBankOp("ReaSampler: delete bank"); // shownBankId is reconciled by the next fingerprint pass. If no named banks remain, // nudge focus to the pool so the selection has a valid home. if (namedBanks().empty()) g_panel.focusedRegion = Region::Pool; @@ -1285,30 +1286,49 @@ void doEvacuateBank(const std::string& bankId) { const Bank* bk = book()->bank(bankId); if (!bk || bk->isPool()) return; if (!book()->evacuate(bankId)) return; - persistBook(); + persistBankOp("ReaSampler: evacuate bank"); invalidatePanel(); } void doActivateBank(const std::string& bankId) { if (!book()) return; if (!book()->setActiveBank(bankId)) return; // rejects an unknown id - persistBook(); + persistBankOp("ReaSampler: activate bank"); invalidatePanel(); } // Move or copy `sampleIds` from `srcBankId` to `destBankId` (index-only). Both pass // ids straight to the model op (no BankIndex& cached across the loop's mutations). +// +// NO-OP GUARDRAIL — VERB-AWARE (matches the action layer's doBankTransferSelected): +// * MOVE collapse: the source entry WAS removed (bank_book removes unconditionally +// before the dest add collapses on hash), so the index DID mutate — counts. +// * COPY collapse: the source is left intact AND the dest already held the hash, +// so NOTHING changed — a true index no-op. Must NOT open an undo point. +// Hence: copy counts only real gains (Copied); move counts gains OR collapses. void transferSamples(const std::vector& sampleIds, const std::string& srcBankId, const std::string& destBankId, bool copy) { if (!book()) return; if (sampleIds.empty() || srcBankId == destBankId) return; if (!book()->bank(srcBankId) || !book()->bank(destBankId)) return; + int ok = 0, collapsed = 0; for (const std::string& sid : sampleIds) { - if (copy) book()->copySample(sid, srcBankId, destBankId); - else book()->moveSample(sid, srcBankId, destBankId); + const TransferResult r = + copy ? book()->copySample(sid, srcBankId, destBankId) + : book()->moveSample(sid, srcBankId, destBankId); + switch (r) { + case TransferResult::Moved: + case TransferResult::Copied: ++ok; break; + case TransferResult::Collapsed: ++collapsed; break; + default: break; + } } - persistBook(); + const bool mutated = copy ? (ok > 0) : (ok > 0 || collapsed > 0); + if (!mutated) return; // nothing changed — no persist, no undo point + + const char* label = copy ? "ReaSampler: copy sample(s)" : "ReaSampler: move sample(s)"; + persistBankOp(label); // The selection indexed into the source; after a move those indices are stale, so // clear it (the fingerprint pass will also clear, but do it now for immediacy). g_panel.selection = Selection{}; @@ -1357,7 +1377,7 @@ void removeSamples(const std::vector& sampleIds, ++removed; if (removed == 0) return; // nothing changed — no persist, no undo point - persistBook(); + persistBankOp("ReaSampler: remove sample(s)"); // The selection indexed into the source; after a remove those indices are stale, so // clear it (the fingerprint pass will also clear, but do it now for immediacy). g_panel.selection = Selection{};