Split unidentified from missing FX in Design View drop report, fix message + console pop

Distinguish no-GUID-at-capture from identity-no-longer-live; rewrite the drop
message to state the real recovery step; mark FX-GUID stability [verify — DAW];
guard mismatched fx/fxOffline lengths; the report never force-opens now.
This commit is contained in:
2026-08-02 18:51:41 -04:00
parent 5f6efb7cc3
commit d5ed4f6e53
7 changed files with 109 additions and 31 deletions
+1 -1
View File
@@ -97,7 +97,7 @@ settled 2026-07-23):
## Modules
- `view_mode_model` — Design View mode system: mode registry, GUID-keyed membership, folder-tree-aware visibility derivation, snapshot-based park/restore planner, the per-mode `SoloCache` it owns, JSON round-trip.
- `fx_offline` — the per-FX offline snapshot's KEY and its restore resolution: `FxKeying` (Identity / Slot), `FxOfflineState`, the planned `FxOfflineOp`, and `resolveFxRestore`, which matches each captured state to the FX it came from against the chain as it stands at restore time. An identity that is no longer live is DROPPED and counted (`FxRestoreDrops`, reported through `describeFxRestoreDrops`), never re-pointed at a slot — the slot fallback is precisely the reorder bug identity keying exists to close. Slot keying survives only for snapshots lifted from a pre-identity `view_state` and for park plans, where every live slot is the target by construction.
- `fx_offline` — the per-FX offline snapshot's KEY and its restore resolution: `FxKeying` (Identity / Slot), `FxOfflineState`, the planned `FxOfflineOp`, and `resolveFxRestore`, which matches each captured state to the FX it came from against the chain as it stands at restore time. An identity that is no longer live is DROPPED and counted (`FxRestoreDrops`, reported through `describeFxRestoreDrops`), never re-pointed at a slot — see the `FxKeying` and `resolveFxRestore` comments in `fx_offline.h` for why. Slot keying survives only for snapshots lifted from a pre-identity `view_state` and for park plans, where every live slot is the target by construction.
- `solo_cache` — the per-mode solo surface: `SoloCache` (mode id → GUID → raw `I_SOLO`), the soloed-subset filter, and `planSoloRestore`, whose two drop rules (dead GUID, not visible in the incoming mode) and their reasoning live in its header.
- `view_tree` — pure `I_FOLDERDEPTH`→FolderTree helper for the Design View shell.
- `mode_switch` — REAPER-free segment layout + hit-test for the bank_panel's Design View mode switch.
+32 -9
View File
@@ -1,5 +1,6 @@
#include "core/view/fx_offline.h"
#include <cstddef>
#include <map>
namespace reasampler {
@@ -28,10 +29,16 @@ FxRestoreResolution resolveFxRestore(const std::vector<FxOfflineOp>& planned,
for (const FxOfflineOp& op : planned) {
if (op.keying == FxKeying::Identity) {
// An empty identity (REAPER reported none at capture) is unresolvable
// like any other miss — it does not get to use its slot instead.
if (op.fxGuid.empty()) {
// REAPER reported no GUID at capture time — distinct from a real
// captured identity going missing: the FX may still be live, we
// just never had a name for it. Counted separately so the report
// never claims it was deleted (see describeFxRestoreDrops).
++res.drops.unidentified;
continue;
}
auto it = byGuid.find(op.fxGuid);
if (op.fxGuid.empty() || it == byGuid.end()) {
if (it == byGuid.end()) {
++res.drops.missingIdentity;
continue;
}
@@ -53,16 +60,32 @@ std::string describeFxRestoreDrops(const FxRestoreDrops& drops, int trackCount)
std::string msg = "ReaSampler: Design View restore dropped " +
std::to_string(drops.total()) + " captured FX offline state(s) on " +
std::to_string(trackCount) + " track(s) -- ";
std::vector<std::string> clauses;
if (drops.missingIdentity > 0) {
msg += std::to_string(drops.missingIdentity) +
" FX no longer in the chain (deleted or replaced while parked)";
clauses.push_back(std::to_string(drops.missingIdentity) +
" FX no longer in the chain (deleted or replaced while parked)");
}
if (drops.unidentified > 0) {
clauses.push_back(std::to_string(drops.unidentified) +
" FX REAPER could not identify at capture time (no GUID reported), "
"so it could not be matched now");
}
if (drops.slotOutOfRange > 0) {
if (drops.missingIdentity > 0) msg += ", ";
msg += std::to_string(drops.slotOutOfRange) +
" from a project saved before FX identity was recorded, whose slot no longer exists";
clauses.push_back(std::to_string(drops.slotOutOfRange) +
" from a project saved before FX identity was recorded, whose slot no longer exists");
}
msg += ". Every other FX was left as it was.\n";
for (std::size_t i = 0; i < clauses.size(); ++i) {
if (i) msg += ", ";
msg += clauses[i];
}
// Every dropped entry is still sitting exactly where park left it — offline
// — because the restore that would have flipped it back never ran. Say
// that, not the reassuring-but-wrong "left as it was" (park itself was the
// change; restore is what didn't happen for these).
msg += ". Each was left offline, as park left it, with no snapshot left to "
"restore it -- switch it back on by hand.\n";
return msg;
}
+10 -7
View File
@@ -27,9 +27,9 @@ struct FxOfflineState {
}
};
// One per-FX offline write as PLANNED. The keying travels with the op so an
// entry whose identity is missing can never silently degrade into a slot write
// — that fallback is the reorder bug this keying exists to close.
// One per-FX offline write as PLANNED. The keying travels with the op; see
// FxKeying above and resolveFxRestore below for why a missing identity is
// never re-pointed at a slot.
struct FxOfflineOp {
std::string guid; // track GUID
FxKeying keying = FxKeying::Identity;
@@ -53,21 +53,24 @@ struct FxOfflineWrite {
}
};
// Captured state a restore could not apply. Both counts mean the same act: the
// entry was dropped and no FX was written in its place.
// Captured state a restore could not apply. All three counts mean the same
// act: the entry was dropped and no FX was written in its place.
struct FxRestoreDrops {
int missingIdentity = 0; // identity-keyed entry with no live FX carrying that GUID
int unidentified = 0; // identity-keyed entry whose captured fxGuid was itself empty
int slotOutOfRange = 0; // slot-keyed entry whose capture-time slot no longer exists
int total() const { return missingIdentity + slotOutOfRange; }
int total() const { return missingIdentity + unidentified + slotOutOfRange; }
void add(const FxRestoreDrops& o) {
missingIdentity += o.missingIdentity;
unidentified += o.unidentified;
slotOutOfRange += o.slotOutOfRange;
}
bool operator==(const FxRestoreDrops& o) const {
return missingIdentity == o.missingIdentity && slotOutOfRange == o.slotOutOfRange;
return missingIdentity == o.missingIdentity && unidentified == o.unidentified &&
slotOutOfRange == o.slotOutOfRange;
}
};
+13 -2
View File
@@ -197,7 +197,10 @@ TrackPlan makeParkPlan(const std::string& guid, int fxCount) {
{guid, Flag::FxEnable, 0},
};
// Park has no identity question to answer: it offlines every slot that is
// live right now, so slot keying IS the addressing.
// live right now, so slot keying IS the addressing. In production fxCount
// is always 0 here — planToggle calls this with fxCount=0 and the D2 shell
// expands the real writes itself via TrackFX_GetCount (parkFxOffline in
// shell/view/view.cpp); a nonzero fxCount only exercises this loop in tests.
for (int i = 0; i < fxCount; ++i)
p.fxOffline.push_back(FxOfflineOp{guid, FxKeying::Slot, {}, i, true});
return p;
@@ -619,6 +622,7 @@ bool parseSnapshots(json::Reader& r, std::map<std::string, TrackSnapshot>& snaps
TrackSnapshot snap;
bool haveGuid = false;
std::vector<int> bySlot;
bool haveSlot = false;
std::vector<FxOfflineState> byIdentity;
bool haveIdentity = false;
do {
@@ -629,7 +633,7 @@ bool parseSnapshots(json::Reader& r, std::map<std::string, TrackSnapshot>& snaps
else if (k == "showInMixer") { if (!r.parseInt(snap.showInMixer)) return false; }
else if (k == "mainSend") { if (!r.parseInt(snap.mainSend)) return false; }
else if (k == "fxEnable") { if (!r.parseInt(snap.fxEnable)) return false; }
else if (k == "fxOffline") { if (!r.parseIntArray(bySlot)) return false; }
else if (k == "fxOffline") { if (!r.parseIntArray(bySlot)) return false; haveSlot = true; }
else if (k == "fx") {
if (!parseFxStates(r, byIdentity)) return false;
haveIdentity = true;
@@ -639,6 +643,13 @@ bool parseSnapshots(json::Reader& r, std::map<std::string, TrackSnapshot>& snaps
if (!r.consume('}')) return false;
if (!haveGuid || guid.empty()) return false;
// A blob carrying both arrays at different lengths is not something this
// writer (or any prior version) produces — reject rather than silently
// trusting "fx" over a slot array that disagrees with it; an unreadable
// view_state falls back to a default model per the version-ladder note
// above, which is the same leniency-direction call already made there.
if (haveIdentity && haveSlot && byIdentity.size() != bySlot.size()) return false;
// "fx" wins outright — v2 writes the slot array beside it purely so an
// older build can still read something (see the version ladder above).
if (haveIdentity) {
+9
View File
@@ -103,3 +103,12 @@ applies the resulting lane state to live tracks.
under whatever mode id is active at that point, not the one the user undid back
to. Pre-existing: `snapshots_` already carries this same model-vs-undo split;
the solo cache inherits it rather than introducing it. Not fixed here.
- `fx_offline`'s identity keying (`TrackFX_GetFXGUID`) assumes the GUID stays
attached to its plugin across a chain mutation while parked. That is
`[verify — DAW]` (see `fxGuidString` in `view.cpp`) and SWS issue #802 is a
known reason it might not hold: `SNM_MoveOrRemoveTrackFX` reportedly leaves
the FXID lines behind on reorder rather than moving them with the plugin. If
confirmed, an SWS-driven reorder of a parked track's chain — not a native
drag-reorder — can produce wrong-plugin restores or mass drops through
`resolveFxRestore`. Do not design around this pre-emptively; if native
reorder is clean (the likely case), only the SWS path degrades.
+21 -3
View File
@@ -129,7 +129,18 @@ MediaTrack* resolve(const std::vector<std::pair<std::string, MediaTrack*>>& hand
// The FX's own durable identity, braced exactly like the track GUID keys. Empty
// when REAPER reports none — an FX we cannot name is one we cannot restore, and
// fx_offline treats it that way rather than guessing at its slot.
// fx_offline treats it that way rather than guessing at its slot. Lifetime is
// settled: the string copy is taken immediately and the GUID* is never held
// past this call (reaper_plugin_functions.h:7348 documents no null contract for
// TrackFX_GetFXGUID; treating null as "no identity" is the safe read).
//
// [verify — DAW] STABILITY across a chain mutation is not settled the same way:
// confirm the GUID for one FX instance survives a native drag-reorder, an SWS
// move (SNM_MoveOrRemoveTrackFX — SWS issue #802 reports the FXID lines do not
// follow the plugin after that call, i.e. wrong-plugin restores or mass drops
// through fx_offline on that path specifically), a save/reload round trip, and
// two live instances of one plugin type staying distinguishable. See
// src/shell/view/CLAUDE.md's Gotchas for the SWS-path risk this leaves open.
std::string fxGuidString(MediaTrack* tr, int fx) {
GUID* g = TrackFX_GetFXGUID(tr, fx);
if (!g) return {};
@@ -491,9 +502,16 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
// Captured FX state that could not be applied is REPORTED. Silence here would
// read to the user as "restore worked" while an FX sat at whatever state the
// park left it in.
// park left it in. Sent with the "!SHOW:" prefix (reaper_plugin_functions.h:6536)
// so it never force-opens the console window: applyMode's reapply path also
// runs unattended on project load (see the reconcile comment above), and this
// one call site can't tell that case apart from an interactive toggle/tag-edit
// reapply — both call in with target == active — so splitting loud-on-toggle
// from quiet-on-load would need a flag threaded from every caller, several of
// which are outside this change. Quiet-always is the safe default: the message
// still lands in the console for whoever opens it, on every path.
const std::string fxDropMsg = describeFxRestoreDrops(fxDrops, fxDropTracks);
if (!fxDropMsg.empty()) ShowConsoleMsg(fxDropMsg.c_str());
if (!fxDropMsg.empty()) ShowConsoleMsg(("!SHOW:" + fxDropMsg).c_str());
// MANAGED LANES: drive C_LANEPLAYS so the active mode's lane plays+shows
// and every other managed lane is silenced+hidden. Empty for a D1-only
+23 -9
View File
@@ -82,19 +82,22 @@ static void testReorderedChainRestoresEachPluginItsOwnState() {
// -- 2. FX deleted while parked ----------------------------------------------
static void testDeletedFxDropsExplicitlyAndTouchesNothingElse() {
// B (true) and C (false) carry OPPOSITE captured states — the discriminator.
// A test where both carried `true` couldn't tell "C's own state followed it
// to slot 1" apart from "B's dropped state leaked onto whatever moved there".
const std::vector<FxOfflineOp> ops =
identityOps({{"{A}", false}, {"{B}", true}, {"{C}", true}});
identityOps({{"{A}", false}, {"{B}", true}, {"{C}", false}});
// B was deleted while parked; A and C closed the gap.
const FxRestoreResolution res = resolveFxRestore(ops, {"{A}", "{C}"});
CHECK(res.writes.size() == 2);
CHECK(res.drops.missingIdentity == 1);
CHECK(res.drops.unidentified == 0);
CHECK(res.drops.slotOutOfRange == 0);
CHECK(hasWrite(res, 0, false)); // A
CHECK(hasWrite(res, 1, true)); // C followed its identity to slot 1
// B's captured `true` did NOT land on whatever moved into slot 1.
CHECK(res.writes.size() == 2);
// C's OWN captured `false` landed at slot 1, not B's dropped `true`.
CHECK(hasWrite(res, 1, false));
// The drop is reportable, not silent.
const std::string msg = describeFxRestoreDrops(res.drops, /*trackCount=*/1);
@@ -107,15 +110,17 @@ static void testDeletedFxDropsExplicitlyAndTouchesNothingElse() {
CHECK(describeFxRestoreDrops(FxRestoreDrops{}, 0).empty());
}
static void testDescribeNamesBothDropKinds() {
static void testDescribeNamesAllThreeDropKinds() {
FxRestoreDrops drops;
drops.missingIdentity = 2;
drops.unidentified = 1;
drops.slotOutOfRange = 3;
CHECK(drops.total() == 5);
CHECK(drops.total() == 6);
const std::string msg = describeFxRestoreDrops(drops, /*trackCount=*/2);
CHECK(msg.find("5 captured FX offline state(s) on 2 track(s)") != std::string::npos);
CHECK(msg.find("6 captured FX offline state(s) on 2 track(s)") != std::string::npos);
CHECK(msg.find("2 FX no longer in the chain") != std::string::npos);
CHECK(msg.find("1 FX REAPER could not identify at capture time") != std::string::npos);
CHECK(msg.find("3 from a project saved before FX identity was recorded")
!= std::string::npos);
}
@@ -180,6 +185,10 @@ static void testSlotKeyedSnapshotRestoresByPositionAndBoundsChecks() {
static void testUnresolvableIdentityNeverFallsBackToItsSlot() {
// An identity-keyed entry with NO identity (REAPER reported none at capture)
// is a drop — the slot it happens to carry must not be used as a substitute.
// Counted as `unidentified`, not `missingIdentity`: unlike a real captured
// identity going missing, this FX may still be sitting right there — we
// simply never had a name for it, and the report must say that, not "no
// longer in the chain".
std::vector<FxOfflineOp> ops = identityOps({{"{A}", false}});
ops.push_back(FxOfflineOp{"{TRACK}", FxKeying::Identity, "", /*slot=*/1, /*offline=*/true});
@@ -187,7 +196,12 @@ static void testUnresolvableIdentityNeverFallsBackToItsSlot() {
CHECK(res.writes.size() == 1);
CHECK(hasWrite(res, 0, false));
CHECK(!writesTouch(res, 1)); // {B} would have been the slot-1 victim
CHECK(res.drops.missingIdentity == 1);
CHECK(res.drops.missingIdentity == 0);
CHECK(res.drops.unidentified == 1);
const std::string msg = describeFxRestoreDrops(res.drops, /*trackCount=*/1);
CHECK(msg.find("1 FX REAPER could not identify at capture time") != std::string::npos);
CHECK(msg.find("no longer in the chain") == std::string::npos); // not this FX's story
}
static void testLiveFxWithNoIdentityIsNeverARestoreTarget() {
@@ -225,7 +239,7 @@ static void testDuplicateLiveIdentityResolvesToTheFirstSlotOnly() {
int main() {
testReorderedChainRestoresEachPluginItsOwnState();
testDeletedFxDropsExplicitlyAndTouchesNothingElse();
testDescribeNamesBothDropKinds();
testDescribeNamesAllThreeDropKinds();
testAddedFxIsNotTouched();
testTwoInstancesOfOnePluginKeyIndependently();
testSlotKeyedSnapshotRestoresByPositionAndBoundsChecks();