From d5ff476a19ca3a0c7134f5086a176eac1663a3fb Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 7 Aug 2026 19:51:37 -0400 Subject: [PATCH] Make Design View membership edits undoable: the view_state write moves inside the block All four actions share one body; the point turns on a model diff, so a no-op leaves history and the Undo button alone. The Save-As prompt stays outside. --- src/shell/actions/CLAUDE.md | 12 ++ src/shell/actions/design_view_actions.cpp | 205 ++++++++++++---------- src/shell/view/CLAUDE.md | 46 +++-- src/shell/view/view.cpp | 11 +- src/shell/view/view.h | 19 +- src/shell/view/view_lanes.h | 4 +- src/shell/view/view_park.h | 12 +- 7 files changed, 177 insertions(+), 132 deletions(-) diff --git a/src/shell/actions/CLAUDE.md b/src/shell/actions/CLAUDE.md index 01bc6be..3e1e13e 100644 --- a/src/shell/actions/CLAUDE.md +++ b/src/shell/actions/CLAUDE.md @@ -36,6 +36,18 @@ is owned by other directories and only skinned here. reclaim span is open; the mechanism and its compile-time enforcement are `src/shell/undo/CLAUDE.md`'s. Bank index verbs additionally stay one batched `UNDO_STATE_MISCCFG` point apiece, so one bank operation is one Ctrl-Z. +- **A Design View membership edit's undo point holds its `view_state` write — so the + persist runs INSIDE the block, and the point turns on a MODEL DIFF, not on "REAPER + was written to".** All four membership actions (tag, untag / tag→Arrange, show-both, + the item moves) share one body, `design_view_actions`' `runMembershipEdit`. + Everything the reapply they drive writes is the four driven track flags, which mint + no undo state at all (`src/shell/view/CLAUDE.md`), so a point closed BEFORE the + ext-state write covered no change: the edit was not undoable and rode whatever point + the user's next act minted. Two consequences bind. The persist's modal Save-As half + is split off and runs before the block opens — a modal may not hold a block open. + And a no-op edit records no label, because a recorded label re-titles the Undo button + whether or not REAPER appends an entry. Same shape as `shell/bank_ops`' + `persistBankOp`, for the same reason. - **The prune action is the ONLY file-deletion action in the system**; it opens no undo point (file deletion is not REAPER-undoable). It halts on `blockedByTracking` and prints whichever blockers fired — the malformed ledger, diff --git a/src/shell/actions/design_view_actions.cpp b/src/shell/actions/design_view_actions.cpp index 24bb279..5c947fd 100644 --- a/src/shell/actions/design_view_actions.cpp +++ b/src/shell/actions/design_view_actions.cpp @@ -104,16 +104,13 @@ std::vector selectedTrackGuids() { } // Reapplies the CURRENT active mode so a membership mutation takes visible effect -// immediately (park/unpark/re-derive parents), answering whether it wrote any project -// state — the caller's block mints only if something did. `applied` is deliberately -// discarded: a refused reapply leaves the membership edit standing and unapplied, and -// the user's next edit or mode switch reapplies it (a refusal writes nothing, so it -// reports false here and the block discards). What must NOT be dropped with it is the +// immediately (park/unpark/re-derive parents). `applied` is deliberately discarded: a +// refused reapply leaves the membership edit standing and unapplied, and the user's +// next edit or mode switch reapplies it. What must NOT be dropped with it is the // reclaimer's queue, which is why applyMode invalidates on its refusal path too // (view.cpp). -bool reapplyActiveMode() { - return applyMode(g_session->view(), g_session->view().activeModeId(), nullptr) - .wroteProjectState; +void reapplyActiveMode() { + applyMode(g_session->view(), g_session->view().activeModeId(), nullptr); } constexpr int kFreeModeFixedLanes = 2; // I_FREEMODE value; mirrors the shell's constant @@ -147,33 +144,45 @@ std::vector selectedRetagItems() { // serialization. Every other action keeps the full save. enum class PersistScope { Full, ViewOnly }; -// 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(PersistScope scope) { - if (!g_session->view().membership().empty()) { - ReaProject* proj = EnumProjects(-1, nullptr, 0); - if (proj) { - auto readRppPath = [&]() -> std::string { - std::vector buf(4096, '\0'); - EnumProjects(-1, buf.data(), static_cast(buf.size())); - return std::string(buf.data()); - }; +// The persist's MODAL half, split from the write below so a caller can run the two on +// opposite sides of an undo block. If membership is non-empty and the project is +// unsaved, prompts Save-As (mirrors the flow capture uses) — DAW-ONLY: +// Main_SaveProject(proj, true) blocks until the dialog is dismissed. False means the +// user declined and nothing may be written. +bool ensurePersistTarget() { + if (g_session->view().membership().empty()) return true; - if (readRppPath().empty()) { - Main_SaveProject(proj, true); - if (readRppPath().empty()) { // still empty -> user cancelled - ShowConsoleMsg( - "ReaSampler: Design View state will not persist until " - "the project is saved.\n"); - return; - } - } - } - } - if (scope == PersistScope::ViewOnly) g_session->saveViewStateOnly(); - else g_session->saveToActiveProject(); + ReaProject* proj = EnumProjects(-1, nullptr, 0); + if (!proj) return true; // no active project — the write below no-ops on its own + + auto readRppPath = [&]() -> std::string { + std::vector buf(4096, '\0'); + EnumProjects(-1, buf.data(), static_cast(buf.size())); + return std::string(buf.data()); + }; + + if (!readRppPath().empty()) return true; + + Main_SaveProject(proj, true); + if (!readRppPath().empty()) return true; // still empty -> user cancelled + + ShowConsoleMsg("ReaSampler: Design View state will not persist until " + "the project is saved.\n"); + return false; +} + +// The persist's WRITE half: the bank + Design-View model, so neither is lost across +// save/close/reopen. Returns whether the write was ISSUED — false means no active +// project or an unsaved one, with NOTHING written (session.h owns that contract), which +// is the only reading a caller may turn into a discarded undo point. +bool writePersistedState(PersistScope scope) { + return scope == PersistScope::ViewOnly ? g_session->saveViewStateOnly() + : g_session->saveToActiveProject(); +} + +// The two halves as a caller holding no undo block runs them. +void persistViewState(PersistScope scope) { + if (ensurePersistTarget()) writePersistedState(scope); } // The footer segment already reads disabled while the transport runs, but an action can @@ -211,17 +220,53 @@ void doActivateMode(const std::string& modeId) { bankPanelInvalidate(); // repaint the footer [Arrange|Design] toggle immediately } -// The block each membership-edit action below owns. The reapply they drive opens none -// of its own — nothing it writes mints a point unaided — so a user-fired tag stays one -// Ctrl-Z only because the ACTION spans it, and the label names the act the user -// actually fired rather than the reapply it happened to trigger. Every one of them -// closes the block before persistViewState, which can pop a modal Save-As. +// The drive half of a membership edit: what runs inside the block, answering whether +// it made a project change of its own that a point must hold even with nothing +// persisted. The reapply alone never does — everything it writes is the four driven +// flags, which mint no undo state (src/shell/view/CLAUDE.md). +bool driveReapply() { + reapplyActiveMode(); + return false; +} + +// The item moves' drive: re-drive the same minting/apply path auto-tag uses — mint or +// split lanes for any track whose items now span modes, then reassert active-mode lane +// visibility. A mint moves items between lanes, which is a real project change whether +// or not the persist below lands. +bool driveMintLanesAndReapply() { + const bool minted = mintManagedLanes(g_session->view(), nullptr); + reapplyActiveMode(); + return minted; +} + +// The ONE body behind all four membership edits (this directory's CLAUDE.md owns the +// rule; what follows is why the ORDER is what it is). `mutate` is a pure model edit — +// nothing in it reaches REAPER. // -// The point is recorded only where the reapply actually wrote: re-tagging tracks that -// already sit in that mode writes nothing to the project, and the ("", 0) discard form -// UndoBlock closes on by default is real from a hand-fired action (DAW-measured, -// src/shell/view/CLAUDE.md). The membership move itself is never in the point either -// way — persistViewState runs after the close. +// The prompt sits between the edit and the block because it is pinned on both sides: +// its condition reads the POST-edit membership (a first-ever tag on an unsaved project +// must still prompt), and a modal may not stand inside an open block. The persist then +// runs INSIDE, being the only thing here a point can hold; a re-tag of tracks already +// in that mode re-writes byte-identical ext state, which is why the label turns on the +// model diff and not on the write. +template +void runMembershipEdit(Mutate&& mutate, bool (*drive)(), const std::string& label, + int mask) { + const MembershipIndex before = g_session->view().membership(); + mutate(); + const bool changed = !(g_session->view().membership() == before); + + const bool persistable = ensurePersistTarget(); + + UndoBlock undo(nullptr); + const bool structural = drive(); + const bool persisted = persistable && writePersistedState(PersistScope::Full); + if (structural || (changed && persisted)) + undo.recordPoint(label, mask); + // Otherwise the block closes on its ("", 0) discard form — real from a hand-fired + // action (DAW-measured, src/shell/view/CLAUDE.md) — leaving history and the Undo + // button both untouched. +} // tag() replaces any prior single-mode membership (a leaf lives in one mode; the // cross-mode case is show-both). @@ -229,13 +274,11 @@ void doTag(const std::string& modeId) { const std::vector guids = selectedTrackGuids(); if (guids.empty()) return; // nothing selected — no act, no undo point (cf. doMoveItems) - { - UndoBlock undo(nullptr); - for (const std::string& g : guids) g_session->view().membership().tag(g, modeId); - if (reapplyActiveMode()) - undo.recordPoint("ReaSampler: tag selected tracks -> " + modeId, kViewUndoMask); - } - persistViewState(PersistScope::Full); + runMembershipEdit( + [&] { + for (const std::string& g : guids) g_session->view().membership().tag(g, modeId); + }, + &driveReapply, "ReaSampler: tag selected tracks -> " + modeId, kViewUndoMask); } // Shared body behind "Untag selected" and "Tag -> Arrange" — Arrange is the absence @@ -244,13 +287,11 @@ void doUntag() { const std::vector guids = selectedTrackGuids(); if (guids.empty()) return; - { - UndoBlock undo(nullptr); - for (const std::string& g : guids) g_session->view().membership().untag(g); - if (reapplyActiveMode()) - undo.recordPoint("ReaSampler: untag selected tracks", kViewUndoMask); - } - persistViewState(PersistScope::Full); + runMembershipEdit( + [&] { + for (const std::string& g : guids) g_session->view().membership().untag(g); + }, + &driveReapply, "ReaSampler: untag selected tracks", kViewUndoMask); } // Flips each track's pin independently — the honest semantics of a toggle on a @@ -259,14 +300,12 @@ void doShowBoth() { const std::vector guids = selectedTrackGuids(); if (guids.empty()) return; - { - UndoBlock undo(nullptr); - MembershipIndex& m = g_session->view().membership(); - for (const std::string& g : guids) m.setShowBoth(g, !m.isShowBoth(g)); - if (reapplyActiveMode()) - undo.recordPoint("ReaSampler: toggle show-both on selected tracks", kViewUndoMask); - } - persistViewState(PersistScope::Full); + runMembershipEdit( + [&] { + MembershipIndex& m = g_session->view().membership(); + for (const std::string& g : guids) m.setShowBoth(g, !m.isShowBoth(g)); + }, + &driveReapply, "ReaSampler: toggle show-both on selected tracks", kViewUndoMask); } // Retag the current ITEM selection to `targetMode` (empty => untag -> Arrange @@ -281,28 +320,18 @@ void doMoveItems(const std::string& targetMode) { const std::vector ops = planItemRetag(selected, targetMode); if (ops.empty()) return; // nothing selected, or every selected item was exempt/empty - MembershipIndex& membership = g_session->view().membership(); - - { - UndoBlock undo(nullptr); - 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, then reassert active-mode lane visibility. - // Either half writing is enough to mint; a retag that split no lanes and drove - // no flag leaves nothing for a point to hold. - const bool minted = mintManagedLanes(g_session->view(), nullptr); - const bool reapplied = reapplyActiveMode(); - if (minted || reapplied) - undo.recordPoint(targetMode.empty() - ? std::string("ReaSampler: untag selected items") - : "ReaSampler: move selected items -> " + targetMode, - -1); - } - - persistViewState(PersistScope::Full); + runMembershipEdit( + [&] { + MembershipIndex& membership = g_session->view().membership(); + for (const ItemRetagOp& op : ops) { + if (op.untag) membership.untag(op.guid); + else membership.tag(op.guid, op.modeId); + } + }, + &driveMintLanesAndReapply, + targetMode.empty() ? std::string("ReaSampler: untag selected items") + : "ReaSampler: move selected items -> " + targetMode, + -1); } } // namespace diff --git a/src/shell/view/CLAUDE.md b/src/shell/view/CLAUDE.md index 79b86ae..4347589 100644 --- a/src/shell/view/CLAUDE.md +++ b/src/shell/view/CLAUDE.md @@ -94,14 +94,18 @@ decide membership or mode rules. because a block there could only manufacture the point a project open or a membership edit must not cost. This is why the same `I_FREEMODE` write is blocked at one site and not the other: minting SPLITS a track, `applyLaneOps` only re-states which lane plays. - Second clause, binding every site that does open one: **a block that wrote nothing - closes on the discard form** (`Undo_EndBlock2(proj, "", 0)` — real from a hand-fired - action, per the measurement above), so re-tagging tracks already in that mode leaves - no empty point. The one exception is a REAL SWITCH, which mints written state or not: - it is an explicitly fired action and has to stay undoable even where the plan found - nothing to write. "Wrote" means a REAPER write throughout — model moves persist - through `persistViewState`, outside every block, so no point could carry them - (`ModeApplyResult`, `view.h`). + Second clause, binding every site that does open one: **a block with nothing REAPER + can hold closes on the discard form** (`Undo_EndBlock2(proj, "", 0)` — real from a + hand-fired action, per the measurement above), so re-tagging tracks already in that + mode leaves no empty point and no relabelled Undo button. The one exception is a REAL + SWITCH, which mints written state or not: it is an explicitly fired action and has to + stay undoable even where the plan found nothing to write. "Nothing REAPER can hold" is + NOT "no REAPER write", and reading it that way was a shipped defect: the four driven + flags ARE a REAPER write and mint nothing, so a site predicated on writes names points + REAPER then declines to append — history unchanged, Undo button relabelled. What a + membership edit's point holds is its `view_state` write, which is why that write is + INSIDE the block and the predicate is the model diff + (`shell/actions/design_view_actions`' `runMembershipEdit`). - **ONE mode switch is ONE undo point — the background FX parking included.** The driven flags, the lane state, AND the per-FX offline/online the reclaimer writes minutes-of-plugin-loads later all roll back in a single Ctrl-Z, because they share @@ -222,8 +226,9 @@ decide membership or mode rules. `render_in_place`'s inner reapply. The open half is the criterion above (a reapply re-asserts derived state and is nobody's act); the close half is the nesting rule above (a reapply runs inside its driver's block, so a supersede-close from there - would land at depth 2->1 and hijack it). It reports what it wrote instead, and the - driver holding the block decides — `ModeApplyResult::wroteProjectState`. + would land at depth 2->1 and hijack it). It reports only whether it `applied`; a + driver holding a block decides its point from its OWN model diff, never from what the + reapply wrote, and `ModeApplyResult` no longer offers that answer to be misread. - **A rapid A→B→A flip moves zero plugins.** The reclaimer only starts a park's OFFLINE work once a track has read continuously parked for `kParkHoldMs` (3000ms). A flip inside that window flips the flags twice but the debounce @@ -289,7 +294,7 @@ applies the resulting lane state to live tracks. ## Modules -- `view` — Design View shell: runs the pure planner over the live folder tree and iterates its park/restore plans, applies managed-lane and parent-visibility writes, and on a REAL SWITCH ends any prior span, raises the reclaimer's wakeup, and opens the ONE undo span all of that rides (`fx_reclaim` is what closes it; a reapply opens none and closes none). Reports what it wrote (`ModeApplyResult`) so a caller holding its own block can discard an empty point. Owns the one discriminator (`target != active`) that separates a real switch from a reapply, and with it both the playback gate (`transportBlocksModeSwitch`) and the solo cache/clear/restore seams. **Never touches master or `B_MUTE`.** FX are NOT written here — see `fx_reclaim`. +- `view` — Design View shell: runs the pure planner over the live folder tree and iterates its park/restore plans, applies managed-lane and parent-visibility writes, and on a REAL SWITCH ends any prior span, raises the reclaimer's wakeup, and opens the ONE undo span all of that rides (`fx_reclaim` is what closes it; a reapply opens none and closes none). Reports whether it applied and nothing else (`ModeApplyResult`) — what it wrote is deliberately not an answer it gives, because none of it mints. Owns the one discriminator (`target != active`) that separates a real switch from a reapply, and with it both the playback gate (`transportBlocksModeSwitch`) and the solo cache/clear/restore seams. **Never touches master or `B_MUTE`.** FX are NOT written here — see `fx_reclaim`. - `view_park` — the park surface, FLAG DOMAIN ONLY. `parkTrack` captures the pre-park snapshot (flags AND the FX chain's identity-keyed offline state) when the model holds none, drives the four flags, and EXPOSEs a track that reads park-shaped with no record behind it rather than adopting it; `restoreTrack` writes the flags back from the snapshot and spends its flag half (`releaseSnapshotFlags`). Also `trackFlagParm` (the one `Flag` → REAPER-parameter mapping), `writeIfChanged` (the read-before-write every driven parameter goes through), `fxGuidString` (the one FX-identity read snapshot/reclaim both address FX through), `kViewUndoMask`, and `reportExposedTracks`/`describeExposedTracks` (named tracks, once per changed set per project). - `view_lanes` — managed fixed lanes: `mintManagedLanes` splits multi-mode content onto per-mode lanes, `applyLaneOps` drives `C_LANEPLAYS` for the active mode, `reconcileManagedLanes` recovers ownership from durable `P_LANENAME` on project open. Manual lanes are never minted, driven, renamed or reassigned. - `view_reconcile` — the repair pass for a project state REAPER reloaded under us: `noteAppliedState` records what an apply left driven; `reconcileAppliedFlags` diffs that fingerprint against the reloaded model and hands back any track the reload no longer records but still reads driven — the fix for `UNDO_STATE_TRACKCFG` not carrying `B_SHOWINTCP`/`B_SHOWINMIXER` (DAW-confirmed). It opens no undo block: it runs in the wake of a Ctrl-Z, and its writes are flags only. @@ -339,10 +344,11 @@ applies the resulting lane state to live tracks. unsaved project — land inside the block after all. That is a consequence of the span, not a decision made here; it does not resolve the redo hypothesis below, which turns on capture-time semantics the SDK does not document. `[verify — DAW]` - that a Save-As dialog opening under an open undo block is harmless. On the reapply - path the write stays OUTSIDE every block, as it always did: the reapply opens none, - and a membership action's own `ActionUndoBlock` closes before `persistViewState` for - exactly this modal-Save-As reason. The UNDO direction still comes + that a Save-As dialog opening under an open undo block is harmless. On the MEMBERSHIP + path the write is now deliberately inside the action's own `UndoBlock` — it is the + only thing there a point can hold — and the persist is split so its modal Save-As half + runs BEFORE that block opens (`shell/actions/design_view_actions`, which owns the + reasoning). The UNDO direction still comes out right — the point closed over the ext state as it stood BEFORE the switch, which is what a Ctrl-Z should restore — but by position, not by design. The REDO direction is what this leaves open, and the cost is worse than the @@ -363,9 +369,13 @@ applies the resulting lane state to live tracks. semantics for the state mask; (2) that a redo fires `BeginLoadProjectState` with `isUndo == true` so the session really does reload the stale ext state (asserted in `main.cpp`, never observed). If (2) is false the in-memory model - keeps its snapshots and the redo is consistent. `[verify — DAW]`. Do not "fix" - it by moving `persistViewState` inside the block — it can open a modal Save-As; - whether to split the persist is a separate decision. + keeps its snapshots and the redo is consistent. `[verify — DAW]`. Link (1) is the + same capture-time reading the membership path's fix now rests on, and the nearest + thing to evidence for it is `reaper_plugin.h:1198-1201`: REAPER serializes an + extension's state for a point through `SaveExtensionConfig` with `isUndo` set — i.e. + at point creation, not later. Settling it by moving the SWITCH path's + `persistViewState` is still the wrong move: that one runs with the span open, so its + modal would stand inside a block nothing closes until the drain converges. - A DIFFERENT strand entirely, which the EXPOSE report does NOT cover: an absent or malformed `view_state` (`loadViewModel`, `ext_state_io.cpp`) is not a parked-chain reading at all — it falls back to a DEFAULT model (active mode diff --git a/src/shell/view/view.cpp b/src/shell/view/view.cpp index abedf5d..7a2fe9a 100644 --- a/src/shell/view/view.cpp +++ b/src/shell/view/view.cpp @@ -116,7 +116,6 @@ ModeApplyResult applyModeImpl(ViewModeModel& model, const std::string& targetMod (targetMode ? targetMode->displayName : targetModeId) + " view"; bool laneModeChanged = false; - bool wrote = false; // REAPER writes only — see ModeApplyResult std::vector exposedNames; // named while handles are fresh, reported after the writes // ONLY A REAL SWITCH, both halves — end a prior switch's still-open span, then open @@ -155,7 +154,6 @@ ModeApplyResult applyModeImpl(ViewModeModel& model, const std::string& targetMod MediaTrack* tr = resolve(trackByGuid, guid); if (!tr) continue; // stale/deleted GUID — prune const TrackParkResult r = parkTrack(model, guid, tr, tp.flags); - if (r.wrote) wrote = true; // Named here, not after the block: TrackList_AdjustWindows below can // invalidate the handle. if (r.exposed) exposedNames.push_back(trackDisplayName(tr)); @@ -166,14 +164,13 @@ ModeApplyResult applyModeImpl(ViewModeModel& model, const std::string& targetMod const std::string& guid = tp.flags.front().guid; MediaTrack* tr = resolve(trackByGuid, guid); if (!tr) continue; // stale/deleted GUID — prune - if (restoreTrack(model, guid, tr, tp.flags)) wrote = true; + restoreTrack(model, guid, tr, tp.flags); } // MANAGED LANES: the active mode's lane plays+shows, every other managed lane // is silenced+hidden. Empty on a D1-only project — byte-identical there. const LaneApplyResult lanes = applyLaneOps(trackByGuid, plan.lanes); laneModeChanged = lanes.freeMode; - if (lanes.wrote) wrote = true; // PARENT VISIBILITY (never parked): recomputed every toggle, never recorded. // Only the two visibility flags — never mainSend/FX on a parent. @@ -183,8 +180,8 @@ ModeApplyResult applyModeImpl(ViewModeModel& model, const std::string& targetMod MediaTrack* tr = resolve(trackByGuid, node.guid); if (!tr) continue; // stale GUID — prune const double show = visible.count(node.guid) ? 1.0 : 0.0; - if (writeIfChanged(tr, "B_SHOWINTCP", show)) wrote = true; - if (writeIfChanged(tr, "B_SHOWINMIXER", show)) wrote = true; + writeIfChanged(tr, "B_SHOWINTCP", show); + writeIfChanged(tr, "B_SHOWINMIXER", show); } model.setActiveMode(targetModeId); @@ -225,7 +222,7 @@ ModeApplyResult applyModeImpl(ViewModeModel& model, const std::string& targetMod // an undo rolling the model back underneath it. In memory, never on disk. noteAppliedState(project, model); - return ModeApplyResult{true, wrote}; + return ModeApplyResult{true}; } } // namespace diff --git a/src/shell/view/view.h b/src/shell/view/view.h index 3df9b7b..933d377 100644 --- a/src/shell/view/view.h +++ b/src/shell/view/view.h @@ -29,17 +29,16 @@ bool modeApplyInProgress(); // What one applyMode did. // -// `wroteProjectState` counts REAPER writes ONLY — a driven flag, a lane parameter, -// a parent's visibility. Model moves (a snapshot stored, a record's flag half spent, -// the active mode set) are deliberately excluded: they reach disk through -// persistViewState, which every caller runs OUTSIDE its undo block, so no point can -// carry them and counting them would only mint an empty one. It is the answer a -// caller holding its own block needs for the discard decision (design_view_actions' -// ActionUndoBlock); the solo pair cannot affect it, since only a real switch drives -// solo and a real switch mints regardless. +// Deliberately does NOT report whether it wrote project state. It once did, and a +// caller holding its own block used that as its mint-or-discard predicate — which is +// wrong twice over: everything a reapply writes is the four driven flags, which mint +// no undo state at all, so the answer was true for writes REAPER will not hold and the +// resulting label re-titled the Undo button over an entry that was never appended. What +// a membership edit's point actually holds is its `view_state` write; the caller's own +// model diff is what says whether there was one (design_view_actions' +// runMembershipEdit). Do not reintroduce a wrote-project-state return here. struct ModeApplyResult { - bool applied = false; - bool wroteProjectState = false; + bool applied = false; }; // Captures each about-to-park track's prior flag values into `model`, caches/clears diff --git a/src/shell/view/view_lanes.h b/src/shell/view/view_lanes.h index b8adcc6..ca0bdd5 100644 --- a/src/shell/view/view_lanes.h +++ b/src/shell/view/view_lanes.h @@ -15,8 +15,8 @@ class ReaProject; namespace reasampler { // `freeMode` alone decides whether UpdateTimeline is owed (SDK: required after an -// I_FREEMODE change); `wrote` is a REAPER write, the reading ModeApplyResult -// (view.h) carries through to the caller's undo-point decision. +// I_FREEMODE change); `wrote` is a REAPER write and nothing more — not an undo-point +// predicate (these writes mint no undo state), and read by nothing today. struct LaneApplyResult { bool freeMode = false; bool wrote = false; diff --git a/src/shell/view/view_park.h b/src/shell/view/view_park.h index fa639f6..78d0106 100644 --- a/src/shell/view/view_park.h +++ b/src/shell/view/view_park.h @@ -93,13 +93,11 @@ extern const int kViewUndoMask; // Returns whether it wrote. bool writeIfChanged(MediaTrack* tr, const char* parm, double value); -// What one parkTrack call did. `wrote` is a REAPER write — a driven flag moved — -// and nothing else: storing the snapshot is a MODEL move, and the model reaches disk -// outside every undo block, so counting it would mint a point carrying none of it -// (ModeApplyResult, view.h). The two readings part company on exactly one path — a -// RE-PARK whose restore had already handed the flags back at park-shaped values, which -// stores a fresh snapshot and writes no flag — and the REAPER reading is the one that -// wants: nothing in the project moved, so nothing is there for a point to hold. +// What one parkTrack call did. `wrote` is a REAPER write — a driven flag moved — and +// nothing else; storing the snapshot is a MODEL move and does not count. It is NOT an +// undo-point predicate and never was a sound one: the driven flags mint no undo state, +// so no point can hold what this reports (view.h's ModeApplyResult, which used to carry +// it up, no longer does). Nothing reads it today. // `exposed` is true iff this call handed the track back visible instead of parking or // adopting it; the caller (view.cpp) collects exposed tracks across the whole apply for // the one batched console report.