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.
This commit is contained in:
2026-08-07 19:51:37 -04:00
parent 20fef2a3bf
commit d5ff476a19
7 changed files with 177 additions and 132 deletions
+28 -18
View File
@@ -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