diff --git a/src/app/main.cpp b/src/app/main.cpp index a721162..65469e9 100644 --- a/src/app/main.cpp +++ b/src/app/main.cpp @@ -1,25 +1,16 @@ // main.cpp — the SINGLE translation unit that OWNS the REAPER API pointers. // -// This file is the entire contract between REAPER and the extension: -// * At startup REAPER scans UserPlugins/ for reaper_*.dll|dylib|so and -// dlopen()s each one, then looks up ONE exported symbol: ReaperPluginEntry -// (that name is produced by the REAPER_PLUGIN_ENTRYPOINT macro). -// * REAPER calls it, handing over `rec` — a small dispatch struct. -// - rec->GetFunc(name) resolves any REAPER API function to a pointer -// - rec->Register(what,ptr) plugs OUR callbacks into REAPER -// * REAPERAPI_LoadAPI(rec->GetFunc) walks reaper_plugin_functions.h and -// fills in every global function pointer (ShowConsoleMsg, InsertMedia...). +// REAPER dlopen()s reaper_*.dll|dylib|so from UserPlugins/ and calls the exported +// ReaperPluginEntry, handing over `rec` (rec->GetFunc resolves API pointers, +// rec->Register plugs our callbacks in). Exactly ONE .cpp defines +// REAPERAPI_IMPLEMENT (this one) — that allocates storage for the global API +// pointers every other TU gets `extern`. Never let a second TU define it. // -// Exactly ONE .cpp defines REAPERAPI_IMPLEMENT (this one) — that allocates -// storage for those global pointers. Every other .cpp includes -// reaper_plugin_functions.h WITHOUT the define and gets `extern` declarations. -// -// Since Q-W3 this TU is ONLY pointers + entry + dispatch; since Q-W6 its own -// action family registers through the DATA-DRIVEN TABLE below (kMainActionRows + -// action_registry's registerActionTable/actionTableHandleCommand/ -// unregisterActionTable) — adding a bindable action here means adding ONE row and -// its handler function, nothing else (OCP). The design_view / bank / ingest -// families keep their own register/handle/unregister triples, called from entry. +// This TU is ONLY pointers + entry + dispatch. Its own action family registers +// through the data-driven table below (buildMainActionTable + action_registry) — +// adding a bindable action means adding ONE row and its handler function (OCP). The +// design_view / bank / ingest families keep their own register/handle/unregister +// triples, called from entry. #define REAPERAPI_IMPLEMENT #include "reaper_plugin.h" @@ -32,9 +23,9 @@ #include "core/capture/render_settings.h" // captureActionTable #include "core/version/app_version.h" // appVersion #include "ingest.h" -#include "shell/actions/action_registry.h" // the Q-W6 registration table -#include "shell/actions/bank_actions.h" // multi-bank action family (B3; Q-W4 home) -#include "shell/actions/design_view_actions.h" // Design View action family (D4; Q-W4 home) +#include "shell/actions/action_registry.h" // the registration table +#include "shell/actions/bank_actions.h" // multi-bank action family +#include "shell/actions/design_view_actions.h" // Design View action family #include "shell/capture/capture_batch.h" // batch + recapture action bodies #include "shell/capture/capture_orchestrator.h" // single-capture / realtime / insert action bodies #include "shell/capture/realtime_lifecycle.h" // in-flight realtime state + tick driver @@ -46,20 +37,13 @@ namespace capture = reasampler::capture; // Globals other files reference via `extern`. -REAPER_PLUGIN_HINSTANCE g_hInst = nullptr; // this module's instance handle -reaper_plugin_info_t* g_rec = nullptr; // REAPER's dispatch struct +REAPER_PLUGIN_HINSTANCE g_hInst = nullptr; +reaper_plugin_info_t* g_rec = nullptr; -// Retired command-id SUFFIXES. Kept ONLY to mirror-unregister them on unload so a -// user's stale keybindings are cleaned up. Never re-register these. Composed through -// the channel prefix at unload (channelIdFor) so a beta unload clears beta-qualified -// retired ids and a stable unload clears stable's — each channel cleans up only its -// own family. -// * The M7 four-mode ids (tracks/items/razor WET). -// * CAPTURE_MASTER and CAPTURE_MASTER_REALTIME — the master offline scope and the -// master realtime action are REMOVED (capture is now item + track only; realtime -// taps the selected track). Their shipped ids are retired so old keybindings clear. -// * CAPTURE_ITEM_TAIL and CAPTURE_TRACK_TAIL — the former per-action tail variants -// are REMOVED; tail is now a panel-setting toggle, not a paired action. +// Retired command-id SUFFIXES: kept ONLY to mirror-unregister on unload so a user's +// stale keybindings are cleaned up. Never re-register these. The four-mode WET ids, +// the removed master scope/realtime actions, and the removed per-action tail variants +// (tail is now a panel toggle, not a paired action). static const char* const kRetiredCaptureCmdSuffixes[] = { "CAPTURE_TRACKS_WET", "CAPTURE_ITEMS_WET", @@ -70,35 +54,30 @@ static const char* const kRetiredCaptureCmdSuffixes[] = { "CAPTURE_TRACK_TAIL", }; -// The persistence session (M4): owns the in-memory BankModel and bridges it to -// project ext state. A timer tick drives g_session.poll() to detect project -// load / Save-As; capture adds Samples to g_session.bank() — which (B2) resolves to -// the ACTIVE bank's index inside the session's BankBook; after a capture we serialize -// the book back into the active project's ext state (the `banks` key) so it travels -// with the .rpp. Replaces the M3 session-only g_bank. +// Owns the in-memory BankModel and bridges it to project ext state. A timer tick +// drives g_session.poll() to detect project load / Save-As; capture adds Samples to +// g_session.bank() (resolves to the active bank's index), and we serialize the book +// back into the active project's ext state (the `banks` key) so it travels with the .rpp. static reasampler::ReaSamplerSession g_session; // Command id of the TOGGLE_BANK_PANEL row, resolved from the table once at load so // OnToggleAction's checked-state poll is a single int compare (no per-poll lookup). static int g_cmdToggleBankPanel = 0; -// --- Action handlers (the table's function pointers) -------------------------- -// -// Each is a thin stateless routing shim: (session, per-row arg) -> the action body -// hoisted in Q-W3/Q-W4 (shell/capture/, shell/panel/). The bodies own all behavior; -// these exist only so the table rows can be plain data with flat function pointers. +// Each handler is a thin stateless routing shim: (session, per-row arg) -> the +// action body in shell/capture/ or shell/panel/, existing only so table rows can be +// plain data with flat function pointers. -// Capture scope family: `arg` is the captureActionTable() row index — the table rows -// below are built by iterating that pure taxonomy, so the routing stays 1:1 by -// construction (never a hand-kept parallel list). +// `arg` is the captureActionTable() row index — the table rows below are built by +// iterating that pure taxonomy, so the routing stays 1:1 by construction. static void RunCaptureScopeRow(int arg) { capture::RunCapture(g_session, capture::captureActionTable()[static_cast(arg)]); } static void RunToggleBankPanel(int) { reasampler::bankPanelToggle(); } static void RunCaptureItemAssign(int) { capture::RunCaptureItemAssign(g_session); } -// Insert: `arg` != 0 is the EXPLICIT conform-to-project-tempo opt-in (CONTEXT.md -// §insert: conform is opt-in, never silent); 0 inserts at native length. +// `arg` != 0 is the EXPLICIT conform-to-project-tempo opt-in (never silent); 0 +// inserts at native length. static void RunInsertSelected(int arg) { capture::RunInsertSelected(g_session, arg != 0); } @@ -108,24 +87,15 @@ static void RunCaptureRealtime(int) { capture::RunCaptureRealtimeTrack(g_sessi static void RunCancelRealtime(int) { capture::RunCancelRealtime(g_session); } static void RunRecaptureFromSource(int) { capture::RunRecaptureFromSource(g_session); } static void RunShowVersion(int) { - // On-demand version readout — the ONLY version output on any path (Phase V: no - // unconditional startup print; routine console chatter pops the console window). + // On-demand only — no unconditional startup print (routine console chatter pops + // the console window). ShowConsoleMsg(("ReaSampler " + reasampler::version::appVersion() + "\n").c_str()); } -// --- The registration table (Q-W6) -------------------------------------------- -// -// ONE row per bindable action this TU owns: FOREVER-STABLE id suffix (channel prefix -// composed at register — stable rebuilds the exact shipped id, e.g. -// "CEREBELLUM_REASAMPLER_CAPTURE_TRACK"; beta its isolated forever-family), the -// Actions-list phrase (after the "ReaSampler[ beta]: " lead), the handler, and its -// per-row arg. Registration, hookcommand dispatch, and the unload mirror-unregister -// all iterate this data — adding an action = adding a row + a handler above. -// -// The capture scope rows (CAPTURE_ITEM / CAPTURE_TRACK) come first, sourced from the -// pure captureActionTable() taxonomy (render_settings) — suffix/phrase live in that -// one testable list, and `arg` carries the row index back to RunCapture. The -// remaining rows are this TU's singles, in the pre-table registration order. +// ONE row per bindable action this TU owns: FOREVER-STABLE id suffix, Actions-list +// phrase, handler, per-row arg. Registration, hookcommand dispatch, and the unload +// mirror-unregister all iterate this data. The capture scope rows come first, +// sourced from the pure captureActionTable() taxonomy; the rest are this TU's singles. static std::vector buildMainActionTable() { using reasampler::ActionTableRow; std::vector rows; @@ -135,40 +105,32 @@ static std::vector buildMainActionTable() { rows.push_back(ActionTableRow{cap[i].commandSuffix, cap[i].descriptionPhrase, &RunCaptureScopeRow, static_cast(i)}); - // M5: show/hide the docked bank panel (display-only; never captures/inserts). + // Show/hide the docked bank panel (display-only; never captures/inserts). rows.push_back({"TOGGLE_BANK_PANEL", "toggle bank panel", &RunToggleBankPanel}); - // S8: Item-scope capture + assignment-request write (capture family because it - // leans on the capture render machinery; the other ingest surfaces live in the - // ingest family and the panel drop callback). rows.push_back({"CAPTURE_ITEM_ASSIGN", "capture selected item into bank + assign to active instance", &RunCaptureItemAssign}); - // M6: place the panel's selected sample at the edit cursor. Two variants that - // differ ONLY in InsertOptions — native length vs the explicit conform opt-in. + // Two variants differing ONLY in InsertOptions — native length vs conform opt-in. rows.push_back({"INSERT_SELECTED", "insert selected sample at edit cursor", &RunInsertSelected, 0}); rows.push_back({"INSERT_SELECTED_CONFORM", "insert selected sample at edit cursor (conform to tempo)", &RunInsertSelected, 1}); - // M11: one action fires N captures (per selected item / per razor area); the - // original selection is restored on every exit path. Bank-only, never places. + // One action fires N captures (per selected item / per razor area); the original + // selection is restored on every exit path. Bank-only, never places. rows.push_back({"CAPTURE_BATCH_ITEMS", "batch capture selected items (one per item)", &RunBatchCaptureItems}); rows.push_back({"CAPTURE_BATCH_RAZOR", "batch capture razor areas (one per area)", &RunBatchCaptureRazor}); - // M8: realtime sibling of the offline CAPTURE_TRACK scope — records the selected - // track's own output into a hidden temp track, dialog-free — plus its - // cancel-in-flight companion (stop + restore, non-destructive). + // Realtime sibling of the offline CAPTURE_TRACK scope, plus its cancel-in-flight + // companion (stop + restore, non-destructive). rows.push_back({"CAPTURE_TRACK_REALTIME", "capture selected track (realtime)", &RunCaptureRealtime}); rows.push_back({"CANCEL_REALTIME_CAPTURE", "cancel realtime capture", &RunCancelRealtime}); - // M10: regenerate the selected PROVENANCED sample from its recorded source's - // current state, in place. Bank-only, never places on the timeline. rows.push_back({"RECAPTURE_FROM_SOURCE", "re-capture from source", &RunRecaptureFromSource}); - // Phase V: on-demand version readout for bug reports. rows.push_back({"SHOW_VERSION", "show version", &RunShowVersion}); return rows; @@ -180,74 +142,52 @@ static std::vector buildMainActionTable() { static void OnTimer() { // Advance any in-flight realtime capture FIRST, so a project switch is caught and - // the capture torn down/restored before session.poll() reacts to that switch. - // LOAD-BEARING (CONTEXT.md §Phase Q): the idle fast-path is a SINGLE POINTER - // TEST — the cross-TU drive call is made only when a capture is in flight. + // torn down/restored before session.poll() reacts to that switch. LOAD-BEARING: + // the idle fast-path is a SINGLE POINTER TEST — drive only when a capture is live. if (capture::g_rtCapture) capture::DriveRealtimeCapture(g_session); g_session.poll(); - // D4 reapply-on-open glue. persist stays MODEL-ONLY (it loads the saved view - // model but deliberately does NOT apply visibility — that would couple persist - // to the view shell). Instead poll() raises a one-shot load signal; here — the - // integration layer that already drives both persist and the view shell — we - // drain it and reapply the SAVED active mode's visibility/processing so opening a - // project saved in Design mode parks the Arrange tracks automatically, no manual - // toggle. Fires exactly once per load (consumeLoadSignal clears it); idle ticks - // skip it. proj = nullptr -> REAPER's active project (the one poll just loaded). - // - // The SAME signal re-arms the bank panel's new-content detector: a load must - // re-baseline the detector against the just-loaded project's content so its - // pre-existing tracks are never mis-detected as "new" and mass-tagged into the - // active mode (the reload-mis-tag bug). Notify BEFORE the reapply so the detector's - // re-arm and the model restore ride the one authoritative load event. + // persist stays MODEL-ONLY (loads the saved view model but does not apply + // visibility, to avoid coupling persist to the view shell); poll() raises a + // one-shot load signal that we drain here to reapply the SAVED active mode so a + // project saved in Design mode parks Arrange tracks automatically. The same + // signal re-arms the bank panel's new-content detector — notified BEFORE the + // reapply so re-arm and model restore ride the one load event (otherwise + // pre-existing tracks can be mis-detected as "new" and mass-tagged). if (g_session.consumeLoadSignal()) { reasampler::bankPanelNotifyProjectLoaded(); - // Reconcile the restored lane-ownership index against the live project's lanes - // FIRST (via REAPER's durable P_LANENAME — the cross-session source of truth), - // so a saved lane-split project's managed/manual classification is correct - // before the active mode's lane visibility is reapplied. Never re-mints, never - // mass-tags — it only records managed ownership recovered from lane names. + // Reconcile lane ownership against the live project's lanes (P_LANENAME, + // the cross-session source of truth) BEFORE reapplying visibility. Never + // re-mints, never mass-tags. reasampler::reconcileManagedLanes(g_session.view(), nullptr); reasampler::applyMode(g_session.view(), g_session.view().activeModeId(), nullptr); } - // Reflect a live bank change (capture / project load) in the docked grid. - // Cheap when the bank is unchanged (a fingerprint compare); repaints only on - // an actual change. No-op when the panel is closed. - reasampler::bankPanelRefresh(); + reasampler::bankPanelRefresh(); // cheap fingerprint compare; no-op when unchanged/closed } -// --- 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. +// A Ctrl-Z/Ctrl-Shift-Z rolls back/forward the "reasampler" project ext state on disk +// but keeps the SAME project identity, so the timer's identity poll never re-reads +// ext state on undo/redo — the in-memory book/view would stay stale until +// close+reopen. REAPER's projectconfig fires BeginLoadProjectState on every +// project-state (re)load INCLUDING undo/redo (isUndo == true for both); we hook it. // -// 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. +// TIMING: BeginLoadProjectState fires BEFORE any state restore, so reading +// GetProjExtState here would return the PRE-undo value. Instead we raise a one-shot +// reload request that OnTimer's poll() drains on the NEXT tick, once REAPER has +// finished restoring the block. A normal project open also fires this +// (isUndo=false); ignored here so a normal open flows solely through the timer's +// identity-transition Load path (no double load). 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. +// Intentional no-ops: ReaSampler stores state via project EXT STATE, not this +// extension's own project lines. The struct is registered ONLY for the +// BeginLoadProjectState undo/redo notification. static bool OnProcessExtensionLine(const char* /*line*/, ProjectStateContext* /*ctx*/, bool /*isUndo*/, project_config_extension_t* /*reg*/) { @@ -257,7 +197,6 @@ static bool OnProcessExtensionLine(const char* /*line*/, ProjectStateContext* /* 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. @@ -268,19 +207,15 @@ static project_config_extension_t g_projectConfig{ nullptr, // userData }; -// REAPER calls this for EVERY action fired anywhere; claim only our own id, -// return false otherwise so REAPER keeps looking. This TU's own family dispatches -// through the registration table; the Q-W4 families claim their own ids after it. +// REAPER calls this for EVERY action fired anywhere; claim only our own id, return +// false otherwise so REAPER keeps looking. This TU's own family dispatches through +// the registration table; the other families claim their own ids after it. static bool OnHookCommand(int command, int /*flag*/) { if (command == 0) return false; if (reasampler::actionTableHandleCommand(command)) return true; - // Design View action family (D4). Claims only its own ids; returns false for the - // rest so this hook keeps looking (per the contract). if (reasampler::designViewHandleCommand(command)) return true; - // Multi-bank action family (B3). Same contract: claims only its own ids. if (reasampler::bankHandleCommand(command)) return true; - // S8 ingest action family (Media-Explorer import). Same contract. if (reasampler::ingestHandleCommand(command)) return true; return false; } @@ -299,39 +234,30 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT( { if (!rec) { - // rec == nullptr => REAPER is UNLOADING us. Mirror-unregister every - // callback with the same strings prefixed '-' (per the contract). + // rec == nullptr => REAPER is UNLOADING us. if (g_rec) { // Abort any in-flight realtime capture FIRST, while the API pointers are - // still live — finalize-or-abort + restore so we never leave a temp track, - // an armed track, or an altered transport/cursor in the user's project on - // unload. Commit whatever was captured (best effort) before tearing down. + // still live, so we never leave a temp track, an armed track, or an + // altered transport/cursor in the user's project on unload. capture::AbortRealtimeCaptureForUnload(g_session); 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 - // gaccel + command_id with '-'-prefixed strings. After the hook is gone. reasampler::designViewUnregisterActions(g_rec); - // Tear down the multi-bank action family (B3) — same mirror-unregister. reasampler::bankUnregisterActions(g_rec); - // Tear down the S8 ingest action family — same mirror-unregister. reasampler::ingestUnregisterActions(g_rec); - // Tear down this TU's own family from the registration table (reverse - // table order; each '-command_id' re-presents the SAME interned, - // channel-qualified pointer used at register). + // This TU's own family, reverse table order; each '-command_id' + // re-presents the SAME interned pointer used at register. reasampler::unregisterActionTable(g_rec); // Retire the REMOVED command ids (command_id only — we never held a gaccel - // for them this session). Clears stale user keybindings on unload. Composed - // per channel so a beta clears beta-qualified retired ids, stable its own. + // for them this session). for (const char* suffix : kRetiredCaptureCmdSuffixes) g_rec->Register("-command_id", (void*)reasampler::channelIdFor(suffix)); } - // Destroy the docked window and release cached thumbnails before we drop - // the API pointers (DockWindowRemove/DestroyWindow need them live). + // Before dropping the API pointers: DockWindowRemove/DestroyWindow need them live. reasampler::bankPanelShutdown(); g_rec = nullptr; return 0; @@ -349,13 +275,10 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT( g_hInst = hInstance; g_rec = rec; - // Point the bank panel at the live session BEFORE registering its action, so - // a toggle firing immediately has a session to read (M5). Does not open the - // window — only stores the session pointer. + // Point the bank panel at the live session BEFORE registering its action, so a + // toggle firing immediately has a session to read. Does not open the window. reasampler::bankPanelInit(&g_session); - // Register this TU's whole action family from the table: command_id -> gaccel - // per row, all channel-qualified, all FOREVER-STABLE per channel. { const std::vector rows = buildMainActionTable(); reasampler::registerActionTable(rec, rows.data(), rows.size()); @@ -367,37 +290,23 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT( if (g_cmdToggleBankPanel) rec->Register("toggleaction", (void*)&OnToggleAction); - // Register the Design View action family (D4): toggle/activate mode, tag/untag/ - // show-both selected tracks. Each mints its own command_id + gaccel; the single - // hookcommand below routes them via designViewHandleCommand. Registered before - // the hook so every id is minted first. + // Each family mints its own command_id + gaccel, shares g_session, and is routed + // by the same hookcommand below. Registered before the hook so every id is + // minted first. reasampler::designViewRegisterActions(rec, &g_session); - - // Register the multi-bank action family (B3): create/rename/delete/evacuate bank, - // activate (cycle + pool), move/copy selected samples to a bank, and the two - // full-height layout toggles. Shares g_session with the Design View family; routed - // by the same hookcommand via bankHandleCommand. Registered before the hook. reasampler::bankRegisterActions(rec, &g_session); - - // Register the S8 ingest action family: the Media-Explorer import-into-bank+assign - // action. Shares g_session with the other families; routed by the same hookcommand via - // ingestHandleCommand. (The arrange capture+assign action is a table row above; the - // drop path is a bank_panel callback, not a bindable action.) reasampler::ingestRegisterActions(rec, &g_session); - // One hookcommand routes every ReaSampler action (table + the three families). - // Registered once, after all command ids are minted. rec->Register("hookcommand", (void*)&OnHookCommand); - // Drive project-load / Save-As detection (M4 persist). The timer polls the - // active project each tick; on a project load it reloads the bank from ext - // state, on a Save-As it relocates the bank folder under the new .rpp. + // Drives project-load / Save-As detection: the timer polls the active project + // each tick; on a project load it reloads the bank from ext 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). + // An UNDO/REDO state restore reloads the session's book + view from the restored + // ext state. The timer's identity poll cannot see an undo (same project + // identity), so this hook owns it (see OnBeginLoadProjectState). rec->Register("projectconfig", (void*)&g_projectConfig); return 1; // success — REAPER keeps us loaded diff --git a/src/ext_keys.h b/src/ext_keys.h index 6064c4a..08eaef5 100644 --- a/src/ext_keys.h +++ b/src/ext_keys.h @@ -1,36 +1,27 @@ #pragma once // ext_keys — the SINGLE SOURCE OF TRUTH for the "reasampler" project ext-state // namespace + key names, shared by the extension (writer, via shell/persist) and the -// VST3 instrument (reader, via the bridge). Both sides include this header so the -// wire contract cannot drift between the two artifacts (the S4 reviewer flagged the -// spike's duplicated constants as a drift risk). +// VST3 instrument (reader, via the bridge), so the wire contract cannot drift +// between the two artifacts. // -// PURE HEADER: NO REAPER types, NO VST3 types, NO SWELL, NO vendor/ includes. The key -// spellings are string constants; the NAMESPACE is channel-derived (Phase V, V4) so it -// delegates to the pure app_version module (also REAPER-free / VST3-free). Both the -// REAPER-facing persist shell and the SDK-facing VST bridge include this without pulling -// either SDK. +// PURE HEADER: NO REAPER/VST3/SWELL/vendor types. Key spellings are string +// constants; the namespace is channel-derived (delegates to app_version, also SDK-free). // // FOREVER-STABLE once shipped: these strings key every already-saved project's -// stored state. Changing any of them orphans that state. See shell/persist/ext_state_io.h for the -// per-key retirement / migration semantics — this header only owns the spellings. +// stored state. Changing any of them orphans that state. See +// shell/persist/ext_state_io.h for per-key retirement/migration semantics. #include "core/version/app_version.h" namespace reasampler { -// The ext-state namespace all ReaSampler project state is stored under. CHANNEL-DERIVED -// (Phase V, V4): delegates to the ONE app_version symbol so the extension (writer) and the -// VST3 instrument (reader) resolve the SAME namespace per channel — "reasampler" on stable, -// "reasampler_beta" on the isolated beta build. An accessor (not a constexpr literal) -// because the value is fixed by the channel bit at build time. This is the wire-contract -// reconciliation between S4 (shared ext_keys) and V4 (channel-isolated namespace): without -// it a beta instrument would read the stable namespace and see empty state. +// Channel-derived so the extension (writer) and the VST3 instrument (reader) +// resolve the SAME namespace per channel ("reasampler" / "reasampler_beta") — +// without this a beta instrument would read the stable namespace and see nothing. inline const char* kProjExtNamespace() { return version::extStateNamespace().c_str(); } -// The multi-bank key: the whole serialized BankBook (pool + named banks). This is -// the key the VST3 instrument reads to see the live bank (read-only, S4). ext_state_io -// documents its authority + the legacy-key migration around it. +// The whole serialized BankBook (pool + named banks) — read-only by the VST3 +// instrument. ext_state_io documents the legacy-key migration around it. inline constexpr const char* kProjExtBanksKey = "banks"; // The retired legacy single-bank key (read once on load to migrate into the pool). @@ -45,48 +36,34 @@ inline constexpr const char* kProjExtTailKey = "tail_setting"; // The per-project minted-GUID identity key. inline constexpr const char* kProjExtGuidKey = "project_guid"; -// The S9 BANK-GENERATION key. The EXTENSION stamps a monotonic decimal counter here that it -// bumps on every bank-content mutation that changes what a live instance would PLAY (capture -// add, re-capture-in-place, sample remove, move/copy affecting banks, ingest import). The VST3 -// instrument READS it off the audio thread on a UI-timer cadence and, when the value differs -// from what it last saw, calls reloadInstrument() so a recapture/ingest refreshes playing -// instances hands-free (the S9 change-detection trigger). WIRE-SHARED (instrument reads it); -// the instrument never WRITES it (the extension owns it, same read-only-over-bank rule as the -// assignment request). Additive to the persist blob — an absent stamp reads as generation 0 -// (a pre-S9 project), and the first bump (>= 1) then reads as a change. FOREVER-STABLE once -// shipped: changing this spelling resets every already-shipped instance's change-detection -// baseline (a one-time spurious reload), so it is fixed like every sibling key. +// The EXTENSION stamps a monotonic counter here, bumped on every bank-content +// mutation that changes what a live instance would PLAY. The VST3 instrument reads +// it on a UI-timer cadence and calls reloadInstrument() on a change; it never +// writes this key. Additive: an absent stamp reads as generation 0 (pre-existing +// projects). FOREVER-STABLE — changing the spelling resets every shipped instance's +// change-detection baseline (a one-time spurious reload). inline constexpr const char* kProjExtBankGenKey = "bank_generation"; -// The S8 ingest ASSIGNMENT-REQUEST key. The EXTENSION writes an assignment request here -// after an ingest-with-assign (arrange capture / Media-Explorer import / drop-onto-panel): -// "the active sampler instance should now play THIS sample." The value is the pure -// assignment_request wire format ("rsassign1" + bankId + sampleId + generation) — see -// assignment_request.h for the exact grammar. WIRE-SHARED because the VST3 instrument -// READS it (in a later dispatch, S8 instrument-side follow-up) to update its own selection -// and reload; the instrument never WRITES it (the extension writing its own namespace does -// not violate the instrument's read-only-over-the-bank rule). FOREVER-STABLE once shipped: -// changing this spelling strands any pending request an already-shipped instrument watches. +// The EXTENSION writes an assignment request here after an ingest-with-assign: +// "the active sampler instance should now play THIS sample." Value is the pure +// assignment_request wire format ("rsassign1" + bankId + sampleId + generation). +// The instrument reads it to update its own selection and reload; it never writes +// it. FOREVER-STABLE — changing the spelling strands any pending request an +// already-shipped instrument watches. inline constexpr const char* kProjExtAssignKey = "assign_request"; -// The pS-usage PER-INSTANCE USAGE-RECORD key prefix. The INSTRUMENT writes one key per -// instance — "rsusage_" — carrying the sample_usage wire record of every -// capture that instance holds; the EXTENSION enumerates the prefix at prune-scan time -// and folds live instances' holds into the prune's `referenced` set so a held capture -// can never be pruned. This is the ONE sanctioned instrument-side ext-state write -// (Daniel's ruling — the VST publishes its OWN usage; it never mutates banks/view/ -// tail/assign, and the bridge's write entry point structurally accepts only this -// prefix). WIRE-SHARED in the write->read direction the other keys reverse. The "rs" -// qualifier is deliberate: a future key that happens to start with "usage_" must never -// be swept into the FX-liveness fold (whose abort-on-unreadable rule would then halt -// every prune), so the prefix is namespaced like the wire magics (rsusage1/rsassign1). -// FOREVER-STABLE once shipped: changing the prefix strands every saved project's usage -// records (prune falls back to bank-references-only until instances republish — -// graceful, but the instance-hold protection lapses for stale-saved projects). +// The INSTRUMENT writes one key per instance — "rsusage_" — carrying +// the sample_usage wire record of every capture that instance holds; the EXTENSION +// enumerates the prefix at prune-scan time so a held capture can never be pruned. +// This is the ONE sanctioned instrument-side ext-state write (it never mutates +// banks/view/tail/assign; the bridge's write entry point structurally accepts only +// this prefix). The "rs" qualifier keeps a future "usage_*"-prefixed key from being +// swept into the FX-liveness fold. FOREVER-STABLE — changing the prefix strands +// every saved project's usage records (prune falls back to bank-references-only +// until instances republish). inline constexpr const char* kProjExtUsageKeyPrefix = "rsusage_"; -// The full per-instance usage key for a minted instance GUID (the one composition -// point, shared by the instrument's writer and the extension's enumerator). +// Shared by the instrument's writer and the extension's enumerator. inline std::string usageKeyFor(const std::string& instanceGuid) { return std::string(kProjExtUsageKeyPrefix) + instanceGuid; } diff --git a/src/ingest.cpp b/src/ingest.cpp index 43e288c..6837700 100644 --- a/src/ingest.cpp +++ b/src/ingest.cpp @@ -1,9 +1,6 @@ -// ingest.cpp — the S8 "ingest through the bank" shell (extension side). See ingest.h. -// -// Compiled into the reaper_reasampler MODULE. Includes reaper_plugin_functions.h WITHOUT -// REAPERAPI_IMPLEMENT — main.cpp owns the API pointers; here they are extern -// (CLAUDE.md §contract). REAPER-facing, DAW-verified; the pure serialization it drives -// (assignment_request) is CTest-tested. +// ingest.cpp — see ingest.h. main.cpp owns the API pointers; this TU gets them extern. +// REAPER-facing, DAW-verified; the pure serialization it drives (assignment_request) +// is CTest-tested. #include "ingest.h" @@ -21,7 +18,7 @@ #include "core/model/bank_model.h" // Sample, AddResult, findByHash #include "shell/panel/panel_input.h" // bankPanelRefresh #include "core/capture/capture_paths.h" // deriveBankPaths / projectDirOfRpp -#include "core/util/file_bytes.h" // shared whole-file loader (Q-W1, T2-03) +#include "core/util/file_bytes.h" // shared whole-file loader #include "core/wire/instrument_drop.h" // pure buildInstrumentDropPreset (.vstpreset image for a sampleId) #include "shell/actions/instrument_drop_win.h" // shell loadInstrumentOntoTrack (FX add+apply, no own undo block) #include "shell/persist/session.h" // ReaSamplerSession @@ -46,8 +43,6 @@ namespace reasampler { -// Real-namespace-home using-declarations (Q-W6: the interim core/namespaces.h shim -// is retired; each symbol names its Q-W1 home explicitly). using capture::BankPaths; using capture::buildFloat32Wav; using capture::deriveBankPaths; @@ -64,42 +59,31 @@ using wire::encodeAssignmentRequest; namespace { -// The live session the ingest paths mutate. Set once by ingestRegisterActions and read by -// every ingest body. Not owned here (main.cpp owns g_session). +// Not owned here (main.cpp owns g_session). ReaSamplerSession* g_session = nullptr; -// FOREVER-STABLE ingest action-id SUFFIX (Phase V, V4). The channel prefix is prepended at -// register via channelCommandId; NEVER change a shipped suffix. Only the Media-Explorer -// import registers here — the arrange capture+assign action lives in the capture family in -// main.cpp (it reuses the capture render machinery there), and the drop path is a panel -// callback (ingestDroppedFiles), not a bindable action. +// FOREVER-STABLE suffix — NEVER change after ship. Only the Media-Explorer import +// registers here — the arrange capture+assign action lives in the capture family in +// main.cpp, and the drop path is a panel callback (ingestDroppedFiles), not a +// bindable action. constexpr const char* kIdImportMediaExplorer = "INGEST_IMPORT_MEDIA_EXPLORER"; int g_cmdImportMediaExplorer = 0; gaccel_register_t g_accelImportMediaExplorer{}; -// Durable store of the composed, channel-qualified command-id + label strings. Two scalar -// std::string globals (one action); their c_str() pointers are handed to REAPER at register -// and re-presented at unregister, so these strings must not be mutated after registration. -// Populated once by ingestRegisterActions; stable for the extension lifetime. +// c_str() pointers are handed to REAPER at register and re-presented at unregister, +// so these strings must not be mutated after registration. std::string g_idImportStr; std::string g_labelImportStr; -// --- Project directory -------------------------------------------------------- - -// The current project's directory (parent of its .rpp), forward-slashed, no trailing -// slash — the M4 convention (projectDirOfRpp). Empty for an unsaved/no-active project, -// which makes the import refuse to place a file (no default-location fallback — the -// relative-paths invariant). Read-only. +// Forward-slashed, no trailing slash. Empty for an unsaved/no-active project, which +// makes the import refuse to place a file (relative-paths invariant, no fallback). std::string currentProjectDir() { std::vector buf(4096, '\0'); EnumProjects(-1, buf.data(), static_cast(buf.size())); return projectDirOfRpp(std::string(buf.data())); } -// Whole-file reads (source read + bank-copy validate/hash) go through the shared -// core/util readFileBytes (Q-W1, T2-03): empty on any failure (missing / unreadable). - // Writes a byte buffer to a file. Returns true on success. The caller is responsible for // ensuring the directory exists before calling. bool writeFileBytes(const std::string& path, const std::vector& bytes) { @@ -110,17 +94,13 @@ bool writeFileBytes(const std::string& path, const std::vector& by return f.good(); } -// The 32f WAV build itself lives in the pure wav_codec module (Q-W3, audit §4e / -// T4-10 — one owner of the RIFF layout, CTest-covered): buildFloat32Wav takes the -// interleaved ReaSample (double) frames decoded below and yields the canonical -// bank-format bytes (capture.cpp kRenderFormatWavFloat32; wav_codec.h FORMAT -// ASSUMPTION — the double→float narrowing is the intentional bank contract). +// buildFloat32Wav (wav_codec) takes the interleaved ReaSample (double) frames +// decoded below and yields the canonical bank-format bytes — the double->float +// narrowing is the intentional bank contract. -// Decodes ALL samples from `src` into interleaved double-precision frames. -// Returns empty on a zero-length or silent source (sampleRate < 1, channelCount == 0). -// Uses GetSamples in blocks; advances time_s monotonically. The caller has already -// queried channelCount and sampleRate from the same source; those values are passed in -// to avoid re-querying after GetSamples mutates decoder state. +// Returns empty on a zero-length or silent source. The caller has already queried +// channelCount/sampleRate from the same source (passed in to avoid re-querying +// after GetSamples mutates decoder state). std::vector decodePcmSource(PCM_source* src, int nch, double sampleRate, double lengthSeconds) { if (!src || nch <= 0 || sampleRate < 1.0 || lengthSeconds <= 0.0) return {}; @@ -132,7 +112,6 @@ std::vector decodePcmSource(PCM_source* src, int nch, double sampleRa std::vector out; out.reserve(totalFrames * static_cast(nch)); - // Pull samples in blocks of ~4096 frames; loop until source is exhausted. constexpr int kBlockFrames = 4096; std::vector block(static_cast(kBlockFrames * nch)); @@ -156,41 +135,24 @@ std::vector decodePcmSource(PCM_source* src, int nch, double sampleRa return out; } -// The result of an import-into-bank: the sample id to assign (the existing id on a -// hash-dedup collapse, the new id otherwise) and whether anything was added to the index -// (so the caller opens an undo point only for a real mutation). struct ImportResult { std::string sampleId; // "" on failure (nothing to assign) bool added = false; // true iff a NEW index entry was created (not a collapse) std::string message; // human-readable outcome for the console }; -// Imports one OS-native source file into the ACTIVE bank: convert-if-needed to 32-bit- -// float WAV, write to the project-relative bank folder, index-add, hash-dedup applied. +// Imports one OS-native source file into the ACTIVE bank: convert-if-needed to +// 32-bit-float WAV (the bank contract — a verbatim copy of anything else would be +// unplayable), write to the project-relative bank folder, index-add, hash-dedup. // -// BANK CONTRACT: the instrument (wav_codec parse) expects every bank file to be a canonical -// 32-bit-float WAV (WAVE_FORMAT_IEEE_FLOAT, 32 bits). A verbatim copy of a non-WAV (or -// an integer-PCM or double-float WAV) would be unplayable. This function therefore: -// 1. Checks whether the source IS already a valid 32f WAV (parseWavLayout fast path). -// 2. If yes: copies it verbatim — one I/O, content unchanged. -// 3. If no: decodes via PCM_source::GetSamples and writes a fresh 32f WAV, preserving -// the source's channel count and sample rate. +// DEDUP ORDERING: the content hash is taken from the CONVERTED bytes AFTER building +// the buffer but BEFORE writing to disk, so a re-import of the same source (or of a +// WAV matching a captured file's content) collapses without a redundant disk write. +// Hashing the raw source bytes instead would miss this for non-WAV sources, since +// their bytes differ from the converted WAV bytes. // -// DEDUP ORDERING: the content hash is taken from the CONVERTED (bank-format) bytes AFTER -// building the file buffer but BEFORE writing to disk. This means: -// * Re-importing the same source file yields the same converted bytes → same hash → -// dedup fires → no redundant disk write (matching the DEDUP-BEFORE-DISK design). -// * An imported WAV whose audio-content hash matches a captured WAV also deduplicates -// correctly (hashWavContent is chunk-aware for both). -// * The pre-conversion hash shortcut (hash the raw source bytes) is not used: a non-WAV -// source's bytes would produce a different hash from the converted WAV bytes, so two -// imports of the same mp3 would NOT dedup — which is wrong. Hashing post-conversion -// is correct. -// -// NON-DESTRUCTIVE: the source file is never modified or moved — only read. -// Records the written file in the owned-file manifest (Phase B B-cap) so Phase R prune can -// attribute it. Does NOT persist or open an undo point — the caller batches that (a -// multi-file drop is one undo point, one persist). +// NON-DESTRUCTIVE: the source file is only read. Does NOT persist or open an undo +// point — the caller batches that (a multi-file drop is one undo point, one persist). ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { ImportResult out; @@ -212,17 +174,14 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { return out; } - // Read source bytes; needed to check whether it is already a 32f WAV. const std::vector srcBytes = readFileBytes(absoluteSourcePath); if (srcBytes.empty()) { out.message = "file is empty or unreadable: " + absoluteSourcePath; return out; } - // Probe the source's audio geometry via PCM_source. Needed for conversion AND for - // populating the Sample's metadata. A file REAPER cannot open leaves geometry at - // zero — the sample still imports if the WAV-fast-path succeeds; the geometry - // is simply unknown, the honest default. + // A file REAPER cannot open leaves geometry at zero — the sample still imports + // if the WAV-fast-path succeeds; the geometry is simply unknown, the honest default. int channelCount = 0; int sampleRate = 0; double lengthSeconds = 0.0; @@ -235,22 +194,16 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { if (isQN) lengthSeconds = 0.0; // QN-length source has no seconds length to store } - // Determine whether a verbatim copy suffices (fast path) or a conversion is needed. - // parseWavLayout validates that the source is a canonical 32-bit-float RIFF/WAVE; any - // other format (mp3, aiff, integer PCM, 16-bit WAV, etc.) takes the decode+rewrite path. + // parseWavLayout validates a canonical 32-bit-float RIFF/WAVE; any other format + // (mp3, aiff, integer PCM, 16-bit WAV, etc.) takes the decode+rewrite path. const WavLayout layout = parseWavLayout(srcBytes); const bool isFloat32Wav = layout.valid; - // Build the bank-format bytes in memory (the "converted" bytes), which we hash for dedup - // BEFORE writing to disk so a re-import of the same source skips the disk write. std::vector bankBytes; if (isFloat32Wav) { - // Fast path: already canonical — bank bytes ARE the source bytes. bankBytes = srcBytes; if (srcHandle) PCM_Source_Destroy(srcHandle); } else { - // Conversion path: decode all samples then write a fresh 32f WAV. - // PCM_source is opened on the source path (not a copy); we already have srcHandle. std::vector decoded; if (srcHandle && channelCount > 0 && sampleRate > 0 && lengthSeconds > 0.0) { decoded = decodePcmSource(srcHandle, channelCount, @@ -259,10 +212,8 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { if (srcHandle) PCM_Source_Destroy(srcHandle); if (decoded.empty()) { - // No decodable audio. The source is on disk (valid path, REAPER could open it) - // but yielded no samples — e.g. a MIDI file, a zero-length audio file, or a - // format REAPER does not support. Fail loudly: we must not write a silent WAV - // and pretend the import succeeded. + // e.g. a MIDI file, zero-length audio, or an unsupported format. Fail + // loudly rather than write a silent WAV and pretend the import succeeded. out.message = "could not decode audio samples from: " + fs::path(absoluteSourcePath).filename().string() + " (unsupported format or no audio data)"; @@ -275,20 +226,17 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { static_cast(sampleRate), frameCount, decoded); } - // srcHandle is destroyed above in both branches. - // Hash the converted (bank-format) bytes for dedup. WAV-aware hash (hashWavContent) - // so a re-import of the same source deduplicates against a previously-captured or - // previously-imported sample with identical audio content, even if non-audio RIFF - // chunks differ. Empty hash (unhashable) is treated as "not dedupable" (safe direction: - // copies + adds rather than silently collapsing onto an unrelated entry). + // WAV-aware hash so a re-import deduplicates against a previously-captured or + // previously-imported sample with identical audio content, even if non-audio + // RIFF chunks differ. Empty (unhashable) is "not dedupable" — copies + adds + // rather than silently collapsing onto an unrelated entry. const std::string contentHash = hashWavContent(bankBytes); BankBook& book = g_session->book(); - // Dedup-before-disk: if the active bank already holds this audio content, assign the - // existing sample's id and skip the disk write (no redundant on-disk duplicate). - // Empty hashes never match (findByHash treats "" as non-participating). + // Dedup-before-disk: skip the write entirely if the active bank already holds + // this content. if (!contentHash.empty()) { if (const Sample* existing = book.activeIndex().findByHash(contentHash)) { out.sampleId = existing->id; @@ -298,14 +246,12 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { } } - // Derive the destination path. The stem comes from the source file name; a timestamp - // uniqueTag avoids collision with a prior import of a same-named file. + // A timestamp uniqueTag avoids collision with a prior import of a same-named file. const std::string sourceStem = fs::path(absoluteSourcePath).stem().string(); const std::int64_t nowSec = static_cast(std::time(nullptr)); const std::string uniqueTag = std::to_string(nowSec); const BankPaths paths = deriveBankPaths(projectDir, sourceStem, uniqueTag); - // Ensure the bank folder exists, then write the (converted) bank bytes. fs::create_directories(paths.absoluteDir, ec); // idempotent; ec ignored (write reports) const std::string destPath = paths.absoluteDir + "/" + paths.fileName; if (!writeFileBytes(destPath, bankBytes)) { @@ -313,10 +259,8 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { return out; } - // Build the Sample. Import is NOT a capture — sourceMode/range/tail do not apply; we - // record what we know (path, hash, geometry, name) and leave capture-only fields at - // their defaults. rootNote/loop stay empty: an imported file is not a single played - // note, so we do not guess a root note. + // Import is NOT a capture — capture-only fields stay at defaults. rootNote/loop + // stay empty: an imported file is not a single played note, so we do not guess. Sample s; s.id = "imp-" + uniqueTag + "-" + paths.fileName; s.displayName = sourceStem.empty() ? std::string("import") : sourceStem; @@ -329,10 +273,8 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { s.createdTimestamp = nowSec; const AddResult r = book.activeIndex().add(s); - // Record the written file as owned regardless of the add outcome — the tool WROTE it, so - // Phase R prune must attribute it. (A Collapsed result here would mean another sample in - // the active bank matched the hash after we passed the pre-write dedup check — a narrow - // race window. Record + handle both honestly.) + // Record as owned regardless of outcome — the tool WROTE the file, so prune must + // attribute it even in the narrow Collapsed race below. g_session->owned().add(paths.relativePath); switch (r) { @@ -343,8 +285,7 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { paths.relativePath; break; case AddResult::Collapsed: { - // The hash matched an existing entry (a race against our pre-write dedup check, - // or an empty-hash edge). Assign the existing entry's id. + // A race against the pre-write dedup check (or an empty-hash edge). const Sample* existing = contentHash.empty() ? nullptr : book.activeIndex().findByHash(contentHash); out.sampleId = existing ? existing->id : std::string{}; @@ -354,40 +295,30 @@ ImportResult importFileIntoActiveBank(const std::string& absoluteSourcePath) { } case AddResult::RejectedAbsolutePath: case AddResult::RejectedEmptyId: - // deriveBankPaths always yields a relative path and a non-empty id above, so - // these are unreachable in practice — reported honestly rather than silently. + // Unreachable in practice (deriveBankPaths always yields a relative path + // and non-empty id) — reported honestly rather than silently. out.message = "index rejected the import (internal path/id error)"; break; } return out; } -// --- Media-Explorer import action -------------------------------------------- - -// Import the Media Explorer's current last-played/selected file into the active bank, then -// add a ReaSampler 9000 instrument to the FIRST SELECTED TRACK pre-loaded with that sound. -// No new track is created; no routing changes are made — "new sound, existing track." -// No assignment_request is written on this path. +// Imports the Media Explorer's last-played/selected file into the active bank, then +// adds a ReaSampler 9000 instrument to the FIRST SELECTED TRACK pre-loaded with that +// sound — no new track, no routing changes. No assignment_request write. // -// Single-file, pull-on-action: MediaExplorerGetLastPlayedFileInfo returns the ONE last-played -// file (the whole ME contract — no enumerate-selected API). The selection RANGE it reports is -// deliberately IGNORED here: an import brings the whole file into the bank (the range is a -// preview hint, and the fields are [0,1] fractions, not seconds — see the DAW-verify note); -// a user wanting a sub-range captures it via the arrange path instead. +// Single-file, pull-on-action (MediaExplorerGetLastPlayedFileInfo — no +// enumerate-selected API). The selection RANGE it reports is deliberately IGNORED: +// an import brings the whole file in ([0,1] fraction fields are a preview hint, not +// seconds); a sub-range user captures via the arrange path instead. // -// LOAD-BEARING (CLAUDE.md): this adds ONE FX instance to the user's existing selected track. -// It NEVER inserts a timeline item and NEVER creates a track. Persist ordering is critical — -// the fresh instance's setState -> reloadInstrument reads the bank from project ext-state, so -// the sample MUST be persisted (generation bumped when something new landed) BEFORE +// LOAD-BEARING: NEVER inserts a timeline item, NEVER creates a track. Persist +// ordering is critical — the fresh instance's setState -> reloadInstrument reads the +// bank from project ext-state, so the sample MUST be persisted BEFORE // loadInstrumentOntoTrack adds the FX, or the instance cannot resolve the sampleId. -// Undo-wrapped: persist + FX-add + inject = one Ctrl-Z. -// -// No selected track: the bank import still proceeds (sound is now in the bank), but no -// instrument is placed and a clear console message explains why. void doImportFromMediaExplorer() { - // filemode/sel/pitch/vol/rate/bpm/extrainfo are read but only the filename is used for - // the import. selstart/selend are [0,1] fractions (SDK header) — a preview hint, not a - // bank-relevant range; left unused. extrainfo is documented "currently unused". + // Only the filename is used; selstart/selend are [0,1] fractions (a preview + // hint, not a bank-relevant range), extrainfo is documented "currently unused". std::vector nameBuf(4096, '\0'); int filemode = 0; double selStart = 0.0, selEnd = 0.0; @@ -406,21 +337,17 @@ void doImportFromMediaExplorer() { const ImportResult r = importFileIntoActiveBank(path); if (r.sampleId.empty()) { - // Import refused (unsaved project / undecodable / write failure). Report and stop — - // no instrument is placed. ShowConsoleMsg(("ReaSampler ingest: Media Explorer import failed -- " + r.message + ".\n").c_str()); return; } - // Resolve the first selected track. GetSelectedTrack(nullptr, 0): proj=nullptr=active - // project, seltrackidx=0=first selected (ignores master). Returns null when nothing is - // selected — directive: existing track only, never alter the graph. + // GetSelectedTrack ignores the master; null means nothing selected — existing + // track only, never alter the graph. MediaTrack* target = GetSelectedTrack(nullptr, 0); if (!target) { - // Sound landed in the bank; no instrument placed because there is no selected track. - // The bank import is kept (sound is available in the bank browser) and generation is - // bumped so any open VST3 browser instances refresh to show the new sound. + // Bank import is kept (sound is in the bank browser); generation is bumped + // so any open VST3 browser instances refresh to show the new sound. if (r.added) { Undo_BeginBlock2(nullptr); g_session->bumpBankGeneration(); @@ -439,24 +366,15 @@ void doImportFromMediaExplorer() { return; } - // Build the pre-loaded instrument payload (a .vstpreset image) for the resolved sampleId. - // Valid for BOTH the fresh import and the dedup case (added == false but a real sampleId) - // — the user asked for a player, and a valid sampleId is sufficient to pre-select the sound. + // Valid for BOTH the fresh import and the dedup case (added == false but a real + // sampleId is sufficient to pre-select the sound). const std::vector preset = buildInstrumentDropPreset(r.sampleId); - // One undo point for the whole gesture. Persist happens INSIDE the block and BEFORE the - // FX add so the new instance's setState -> reloadInstrument sees the just-persisted sample. - // The generation is bumped only when something NEW landed (a dedup collapse mutated nothing, - // so it needs neither a bump nor a persist to resolve — the sample is already in ext-state). - // If saveToActiveProject() no-ops (unsaved project), close with an empty label + zero flag so - // REAPER discards the undo entry (the house pattern from persistBankOp). importFileIntoActiveBank - // already refuses on an unsaved project, so in practice the persist here succeeds. + // Persist happens INSIDE the block and BEFORE the FX add so the new instance's + // setState -> reloadInstrument sees the just-persisted sample. Undo_BeginBlock2(nullptr); - bool persisted = true; // true when nothing needed persisting (dedup) — governs the label path + bool persisted = true; // true when nothing needed persisting (dedup) if (r.added) { - // S9: a new sample landed in the active bank -> bump inside the block so the stamped - // generation is what the fresh instance (and any other live instances) resolve against, - // and undo rolls the generation back with the banks key. g_session->bumpBankGeneration(); persisted = g_session->saveToActiveProject(); } @@ -465,9 +383,8 @@ void doImportFromMediaExplorer() { Undo_EndBlock2(nullptr, "ReaSampler: import from Media Explorer into selected track", UNDO_STATE_MISCCFG); else - // Either the FX add/inject failed (loadInstrumentOntoTrack already rolled the FX back — - // no orphan) or the project was unsaved (persist no-op): discard the undo entry so no - // empty point is recorded. + // Either the FX add/inject failed (already rolled back, no orphan) or the + // project was unsaved (persist no-op): discard so no empty point is recorded. Undo_EndBlock2(nullptr, "", 0); bankPanelRefresh(); @@ -481,30 +398,26 @@ void doImportFromMediaExplorer() { } // namespace -// --- Assignment-request write ------------------------------------------------ - void ingestAssignActiveInstance(const std::string& bankId, const std::string& sampleId) { if (!g_session || sampleId.empty()) return; // nothing to assign AssignmentRequest req; req.bankId = bankId; req.sampleId = sampleId; - // Monotonic disambiguator: a wall-clock unix-epoch stamp so the reader tells a fresh - // assign (even re-assigning the SAME id) from a stale value. NOT the S9 bank-generation - // counter (a separate point) — this field is self-contained to the request. + // Monotonic wall-clock stamp so the reader tells a fresh assign (even + // re-assigning the SAME id) from a stale value — self-contained to the request, + // not the bank-generation counter. req.generation = static_cast(std::time(nullptr)); g_session->writeAssignmentRequest(encodeAssignmentRequest(req)); } -// --- Drop-onto-panel ingest -------------------------------------------------- - +// Bank-fill only; no assignment_request is written (the drop has no effect on what +// any live instance plays). Batch the persist + undo point: many imports are ONE +// undo entry. void ingestDroppedFiles(const std::vector& absolutePaths) { if (!g_session || absolutePaths.empty()) return; - // Import ALL dropped files into the active bank — bank-fill only. No assignment_request - // is written on this path; the drop has no effect on what any live instance plays. - // Batch the persist + undo point: many imports are ONE undo entry. int importedNew = 0; int importedTotal = 0; std::string lastFailure; @@ -519,14 +432,9 @@ void ingestDroppedFiles(const std::vector& absolutePaths) { if (r.added) ++importedNew; } - // One undo point for the whole drop, opened only if a NEW index entry was created (a - // drop that only re-hit existing content mutated nothing on the index). - // If saveToActiveProject() no-ops (unsaved project), we close with an empty label + zero - // flag so REAPER discards the undo entry (house pattern from persistBankOp). + // One undo point for the whole drop, opened only if a NEW index entry was created. if (importedNew > 0) { Undo_BeginBlock2(nullptr); - // S9: one coalesced generation bump for the whole drop (>=1 new sample landed) so - // open VST3 browser instances refresh to show the newly available sounds. g_session->bumpBankGeneration(); const bool persisted = g_session->saveToActiveProject(); if (persisted) @@ -539,7 +447,6 @@ void ingestDroppedFiles(const std::vector& absolutePaths) { (importedTotal == 1 ? " file" : " files") + " into the bank.\n"; ShowConsoleMsg(msg.c_str()); } else if (importedTotal > 0) { - // All dropped files were already in the bank (deduplicated); nothing changed. bankPanelRefresh(); ShowConsoleMsg("ReaSampler ingest: all dropped files already in the bank.\n"); } else { @@ -549,10 +456,8 @@ void ingestDroppedFiles(const std::vector& absolutePaths) { } } -// --- Action registration ------------------------------------------------------ - void ingestRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session) { - g_session = session; // shared with the capture / bank / Design-View families + g_session = session; g_idImportStr = channelCommandId(kIdImportMediaExplorer); g_cmdImportMediaExplorer = rec->Register("command_id", (void*)g_idImportStr.c_str()); @@ -571,8 +476,7 @@ bool ingestHandleCommand(int command) { } void ingestUnregisterActions(reaper_plugin_info_t* rec) { - // Mirror-unregister with '-'-prefixed strings; the '-command_id' re-presents the SAME - // interned channel-qualified id used at register (g_idImportStr). + // '-command_id' re-presents the SAME interned id used at register (g_idImportStr). rec->Register("-gaccel", (void*)&g_accelImportMediaExplorer); rec->Register("-command_id", (void*)g_idImportStr.c_str()); g_session = nullptr; diff --git a/src/ingest.h b/src/ingest.h index a6b2203..3c4fa4b 100644 --- a/src/ingest.h +++ b/src/ingest.h @@ -1,77 +1,50 @@ #pragma once -// ingest — the S8 "ingest through the bank" shell (EXTENSION side). -// -// Compiled into the reaper_reasampler MODULE. REAPER-facing (PCM_Source metadata reads, -// Media-Explorer query, ext-state assignment write, action registration), so it is -// DAW-verified, not unit-tested; the pure serialization it drives lives in -// assignment_request (tested in CTest). -// -// -- The one gesture (CONTEXT.md §Ingest through the bank) -------------------- +// ingest — the "ingest through the bank" shell (EXTENSION side). REAPER-facing +// (PCM_Source metadata reads, Media-Explorer query, ext-state assignment write, +// action registration), so DAW-verified, not unit-tested; the pure serialization it +// drives lives in assignment_request (CTest). // // Loading a sample into the sampler is ONE gesture: capture/import-into-bank AND -// auto-assign to the active sampler instance. The EXTENSION owns ingest (it has arrange -// access, Media-Explorer access, and the drop-target surface on its own panels); the -// instrument stays a READ-ONLY bank consumer. Three ingest surfaces: +// auto-assign to the active sampler instance. The EXTENSION owns ingest (arrange +// access, Media-Explorer access, drop-target surface); the instrument stays a +// READ-ONLY bank consumer. Three surfaces: (1) arrange capture -> bank -> assign, +// (2) Media-Explorer import -> bank -> assign (single-file, pull-on-action), (3) +// drop-onto-panel -> bank -> assign (multi-file: import all, assign the first). // -// 1. Arrange capture -> bank -> assign (a bindable action; reuses the capture path). -// 2. Media-Explorer import -> bank -> assign (a bindable action; single-file, pull-on- -// action via MediaExplorerGetLastPlayedFileInfo). -// 3. Drop-onto-panel -> bank -> assign (an OS file drop on the docked bank_panel HWND; -// multi-file: import all, assign the first). +// LOAD-BEARING: ingest NEVER inserts a timeline item — capture writes a file + index +// entry, import copies a file + adds an index entry, assignment is a bank-index + +// instance-selection act, not a placement. Any InsertMedia call here is a bug. // -// -- The load-bearing principle (restated) ----------------------------------- -// -// Ingest NEVER inserts a timeline item. Capture writes a file + an index entry; import -// copies a file + adds an index entry; assignment is a bank-index + instance-selection -// act, not a placement. Any path here that calls InsertMedia would be a bug. -// -// -- Import semantics --------------------------------------------------------- -// -// A Media-Explorer/drop import is a FILE COPY into the project-relative bank folder + -// an index add, mirroring how a capture lands (relative-paths-only, hash-dedup). If the -// active bank already holds the imported content (by content hash), the import collapses -// onto the existing sample and assigns THAT sample's id — no redundant on-disk copy. +// Import is a FILE COPY into the project-relative bank folder + an index add +// (relative-paths-only, hash-dedup). If the active bank already holds the content +// (by hash), the import collapses onto the existing sample instead of duplicating. #include #include -// Forward declarations keep this header REAPER-free at its own boundary (the .cpp pulls -// the SDK). reaper_plugin_info_t is REAPER's dispatch struct; ReaSamplerSession owns the -// book + persist bridge the ingest paths mutate. +// Forward declarations keep this header REAPER-free (the .cpp pulls the SDK). struct reaper_plugin_info_t; namespace reasampler { class ReaSamplerSession; -// Registers the S8 ingest action family (command_id/gaccel per the house contract), -// mirror of bankRegisterActions. `session` is the live session the ingest paths mutate -// (shared with the capture / bank / Design-View families). The single hookcommand in -// main.cpp routes fired ids here via ingestHandleCommand. +// `session` is shared with the capture / bank / Design-View families; the single +// hookcommand in main.cpp routes fired ids here via ingestHandleCommand. void ingestRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session); -// Routes a fired command id to its ingest action. Returns true iff it was one of ours -// (claim-only, per the hookcommand contract); false otherwise so the hook keeps looking. bool ingestHandleCommand(int command); -// Mirror-unregisters the ingest action family on unload (the '-'-prefixed strings). void ingestUnregisterActions(reaper_plugin_info_t* rec); -// Write the S8 assignment request for a just-ingested sample: "the active sampler -// instance should now play (bankId, sampleId)." Encodes the pure assignment_request value -// (with a fresh monotonic generation stamp) and routes it to ext state via the session. -// Called by EVERY ingest surface after the sample lands in the bank — the arrange -// capture+assign action (main.cpp, alongside the capture machinery it reuses), the ME -// import action, and the drop path. A no-op-safe write: if there is no saved/active -// project the request is silently dropped (nothing to signal into), matching the -// book/manifest quiet-persist idiom. `sampleId` empty -> no write (nothing to assign). +// "The active sampler instance should now play (bankId, sampleId)." Called by EVERY +// ingest surface after the sample lands in the bank. No-op-safe: an unsaved/no-active +// project silently drops the write; `sampleId` empty -> no write. void ingestAssignActiveInstance(const std::string& bankId, const std::string& sampleId); -// Ingest OS-dropped files onto a ReaSampler surface (S8 drop path). Called by the -// bank_panel's WM_DROPFILES handler with the dropped file paths (absolute, OS-native). -// Imports EVERY file into the active bank (copy + index add, hash-dedup) and assigns the -// FIRST successfully-imported sample to the active instance. A no-op on an empty list or -// an unsaved/no-active project (nothing to import into). Reports outcomes to the console. +// Called by the bank_panel's WM_DROPFILES handler. Imports EVERY file into the +// active bank (hash-dedup) and assigns the FIRST successfully-imported sample to the +// active instance. No-op on an empty list or an unsaved/no-active project. void ingestDroppedFiles(const std::vector& absolutePaths); } // namespace reasampler diff --git a/src/resource.h b/src/resource.h index 1064b9a..63615cb 100644 --- a/src/resource.h +++ b/src/resource.h @@ -1,10 +1,8 @@ #pragma once -// resource.h — dialog/control ids for ReaSampler's SWELL dialogs. -// -// Shared by resource.rc (Windows resource compiler) and, on macOS/Linux, by the -// SWELL resgen-generated source (see CLAUDE.md §SWELL dialog resources). Keep the -// numeric ids stable and unique across the extension. +// resource.h — dialog/control ids for ReaSampler's SWELL dialogs. Shared by +// resource.rc and, on macOS/Linux, the SWELL resgen-generated source. Keep ids +// stable and unique across the extension. -// The docked bank panel (M5). A bare owner-drawn child dialog: it carries no -// controls — the panel shell (shell/panel/panel_render.cpp) paints the whole client area with LICE. +// A bare owner-drawn child dialog with no controls — panel_render.cpp paints the +// whole client area with LICE. #define IDD_BANK_PANEL 1000 diff --git a/src/shell/actions/action_registry.cpp b/src/shell/actions/action_registry.cpp index 00ece1a..06eff0f 100644 --- a/src/shell/actions/action_registry.cpp +++ b/src/shell/actions/action_registry.cpp @@ -1,6 +1,5 @@ -// action_registry.cpp — shared registration plumbing (Q-W4) + the registration -// table (Q-W6). See action_registry.h. Needs no REAPER API pointers: rec->Register -// is a member call on the dispatch struct REAPER hands the entry point. +// action_registry.cpp — see action_registry.h. Needs no REAPER API pointers: +// rec->Register is a member call on the dispatch struct REAPER hands the entry point. #include "shell/actions/action_registry.h" @@ -17,16 +16,12 @@ namespace { using version::channelActionName; using version::channelCommandId; -// Durable store of composed, channel-qualified strings (ids + labels). A std::deque -// never invalidates references on push_back, so a c_str() handed to REAPER (a -// command_id at register, a gaccel desc for its lifetime) stays valid until process -// exit. Memoized by suffix so register and the mirror-unregister get the SAME id -// pointer for a given action. +// std::deque never invalidates references on push_back, so a c_str() handed to +// REAPER stays valid until process exit. Memoized by suffix so register and +// mirror-unregister get the SAME id pointer. std::deque g_strStore; -// One registered table row: the row data plus the registry-owned registration -// artifacts (interned id, minted cmd, gaccel storage REAPER holds a pointer to). -// A std::deque so element addresses never move after push_back — REAPER keeps each +// std::deque so element addresses never move after push_back — REAPER keeps each // &accel until the mirror-unregister. struct TableEntry { ActionTableRow row; diff --git a/src/shell/actions/action_registry.h b/src/shell/actions/action_registry.h index 9f45fbe..e92c7d4 100644 --- a/src/shell/actions/action_registry.h +++ b/src/shell/actions/action_registry.h @@ -1,28 +1,10 @@ #pragma once -// action_registry — shared registration plumbing + the Q-W6 registration TABLE. -// -// Two layers, one TU: -// -// * The Q-W4 plumbing (channelIdFor / registerAction): the durable interned-string -// store the action families register through, so a composed command id keeps ONE -// stable pointer from register to the mirror-unregister, and the -// register-a-command_id-then-gaccel sequence has one implementation. The -// design_view / bank / ingest families still register row-by-row through this. -// -// * The Q-W6 registration TABLE (ActionTableRow + registerActionTable / -// actionTableHandleCommand / actionTableCommandId / unregisterActionTable): the -// data-driven home of main.cpp's own action family (capture scopes, panel toggle, -// insert, batch, realtime, recapture, version). One row = one action (FOREVER- -// STABLE id suffix, display phrase, flat function-pointer handler); registration -// iterates the rows, hookcommand dispatch walks the same rows, and unload -// mirror-unregisters from them — adding an action touches the table only (OCP). -// Handlers are plain function pointers (a static dispatch walk, no std::function, -// no virtual — the §3 performance guardrail); gaccel + interned-id storage is -// owned here for the module lifetime, so REAPER's held pointers stay valid and -// the '-command_id' unregister re-presents the IDENTICAL pointer registered. -// -// Includes reaper_plugin.h (gaccel_register_t / reaper_plugin_info_t full defs); -// only the action-family TUs and main.cpp include this header. +// action_registry — shared REAPER registration plumbing, plus a data-driven action +// table (ActionTableRow) so adding an action means adding one row, not touching +// register/dispatch/unregister separately (OCP). Interned command-id/label strings +// persist for the module lifetime: REAPER holds those pointers, and an unregister +// must re-present the SAME one. Handlers are flat function pointers, never +// std::function/virtual (hot-path-adjacent dispatch discipline). #include @@ -30,27 +12,19 @@ namespace reasampler { -// Returns the channel-qualified command id for `suffix`, interning it once for the -// process lifetime. Called by BOTH registerAction and each family's unregister path, -// so a '-command_id' presents the IDENTICAL string pointer registered earlier. +// Interns the channel-qualified command id for `suffix` once per process, so a +// '-command_id' unregister presents the IDENTICAL pointer registered earlier. const char* channelIdFor(const char* suffix); -// Mints a command id from a channel-qualified SUFFIX and registers its gaccel -// (Actions-list entry with a channel-qualified label PHRASE). Returns the command id -// (0 on failure). Both the composed id and label are interned durably — REAPER holds -// the desc pointer, and the id must survive to the mirror-unregister. The gaccel -// storage itself is caller-owned (file-scope in the family TU). +// Mints a command id from `suffix`, registers its gaccel with label `phrase`. +// Returns the id (0 on failure); gaccel storage is caller-owned. int registerAction(reaper_plugin_info_t* rec, const char* suffix, gaccel_register_t& accel, const char* phrase); -// --- The registration table (Q-W6) ------------------------------------------- +// --- The registration table --------------------------------------------------- -// One bindable action. `suffix` and `phrase` are the channel-AGNOSTIC pieces (the -// registry composes the full id/label via channelCommandId / channelActionName); -// both must have static storage duration (string literals, or a pure static table -// like captureActionTable()). `run` fires when the minted command does; `arg` is an -// opaque per-row value passed through to it (e.g. a captureActionTable row index, or -// a bool-like flag), so sibling actions can share one handler without captures. +// `suffix`/`phrase` are channel-agnostic and must have static storage duration. +// `arg` is an opaque per-row value so sibling actions can share one handler. struct ActionTableRow { const char* suffix; // FOREVER-STABLE command-id suffix — never change shipped const char* phrase; // Actions-list display phrase (after the channel prefix) @@ -58,26 +32,19 @@ struct ActionTableRow { int arg = 0; // opaque per-row handler argument }; -// Registers every row (command_id -> gaccel, via the same interning plumbing as -// registerAction) in table order. Rows are COPIED into registry-owned storage whose -// element addresses never move (REAPER holds each gaccel pointer until unload). -// Call once at load; a failed command_id mint (cmd 0) leaves that row inert but -// still mirror-unregistered on unload (harmless, matches the pre-table behavior). +// Rows are copied into registry-owned storage whose addresses never move (REAPER +// holds each gaccel pointer until unload). void registerActionTable(reaper_plugin_info_t* rec, const ActionTableRow* rows, std::size_t count); -// Dispatches one fired command: fires the matching row's handler and returns true; -// false when the command belongs to no table row (caller's hookcommand keeps -// looking, per the claim-only contract). A flat walk over the registered rows. bool actionTableHandleCommand(int command); -// The minted command id for `suffix` (0 when unregistered / mint failed). For the -// callers that need a raw command id outside dispatch — e.g. the toggleaction -// checked-state hook resolving TOGGLE_BANK_PANEL once at load. +// 0 when unregistered / mint failed — for callers needing a raw id outside dispatch +// (e.g. the toggleaction checked-state hook). int actionTableCommandId(const char* suffix); -// Mirror-unregisters every table row (reverse table order): '-gaccel' with the same -// held storage, '-command_id' with the SAME interned pointer used at register. +// Mirror-unregisters every table row (reverse order): '-gaccel' with the held +// storage, '-command_id' with the SAME interned pointer used at register. void unregisterActionTable(reaper_plugin_info_t* rec); } // namespace reasampler diff --git a/src/shell/actions/bank_actions.cpp b/src/shell/actions/bank_actions.cpp index 32e854d..f5c3a63 100644 --- a/src/shell/actions/bank_actions.cpp +++ b/src/shell/actions/bank_actions.cpp @@ -1,23 +1,17 @@ -// bank_actions.cpp — the multi-bank bindable action family (Phase B3; Q-W4 split of -// actions.cpp). See bank_actions.h. +// bank_actions.cpp — see bank_actions.h. // -// Q-W4 dedupe / Q-W6 seam: each mutating handler is a THIN UX SKIN — text prompts -// (promptBankName), name resolution, and console feedback — over the promptless -// bankOp* inner verbs homed in shell/bank_ops (model op + persistBankOp, one bank op -// = one Ctrl-Z), driven against this family's registered session. The book's rules -// (pool privileges, collapse-by-hash, active-fallback-to-pool) all live in -// bank_book; these handlers only drive the verbs and react to the boolean. +// Each mutating handler is a thin UX skin — text prompts, name resolution, console +// feedback — over the promptless bankOp* verbs in shell/bank_ops (model op + +// persistBankOp, one bank op = one Ctrl-Z). Pool privileges / collapse-by-hash / +// active-fallback-to-pool live in bank_book; handlers only drive the verbs. // -// REFERENCE-INVALIDATION GUARDRAIL (B2 review): book().activeIndex() / bank()->index -// return a reference INTO the book's internal vector, which a create/delete can -// reallocate. No handler here caches a BankModel& (or a Bank*) across a structural -// mutation — each resolves ids to strings up front and re-resolves after any -// create/delete. Move/copy pass ids (not references) straight to the verbs. +// REFERENCE-INVALIDATION GUARDRAIL: book().activeIndex() / bank()->index return a +// reference INTO the book's internal vector, which a create/delete can reallocate. +// No handler caches a BankModel&/Bank* across a structural mutation — ids are +// resolved to strings up front and re-resolved after any create/delete. // -// Compiled into the reaper_reasampler MODULE. Includes reaper_plugin_functions.h -// WITHOUT REAPERAPI_IMPLEMENT — main.cpp owns the API pointers (CLAUDE.md §contract). -// The action ids are minted from FOREVER-STABLE strings; user keybindings key off -// them, so they must never change after ship. +// main.cpp owns the API pointers; this TU gets them extern. Action ids are minted +// from FOREVER-STABLE strings — never change one after ship. #include "shell/actions/bank_actions.h" @@ -42,11 +36,9 @@ namespace reasampler { namespace { -// FOREVER-STABLE multi-bank action-id SUFFIXES (Phase V, V4). The channel family prefix is -// prepended at register via channelCommandId (as with the Design View family) — stable -// rebuilds the shipped id, beta the isolated one. NEVER change a shipped suffix. -// Each suffix + the stable prefix must byte-match the pre-V4 shipped literal exactly -// (e.g. "BANK_REMOVE_SELECTED" -> "CEREBELLUM_REASAMPLER_BANK_REMOVE_SELECTED"). +// FOREVER-STABLE action-id SUFFIXES: the channel prefix is prepended at register +// (channelCommandId); NEVER change a shipped suffix — user keybindings key off the +// composed id. constexpr const char* kIdBankCreate = "BANK_CREATE"; constexpr const char* kIdBankRename = "BANK_RENAME"; constexpr const char* kIdBankDelete = "BANK_DELETE"; @@ -58,14 +50,10 @@ constexpr const char* kIdBankCopySel = "BANK_COPY_SELECTED"; constexpr const char* kIdBankRemoveSel = "BANK_REMOVE_SELECTED"; constexpr const char* kIdBankPoolFull = "BANK_POOL_FULLHEIGHT"; constexpr const char* kIdBankBanksFull = "BANK_BANKS_FULLHEIGHT"; -// Phase R (Reclaim), R2: the FOREVER-STABLE "Prune bank folder" id. Registered NOW so -// in-DAW dry-run verification is possible; R2 behaviour is REPORT-ONLY (no deletion), -// and R3 extends the confirm-and-delete step behind this SAME id — never a throwaway id. constexpr const char* kIdBankPruneFolder = "BANK_PRUNE_FOLDER"; -// The live session the actions read (name resolution, member counts, prune) and -// pass to the bankOp* verbs by reference (bankHandleCommand guards it non-null -// before any handler runs). Set once by bankRegisterActions; not owned here. +// Not owned here; set once by bankRegisterActions. bankHandleCommand guards it +// non-null before any handler runs. ReaSamplerSession* g_session = nullptr; int g_cmdBankCreate = 0; @@ -96,25 +84,17 @@ gaccel_register_t g_accelBankPoolFull{}; gaccel_register_t g_accelBankBanksFull{}; gaccel_register_t g_accelBankPruneFolder{}; -// Resolves a user-typed bank reference (a display name) to a bank id, scanning the -// book's banks in ordinal order. Exact match on displayName; "Pool" resolves the pool. -// Returns "" when no bank carries that name. Kept in the action layer (not the model) -// — it is UI name-resolution, not a model rule. First-match is unambiguous BY -// CONSTRUCTION: the model enforces unique display names (trimmed + case-insensitive), -// so at most one bank can carry a given name — no duplicate can shadow another here. +// Resolves a user-typed display name to a bank id ("" if none matches). UI name +// resolution, not a model rule — kept here rather than the model. Unambiguous by +// construction: the model enforces unique display names. std::string bankIdByDisplayName(const std::string& name) { for (const Bank& b : g_session->book().banks()) if (b.displayName == name) return b.id; return {}; } -// -- Action bodies (thin UX skins over the bankOp* verbs) ------------------- - -// Create a named bank: prompt for a display name; the verb mints a stable GUID id, -// creates it in the model, persists. The new bank is NOT auto-activated (create and -// activate are distinct acts — mirrors capture/placement separation). The model -// rejects a duplicate display name (trimmed + case-insensitive, incl. "Pool"); the -// create then fails and the user is told the name is taken. +// The new bank is NOT auto-activated (create and activate are distinct acts, +// mirroring capture/placement separation). void doBankCreate() { std::string name; if (!promptBankName("ReaSampler: create bank", "Bank name:", "", name)) return; @@ -126,9 +106,8 @@ void doBankCreate() { } } -// Rename a bank: prompt for which bank (by current display name) and the new name. -// The pool is un-renamable (the model rejects it). Two prompts keep the bindable form -// self-contained; the panel renames in place on a tab. +// Two prompts (which bank, then the new name) keep this bindable form +// self-contained; the panel renames in place on a tab instead. void doBankRename() { std::string which; if (!promptBankName("ReaSampler: rename bank", "Bank to rename (current name):", "", @@ -142,18 +121,13 @@ void doBankRename() { std::string newName; if (!promptBankName("ReaSampler: rename bank", "New name:", which, newName)) return; if (!bankOpRename(*g_session, id, newName)) { - // The verb rejects the pool (un-renamable) or a name already used by another - // bank (unique display names, trimmed + case-insensitive). ShowConsoleMsg("ReaSampler: cannot rename that bank (the pool is un-renamable, " "or another bank already uses that name).\n"); } } -// Delete a named bank. Bindable safe-form of the confirm-on-non-empty guardrail: -// prompt for the bank; if it holds members, a YESNO ShowMessageBox names evacuate as -// the alternative before dropping them (a plain delete orphans those members' files -// until prune — CONTEXT.md §delete). An empty bank deletes with no prompt. The richer -// panel confirm (naming evacuate inline, with a one-click evacuate) lives in the panel. +// If the bank holds members, confirm first (a plain delete orphans those members' +// files until prune); an empty bank deletes with no prompt. void doBankDelete() { std::string which; if (!promptBankName("ReaSampler: delete bank", "Bank to delete:", "", which)) return; @@ -162,15 +136,13 @@ void doBankDelete() { ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str()); return; } - // Pool early-out: the pool is un-deletable (the model rejects it). Catch it here, - // BEFORE the non-empty confirm, so typing "Pool" never shows a misleading - // "delete anyway?" prompt for an operation the model will refuse regardless. + // Catch the pool BEFORE the non-empty confirm, so typing "Pool" never shows a + // misleading "delete anyway?" for an operation the model will refuse regardless. if (id == kPoolBankId) { ShowConsoleMsg("ReaSampler: the pool cannot be deleted.\n"); return; } - // Read member count BEFORE deleting (the Bank* is invalidated by the delete; we do - // not cache it — resolve size to an int up front). + // Read member count before deleting — the Bank* is invalidated by the delete. const Bank* b = g_session->book().bank(id); if (!b) return; // race-safe: id resolved above but re-check const std::size_t members = b->index.size(); @@ -184,16 +156,14 @@ void doBankDelete() { const int r = ShowMessageBox(msg.c_str(), "ReaSampler: delete non-empty bank", 4); if (r != 6) return; // 6 == YES; anything else cancels (SDK ~6544) } - // S9: bump only when the deleted bank held samples — dropping them changes what a live - // instance referencing one could play. Deleting an EMPTY bank is purely organizational. + // Bump only when the deleted bank held samples — dropping them changes what a + // live instance referencing one could play. if (!bankOpDelete(*g_session, id, /*bumpGeneration=*/members > 0)) { ShowConsoleMsg("ReaSampler: cannot delete that bank (the pool is un-deletable).\n"); } } -// Evacuate a named bank: move every member back to the pool (index-only, collapse by -// hash), leaving the bank empty. The pool is un-evacuable (the verb rejects it). The -// intended "keep the samples" companion to delete. +// The "keep the samples" companion to delete: moves every member back to the pool. void doBankEvacuate() { std::string which; if (!promptBankName("ReaSampler: evacuate bank", "Bank to evacuate to the pool:", "", @@ -210,10 +180,8 @@ void doBankEvacuate() { } } -// Cycle the active bank forward in ordinal order (pool -> named -> ... -> pool), -// via the pure nextBankId helper. Activating a bank changes the CAPTURE TARGET (the -// next capture lands in the newly-active bank — B2's book().activeIndex() seam) and -// never touches the timeline. The verb persists so the active id travels with the .rpp. +// Cycles the active bank (pool -> named -> ... -> pool). Activating changes the +// CAPTURE TARGET only — never touches the timeline. void doBankActivateNext() { std::vector ids; ids.reserve(g_session->book().size()); @@ -223,20 +191,13 @@ void doBankActivateNext() { bankOpActivate(*g_session, target); } -// Activate the pool directly (the common "back to the default target" jump). Bindable -// direct-by-id form; a general activate-bank-by-name/menu is a panel affordance. void doBankActivatePool() { bankOpActivate(*g_session, kPoolBankId); } -// Move or copy the panel's selected samples into a named destination bank (prompted -// by display name). The SOURCE is the bank the selection lives in — the focused -// region's displayed bank (bankPanelSelectedSourceBankId), which under B4's vertical -// split is NOT necessarily the active/capture-target bank (active ≠ shown). Both are -// index-only (files never relocate); the verb owns the verb-aware no-op guardrail and -// destination collapse-by-hash. The panel's "move to bank" menu drives the same verb -// with a menu-chosen destination — this bindable form is the same operation with a -// text-prompt destination. +// SOURCE is the bank the selection lives in (bankPanelSelectedSourceBankId), which is +// NOT necessarily the active/capture-target bank — the vertical split can show a +// different bank than the one active for capture. void doBankTransferSelected(bool copy) { const std::vector selected = bankPanelSelectedSampleIds(); if (selected.empty()) { @@ -253,7 +214,6 @@ void doBankTransferSelected(bool copy) { ShowConsoleMsg(("ReaSampler: no bank named \"" + destName + "\".\n").c_str()); return; } - // Source = the bank the selection lives in (the focused region's displayed bank). const std::string srcId = bankPanelSelectedSourceBankId(); if (srcId == destId) { ShowConsoleMsg("ReaSampler: source and destination are the same bank.\n"); @@ -262,10 +222,8 @@ void doBankTransferSelected(bool copy) { bankOpTransfer(*g_session, selected, srcId, destId, copy); } -// Remove the panel's selected samples from the SOURCE bank (the focused region's -// displayed bank — same source as move/copy). Index-only and non-destructive to the -// file (orphaned until Phase R prune); silent, with the batched undo as recovery — -// see bankOpRemove for the full contract. +// Index-only and non-destructive to the file (orphaned until prune); silent, with +// the batched undo as recovery. void doBankRemoveSelected() { const std::vector selected = bankPanelSelectedSampleIds(); if (selected.empty()) { @@ -307,8 +265,6 @@ void bankRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session) "toggle pool full-height"); g_cmdBankBanksFull = registerAction(rec, kIdBankBanksFull, g_accelBankBanksFull, "toggle banks full-height"); - // Phase R, R2: the "Prune bank folder" action (report-only in this wave; R3 extends - // the confirm-and-delete step behind this SAME forever-stable id). g_cmdBankPruneFolder = registerAction(rec, kIdBankPruneFolder, g_accelBankPruneFolder, "prune bank folder"); } @@ -335,8 +291,8 @@ bool bankHandleCommand(int command) { int bankPruneCommandId() { return g_cmdBankPruneFolder; } void bankUnregisterActions(reaper_plugin_info_t* rec) { - // Mirror-unregister with '-'-prefixed strings, reverse of registration order. Each - // '-command_id' re-presents the same interned channel-qualified id (channelIdFor). + // Reverse of registration order; each '-command_id' re-presents the same + // interned id (channelIdFor). rec->Register("-gaccel", (void*)&g_accelBankPruneFolder); rec->Register("-command_id", (void*)channelIdFor(kIdBankPruneFolder)); rec->Register("-gaccel", (void*)&g_accelBankBanksFull); diff --git a/src/shell/actions/bank_actions.h b/src/shell/actions/bank_actions.h index f8221e3..ac86344 100644 --- a/src/shell/actions/bank_actions.h +++ b/src/shell/actions/bank_actions.h @@ -1,45 +1,29 @@ #pragma once -// bank_actions — the multi-bank bindable action family (Phase B3; Q-W4 split of -// actions.h). The bindable action set that drives the multi-bank workflow: create / -// rename / delete / evacuate a bank, activate a bank (direct pool + cycle), move / -// copy / remove the panel's selected samples, the two vertical-split full-height -// toggles, and the Phase R prune action's registration + dispatch (its guarded body -// lives in prune_action). Q-W4 dedupe: every mutating handler here is a THIN UX skin -// (text prompts + console messages) over the promptless bankOp* verbs homed in -// panel_bank_ops — one implementation home for each mutation, two UX skins (this -// family prompts for which bank; the panel acts on a clicked tab). +// bank_actions — the multi-bank bindable action family: create/rename/delete/ +// evacuate a bank, activate (direct pool + cycle), move/copy/remove the panel's +// selected samples, the two vertical-split full-height toggles, and the prune +// action's registration + dispatch (guarded body in prune_action). Every mutating +// handler is a thin UX skin over the promptless bankOp* verbs in bank_ops (this +// family prompts for a bank name; the panel acts on a clicked tab). // -// Same registration/routing/unload contract as the Design View family -// (design_view_actions); both share main.cpp's single hookcommand, and each family's -// Handle claims only its own ids. This header is SDK-free. +// Same registration/routing/unload contract as design_view_actions. SDK-free header. -// Forward-declared at GLOBAL scope (matches reaper_plugin.h's typedef) so this -// header stays SDK-free; the .cpp includes the real definition. -struct reaper_plugin_info_t; +struct reaper_plugin_info_t; // global scope, matches reaper_plugin.h's typedef namespace reasampler { class ReaSamplerSession; -// Registers the multi-bank family against `rec`. `session` is the live session (must -// outlive registration). Call exactly once at load — pass the SAME session pointer -// the Design View family receives. +// `session` must outlive registration — pass the SAME pointer the Design View +// family receives. void bankRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session); -// Services one fired command for the multi-bank family. True iff it was one of this -// family's ids (and handled); false otherwise so the caller's hookcommand keeps -// looking. Safe for any command. bool bankHandleCommand(int command); -// Mirror-unregisters the multi-bank family with '-'-prefixed strings. Call once on -// rec==nullptr (before the session is torn down). void bankUnregisterActions(reaper_plugin_info_t* rec); -// The registered command id for the "Prune bank folder" action (Phase R, R3), or 0 -// before registration. The bank_panel prune button fires the action THROUGH this id -// via Main_OnCommand (fork R-E: the button dispatches the command, it does not call -// the session directly) so the panel affordance and the bindable action share one -// guarded code path. +// The bank_panel prune button fires THROUGH this id (Main_OnCommand) rather than +// calling the session directly, so button and bindable action share one guarded path. int bankPruneCommandId(); } // namespace reasampler diff --git a/src/shell/actions/design_view_actions.cpp b/src/shell/actions/design_view_actions.cpp index a05dfb9..5e0d4ca 100644 --- a/src/shell/actions/design_view_actions.cpp +++ b/src/shell/actions/design_view_actions.cpp @@ -1,24 +1,16 @@ -// design_view_actions.cpp — the Design View action family (Phase D4; Q-W4 split of -// actions.cpp). See design_view_actions.h. +// design_view_actions.cpp — see design_view_actions.h. // -// Compiled into the reaper_reasampler MODULE. Includes reaper_plugin_functions.h -// WITHOUT REAPERAPI_IMPLEMENT — main.cpp is the one TU that defines the API pointers -// (CLAUDE.md §contract). The action ids are minted from FOREVER-STABLE strings (the -// same CEREBELLUM_REASAMPLER_ family prefix main.cpp uses); user keybindings key off -// them, so they must never change after ship. +// main.cpp owns the API pointers; this TU gets them extern. Action ids are minted +// from FOREVER-STABLE strings — never change one after ship. // -// Each action: -// 1. mutates the session's ViewModeModel (membership tag/untag/show-both, or the -// active mode via toggle/activate) — the pure D1 state, -// 2. reapplies the active mode through the D2 view shell (applyMode) so the change -// takes visible effect immediately (tagging a track into Design while in Arrange -// parks it right away; a mode change re-partitions and re-parks in one step). +// Each action mutates the session's ViewModeModel (membership tag/untag/show-both, or +// the active mode via toggle/activate), then reapplies the active mode through the +// view shell (applyMode) so the change takes visible effect immediately. // // Selection-driven mutations iterate the CURRENT REAPER track selection -// (CountSelectedTracks/GetSelectedTrack — both ignore the master, which is correct: -// the master is never tagged) and resolve each track to its canonical GUID key via -// the shared guidString helper, so the keys match exactly what the D2 shell / view -// tree key on (the cross-module key contract). +// (CountSelectedTracks/GetSelectedTrack ignore the master, which is correct — the +// master is never tagged) and resolve each track to its canonical GUID key so the +// keys match what the view shell / view tree key on. #include "shell/actions/design_view_actions.h" @@ -55,11 +47,8 @@ using view::isOnManualLane; namespace { -// FOREVER-STABLE action-id SUFFIXES (Phase V, V4). The channel family prefix is prepended -// at register time via channelCommandId (app_version), so stable rebuilds the exact shipped -// id ("CEREBELLUM_REASAMPLER_VIEW_TOGGLE_MODE") and beta yields the isolated forever-family -// id ("CEREBELLUM_REASAMPLER_BETA_VIEW_TOGGLE_MODE"). Each composed id is minted into a -// persistent command id user keybindings key off — NEVER change a shipped suffix after ship. +// FOREVER-STABLE action-id SUFFIXES: the channel prefix is prepended at register +// (channelCommandId) — NEVER change a shipped suffix. constexpr const char* kIdToggleMode = "VIEW_TOGGLE_MODE"; constexpr const char* kIdActivateArrange = "VIEW_ACTIVATE_ARRANGE"; constexpr const char* kIdActivateDesign = "VIEW_ACTIVATE_DESIGN"; @@ -67,17 +56,14 @@ constexpr const char* kIdTagDesign = "VIEW_TAG_DESIGN"; constexpr const char* kIdTagArrange = "VIEW_TAG_ARRANGE"; constexpr const char* kIdUntag = "VIEW_UNTAG"; constexpr const char* kIdShowBoth = "VIEW_SHOW_BOTH"; -// D2 Wave 3-B item-level mode moves — the item analog of the track tag family. Same -// FOREVER-STABLE contract (suffix composed with the channel prefix) — NEVER change these. +// Item-level mode moves — the item analog of the track tag family above. constexpr const char* kIdMoveItemsDesign = "VIEW_MOVE_ITEMS_DESIGN"; constexpr const char* kIdMoveItemsArrange = "VIEW_MOVE_ITEMS_ARRANGE"; constexpr const char* kIdUntagItems = "VIEW_UNTAG_ITEMS"; -// The live session the actions mutate. Set once by designViewRegisterActions and -// read by the hookcommand handler. Not owned here (main.cpp owns g_session). +// Not owned here (main.cpp owns g_session). ReaSamplerSession* g_session = nullptr; -// Minted command ids (0 until registration succeeds). Compared in the handler. int g_cmdToggleMode = 0; int g_cmdActivateArrange = 0; int g_cmdActivateDesign = 0; @@ -102,9 +88,6 @@ gaccel_register_t g_accelMoveItemsDesign{}; gaccel_register_t g_accelMoveItemsArrange{}; gaccel_register_t g_accelUntagItems{}; -// Collects the canonical GUID keys of the current track selection. Empty if nothing -// is selected. CountSelectedTracks/GetSelectedTrack ignore the master (SDK), which is -// exactly right — the master is never a tagged leaf. std::vector selectedTrackGuids() { std::vector guids; const int n = CountSelectedTracks(nullptr); // nullptr = active project @@ -118,22 +101,17 @@ std::vector selectedTrackGuids() { return guids; } -// Reapplies the model's CURRENT active mode to the active project so a membership -// mutation takes visible effect immediately (park/unpark/re-derive parents). Called -// after every tag/untag/show-both. `proj = nullptr` -> REAPER's active project. +// Reapplies the CURRENT active mode so a membership mutation takes visible effect +// immediately (park/unpark/re-derive parents). void reapplyActiveMode() { applyMode(g_session->view(), g_session->view().activeModeId(), nullptr); } -// Track fixed-lane mode value (I_FREEMODE=2). Mirrors the shell's constant; used only to -// decide whether an item's lane name is meaningful for the manual-lane read. -constexpr int kFreeModeFixedLanes = 2; +constexpr int kFreeModeFixedLanes = 2; // I_FREEMODE value; mirrors the shell's constant -// Collects the current media-item selection as the pure decision's input: each selected -// item's GUID plus whether it sits on a MANUAL lane (⇒ EXEMPT — never retagged/re-laned). -// The manual-lane read follows the shared pure predicate exactly as the shell's readers -// do: only on a fixed-lane track (I_FREEMODE==2) is the item's lane name read; on a normal -// track isOnManualLane returns false for the empty name, so the P_LANENAME read is skipped. +// Each selected item's GUID plus whether it sits on a MANUAL lane (EXEMPT — never +// retagged/re-laned). The lane name is read only on a fixed-lane track; on a normal +// track the shared predicate returns false for an empty name, so the read is skipped. // Items whose GUID cannot be read are dropped (an empty GUID must never be retagged). std::vector selectedRetagItems() { std::vector items; @@ -148,33 +126,18 @@ std::vector selectedRetagItems() { MediaTrack* tr = GetMediaItemTrack(it); const bool fixedLane = tr && static_cast(GetMediaTrackInfo_Value(tr, "I_FREEMODE")) == kFreeModeFixedLanes; - // Only read the lane name on a fixed-lane track; the pure predicate handles the - // normal-track case (returns false) so we pass an empty name and skip the read. const std::string laneNm = fixedLane ? itemLaneName(tr, it) : std::string{}; items.push_back(RetagItem{std::move(g), isOnManualLane(fixedLane, laneNm)}); } return items; } -// Persists both the bank and the Design-View model to the active project's ext -// state. Called after every state-changing Design View action so the view model -// is not lost across save/close/reopen. Marking the project dirty is correct — -// a Design View mutation is a project-level change the user should be prompted -// to save. -// -// When the membership index is non-empty AND the project is unsaved, we prompt -// the user to Save-As before persisting — mirroring the flow capture uses. -// Gate: if membership is empty (no tracks tagged), skip the prompt entirely; -// saveToActiveProject will no-op for an unsaved project, which is correct. -// -// DAW-ONLY ASSUMPTION: Main_SaveProject(proj, true) opens a Save-As dialog and -// blocks until the user dismisses it. The blocking behaviour and dialog -// appearance can only be confirmed in a running REAPER (same caveat as capture). +// Persists the bank + Design-View model after every state-changing action so the +// view model is not lost across save/close/reopen. If membership is non-empty and +// the project is unsaved, prompts Save-As first (mirrors the flow capture uses) — +// DAW-ONLY: Main_SaveProject(proj, true) blocks until the dialog is dismissed. void persistViewState() { if (!g_session->view().membership().empty()) { - // At least one track is tagged — worth persisting. Check whether the - // project is saved and, if not, prompt Save-As so saveToActiveProject - // can write ext state. Mirrors capture's readRppPath idiom exactly. ReaProject* proj = EnumProjects(-1, nullptr, 0); if (proj) { auto readRppPath = [&]() -> std::string { @@ -184,15 +147,11 @@ void persistViewState() { }; if (readRppPath().empty()) { - // Project is unsaved — prompt Save-As. Main_SaveProject(proj, true); - // Re-read: still empty means the user cancelled. - if (readRppPath().empty()) { + if (readRppPath().empty()) { // still empty -> user cancelled ShowConsoleMsg( "ReaSampler: Design View state will not persist until " "the project is saved.\n"); - // The in-session tag state is left as-is — the mode change - // already applied and remains valid for this session. return; } } @@ -201,11 +160,8 @@ void persistViewState() { g_session->saveToActiveProject(); } -// -- Action bodies --------------------------------------------------------- - -// Toggle: cycle to the next mode in ordinal order (Arrange <-> Design with two -// seeds; scales to cycle-through-all for >2 modes with no change here). applyMode -// itself sets the model's active mode, so we only compute the target and apply. +// Cycle to the next mode in ordinal order. applyMode itself sets the model's active +// mode, so we only compute the target and apply. void doToggleMode() { const std::string target = nextModeId(g_session->view().modes(), g_session->view().activeModeId()); @@ -223,9 +179,8 @@ void doActivateMode(const std::string& modeId) { bankPanelInvalidate(); // repaint the footer [Arrange|Design] toggle immediately } -// Tag the selection's leaves into `modeId`, then reapply so the change is immediate. // tag() replaces any prior single-mode membership (a leaf lives in one mode; the -// cross-mode case is show-both), matching the D1 contract. +// cross-mode case is show-both). void doTag(const std::string& modeId) { for (const std::string& g : selectedTrackGuids()) g_session->view().membership().tag(g, modeId); @@ -233,9 +188,8 @@ void doTag(const std::string& modeId) { persistViewState(); } -// Untag the selection entirely (return each to the Arrange default). This is the -// shared body behind both "Untag selected" and "Tag -> Arrange" (Arrange = the -// absence of a tag), so the two actions are the same act by definition. +// Shared body behind "Untag selected" and "Tag -> Arrange" — Arrange is the absence +// of a tag, so the two actions are the same act. void doUntag() { for (const std::string& g : selectedTrackGuids()) g_session->view().membership().untag(g); @@ -243,11 +197,8 @@ void doUntag() { persistViewState(); } -// Toggle the per-track show-both pin for the selection. Read the CURRENT pin of each -// track and flip it independently (a mixed selection converges toward "all on" then -// "all off" only if uniform; per-track flip is the honest semantics of a toggle on a -// multi-selection). show-both leaves are never parked (D1), so reapply reflects the -// change immediately. +// Flips each track's pin independently — the honest semantics of a toggle on a +// multi-selection (a mixed selection converges toward uniform only if it already was). void doShowBoth() { MembershipIndex& m = g_session->view().membership(); for (const std::string& g : selectedTrackGuids()) @@ -256,19 +207,12 @@ void doShowBoth() { persistViewState(); } -// -- Item-level mode moves (D2 Wave 3-B) ----------------------------------- +// Retag the current ITEM selection to `targetMode` (empty => untag -> Arrange +// default). planItemRetag decides which items to retag (manual-lane items are +// EXEMPT), upholding the managed-lanes-only invariant. Wrapped in ONE Undo block. // -// Retag the current ITEM selection to `targetMode` (empty ⇒ untag → Arrange default), -// then re-drive the minting + apply path so each moved item lands on its target mode's -// managed lane and the active-mode lane visibility is reasserted. The pure planItemRetag -// decides which selected items to retag (manual-lane items are EXEMPT — never retagged, -// never re-laned), upholding the managed-lanes-only invariant even under this explicit -// user action. The whole structural act is wrapped in ONE Undo block with a descriptive -// label (the inner blocks mintManagedLanes / applyMode open nest harmlessly under it). -// -// UNDO/SAVE ordering: persistViewState may pop a Save-As dialog (Main_SaveProject) which -// must NOT sit inside the Undo block, so we close the block first, then persist — the same -// separation the track actions rely on (they persist outside applyMode's own block). +// UNDO/SAVE ordering: persistViewState may pop a Save-As dialog, which must NOT sit +// inside the Undo block, so we close the block first, then persist. void doMoveItems(const std::string& targetMode) { const std::vector selected = selectedRetagItems(); const std::vector ops = planItemRetag(selected, targetMode); @@ -277,15 +221,12 @@ void doMoveItems(const std::string& targetMode) { MembershipIndex& membership = g_session->view().membership(); Undo_BeginBlock2(nullptr); - // Apply the pure decision's membership writes: tag into targetMode, or untag. for (const ItemRetagOp& op : ops) { if (op.untag) membership.untag(op.guid); else membership.tag(op.guid, op.modeId); } - // Re-drive the SAME minting/apply path auto-tag uses: mint/split lanes for any track - // whose items now span modes and assign each moved item to its mode's managed lane, - // then reassert the active mode's lane visibility. Manual lanes stay untouched - // (mintManagedLanes reports their items exempt and never mints over them). + // Re-drive the same minting/apply path auto-tag uses: mint/split lanes for any + // track whose items now span modes, then reassert active-mode lane visibility. mintManagedLanes(g_session->view(), nullptr); reapplyActiveMode(); @@ -303,8 +244,6 @@ void doMoveItems(const std::string& targetMode) { void designViewRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session) { g_session = session; - // command_id -> gaccel for each. The single hookcommand that routes these lives - // in main.cpp (one hook per extension); designViewHandleCommand services them. g_cmdToggleMode = registerAction(rec, kIdToggleMode, g_accelToggleMode, "toggle Design View mode"); g_cmdActivateArrange = registerAction(rec, kIdActivateArrange, g_accelActivateArrange, @@ -320,7 +259,6 @@ void designViewRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* ses g_cmdShowBoth = registerAction(rec, kIdShowBoth, g_accelShowBoth, "show both for selected tracks"); - // Item-level mode moves (D2 W3-B): the item analog of the track tag family. g_cmdMoveItemsDesign = registerAction(rec, kIdMoveItemsDesign, g_accelMoveItemsDesign, "move selected items -> Design"); g_cmdMoveItemsArrange = registerAction(rec, kIdMoveItemsArrange, g_accelMoveItemsArrange, @@ -336,13 +274,10 @@ bool designViewHandleCommand(int command) { if (command == g_cmdActivateArrange) { doActivateMode(kArrangeModeId); return true; } if (command == g_cmdActivateDesign) { doActivateMode(kDesignModeId); return true; } if (command == g_cmdTagDesign) { doTag(kDesignModeId); return true; } - // Tag -> Arrange and Untag are the same act (Arrange = the absence of a tag). if (command == g_cmdTagArrange) { doUntag(); return true; } if (command == g_cmdUntag) { doUntag(); return true; } if (command == g_cmdShowBoth) { doShowBoth(); return true; } - // Item-level moves. Move -> Arrange and Untag items collapse to the same act (an - // empty target ⇒ untag ⇒ Arrange default), mirroring the track-level pairing above. if (command == g_cmdMoveItemsDesign) { doMoveItems(kDesignModeId); return true; } if (command == g_cmdMoveItemsArrange) { doMoveItems(std::string{}); return true; } if (command == g_cmdUntagItems) { doMoveItems(std::string{}); return true; } @@ -351,11 +286,8 @@ bool designViewHandleCommand(int command) { } void designViewUnregisterActions(reaper_plugin_info_t* rec) { - // Mirror-unregister with '-'-prefixed strings, per the contract's unload rule. - // gaccel first, then the command_id string (reverse of registration order — the item - // moves registered last, so they tear down first). - // Each '-command_id' re-presents the SAME interned, channel-qualified id (channelIdFor - // returns the memoized pointer registered above), so the unregister matches exactly. + // Reverse registration order; each '-command_id' re-presents the SAME interned + // pointer channelIdFor returned above. rec->Register("-gaccel", (void*)&g_accelUntagItems); rec->Register("-command_id", (void*)channelIdFor(kIdUntagItems)); rec->Register("-gaccel", (void*)&g_accelMoveItemsArrange); diff --git a/src/shell/actions/design_view_actions.h b/src/shell/actions/design_view_actions.h index 4006516..6352feb 100644 --- a/src/shell/actions/design_view_actions.h +++ b/src/shell/actions/design_view_actions.h @@ -1,38 +1,26 @@ #pragma once -// design_view_actions — the Design View action family (Phase D4; Q-W4 split of -// actions.h). Registers the bindable actions that drive the mode workflow and wires -// them end-to-end: toggle/activate a mode, tag/untag/show-both the current track -// selection, and the item-level mode moves (D2 W3-B). Each action mutates the -// session's ViewModeModel (D1, via persist's ReaSamplerSession) and then reapplies -// the active mode through the view shell (D2) so the change takes effect immediately. -// -// REAPER-facing shell: the .cpp includes reaper_plugin_functions.h WITHOUT -// REAPERAPI_IMPLEMENT (main.cpp owns the API pointers — CLAUDE.md §contract). This -// header is SDK-free; main.cpp calls register/handle/unregister and nothing else. +// design_view_actions — the Design View bindable action family: toggle/activate a +// mode, tag/untag/show-both the current track selection, and the item-level mode +// moves. Each action mutates the session's ViewModeModel then reapplies the active +// mode through the view shell so the change takes effect immediately. SDK-free +// header; main.cpp calls register/handle/unregister and nothing else. -// Forward-declared at GLOBAL scope (matches reaper_plugin.h's typedef struct -// reaper_plugin_info_t) so this header stays SDK-free; the .cpp includes the real -// definition. Declared before the namespace so it is the global type, not a -// namespace-local shadow. +// Forward-declared at GLOBAL scope (matches reaper_plugin.h's typedef) so this +// header stays SDK-free; the .cpp includes the real definition. struct reaper_plugin_info_t; namespace reasampler { class ReaSamplerSession; -// Registers the Design View action family against `rec` (command_id + gaccel + -// hookcommand-routing is owned by the caller's single hookcommand). `session` is the -// live session the actions mutate; it must outlive the registration. Idempotent is -// NOT promised — call exactly once at load, mirror-unregister once at unload. +// `session` must outlive registration. Not idempotent — call exactly once at load, +// mirror-unregister once at unload. void designViewRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session); -// Services one fired command. Returns true iff `command` is one of this module's -// action ids (and it was handled); false otherwise so the caller's hookcommand keeps -// looking (per the contract: claim only our own ids). Safe to call for any command. +// True iff `command` is one of this module's ids (and handled); false otherwise so +// the caller's hookcommand keeps looking. bool designViewHandleCommand(int command); -// Mirror-unregisters everything designViewRegisterActions registered, with the -// '-'-prefixed strings (per the contract's unload rule). Call once on rec==nullptr. void designViewUnregisterActions(reaper_plugin_info_t* rec); } // namespace reasampler diff --git a/src/shell/actions/drag_out_win.cpp b/src/shell/actions/drag_out_win.cpp index 8316a86..71b92f1 100644 --- a/src/shell/actions/drag_out_win.cpp +++ b/src/shell/actions/drag_out_win.cpp @@ -1,13 +1,7 @@ -// drag_out_win — OS/COM initiation of native OS drag-out (M11). See drag_out_win.h. -// -// Windows path (primary): a hand-rolled minimal IDataObject exposing exactly one format, -// CF_HDROP, plus a minimal IDropSource, handed to OLE DoDragDrop with a COPY-ONLY effect -// mask. We roll our own rather than pull in a helper because the object is tiny (one -// format, one medium) and the copy-only guarantee must be structural and auditable in one -// place. mac/linux route to SWELL's file-list drag behind the same seam. -// -// Compiled into the reaper_reasampler MODULE. No REAPER API is used here (pure OS/COM); it -// is a leaf the bank_panel calls. +// drag_out_win.cpp — see drag_out_win.h. Hand-rolled IDataObject/IDropSource rather +// than a helper library: the object is tiny (one format, one medium) and the +// copy-only guarantee must be structural and auditable in one place. No REAPER API +// used here (pure OS/COM). #include "shell/actions/drag_out_win.h" @@ -29,8 +23,8 @@ namespace { HGLOBAL buildHDrop(const std::vector& paths) { if (paths.empty()) return nullptr; - // 1) Convert each UTF-8 path to wide, normalizing '/' -> '\\' (the panel stores paths - // slash-normalized for its own resolution; CF_HDROP wants native backslashes). + // Convert each UTF-8 path to wide, normalizing '/' -> '\\' (the panel stores paths + // slash-normalized; CF_HDROP wants native backslashes). std::vector wide; wide.reserve(paths.size()); std::size_t totalChars = 0; // characters incl. each path's terminating NUL @@ -40,8 +34,7 @@ HGLOBAL buildHDrop(const std::vector& paths) { if (need <= 0) continue; // unconvertible path — skip rather than emit garbage std::wstring w(static_cast(need), L'\0'); MultiByteToWideChar(CP_UTF8, 0, p.c_str(), -1, &w[0], need); - // `need` includes the NUL; drop it from the string length, we re-add it in the buffer. - if (!w.empty() && w.back() == L'\0') w.pop_back(); + if (!w.empty() && w.back() == L'\0') w.pop_back(); // re-added below for (wchar_t& c : w) if (c == L'/') c = L'\\'; totalChars += w.size() + 1; // + the per-path NUL wide.push_back(std::move(w)); @@ -200,10 +193,9 @@ private: bool initiateDragOut(HWND__* /*panelHwnd*/, const std::vector& absolutePaths) { if (absolutePaths.empty()) return false; - // REAPER's main thread is already OLE-initialized (it hosts OLE drag targets), so we do - // NOT call OleInitialize here — a nested OleInitialize on an already-initialized STA is - // harmless-but-unnecessary, and OleUninitialize pairing across a REAPER-owned apartment - // is the kind of thing that bites. DoDragDrop works on the already-initialized STA. + // REAPER's main thread is already OLE-initialized (it hosts OLE drag targets); we + // deliberately do NOT call OleInitialize — pairing OleUninitialize across a + // REAPER-owned apartment is the kind of thing that bites. HGLOBAL hdrop = buildHDrop(absolutePaths); if (!hdrop) return false; @@ -211,9 +203,8 @@ bool initiateDragOut(HWND__* /*panelHwnd*/, const std::vector& abso auto* source = new DropSource(); DWORD effect = 0; - // COPY-ONLY (invariant #1): the allowed-effects mask is DROPEFFECT_COPY alone. MOVE is - // NEVER offered, so no drop target can relocate (delete) the bank file — only prune - // deletes bank bytes (Phase R boundary). + // COPY-ONLY: the allowed-effects mask is DROPEFFECT_COPY alone. MOVE is never + // offered, so no drop target can relocate (delete) the bank file. const HRESULT hr = DoDragDrop(data, source, DROPEFFECT_COPY, &effect); source->Release(); @@ -232,12 +223,9 @@ bool initiateDragOut(HWND__* /*panelHwnd*/, const std::vector& abso namespace reasampler { -// SWELL provides a file-list drag surface (SWELL_InitiateDragDropOfFileList, verified in -// vendor/WDL/WDL/swell/swell-functions.h). It takes a C-string array + count and initiates -// a copy-style file drag from the given window. Unlike OLE it exposes no per-source effect -// mask, so the copy-only guarantee rests on SWELL's copy semantics rather than an explicit -// DROPEFFECT_COPY mask — an honest platform difference, not a faked equivalence. Windows is -// the exact-control path (D5: Windows is the shipping target). +// SWELL_InitiateDragDropOfFileList initiates a copy-style file drag from the given +// window. Unlike OLE it exposes no per-source effect mask, so the copy-only +// guarantee here rests on SWELL's copy semantics rather than an explicit mask. bool initiateDragOut(HWND__* panelHwnd, const std::vector& absolutePaths) { if (absolutePaths.empty() || !panelHwnd) return false; diff --git a/src/shell/actions/drag_out_win.h b/src/shell/actions/drag_out_win.h index 04ccb6b..1179a7c 100644 --- a/src/shell/actions/drag_out_win.h +++ b/src/shell/actions/drag_out_win.h @@ -1,25 +1,17 @@ #pragma once -// drag_out_win — the OS/COM initiation half of native OS drag-out (Milestone 11). The pure -// gesture-boundary decision and path-list assembly live in drag_out.*; THIS is the platform -// shell that hands a resolved, existing-file path list to the operating system's drag-drop -// machinery so the user can drop bank samples into Explorer / another app / another DAW. +// drag_out_win — the OS/COM initiation half of native OS drag-out: hands a resolved +// existing-file path list to the OS's drag-drop machinery so the user can drop bank +// samples into Explorer / another app / another DAW. The pure gesture-boundary +// decision + path-list assembly live in drag_out.*. // -// ONE seam, platform-forked inside the .cpp: -// * Windows (primary — Daniel's target): OLE DoDragDrop with a minimal IDataObject -// carrying CF_HDROP (absolute paths, double-null-terminated wide list) and a minimal -// IDropSource. COPY-ONLY is STRUCTURAL: the IDataObject offers DROPEFFECT_COPY and the -// effect mask passed to DoDragDrop is DROPEFFECT_COPY alone — MOVE is never offered, so -// no target can pull the bank file out of the bank folder (invariant #1: a move would -// delete bank bytes, and per the Phase R boundary ONLY prune deletes files). -// * macOS/Linux (SWELL): SWELL_InitiateDragDropOfFileList (verified present in -// vendor/WDL/WDL/swell/swell-functions.h) behind the same seam. SWELL's file-list drag -// is a copy-style file drag; it exposes no per-source effect mask the way OLE does, so -// the copy-only guarantee there rests on SWELL's copy semantics rather than an explicit -// mask — noted honestly, not faked. Windows is where the mask control is exact. +// COPY-ONLY is STRUCTURAL on Windows: DoDragDrop's effect mask is DROPEFFECT_COPY +// alone — MOVE is never offered, so no target can pull a file out of the bank folder +// (only prune deletes bank bytes). macOS/Linux route through SWELL's file-list drag, +// which exposes no per-source effect mask, so there the copy-only guarantee rests on +// SWELL's copy semantics rather than an explicit mask. // -// NON-DESTRUCTIVE (invariant #2): initiating a drag reads nothing but the path list and -// mutates no sample / index / selection. A cancelled or failed drag changes nothing — the -// OS layer here neither writes ext-state nor touches the book. +// NON-DESTRUCTIVE: initiating a drag reads nothing but the path list; a cancelled or +// failed drag mutates no sample/index/selection. #include #include @@ -28,16 +20,10 @@ struct HWND__; // avoid dragging windows.h into every includer; the shell casts namespace reasampler { -// Initiates a native OS drag-out of `absolutePaths` (already resolved, existing, de-duped — -// the pure drag_out::assemblePathList output) from the panel window `panelHwnd`. COPY-ONLY; -// see the header note. A no-op when the path list is empty (nothing draggable — the caller -// checks this too, but the guard is repeated here so a direct call is safe). -// -// BLOCKING on Windows: OLE DoDragDrop runs its own modal message loop until the drop or -// cancel, then returns — the caller's gesture state should be reset AFTER this returns. -// Returns true if a drop was accepted (DROPEFFECT_COPY), false on cancel / failure / -// empty input. The return is advisory (a failed drag is visible by nothing happening — -// the caller does not surface an error, per the brief's no-console-output constraint). +// `absolutePaths` must already be resolved/existing/de-duped (drag_out::assemblePathList +// output). No-op when empty. BLOCKING on Windows: OLE DoDragDrop runs its own modal +// message loop until drop/cancel. Returns true iff the drop was accepted +// (DROPEFFECT_COPY); the return is advisory — a failed drag surfaces no error. bool initiateDragOut(HWND__* panelHwnd, const std::vector& absolutePaths); } // namespace reasampler diff --git a/src/shell/actions/instrument_drop_win.cpp b/src/shell/actions/instrument_drop_win.cpp index fa2503d..b6b0213 100644 --- a/src/shell/actions/instrument_drop_win.cpp +++ b/src/shell/actions/instrument_drop_win.cpp @@ -1,7 +1,5 @@ -// instrument_drop_win — the REAPER shell for S17 drop-and-load. See instrument_drop_win.h. -// -// Compiled into the reaper_reasampler MODULE. Includes reaper_plugin_functions.h WITHOUT -// REAPERAPI_IMPLEMENT (main.cpp owns the pointers; here they are extern via the WANT list). +// instrument_drop_win.cpp — see instrument_drop_win.h. main.cpp owns the API +// pointers; this TU gets them extern via the WANT list. #include "shell/actions/instrument_drop_win.h" @@ -35,32 +33,24 @@ using wire::infoNamesFxHotspot; namespace { -// Write `bytes` to a fresh uniquely-named .vstpreset in the OS temp dir and return its path; -// returns an empty path on any failure. The .vstpreset extension is load-bearing — -// TrackFX_SetPreset's full-path form is documented for .vstpreset files (VST3). The file is -// transient: the caller deletes it right after the SetPreset call. +// Writes `bytes` to a fresh uniquely-named .vstpreset in the OS temp dir; empty path +// on any failure. The .vstpreset extension is load-bearing — TrackFX_SetPreset's +// full-path form is documented for .vstpreset files (VST3). Transient: the caller +// deletes it right after the SetPreset call. // -// The temp filename embeds the process ID so two concurrent REAPER instances (e.g. stable + -// beta) cannot collide in the shared OS temp dir, and one instance's cleanup cannot -// accidentally delete another's in-flight file. -// -// Non-throwing: every std::filesystem call uses the error_code overload. The whole body is -// wrapped in try/catch to guarantee no exception crosses the REAPER C callback boundary -// (the same discipline the prune shell uses — see prune_fs.cpp's non-throwing scan comment). -// -// Returns the path object (not a narrow string) so the caller can: -// (a) pass path.u8string() to TrackFX_SetPreset — UTF-8 on MSVC, not ACP-converted, -// so a temp dir with accented or CJK user-name bytes is handled correctly; -// (b) delete via the retained path object — not via re-parsing the narrow string — -// so the cleanup cannot leak if the conversion above were to round-trip incorrectly. +// Non-throwing: every std::filesystem call uses the error_code overload, and the +// whole body is try/catch-wrapped so no exception crosses the REAPER callback +// boundary. Returns the path object (not a narrow string) so the caller can pass +// path.u8string() to TrackFX_SetPreset (UTF-8, not ACP-converted) and delete via the +// same retained path — never a re-parsed narrow string. std::filesystem::path writeTempPreset(const std::vector& bytes) { try { static std::atomic counter{0}; std::error_code ec; const std::filesystem::path dir = std::filesystem::temp_directory_path(ec); if (ec) return {}; - // PID in the name keeps files from distinct REAPER instances distinct in the shared - // temp dir — prevents cross-instance collisions and spurious post-apply deletions. + // PID in the name: two concurrent REAPER instances (stable + beta) cannot + // collide in the shared temp dir. const std::string name = "reasampler_drop_" + std::to_string(GetCurrentProcessId()) + "_" + std::to_string(counter.fetch_add(1)) + ".vstpreset"; @@ -85,10 +75,8 @@ std::filesystem::path writeTempPreset(const std::vector& bytes) { FxDropTarget resolveFxDropTarget(int screenX, int screenY) { FxDropTarget out; char info[256] = {0}; - // GetThingFromPoint returns the track under the point (may be null for a non-track thing) - // and fills `info` with what was hit. A non-empty info OR a non-null track means the point - // is over REAPER's own UI; a null track with an empty info means the pointer has left - // REAPER entirely (over another app / the desktop) — the OsDrag boundary. + // A non-empty info OR a non-null track means the point is over REAPER's own UI; + // a null track with empty info means the pointer has left REAPER entirely. MediaTrack* track = GetThingFromPoint(screenX, screenY, info, sizeof(info)); out.track = track; out.overReaperUi = (track != nullptr) || (info[0] != '\0'); @@ -101,46 +89,32 @@ FxDropTarget resolveFxDropTarget(int screenX, int screenY) { bool loadInstrumentOntoTrack(MediaTrack* track, const std::vector& presetBytes) { if (!track || presetBytes.empty()) return false; - // Materialize the .vstpreset FIRST so an I/O failure leaves the track untouched (no FX - // added yet — nothing to roll back). + // Materialize the .vstpreset FIRST so an I/O failure leaves the track untouched + // (no FX added yet — nothing to roll back). const std::filesystem::path presetPath = writeTempPreset(presetBytes); if (presetPath.empty()) return false; - // The CHANNEL-correct FX name: "VST3:ReaSampler 9000" on stable, "VST3:ReaSampler 9000 - // beta" on beta. Sourcing it from app_version::vstPluginName() (the same accessor the VST - // factory display name derives from) keeps the pairing invariant intact — a beta extension - // drops the beta VST, a stable extension the stable VST — with no literal to drift. (The - // preset's class ID forks by the same channel bit inside buildInstrumentDropPreset.) + // Channel-correct FX name ("VST3:ReaSampler 9000[ beta]") sourced from the same + // accessor the VST factory display name derives from, so the pairing invariant + // (beta extension <-> beta VST) has no literal to drift. const std::string fxName = "VST3:" + vstPluginName(); - // Negative `instantiate` => always create a NEW instance (verified in the header). recFX - // = false: a normal track FX chain instance, not a record/monitoring FX. + // Negative `instantiate` => always create a NEW instance. recFX = false: a + // normal track FX chain instance, not a record/monitoring FX. const int fxIndex = TrackFX_AddByName(track, fxName.c_str(), /*recFX=*/false, /*instantiate=*/-1); bool ok = fxIndex >= 0; - // Apply the dragged capture's component state through the DOCUMENTED channel: a full - // .vstpreset path handed to TrackFX_SetPreset (SDK: "Full paths to .vstpreset files are - // also supported for VST3 plug-ins"). REAPER parses the Steinberg container and feeds the - // 'Comp' chunk to the instance's setState — the same bytes the instrument's own - // serializer produced (instrument_drop::buildInstrumentDropPreset -> - // sample_map::serializeComponentState). Unlike the former "vst_chunk" named-config-parm - // write, a failure here is REPORTED (false), not silently ignored. - // - // u8string() gives UTF-8 bytes on MSVC (not ACP-converted), so a temp dir under an - // accented or CJK user-name is handled correctly by REAPER's path APIs. + // u8string() gives UTF-8 bytes on MSVC (not ACP-converted), so a temp dir under + // an accented or CJK user-name is handled correctly by REAPER's path APIs. if (ok) ok = TrackFX_SetPreset(track, fxIndex, presetPath.u8string().c_str()); - // The preset file is transient regardless of outcome; delete via the retained path object - // (not a re-parsed narrow string) so cleanup cannot leak even if the UTF-8 conversion - // round-trip were incorrect. std::error_code ec; - std::filesystem::remove(presetPath, ec); + std::filesystem::remove(presetPath, ec); // transient regardless of outcome + // All-or-nothing: if the preset apply fails, remove the FX instance we just + // added so the track is left exactly as it was. if (!ok && fxIndex >= 0) { - // All-or-nothing: if the preset apply fails, remove the empty FX instance we just - // added so the track is left exactly as it was. TrackFX_Delete signature (verified - // in reaper_plugin_functions.h:7236): bool TrackFX_Delete(MediaTrack*, int fx). TrackFX_Delete(track, fxIndex); } return ok; @@ -149,11 +123,8 @@ bool loadInstrumentOntoTrack(MediaTrack* track, const std::vector& bool performInstrumentDrop(MediaTrack* track, const std::vector& presetBytes) { if (!track || presetBytes.empty()) return false; - // One undo point for the whole gesture (mirrors the bank-verb undo discipline). Both the - // FX add and the state apply are REAPER-undoable, so Ctrl-Z removes the instance cleanly. Undo_BeginBlock2(nullptr); const bool ok = loadInstrumentOntoTrack(track, presetBytes); - // The undo label reflects the placement-of-the-player framing (not a capture, not an insert). Undo_EndBlock2(nullptr, "ReaSampler: drop capture onto FX chain", -1); return ok; } diff --git a/src/shell/actions/instrument_drop_win.h b/src/shell/actions/instrument_drop_win.h index 20a65cb..5e4e675 100644 --- a/src/shell/actions/instrument_drop_win.h +++ b/src/shell/actions/instrument_drop_win.h @@ -1,69 +1,51 @@ #pragma once -// instrument_drop_win — the REAPER-facing shell half of S17 drop-and-load. The pure gesture -// decision lives in drag_out (DragGesture::InstrumentDrop) and the pure payload construction -// in instrument_drop; THIS is the platform shell that (a) resolves a screen point to a track -// + its FX-surface hotspot via REAPER's hit-test API, and (b) on release adds a ReaSampler -// 9000 instance to that track and applies the dragged capture as its component state via a -// temp .vstpreset + TrackFX_SetPreset (S-GA-DropFX: the earlier "vst_chunk" named-config-parm -// write was silently unappliable — see instrument_drop.h for the diagnosis). +// instrument_drop_win — the REAPER-facing shell half of drop-and-load: (a) resolves +// a screen point to a track + its FX-surface hotspot via REAPER's hit-test API, and +// (b) on release adds a ReaSampler 9000 instance and applies the dragged capture as +// its component state via a temp .vstpreset + TrackFX_SetPreset (the earlier +// "vst_chunk" named-config-parm write was silently unappliable for VST3 — don't +// revert to it). The pure gesture decision lives in drag_out; the pure payload +// construction in instrument_drop. // -// Compiled into the reaper_reasampler MODULE. REAPER-facing (GetThingFromPoint, TrackFX_*, -// Undo_*), so DAW-verified, not unit-tested; the pure decision + preset it drives are CTest'd. -// -// LOAD-BEARING (CONTEXT.md §Drop-and-load): this is an EXPLICIT user placement-of-the-player -// gesture — it adds a READER of the bank on a track and points it at one already-captured -// sample. It NEVER captures, NEVER writes the bank, and NEVER inserts a timeline item. The -// only writes are: a new FX instance on the target track + that instance's own component -// state — both REAPER-undoable, wrapped in one undo block so the whole gesture is one Ctrl-Z -// — plus a transient .vstpreset in the OS temp dir, deleted before returning. +// LOAD-BEARING: an EXPLICIT user placement-of-the-player gesture — adds a READER of +// the bank on a track, pointed at an already-captured sample. NEVER captures, NEVER +// writes the bank, NEVER inserts a timeline item. The only writes are a new FX +// instance + its component state, both wrapped in one undo block (one Ctrl-Z), plus +// a transient .vstpreset deleted before returning. #include #include -// Opaque REAPER track handle at the boundary so includers don't need the SDK. The SDK -// declares it as a class (reaper_plugin.h) — match that spelling so the mangled name agrees. +// Declared as a class (matching reaper_plugin.h) so the mangled name agrees, without +// pulling in the SDK. class MediaTrack; namespace reasampler { -// The result of hit-testing a screen point during a live InstrumentDrop drag. struct FxDropTarget { MediaTrack* track = nullptr; // the track under the pointer (null if none / not a track) bool overReaperUi = false; // the point is over REAPER's own window/UI at all bool overFxHotspot = false; // specifically over this track's FX button/chain surface - // A valid drop target: a resolved track whose FX hotspot is under the pointer. bool valid() const { return track != nullptr && overFxHotspot; } }; -// Hit-test a screen point (REAPER screen coords) to an FX drop target. Wraps -// GetThingFromPoint, whose info string tells us what was hit ("tcp.fx*"/"mcp.fx*" for the -// TCP/MCP FX button family; "fx_chain"/"fx_N" for the FX-chain and floating-FX windows; bare -// "tcp"/"mcp" or other sub-element tokens for non-FX track-panel regions). `overReaperUi` is -// the shell-supplied predicate the pure drag_out::decideGesture consumes (true when the point -// is over REAPER's own UI — i.e. GetThingFromPoint returned a track OR a recognizable -// non-track thing, false when the pointer has left REAPER entirely). `overFxHotspot` is true -// only when the info string names a genuine FX-bearing surface — decided by the pure -// instrument_drop::infoNamesFxHotspot from the SDK's own hit-test string. +// Wraps GetThingFromPoint, whose info string tells us what was hit ("tcp.fx*"/ +// "mcp.fx*" for the TCP/MCP FX button family; "fx_chain"/"fx_N" for the FX-chain and +// floating windows). `overReaperUi` is true when the point is over REAPER's own UI +// at all; `overFxHotspot` is true only for a genuine FX-bearing surface (decided by +// the pure instrument_drop::infoNamesFxHotspot). FxDropTarget resolveFxDropTarget(int screenX, int screenY); -// Perform the drop on `track`: add a fresh ReaSampler 9000 instance and apply `presetBytes` -// (the instrument_drop::buildInstrumentDropPreset output — a .vstpreset image) as its -// component state so it plays the dragged capture. Wraps the add + apply in one REAPER undo -// block (mirrors the bank-verb undo discipline). Returns true on success (the FX was added -// and the preset applied), false on any failure. All-or-nothing: if the preset apply fails -// after a successful add, the freshly-added FX instance is removed via TrackFX_Delete before -// returning false, leaving the track exactly as it was (no orphaned empty-state FX). -// NEVER inserts a timeline item; the ONLY persistent mutations are the FX instance + its -// state, both undoable. +// Adds a fresh ReaSampler 9000 instance to `track` and applies `presetBytes` as its +// component state. Wraps add + apply in one REAPER undo block. All-or-nothing: if +// the preset apply fails after a successful add, the FX instance is removed via +// TrackFX_Delete before returning false, leaving the track exactly as it was. bool performInstrumentDrop(MediaTrack* track, const std::vector& presetBytes); -// Add a fresh ReaSampler 9000 instance to `track` and apply `presetBytes` as its component -// state. Same all-or-nothing add+apply contract as performInstrumentDrop (rolls the FX back -// via TrackFX_Delete on apply failure), but does NOT open its own undo block — the caller owns -// the undo grouping so the whole gesture (persist + FX-add + apply) collapses to -// one Ctrl-Z. This is the shared inner half performInstrumentDrop wraps in its own block. -// Returns true on success, false on any failure. NEVER inserts a timeline item. +// Same all-or-nothing add+apply contract as performInstrumentDrop but does NOT open +// its own undo block — the caller owns the undo grouping so persist + FX-add + apply +// collapses to one Ctrl-Z. The shared inner half performInstrumentDrop wraps. bool loadInstrumentOntoTrack(MediaTrack* track, const std::vector& presetBytes); } // namespace reasampler diff --git a/src/shell/actions/prune_action.cpp b/src/shell/actions/prune_action.cpp index a0b037d..0fa4240 100644 --- a/src/shell/actions/prune_action.cpp +++ b/src/shell/actions/prune_action.cpp @@ -1,8 +1,5 @@ -// prune_action.cpp — the "Prune bank folder" action body (Phase R3; Q-W4 split of -// actions.cpp). See prune_action.h for the contract this TU preserves. -// -// Compiled into the reaper_reasampler MODULE. Includes reaper_plugin_functions.h -// WITHOUT REAPERAPI_IMPLEMENT (main.cpp owns the API pointers — CLAUDE.md §contract). +// prune_action.cpp — see prune_action.h for the contract this TU preserves. +// main.cpp owns the API pointers; this TU gets them extern. #include "shell/actions/prune_action.h" @@ -18,25 +15,17 @@ namespace reasampler { -// Prune bank folder — Phase R (Reclaim), R3: the guarded DESTRUCTIVE step, and the SOLE -// file-deletion entry in ReaSampler. Dry-run FIRST (compute the orphan set, read-only), -// then — only when orphans exist — a blocking CONFIRM showing the SPECIFIC manifest -// (count + reclaimable bytes + the file list, truncated consistent with the 64-cap), then -// on explicit Yes delete EXACTLY that set (session.pruneReclaim, which recomputes the -// pure core fresh and deletes confirmed ∩ freshOrphans — trash-preferred, unlink fallback). -// Zero orphans => informational only, NO confirm ever shown. Cancel deletes nothing. -// -// The full (untruncated) orphan set is captured here for the delete; the dry-run's -// truncated list is only the confirm's readout. No ext-state is written and no undo point -// is opened (file deletion is not REAPER-undoable and pruneReclaim mutates no project -// state) — a Ctrl-Z after a prune correctly cannot claim to restore deleted files. +// The guarded DESTRUCTIVE step and the SOLE file-deletion entry in ReaSampler. +// Dry-run FIRST (read-only); only when orphans exist, a blocking CONFIRM with the +// manifest; on explicit Yes, delete EXACTLY that set (recomputed fresh — confirmed ∩ +// freshOrphans). Zero orphans => informational only, no confirm shown. No ext-state +// write, no undo point (file deletion is not REAPER-undoable). void doBankPruneFolder(ReaSamplerSession& session) { const reclaim::PruneReport report = session.pruneDryRun(); - // pS-usage FAIL-SAFE: a present instance-usage record could not be read — the - // protected set is unknowable, so the prune HALTS outright (deletes nothing) rather - // than proceed with degraded protection. Distinct from "no orphans": the user must - // know the prune refused to run and why. + // FAIL-SAFE: an unreadable instance-usage record makes the protected set + // unknowable, so the prune HALTS outright rather than proceed with degraded + // protection. if (report.abortedUnreadableUsage) { std::string msg = "ReaSampler prune: ABORTED -- one or more instance usage records could not " @@ -58,13 +47,10 @@ void doBankPruneFolder(ReaSamplerSession& session) { return; } - // The EXACT set the delete will target — full, untruncated, so what the confirm - // summarises (count + bytes) matches what pruneReclaim reclaims. Captured before the - // confirm so the confirm and the delete reason about the same enumeration. + // The EXACT (untruncated) set the delete will target, captured before the confirm + // so confirm and delete reason about the same enumeration. const std::vector orphanSet = session.pruneOrphanSet(); - // Confirm-with-manifest: count + bytes exact; the file list is the dry-run's 64-capped - // list (the same clip the R2 readout used), with a "N more not shown" tail when clipped. std::string msg = "ReaSampler prune will PERMANENTLY reclaim " + std::to_string(report.count) + " orphaned file(s), freeing " + std::to_string(report.totalBytes) + " bytes.\n\n" @@ -84,7 +70,6 @@ void doBankPruneFolder(ReaSamplerSession& session) { return; } - // Confirmed -> delete exactly the confirmed set (recomputed fresh, stale entries skipped). const reclaim::PruneDeletionResult del = session.pruneReclaim(orphanSet); std::string done = "ReaSampler prune: reclaimed " + diff --git a/src/shell/actions/prune_action.h b/src/shell/actions/prune_action.h index fed17df..eae1a4e 100644 --- a/src/shell/actions/prune_action.h +++ b/src/shell/actions/prune_action.h @@ -1,15 +1,12 @@ #pragma once -// prune_action — the "Prune bank folder" action body (Phase R3; Q-W4 split of -// actions.cpp). This is the SOLE file-deletion action in ReaSampler, isolated in its -// own TU so the deletion authority is one obvious module on the actions side (its -// persist-side counterpart concentrates into prune_fs in Q-W5). Registration and -// hookcommand routing for its FOREVER-STABLE id (BANK_PRUNE_FOLDER) stay with the -// bank family (bank_actions) — one registration flow, one guarded body here. +// prune_action — the "Prune bank folder" action body: the SOLE file-deletion action +// in ReaSampler, isolated in its own TU so the deletion authority is one obvious +// module. Registration/dispatch for its FOREVER-STABLE id (BANK_PRUNE_FOLDER) stay +// with bank_actions; one guarded body here. // // Contract (preserve exactly): dry-run first; abort outright on unreadable usage -// records (pS-usage fail-safe); confirm-with-manifest before any deletion; opens NO -// undo point and writes NO ext state (file deletion is not REAPER-undoable). Routes -// to persist's public session API only (pruneDryRun / pruneOrphanSet / pruneReclaim). +// records (fail-safe); confirm-with-manifest before any deletion; opens NO undo +// point and writes NO ext state (file deletion is not REAPER-undoable). namespace reasampler { diff --git a/src/shell/bank_ops/bank_ops.cpp b/src/shell/bank_ops/bank_ops.cpp index 55dfe4e..134715e 100644 --- a/src/shell/bank_ops/bank_ops.cpp +++ b/src/shell/bank_ops/bank_ops.cpp @@ -1,16 +1,11 @@ -// bank_ops.cpp — the promptless bank-verb seam (Q-W6 lift; see bank_ops.h for the -// contract). The ONE implementation home of the bank verbs (create / rename / -// delete / evacuate / activate / move / copy / remove): each mutates the given -// session's book() then persists via persistBankOp() (one bank op = one Ctrl-Z; a -// true index no-op opens NO undo point). It DOES mutate the bank BOOK — but only -// the index/model + ext-state, never the arrange, never a sample file on disk -// (bank ops are index-only; files stay put — CONTEXT.md §Multi-bank). -// REFERENCE-INVALIDATION GUARDRAIL: after a STRUCTURAL mutation any -// Bank*/BankModel& is invalid — verbs take ids and resolve fresh per model call. +// bank_ops.cpp — see bank_ops.h for the contract. The ONE implementation home of the +// bank verbs: each mutates the given session's book() then persists via +// persistBankOp() (one bank op = one Ctrl-Z; a true index no-op opens NO undo +// point). Index/model + ext-state only — never the arrange, never a file on disk. +// REFERENCE-INVALIDATION GUARDRAIL: after a STRUCTURAL mutation any Bank*/BankModel& +// is invalid — verbs take ids and resolve fresh per model call. // -// Compiled into the reaper_reasampler MODULE. Includes reaper_plugin_functions.h -// WITHOUT REAPERAPI_IMPLEMENT — main.cpp owns the API pointers; here they are -// extern (CLAUDE.md §contract). DAW-verified, not unit tested. +// main.cpp owns the API pointers; this TU gets them extern. DAW-verified, not unit tested. #include "shell/bank_ops/bank_ops.h" @@ -31,9 +26,7 @@ namespace reasampler { namespace { -// Mints a fresh, genuine REAPER GUID string as a stable bank id (the B2/model -// design: ids are caller-supplied and stable; the model stays pure and mints none). -// Distinct from a track GUID by origin only — both are canonical guidToString output. +// Ids are caller-supplied and stable; the model stays pure and mints none. std::string mintBankId() { GUID g{}; genGuid(&g); @@ -44,35 +37,20 @@ std::string mintBankId() { } // namespace -// Persists a completed bank-index verb as a SINGLE batched REAPER undo point (R-B) — -// one bank op = one Ctrl-Z. +// WHY THIS WRAPS: a bank verb mutates ONLY our project ext-state, which REAPER's +// undo system captures iff UNDO_STATE_MISCCFG is set (the SDK documents MISCCFG as +// covering extensions' project ext-state). We pass exactly UNDO_STATE_MISCCFG, not +// -1/UNDO_STATE_ALL — a bank verb touches no tracks/FX/items, so snapshotting them +// would be both heavier and wrong. Persist runs INSIDE the block so the post-mutation +// ext-state is the block's "after" image. // -// WHY THIS WRAPS AND saveToActiveProject() 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. The persist runs -// INSIDE the block so the post-mutation ext-state is the block's "after" image. -// -// UNSAVED-PROJECT GUARDRAIL: on an unsaved / no-active project saveToActiveProject() -// 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. (Quiet persist by design — mirrors the CAPTURE path, NOT the -// Design-View path; deliberately NO Save-As prompt.) +// UNSAVED-PROJECT GUARDRAIL: on an unsaved/no-active project saveToActiveProject() +// no-ops; we still CLOSE the block, but with an empty label + zero flag so REAPER +// discards the point instead of recording a no-effect undo entry. Quiet persist by +// design (mirrors capture, not Design-View) — deliberately no Save-As prompt. void persistBankOp(ReaSamplerSession& session, const char* label, bool bumpGeneration) { Undo_BeginBlock2(nullptr); - // S9: bump the bank-generation counter INSIDE the block, before the persist, so the - // fresh generation rides the same ext-state write (saveToActiveProject() stamps - // bankGeneration()). Bumped only for content-changing verbs (the caller decides); a - // pure-organizational verb passes false and leaves the counter be, so a - // rename/activate does not needlessly refresh live instances. if (bumpGeneration) session.bumpBankGeneration(); const bool persisted = session.saveToActiveProject(); if (persisted) @@ -109,7 +87,6 @@ bool bankOpDelete(ReaSamplerSession& session, const std::string& bankId, bool bankOpEvacuate(ReaSamplerSession& session, const std::string& bankId) { if (!session.book().evacuate(bankId)) return false; // pool is a destination, not a source - // S9: evacuate moves members between banks (bank membership changes) -> bump. persistBankOp(session, "ReaSampler: evacuate bank", /*bumpGeneration=*/true); return true; } @@ -120,14 +97,10 @@ bool bankOpActivate(ReaSamplerSession& session, const std::string& bankId) { return true; } -// 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; move counts gains OR collapses. Ids pass -// straight to the model op — no BankModel& cached across the loop's mutations. +// NO-OP GUARDRAIL, verb-aware: a MOVE collapse still removed the source entry (the +// index DID mutate), but a COPY collapse left the source intact AND the dest already +// held the hash (a true no-op) — so copy counts only real gains, move counts gains +// OR collapses. bool bankOpTransfer(ReaSamplerSession& session, const std::vector& sampleIds, const std::string& srcBankId, const std::string& destBankId, @@ -151,18 +124,14 @@ bool bankOpTransfer(ReaSamplerSession& session, } const bool mutated = copy ? (ok > 0) : (ok > 0 || collapsed > 0); if (!mutated) return false; // nothing changed — no persist, no undo point - // S9: a move/copy changes bank membership (a sample arrives in / leaves a bank an - // instance may reference) -> bump so assigned instances refresh hands-free. persistBankOp(session, copy ? "ReaSampler: copy sample(s)" : "ReaSampler: move sample(s)", /*bumpGeneration=*/true); return true; } -// Index-only, this-bank scope (fork R-A: the sole surfaced verb; RemoveScope::AllBanks -// stays latent in the model). Non-destructive to the file: a last-reference remove -// leaves the file on disk, orphaned until Phase R prune — remove NEVER deletes bytes -// (the manifest is untouched). Silent: recoverability is the batched undo (R-B). +// Index-only, this-bank scope; non-destructive to the file (a last-reference remove +// leaves the file orphaned until prune). Silent: recoverability is the batched undo. bool bankOpRemove(ReaSamplerSession& session, const std::vector& sampleIds, const std::string& srcBankId) { @@ -174,8 +143,6 @@ bool bankOpRemove(ReaSamplerSession& session, RemoveResult::Removed) ++removed; if (removed == 0) return false; // every id already absent — no undo point - // S9: a remove drops a sample from a bank (an instance referencing it must refresh — - // it will resolve to silence, per the stale-id policy) -> bump. persistBankOp(session, "ReaSampler: remove sample(s)", /*bumpGeneration=*/true); return true; } diff --git a/src/shell/bank_ops/bank_ops.h b/src/shell/bank_ops/bank_ops.h index 659efa0..a6eaeaa 100644 --- a/src/shell/bank_ops/bank_ops.h +++ b/src/shell/bank_ops/bank_ops.h @@ -1,22 +1,14 @@ #pragma once -// bank_ops — the promptless bank-verb seam (Q-W6 lift of the Q-W4 single-owner -// verbs out of shell/panel/panel_bank_ops into a NON-UI home). Each verb is a model -// op on the given session's BankBook + persistBankOp (undo-batched ext-state -// persist) — NO prompts, NO message boxes, NO panel-state nudges, NO panel-global -// reads. The two UX surfaces consume these as thin skins: +// bank_ops — the promptless bank-verb seam. Each verb is a model op on the given +// session's BankBook + persistBankOp (undo-batched ext-state persist) — NO prompts, +// NO message boxes, NO panel-state nudges. Two UX surfaces consume these as thin +// skins: shell/panel/panel_bank_ops (menu prompts/confirms/repaints) and +// shell/actions/bank_actions (bindable family, text prompts/console feedback). // -// * shell/panel/panel_bank_ops — the panel's menu handlers (prompts / confirms / -// repaints), passing the panel's live session. -// * shell/actions/bank_actions — the bindable family (text prompts / console -// feedback), passing its registered session. -// -// The session arrives BY REFERENCE: there is exactly one session pointer question -// per call site (the caller's), so a missing session can never be half-reported as -// a model rejection from in here (the Q-W4 review's fail-safe-collapse concern). -// Every verb returns whether the model accepted the mutation — a rejected op -// persists nothing and opens no undo point. -// -// REAPER-facing (persist + undo blocks + GUID minting) but SDK-free in this header. +// The session arrives BY REFERENCE, so a missing session can never be +// half-reported as a model rejection from in here. Every verb returns whether the +// model accepted the mutation — a rejected op persists nothing and opens no undo +// point. REAPER-facing (persist + undo blocks + GUID minting) but SDK-free header. #include #include @@ -25,58 +17,47 @@ namespace reasampler { class ReaSamplerSession; -// Mints a stable GUID bank id, creates `name` in the book. Returns the new bank id, -// or "" when the model rejects the name (duplicate, trimmed + case-insensitive). -// Create is purely organizational — no generation bump. +// Returns "" when the model rejects the name (duplicate, trimmed + case-insensitive). +// Purely organizational — no generation bump. std::string bankOpCreate(ReaSamplerSession& session, const std::string& name); -// Renames `bankId`. False when the model rejects (pool un-renamable / name in use). +// False when the model rejects (pool un-renamable / name in use). bool bankOpRename(ReaSamplerSession& session, const std::string& bankId, const std::string& newName); -// Deletes `bankId`. False when the model rejects (pool un-deletable). The caller -// passes `bumpGeneration` from the member count it read BEFORE any evacuate/delete -// (an evacuate-then-delete flow must still bump on the ORIGINAL membership). +// False when the model rejects (pool un-deletable). `bumpGeneration` should be the +// member count read BEFORE any evacuate/delete (an evacuate-then-delete flow must +// still bump on the ORIGINAL membership). bool bankOpDelete(ReaSamplerSession& session, const std::string& bankId, bool bumpGeneration); -// Evacuates `bankId`'s members to the pool. False when the model rejects (the pool -// itself). Bumps the generation (membership changed). +// False when the model rejects (the pool itself). Bumps the generation. bool bankOpEvacuate(ReaSamplerSession& session, const std::string& bankId); -// Activates `bankId` as the capture target. False on an unknown id. No bump. +// False on an unknown id. No bump. bool bankOpActivate(ReaSamplerSession& session, const std::string& bankId); -// Moves (copy=false) or copies (copy=true) `sampleIds` from `srcBankId` to -// `destBankId` (index-only; files never relocate). Returns whether the index -// actually mutated — the verb-aware no-op guardrail: a COPY collapse changes -// nothing (no undo point); a MOVE collapse did remove the source entry (counts). -// Persists ONE undo point ("move/copy sample(s)") only when mutated. +// Index-only; files never relocate. Returns whether the index actually mutated — a +// COPY collapse changes nothing (no undo point), a MOVE collapse did remove the +// source entry (counts). Persists one undo point only when mutated. bool bankOpTransfer(ReaSamplerSession& session, const std::vector& sampleIds, const std::string& srcBankId, const std::string& destBankId, bool copy); -// Removes `sampleIds` from `srcBankId` (index-only, this-bank scope; never deletes -// bytes). Returns whether anything was removed; persists one undo point when so. +// Index-only, this-bank scope; never deletes bytes. Persists one undo point when +// anything was removed. bool bankOpRemove(ReaSamplerSession& session, const std::vector& sampleIds, const std::string& srcBankId); -// Persists a completed bank-index verb as a single REAPER undo point (R-B). -// Wraps the session persist (SetProjExtState) in a Begin/End block with -// UNDO_STATE_MISCCFG so the bank op is one Ctrl-Z. On an unsaved / no-active project -// the persist 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. +// Wraps the session persist in a Begin/End undo block (UNDO_STATE_MISCCFG) so the +// bank op is one Ctrl-Z; on an unsaved/no-active project the block closes empty +// (REAPER discards it). Call ONLY after an effective mutation. // -// S9 bank-generation bump: pass `bumpGeneration = true` for a verb that changes what a -// live instance would PLAY — move / copy / remove / evacuate / delete-with-members. Leave -// it false (the default) for a PURELY ORGANIZATIONAL verb — create / rename / activate / -// reorder. The bump (when requested) happens INSIDE the block, BEFORE the persist, so -// the stamped counter rides the same ext-state write and undo captures the pre/post -// generation with the rest of the blob. +// `bumpGeneration = true` for a verb that changes what a live instance would PLAY; +// leave false for a purely organizational verb. The bump happens INSIDE the block, +// before the persist, so the stamped counter rides the same ext-state write. void persistBankOp(ReaSamplerSession& session, const char* label, bool bumpGeneration = false);