diff --git a/src/actions.cpp b/src/actions.cpp index 0754cd8..f180011 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -453,7 +453,41 @@ gaccel_register_t g_accelBankBanksFull{}; // persists. This is an intentional divergence from persistViewState (above), which // DOES prompt Save-As on an unsaved project; do not "align" the two — a bank mutation // follows capture's quiet-persist idiom, a Design-View mutation follows the prompt idiom. -void persistBook() { g_session->saveToActiveProject(); } +// Returns whether a persist actually happened (false on an unsaved/no-active project), +// so persistBankOp can skip its undo block when nothing was written. +bool persistBook() { return g_session->saveToActiveProject(); } + +// Persists a completed bank index verb (create/rename/reorder/delete/evacuate/ +// move/copy) as a SINGLE batched REAPER undo point (R-B) — one bank op = one Ctrl-Z. +// +// 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 +} // 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 @@ -523,7 +557,7 @@ void doBankCreate() { .c_str()); return; } - persistBook(); + persistBankOp("ReaSampler: create bank"); ShowConsoleMsg(("ReaSampler: created bank \"" + name + "\".\n").c_str()); } @@ -549,7 +583,7 @@ void doBankRename() { "or another bank already uses that name).\n"); return; } - persistBook(); + persistBankOp("ReaSampler: rename bank"); ShowConsoleMsg(("ReaSampler: renamed \"" + which + "\" -> \"" + newName + "\".\n") .c_str()); } @@ -593,7 +627,7 @@ void doBankDelete() { ShowConsoleMsg("ReaSampler: cannot delete that bank (the pool is un-deletable).\n"); return; } - persistBook(); + persistBankOp("ReaSampler: delete bank"); ShowConsoleMsg(("ReaSampler: deleted bank \"" + which + "\".\n").c_str()); } @@ -615,7 +649,7 @@ void doBankEvacuate() { "destination, not a source).\n"); return; } - persistBook(); + persistBankOp("ReaSampler: evacuate bank"); ShowConsoleMsg(("ReaSampler: evacuated \"" + which + "\" to the pool.\n").c_str()); } @@ -630,7 +664,7 @@ void doBankActivateNext() { const std::string target = nextBankId(ids, g_session->book().activeBankId()); if (target.empty()) return; // degenerate (no banks) — cannot happen (pool seeded) if (!g_session->book().setActiveBank(target)) return; - persistBook(); + persistBankOp("ReaSampler: activate bank"); const Bank* b = g_session->book().bank(target); ShowConsoleMsg(("ReaSampler: active bank -> \"" + (b ? b->displayName : target) + "\".\n") @@ -641,7 +675,7 @@ void doBankActivateNext() { // direct-by-id form; a general activate-bank-by-name/menu is a B4 affordance. void doBankActivatePool() { if (!g_session->book().setActiveBank(kPoolBankId)) return; - persistBook(); + persistBankOp("ReaSampler: activate bank"); ShowConsoleMsg("ReaSampler: active bank -> \"Pool\".\n"); } @@ -692,7 +726,19 @@ void doBankTransferSelected(bool copy) { case TransferResult::RejectedSameBank: break; } } - persistBook(); + // No-op guardrail — VERB-AWARE (a collapse means different things per verb): + // * MOVE collapse: the source entry WAS removed (bank_book moveSample removes + // unconditionally before the dest add collapses on hash), so the index DID + // mutate — it counts toward opening an undo point. + // * COPY collapse: the source is left intact AND the dest already held the hash, + // so NOTHING changed — a true index no-op. It must NOT open an undo point. + // Hence: copy counts only real gains (ok); move counts gains OR collapses. + const bool mutated = copy ? (ok > 0) : (ok > 0 || collapsed > 0); + if (mutated) { + const std::string label = + std::string("ReaSampler: ") + verb + " sample(s)"; + persistBankOp(label.c_str()); + } std::string log = std::string("ReaSampler: ") + verb + " -> \"" + destName + "\": " + std::to_string(ok) + " " + verb + "d"; if (collapsed) log += ", " + std::to_string(collapsed) + " collapsed on hash"; diff --git a/src/main.cpp b/src/main.cpp index 2223f07..04a4974 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -241,6 +241,56 @@ static void OnTimer() reasampler::bankPanelRefresh(); } +// --- projectconfig hook: reload the session on undo/redo (R-B) --------------- +// A Ctrl-Z / Ctrl-Shift-Z rolls back / forward the "reasampler" project ext state on +// disk but keeps the SAME project identity (ReaProject*/GUID/.rpp path), so the timer's +// identity poll reads it as NoOp and never re-reads ext state — the in-memory book/view +// would stay stale until close+reopen. REAPER's projectconfig extension fires +// BeginLoadProjectState on every project-state (re)load, INCLUDING an undo/redo restore +// (isUndo == true for both). We hook it to drive a session reload. +// +// TIMING (the crux): BeginLoadProjectState is documented (reaper_plugin.h ~1203) as +// firing BEFORE any state restore. Reading GetProjExtState synchronously here would +// return the PRE-undo value. So we do NOT read here — we raise a one-shot reload request +// (g_session.requestReload()) that OnTimer's poll() drains on the NEXT tick, by which +// point REAPER has finished restoring the block and GetProjExtState returns +// the POST-undo value. Deterministic, event-driven — NOT ext-state content polling. +// +// GATED ON isUndo: a normal project open also fires BeginLoadProjectState (isUndo=false); +// we ignore that here so a normal open flows solely through the timer's identity-transition +// Load path (no double load). Only undo/redo (isUndo=true) requests the reload. +static void OnBeginLoadProjectState(bool isUndo, project_config_extension_t* /*reg*/) +{ + if (isUndo) + g_session.requestReload(); +} + +// ProcessExtensionLine / SaveExtensionConfig are intentional no-ops: ReaSampler stores +// its state via project EXT STATE (SetProjExtState/GetProjExtState under "reasampler"), +// which REAPER persists in its own RPP block — NOT via this extension's own +// project lines. We register the struct ONLY for the BeginLoadProjectState undo/redo +// notification. Returning false from ProcessExtensionLine means "not our line" so REAPER +// keeps dispatching (we claim none). SaveExtensionConfig writes nothing. +static bool OnProcessExtensionLine(const char* /*line*/, ProjectStateContext* /*ctx*/, + bool /*isUndo*/, project_config_extension_t* /*reg*/) +{ + return false; // we own no project lines — ext state carries our data +} + +static void OnSaveExtensionConfig(ProjectStateContext* /*ctx*/, bool /*isUndo*/, + project_config_extension_t* /*reg*/) +{ + // Nothing to write: our data rides in ext state, not project lines. +} + +// Storage must outlive registration — REAPER holds this pointer until we unregister it. +static project_config_extension_t g_projectConfig{ + &OnProcessExtensionLine, + &OnSaveExtensionConfig, + &OnBeginLoadProjectState, + nullptr, // userData +}; + // --- Scope-action source resolution ----------------------------------------- // The three scope actions (item / track / master) each resolve to (1) an exact // render range in project seconds — razor-else-time, inferred here — and (2) the @@ -786,6 +836,7 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT( } g_rec->Register("-timer", (void*)&OnTimer); + g_rec->Register("-projectconfig", (void*)&g_projectConfig); g_rec->Register("-toggleaction", (void*)&OnToggleAction); g_rec->Register("-hookcommand", (void*)&OnHookCommand); // Tear down the Design View action family (D4) — mirror-unregisters each @@ -957,6 +1008,12 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT( // state, on a Save-As it relocates the bank folder under the new .rpp. rec->Register("timer", (void*)&OnTimer); + // Register the projectconfig hook so an UNDO/REDO state restore reloads the + // session's book + view from the restored ext state (R-B). The timer's identity + // poll cannot see an undo (same project identity), so this hook owns undo/redo; it + // requests a deferred reload that the next timer tick drains (see the hook comment). + rec->Register("projectconfig", (void*)&g_projectConfig); + ShowConsoleMsg("ReaSampler loaded.\n"); return 1; // success — REAPER keeps us loaded diff --git a/src/persist.cpp b/src/persist.cpp index d9c755f..2b43baa 100644 --- a/src/persist.cpp +++ b/src/persist.cpp @@ -51,11 +51,23 @@ // (after relocating, or on the forked-sibling Load branch) so identities diverge. // // Rationale for the timer: the brief mandates ext-state storage (rules out the -// projectconfig .rpp-line hook), and the timer composes cleanly with ext-state -// while covering both load and Save-As detection in one place. The -// `projectconfig` BeginLoadProjectState hook is a deterministic alternative for -// pure load detection but would still need the timer (or Main_SaveProject -// post-hook) for Save-As path-change detection — surfaced in the handoff. +// projectconfig .rpp-line hook for STORAGE), and the timer composes cleanly with +// ext-state while covering identity-transition load + Save-As detection in one +// place. +// +// DIVISION OF LABOUR (R-B undo): +// * Identity-transition poll (this file, classifyProjectTransition) owns +// open / tab-switch / new / forked-sibling / Save-As-relocation — every case +// where the project OF RECORD changes. +// * The `projectconfig` hook (main.cpp registers project_config_extension_t; +// BeginLoadProjectState with isUndo) owns UNDO/REDO — where the project +// identity is unchanged but its ext state rolled back/forward on disk. The +// identity poll sees NoOp there and would never re-read ext state, so the hook +// requests a reload (requestReload) that poll() drains on the next tick, once +// REAPER has restored the block. See requestReload / the poll drain. +// The hook fires on undo AND redo (isUndo true for both), and on normal open +// (isUndo false) — but we set the reload flag ONLY for isUndo, so a normal open +// flows solely through the identity-transition Load path and never double-loads. // // NON-DESTRUCTIVE: this module writes ONLY our own ext-state key and moves ONLY // our own reasampler_bank/ folder. It never touches the user's media, items, or @@ -172,11 +184,11 @@ bool relocateBankFolder(const std::string& oldBankDir, } // namespace -void ReaSamplerSession::saveToActiveProject() { +bool ReaSamplerSession::saveToActiveProject() { std::string rppPath; void* proj = readActiveProject(rppPath); - if (!proj) return; // no active project — nothing to persist - if (rppPath.empty()) return; // unsaved project — no .rpp to store into + if (!proj) return false; // no active project — nothing to persist + if (rppPath.empty()) return false; // unsaved project — no .rpp to store into // Phase B: the whole book (pool as bank-zero + named banks) is authoritative and // rides in the `banks` key. @@ -206,6 +218,7 @@ void ReaSamplerSession::saveToActiveProject() { kProjExtTailKey, tailJson.c_str()); MarkProjectDirty(static_cast(proj)); + return true; } namespace { @@ -339,6 +352,13 @@ bool ReaSamplerSession::consumeLoadSignal() { return pending; } +void ReaSamplerSession::requestReload() { + // Set-only; poll() drains it on the next tick (see the poll() drain block for why + // the read is deferred past the projectconfig callback). Cheap and idempotent — + // multiple undo/redo callbacks before the next tick collapse to one reload. + reloadRequested_ = true; +} + void ReaSamplerSession::poll() { std::string rppPath; void* proj = readActiveProject(rppPath); @@ -355,6 +375,29 @@ void ReaSamplerSession::poll() { lastProject_ = proj; lastGuid_ = ensureProjectGuid(proj, rppPath, currentGuid); lastRppPath_ = rppPath; + reloadRequested_ = false; // priming already loaded — a co-tick request is moot + return; + } + + // Undo/redo reload (owner: the projectconfig hook, NOT the identity classifier + // below). An undo/redo keeps the SAME project identity — same ReaProject*, GUID, + // and .rpp path — so classifyProjectTransition would return NoOp and never re-read + // ext state, leaving book_/view_ stale after the on-disk ext state rolled back. + // The projectconfig BeginLoadProjectState callback (isUndo) raised reloadRequested_ + // one or more ticks ago; by NOW REAPER has finished restoring the project's + // block, so GetProjExtState returns the POST-undo value. Reload from the + // current active project and identity-adopt it (no relocation — the path is + // unchanged), then return. loadFromProject raises loadPending_, so the existing + // consumeLoadSignal() glue re-baselines the panel detector and reapplies the active + // mode; bankPanelRefresh's fingerprint pass then repaints the restored book. This is + // the ONLY undo/redo reload path — the timer never polls ext-state CONTENT to detect + // an undo (Daniel's directive: the hook drives it, not a poll heuristic). + if (reloadRequested_) { + reloadRequested_ = false; + loadFromProject(proj, projectDirOf(rppPath)); + lastProject_ = proj; + lastGuid_ = ensureProjectGuid(proj, rppPath, currentGuid); + lastRppPath_ = rppPath; return; } diff --git a/src/persist.h b/src/persist.h index de3743d..c74c8df 100644 --- a/src/persist.h +++ b/src/persist.h @@ -126,13 +126,35 @@ public: // to the active project's ext state (namespace "reasampler"), and clear the retired // legacy `bank_index` key. Non-destructive beyond writing our own ext-state keys. // Safe to call when there is no active/saved project (it no-ops). - void saveToActiveProject(); + // + // Returns true iff a persist actually happened (an active, SAVED project existed); + // false when it no-op'd (no active project, or an unsaved one with no .rpp). Lets a + // caller wrapping this in an undo block skip the block when nothing was written, so + // no dangling no-effect undo entry is opened on an unsaved project. + bool saveToActiveProject(); // Poll the active project. Detects a project load (active project changed) // and a Save-As (active project's .rpp path changed) and reacts accordingly. // Intended to be driven by REAPER's "timer" register. Idempotent per tick. + // + // Also drains a pending undo/redo reload (requestReload): a Ctrl-Z / Ctrl-Shift-Z + // keeps the SAME project identity (same ReaProject*/GUID/.rpp path), so the + // identity classifier below reads it as NoOp and would never re-read ext state. + // The projectconfig hook (main.cpp) raises the reload flag on an undo/redo state + // restore; poll() honours it FIRST — reloading book_ + view_ + tail_ from the + // (now-restored) ext state of the current project — before the identity check, so + // the undo is reflected in-session without any content polling. void poll(); + // Request a reload of book_ + view_ + tail_ from the CURRENT active project's ext + // state on the next poll() tick. Raised by the projectconfig hook (main.cpp) ONLY + // on an undo/redo state restore (isUndo). Deferred (a flag, not an immediate read) + // because the projectconfig callback fires BEFORE REAPER has restored the project's + // block — reading GetProjExtState synchronously there would return the + // PRE-undo value. Draining it on the next timer tick reads the restored value. This + // is REAPER-facing shell state; the request itself carries no REAPER types. + void requestReload(); + // Load signal for the D4 reapply-on-open glue. poll() raises this whenever it // (re)loads the view model from a project — prime, a project switch/open, or a // forked-sibling load. consumeLoadSignal() returns true ONCE per load and clears @@ -171,6 +193,7 @@ private: std::string lastRppPath_; // .rpp path last seen for lastProject_ bool primed_ = false; // false until the first poll() observes state bool loadPending_ = false; // raised by loadFromProject; drained by consumeLoadSignal + bool reloadRequested_ = false; // raised by requestReload (projectconfig undo/redo); drained by poll // Load the book from the given project's ext state (the `banks` key, else the // legacy `bank_index` key migrated into the pool) and resolve bank paths against