From a07b28fd658fcdd08004b7f234c57096e30e170d Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 26 Jul 2026 06:59:26 -0400 Subject: [PATCH] feat(persist): reload book+view on undo/redo via projectconfig hook (R-B) BeginLoadProjectState(isUndo) requests a deferred reload the timer drains next tick, once REAPER has restored ext-state. Hook owns undo/redo; identity poll still owns open/tab-switch/save-as. Fixes copy-collapse over-count (verb-aware guard) and dangling undo point on unsaved-project bank ops. --- src/actions.cpp | 35 +++++++++++++++++++++-------- src/main.cpp | 57 +++++++++++++++++++++++++++++++++++++++++++++++ src/persist.cpp | 59 ++++++++++++++++++++++++++++++++++++++++++------- src/persist.h | 25 ++++++++++++++++++++- 4 files changed, 158 insertions(+), 18 deletions(-) diff --git a/src/actions.cpp b/src/actions.cpp index bf3bd07..f180011 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -453,7 +453,9 @@ 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. @@ -469,11 +471,22 @@ void persistBook() { g_session->saveToActiveProject(); } // // 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. +// 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); - persistBook(); - Undo_EndBlock2(nullptr, label, UNDO_STATE_MISCCFG); + 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. @@ -713,11 +726,15 @@ void doBankTransferSelected(bool copy) { case TransferResult::RejectedSameBank: break; } } - // No-op guardrail: if nothing actually changed the index (every selected sample was - // absent, or all were pre-checked rejects), don't open an undo point. `collapsed` - // counts a hash-collapse — that DID mutate the index (source entry removed on a - // move, or dest already held the hash), so it belongs inside the undo point. - if (ok > 0 || collapsed > 0) { + // 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()); 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