diff --git a/CMakeLists.txt b/CMakeLists.txt index c057d46..3b55909 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1129,7 +1129,7 @@ add_library(reaper_reasampler MODULE src/shell/actions/design_view_actions.cpp src/shell/actions/bank_actions.cpp src/shell/actions/prune_action.cpp - src/ingest.cpp + src/shell/actions/ingest.cpp src/core/model/bank_book.cpp src/core/model/bank_book_json.cpp src/core/model/owned_manifest.cpp diff --git a/src/app/main.cpp b/src/app/main.cpp index 65469e9..2361822 100644 --- a/src/app/main.cpp +++ b/src/app/main.cpp @@ -22,7 +22,7 @@ #include "core/capture/render_settings.h" // captureActionTable #include "core/version/app_version.h" // appVersion -#include "ingest.h" +#include "shell/actions/ingest.h" #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 diff --git a/src/core/ui/drag_out.cpp b/src/core/ui/drag_out.cpp index 1a84c94..c817a31 100644 --- a/src/core/ui/drag_out.cpp +++ b/src/core/ui/drag_out.cpp @@ -48,4 +48,11 @@ PathList assemblePathList(const std::vector& resolved) { return out; } +OsHandoff decideOsHandoff(const std::vector& paths) { + OsHandoff out; + out.startOsDrag = !paths.empty(); + out.releaseInternalDrag = out.startOsDrag; + return out; +} + } // namespace reasampler::ui diff --git a/src/core/ui/drag_out.h b/src/core/ui/drag_out.h index b859945..5c6ccf0 100644 --- a/src/core/ui/drag_out.h +++ b/src/core/ui/drag_out.h @@ -69,4 +69,19 @@ struct PathList { // exact-string — the shell normalizes case/slashes upstream if it wants Windows-style dedup. PathList assemblePathList(const std::vector& resolved); +// --- OS hand-off ordering ----------------------------------------------------- + +// The two side effects the shell performs when a drag crosses out of REAPER. They are ONE +// decision, not two: winding the internal drag down (release capture, clear drag state) for a +// hand-off that then cannot happen consumes the gesture — the user sees a drag that silently +// did nothing and drags again. Never release without starting. +struct OsHandoff { + bool startOsDrag = false; // hand `paths` to the OS drag initiator + bool releaseInternalDrag = false; // first wind down mouse capture + panel drag state +}; + +// Decides the hand-off from the assembled path list. Empty (everything stale/unresolvable) +// means the internal drag stays live rather than dying half-torn-down. +OsHandoff decideOsHandoff(const std::vector& paths); + } // namespace reasampler::ui diff --git a/src/core/wire/instrument_drop.cpp b/src/core/wire/instrument_drop.cpp index 3732b32..9c3cfda 100644 --- a/src/core/wire/instrument_drop.cpp +++ b/src/core/wire/instrument_drop.cpp @@ -76,6 +76,17 @@ std::vector buildInstrumentDropPreset(const std::string& sampleId) return buildVstPresetBytes(vstClassIdHex(), instrumentDropStateBytes(sampleId)); } +DropOutcome decideDropOutcome(const DropAttempt& attempt) { + DropOutcome out; + if (attempt.addedFxIndex < 0) return out; // add failed — nothing exists to roll back + if (!attempt.presetApplied) { + out.rollbackFxIndex = attempt.addedFxIndex; + return out; + } + out.loaded = true; + return out; +} + bool infoNamesFxHotspot(const std::string& info) { // See the header contract for the prefix rule and the embed-strip exclusion. auto startsWith = [&info](const char* p) { return info.rfind(p, 0) == 0; }; diff --git a/src/core/wire/instrument_drop.h b/src/core/wire/instrument_drop.h index 7363964..27b2296 100644 --- a/src/core/wire/instrument_drop.h +++ b/src/core/wire/instrument_drop.h @@ -68,4 +68,25 @@ bool infoNamesFxHotspot(const std::string& info); // assert the capture is selected. Not called by the shell. std::vector instrumentDropStateBytes(const std::string& sampleId); +// --- All-or-nothing rollback -------------------------------------------------- + +// What the shell observed while executing one drop, reduced to the two REAPER +// results the contract turns on. +struct DropAttempt { + int addedFxIndex = -1; // TrackFX_AddByName's return; < 0 = nothing was created + bool presetApplied = false; // TrackFX_SetPreset's return +}; + +// The verdict. `rollbackFxIndex >= 0` obliges the caller to TrackFX_Delete it before +// returning — an instance whose capture never landed must not survive the drop. +struct DropOutcome { + bool loaded = false; + int rollbackFxIndex = -1; +}; + +// Pure so the contract is provable without a DAW: the rollback obligation is decided +// here, not inline in the shell, and holds identically for every drop surface (FX +// button, FX chain/container, Media-Explorer import). +DropOutcome decideDropOutcome(const DropAttempt& attempt); + } // namespace reasampler::wire diff --git a/src/shell/actions/CLAUDE.md b/src/shell/actions/CLAUDE.md index 338801c..f28e6ce 100644 --- a/src/shell/actions/CLAUDE.md +++ b/src/shell/actions/CLAUDE.md @@ -37,13 +37,11 @@ is owned by other directories and only skinned here. ## Gotchas -- **Structural wart, not yet fixed:** `ingest.cpp` / `ingest.h`, plus `ext_keys.h` - and `resource.h`, physically live at `src/` root rather than under - `shell/actions/` — Phase Q's reorg did not re-home these files into - `core/`/`shell/`/`app/`. `ingest` is documented here as its nearest sibling by - role, but the files themselves are not in this directory. This is a code - organization issue, not a documentation one — see Open questions in the - originating dispatch report. +- **Structural wart, partly closed:** `ingest.cpp` / `ingest.h` now live in this + directory. `ext_keys.h` and `resource.h` still sit at `src/` root: `ext_keys.h` + is consumed mostly from `shell/instrument/`, so it is not this directory's to + claim, and `resource.h` is a build input paired with `src/resource.rc` (the SWELL + resgen step) rather than a shell module. - Media-Explorer import is single-file, pull-on-action (`OpenMediaExplorer` + `MediaExplorerGetLastPlayedFileInfo`) — there is no enumerate-selected-files or register-a-drop-handler API on the Media Explorer surface. diff --git a/src/shell/actions/drag_out_win.cpp b/src/shell/actions/drag_out_win.cpp index 71b92f1..6abe66a 100644 --- a/src/shell/actions/drag_out_win.cpp +++ b/src/shell/actions/drag_out_win.cpp @@ -62,6 +62,29 @@ HGLOBAL buildHDrop(const std::vector& paths) { return h; } +// COM reference counts MUST be interlocked. A CF_HDROP target is free to marshal the data +// object into another apartment and finish the copy on a background thread AFTER DoDragDrop +// has returned (Explorer's async file copy does exactly this). A plain ++/-- there races the +// source thread's post-DoDragDrop Release: one lost increment destroys the object — and with +// it the source HGLOBAL — before the target reads it, and the drop lands with no file. That +// race is intermittent and a retry usually wins it; do not "simplify" these back. +inline ULONG comAddRef(volatile LONG& refs) { + return static_cast(InterlockedIncrement(&refs)); +} + +// DoDragDrop requires the calling thread to be OLE-initialized — CoInitialize alone is not +// enough, and an uninitialized thread fails the call outright, so the drag never starts. +// Relying on REAPER having done it is a first-use hazard: whether it has depends on what else +// ran first in the session. OleInitialize is per-thread refcounted, so this is additive to +// whatever the host did; we deliberately never OleUninitialize — the extension lives for the +// process, and unbalancing a REAPER-owned apartment is the hazard worth avoiding, not this. +// RPC_E_CHANGED_MODE means the thread joined an MTA, where OLE drag-drop is unavailable. +bool ensureOleForThisThread() { + static thread_local int state = 0; // 0 untried, 1 ready, -1 unavailable + if (state == 0) state = SUCCEEDED(OleInitialize(nullptr)) ? 1 : -1; + return state > 0; +} + // Minimal IDropSource: continue until the (left) button releases or Escape cancels; always // request the copy cursor. This is the standard textbook drop source — no custom feedback. class DropSource final : public IDropSource { @@ -76,11 +99,11 @@ public: *ppv = nullptr; return E_NOINTERFACE; } - ULONG STDMETHODCALLTYPE AddRef() override { return ++refs_; } + ULONG STDMETHODCALLTYPE AddRef() override { return comAddRef(refs_); } ULONG STDMETHODCALLTYPE Release() override { - const ULONG r = --refs_; + const LONG r = InterlockedDecrement(&refs_); if (r == 0) delete this; - return r; + return static_cast(r); } // IDropSource HRESULT STDMETHODCALLTYPE QueryContinueDrag(BOOL escapePressed, DWORD keyState) override { @@ -92,7 +115,7 @@ public: return DRAGDROP_S_USEDEFAULTCURSORS; // let OLE draw the standard copy cursor } private: - ULONG refs_ = 1; + volatile LONG refs_ = 1; }; // Minimal IDataObject exposing exactly one format (CF_HDROP / TYMED_HGLOBAL). The HDROP is @@ -112,11 +135,11 @@ public: *ppv = nullptr; return E_NOINTERFACE; } - ULONG STDMETHODCALLTYPE AddRef() override { return ++refs_; } + ULONG STDMETHODCALLTYPE AddRef() override { return comAddRef(refs_); } ULONG STDMETHODCALLTYPE Release() override { - const ULONG r = --refs_; + const LONG r = InterlockedDecrement(&refs_); if (r == 0) delete this; - return r; + return static_cast(r); } // IDataObject — the two that matter for a drag source. @@ -184,7 +207,7 @@ private: (fe.tymed & TYMED_HGLOBAL) && fe.dwAspect == DVASPECT_CONTENT; } - ULONG refs_ = 1; + volatile LONG refs_ = 1; HGLOBAL hdrop_ = nullptr; }; @@ -192,10 +215,8 @@ private: bool initiateDragOut(HWND__* /*panelHwnd*/, const std::vector& absolutePaths) { if (absolutePaths.empty()) return false; + if (!ensureOleForThisThread()) return false; - // 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; diff --git a/src/ingest.cpp b/src/shell/actions/ingest.cpp similarity index 99% rename from src/ingest.cpp rename to src/shell/actions/ingest.cpp index 6837700..b173fdf 100644 --- a/src/ingest.cpp +++ b/src/shell/actions/ingest.cpp @@ -2,7 +2,7 @@ // REAPER-facing, DAW-verified; the pure serialization it drives (assignment_request) // is CTest-tested. -#include "ingest.h" +#include "shell/actions/ingest.h" #include #include diff --git a/src/ingest.h b/src/shell/actions/ingest.h similarity index 100% rename from src/ingest.h rename to src/shell/actions/ingest.h diff --git a/src/shell/actions/instrument_drop_win.cpp b/src/shell/actions/instrument_drop_win.cpp index b6b0213..8bdf650 100644 --- a/src/shell/actions/instrument_drop_win.cpp +++ b/src/shell/actions/instrument_drop_win.cpp @@ -20,6 +20,7 @@ #define REAPERAPI_WANT_GetThingFromPoint #define REAPERAPI_WANT_TrackFX_AddByName #define REAPERAPI_WANT_TrackFX_Delete +#define REAPERAPI_WANT_TrackFX_GetCount #define REAPERAPI_WANT_TrackFX_SetPreset #define REAPERAPI_WANT_Undo_BeginBlock2 #define REAPERAPI_WANT_Undo_EndBlock2 @@ -29,6 +30,9 @@ namespace reasampler { // Real-namespace-home using-declarations (Q-W6: the namespaces.h shim is retired). using version::vstPluginName; +using wire::decideDropOutcome; +using wire::DropAttempt; +using wire::DropOutcome; using wire::infoNamesFxHotspot; namespace { @@ -99,25 +103,32 @@ bool loadInstrumentOntoTrack(MediaTrack* track, const std::vector& // (beta extension <-> beta VST) has no literal to drift. const std::string fxName = "VST3:" + vstPluginName(); - // 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; + // An EXPLICIT top-level insertion position (instantiate <= -1000 IS the position, -1000 + // = first in chain), not the bare -1. Both always create a new instance; the bare form + // additionally leaves placement to REAPER's ambient FX-chain insert point, which a drop + // onto an FX container/chain-window moves — so the index handed to TrackFX_SetPreset and + // the instance just created stop denoting the same FX and the capture never lands. The + // bare-form retry keeps the reference path alive if the positional form is ever refused. + // recFX = false: a normal track FX chain instance, not a record/monitoring FX. + const int insertPos = TrackFX_GetCount(track); + int fxIndex = TrackFX_AddByName(track, fxName.c_str(), /*recFX=*/false, + /*instantiate=*/-1000 - insertPos); + if (fxIndex < 0) + fxIndex = TrackFX_AddByName(track, fxName.c_str(), /*recFX=*/false, /*instantiate=*/-1); // 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()); + DropAttempt attempt; + attempt.addedFxIndex = fxIndex; + if (fxIndex >= 0) + attempt.presetApplied = TrackFX_SetPreset(track, fxIndex, presetPath.u8string().c_str()); std::error_code 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) { - TrackFX_Delete(track, fxIndex); - } - return ok; + const DropOutcome outcome = decideDropOutcome(attempt); + if (outcome.rollbackFxIndex >= 0) TrackFX_Delete(track, outcome.rollbackFxIndex); + return outcome.loaded; } bool performInstrumentDrop(MediaTrack* track, const std::vector& presetBytes) { diff --git a/src/shell/capture/capture_orchestrator.cpp b/src/shell/capture/capture_orchestrator.cpp index 524e34e..f545702 100644 --- a/src/shell/capture/capture_orchestrator.cpp +++ b/src/shell/capture/capture_orchestrator.cpp @@ -12,7 +12,7 @@ #include "shell/panel/panel_input.h" // bankPanelTailSetting / bankPanelRefresh #include "core/capture/tail_control.h" // TailSetting #include "core/model/provenance.h" // model::Provenance -#include "ingest.h" // ingestAssignActiveInstance +#include "shell/actions/ingest.h" // ingestAssignActiveInstance #include "shell/persist/session.h" // ReaSamplerSession #include "shell/capture/insert.h" // runInsert / InsertRequest #include "shell/capture/realtime_lifecycle.h" // the in-flight realtime state diff --git a/src/shell/panel/panel_drag.cpp b/src/shell/panel/panel_drag.cpp index 9f0d5db..17735d9 100644 --- a/src/shell/panel/panel_drag.cpp +++ b/src/shell/panel/panel_drag.cpp @@ -298,8 +298,13 @@ void onMouseMove(int x, int y) { if (gesture == DragGesture::OsDrag) { // Resolve the payload to existing on-disk paths BEFORE tearing down internal - // drag state (the resolver reads dragSourceBankId / dragSampleIds). + // drag state (the resolver reads dragSourceBankId / dragSampleIds), then let the + // pure rule couple the two side effects: an unresolvable payload must leave the + // internal drag intact rather than wind it down for a hand-off that never runs — + // a half-torn-down drag reads to the user as "the drag did nothing, try again". const std::vector paths = resolveDragPathsForOs(); + const ui::OsHandoff handoff = ui::decideOsHandoff(paths); + if (!handoff.releaseInternalDrag) return; // DoDragDrop runs its own modal loop and takes over mouse capture, so the internal // drag must be fully wound down first. @@ -313,8 +318,7 @@ void onMouseMove(int x, int y) { g_panel.dragPrimaryId.clear(); invalidatePanel(); - // Empty path list -> nothing draggable (all stale/missing); do not start a drag. - if (!paths.empty()) + if (handoff.startOsDrag) initiateDragOut(g_panel.hwnd, paths); // COPY-ONLY; blocking on Windows return; } @@ -377,14 +381,27 @@ void resetDragState() { // OsDragOut is never seen here: the pointer-left-client handoff happens live in onMouseMove. void onLBtnUp(int x, int y) { if (g_panel.dragging) { - // Drop-and-load: a release while hover-tracking a valid FX hotspot instantiates a - // ReaSampler 9000 on that track preloaded with the dragged capture — NOT a bank move, - // NOT an OS drag, NEVER a timeline insert. Takes priority over the in-grid / cross-bank - // drop. Single-capture only, so dragSampleIds.front() is the capture. - if (g_panel.instrumentDropTrack && g_panel.dragSampleIds.size() == 1) { + // Drop-and-load: a release over a valid FX hotspot instantiates a ReaSampler 9000 on + // that track preloaded with the dragged capture — NOT a bank move, NOT an OS drag, + // NEVER a timeline insert. Takes priority over the in-grid / cross-bank drop. + // Single-capture only, so dragSampleIds.front() is the capture. + // + // Re-resolve at the RELEASE point rather than trusting only the hover-tracked target: + // WM_MOUSEMOVE is coalesced, so a fast drag onto a dense surface (an FX chain row, a + // container) can release over a hotspot no processed move ever reported. Strictly + // additive — a release-point miss falls back to the tracked target, so the + // hover-then-release-on-the-FX-button path is untouched. + const bool singleCapture = g_panel.dragSampleIds.size() == 1; + MediaTrack* dropTrack = g_panel.instrumentDropTrack; + if (singleCapture) { + POINT sp{x, y}; + ClientToScreen(g_panel.hwnd, &sp); + const FxDropTarget fx = resolveFxDropTarget(sp.x, sp.y); + if (fx.valid()) dropTrack = fx.track; + } + if (dropTrack && singleCapture) { const std::string sampleId = g_panel.dragSampleIds.front(); - performInstrumentDrop(g_panel.instrumentDropTrack, - buildInstrumentDropPreset(sampleId)); + performInstrumentDrop(dropTrack, buildInstrumentDropPreset(sampleId)); // Read-only over the bank + arrange: the only mutations are the new FX instance + // its state (both undoable in performInstrumentDrop). } else { diff --git a/src/shell/panel/panel_window.cpp b/src/shell/panel/panel_window.cpp index 00b4ce7..9cc90f8 100644 --- a/src/shell/panel/panel_window.cpp +++ b/src/shell/panel/panel_window.cpp @@ -14,7 +14,7 @@ #include "shell/panel/panel_window.h" #include "shell/panel/draw_kit.h" -#include "ingest.h" +#include "shell/actions/ingest.h" #ifdef _WIN32 #include // GET_X_LPARAM / GET_Y_LPARAM (SWELL supplies them on mac/linux) diff --git a/tests/test_drag_out.cpp b/tests/test_drag_out.cpp index 3511c44..ed1cfae 100644 --- a/tests/test_drag_out.cpp +++ b/tests/test_drag_out.cpp @@ -221,6 +221,52 @@ static void testMixedTallies() { CHECK(l.skippedDuplicate == 1); } +// --- OS hand-off ordering ----------------------------------------------------- +// +// The drag-out-lands-without-audio regression: the shell used to wind its internal drag down +// (release capture, clear drag state) BEFORE checking whether the payload had resolved to any +// on-disk file. A payload that resolved to nothing therefore consumed the gesture — the drag +// died half-torn-down and the user saw a drop with no audio and dragged again. The two side +// effects are one decision; these pin that. + +// Nothing draggable -> hand off nothing AND keep the internal drag alive. +static void testEmptyPathsHandsOffNothingAndKeepsInternalDrag() { + const OsHandoff h = decideOsHandoff({}); + CHECK(!h.startOsDrag); + CHECK(!h.releaseInternalDrag); +} + +// A resolvable payload -> release the internal drag, then start the OS drag. +static void testResolvablePayloadHandsOff() { + const OsHandoff one = decideOsHandoff({"C:/proj/bank/a.wav"}); + CHECK(one.startOsDrag); + CHECK(one.releaseInternalDrag); + const OsHandoff many = decideOsHandoff({"a.wav", "b.wav", "c.wav"}); + CHECK(many.startOsDrag); + CHECK(many.releaseInternalDrag); +} + +// The coupling itself: releasing without starting is the defect, so it must be unreachable +// for every input the assembler can produce. +static void testNeverReleasesWithoutStarting() { + const std::vector> inputs = { + {}, {"a.wav"}, {"a.wav", "b.wav"}, {""}, + }; + for (const std::vector& in : inputs) { + const OsHandoff h = decideOsHandoff(in); + CHECK(!(h.releaseInternalDrag && !h.startOsDrag)); + } +} + +// End to end through the assembler: a selection whose every entry is stale/unresolvable +// yields the keep-the-drag verdict, which is exactly the case the shell used to mishandle. +static void testAllSkippedSelectionKeepsInternalDrag() { + const PathList l = assemblePathList({missing("g1.wav"), unresolved()}); + const OsHandoff h = decideOsHandoff(l.paths); + CHECK(!h.startOsDrag); + CHECK(!h.releaseInternalDrag); +} + int main() { testInsidePanelStaysInternal(); testLeavingClientAreaIsOsDrag(); @@ -244,6 +290,11 @@ int main() { testAllSkippedYieldsEmpty(); testMixedTallies(); + testEmptyPathsHandsOffNothingAndKeepsInternalDrag(); + testResolvablePayloadHandsOff(); + testNeverReleasesWithoutStarting(); + testAllSkippedSelectionKeepsInternalDrag(); + if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; } diff --git a/tests/test_instrument_drop.cpp b/tests/test_instrument_drop.cpp index 2a6a934..ff4cda5 100644 --- a/tests/test_instrument_drop.cpp +++ b/tests/test_instrument_drop.cpp @@ -234,6 +234,72 @@ static void testEmbedStripIsNotHotspot() { CHECK(!infoNamesFxHotspot("mcp.fxembed extra")); } +// --- Drop surface parity: container vs FX button ------------------------------ +// +// The container-drop regression (instance loads, capture does not): the two surfaces must be +// one code path carrying one payload. The payload is a function of the capture alone, so the +// surface cannot influence it — and both surfaces must classify as hotspots so both reach it. + +// Every surface a drop can land on admits the SAME capture and yields a payload that decodes +// back to it. The payload takes the capture and nothing else, so the surface cannot influence +// it — walking the surfaces here is what pins that the container family is not second-class. +static void testEveryDropSurfaceCarriesTheCapture() { + const char* surfaces[] = { + "fx_chain", // FX chain window / container list — the reported failing surface + "fx_0", // floating FX window + "tcp.fx", // TCP FX button — the reference path + "mcp.fx", // MCP FX button + }; + const std::string sampleId = "cap-7f3a"; + for (const char* info : surfaces) { + CHECK(infoNamesFxHotspot(info)); + const ParsedPreset p = parsePreset(buildInstrumentDropPreset(sampleId)); + CHECK(p.ok); + CHECK(p.classId == vstClassIdHex()); + const ComponentState cs = deserializeComponentState(p.compChunk, kRate); + CHECK(cs.selectionId == sampleId); + } +} + +// --- All-or-nothing rollback -------------------------------------------------- + +// The add failed: nothing was created, so there is nothing to delete and nothing loaded. +static void testAddFailureLeavesNothingToRollBack() { + const DropOutcome out = decideDropOutcome(DropAttempt{-1, false}); + CHECK(!out.loaded); + CHECK(out.rollbackFxIndex < 0); +} + +// The instance was created but the preset did not apply: the caller MUST delete that exact +// index — an instance without its capture is the orphan the contract forbids. +static void testPresetFailureRollsBackTheCreatedIndex() { + const DropOutcome zero = decideDropOutcome(DropAttempt{0, false}); + CHECK(!zero.loaded); + CHECK(zero.rollbackFxIndex == 0); + const DropOutcome later = decideDropOutcome(DropAttempt{4, false}); + CHECK(!later.loaded); + CHECK(later.rollbackFxIndex == 4); + // Container-addressed indices (0x2000000-flagged) roll back by the same rule — the + // obligation follows the index REAPER handed back, whatever space it names. + const DropOutcome inContainer = decideDropOutcome(DropAttempt{0x2000000 + 5, false}); + CHECK(!inContainer.loaded); + CHECK(inContainer.rollbackFxIndex == 0x2000000 + 5); +} + +// Both halves succeeded: loaded, and nothing to undo. +static void testSuccessKeepsTheInstance() { + const DropOutcome out = decideDropOutcome(DropAttempt{2, true}); + CHECK(out.loaded); + CHECK(out.rollbackFxIndex < 0); +} + +// A "preset applied" report with no instance behind it can never read as loaded. +static void testNoInstanceIsNeverLoaded() { + const DropOutcome out = decideDropOutcome(DropAttempt{-1, true}); + CHECK(!out.loaded); + CHECK(out.rollbackFxIndex < 0); +} + int main() { testClassIdHexPinnedPerChannel(); testPresetRoundTripsThroughInstrumentReader(); @@ -248,6 +314,12 @@ int main() { testNonFxSurfacesAreNotHotspot(); testEmbedStripIsNotHotspot(); + testEveryDropSurfaceCarriesTheCapture(); + testAddFailureLeavesNothingToRollBack(); + testPresetFailureRollsBackTheCreatedIndex(); + testSuccessKeepsTheInstance(); + testNoInstanceIsNeverLoaded(); + if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; }