diff --git a/src/core/instrument/bake/CLAUDE.md b/src/core/instrument/bake/CLAUDE.md index 6f25a7e..4656b90 100644 --- a/src/core/instrument/bake/CLAUDE.md +++ b/src/core/instrument/bake/CLAUDE.md @@ -51,6 +51,9 @@ decision about what the render made obsolete. - **Play mode resets to TRIGGER, not to the value struct's Gate default** — the one classification this track made against the ratified rule rather than reading off it. `bake_reset.cpp` carries the argument at the assignment. +- **`kStageTimeMaxSeconds` (the stage-time ceiling `param_taper` owns) is not a reset-list + candidate at all** — it bounds a knob's taper, is never itself a dialed value, and so has + no disposition to classify against the ratified reset rule. ## Modules diff --git a/tests/test_bake_render.cpp b/tests/test_bake_render.cpp index 5812064..4be2fc1 100644 --- a/tests/test_bake_render.cpp +++ b/tests/test_bake_render.cpp @@ -232,10 +232,12 @@ int main() { } // --- Regression baseline: the neutral render is the source, sample for sample -------- - // DERIVED rather than recorded, so it survives a compiler change: a Trigger voice at its - // own root under Varispeed, with the default flat amp and velocity shapes and no filter, - // reads at ratio exactly 1 — so every printed frame must BE its source frame. An added - // stage, a moved default, or a lost early-out anywhere in the chain moves a sample here. + // A Trigger voice at its own root under Varispeed reads at ratio exactly 1 and hits no + // filter, so every printed frame equals its source frame PROVIDED the amp curve's gain at + // the plan's velocity is exactly 1.0 too (asserted below rather than assumed) — that exact + // value is a property of flat()'s two endpoints cancelling at velocity 100, not a + // guarantee of eval() at an arbitrary velocity. An added stage, a moved default, or a lost + // early-out anywhere in the chain moves a sample here. { SampleData s; s.frames.resize(4000); @@ -249,6 +251,7 @@ int main() { // Shorter than the play span, so the window closes before any note-end shaping. const BakePlan plan = planOf(/*total=*/1000, /*noteOn=*/0, /*noteOff=*/1000); + CHECK(s.velocityCurve.eval(100.0) == 1.0); // names the real cause if this ever fails const BakeAudio audio = renderBake(s, plan, kUnity); CHECK(audio.channelCount == 1); CHECK(audio.frameCount() == 1000); diff --git a/tests/test_bake_reset.cpp b/tests/test_bake_reset.cpp index 23d0f29..157b4f4 100644 --- a/tests/test_bake_reset.cpp +++ b/tests/test_bake_reset.cpp @@ -1,12 +1,9 @@ // Standalone tests for reasampler::instrument::bake::bake_reset — no VST3, no REAPER, no // framework. Same fast assert loop as the sibling pure tests. // -// Covers the ratified reset scope PER PARAMETER, in both directions: every control whose -// effect the render printed comes back at its default, and every mapping fact comes back -// untouched. Asserted field by field rather than by struct equality on purpose — a -// whole-struct compare would pass while silently resetting a survivor, or vice versa. The -// sweep runs over TWO independently dialed fixtures that share only the survivors, which is -// what makes it a property of the survivors alone rather than of one input. +// The ratified reset scope itself is bake/CLAUDE.md's; this sweep checks it field by field +// (never struct equality, which would pass while silently resetting a survivor) over TWO +// independently dialed fixtures, so a pass is a property of the survivors, not of one input. #include "../src/core/instrument/bake/bake_reset.h" @@ -14,6 +11,7 @@ // reset loop LOOKS like on the band is the thing worth asserting, and only that module says. #include "../src/core/instrument/ui/loop_marks.h" +#include #include #include @@ -65,6 +63,9 @@ InstrumentParams dialed() { p.play.pitchEnv.peakSemitones = -7.0; p.play.pitchEnv.shape.attackSeconds = 0.05; p.play.pitchEnv.shape.decaySeconds = 0.15; + p.play.pitchEnv.shape.holdFraction = 0.6; + p.play.pitchEnv.shape.attackCurve = 2.1; + p.play.pitchEnv.shape.decayCurve = 0.4; p.play.pitchVelocityCurve = VelocityCurve::linear(); p.play.playRate = 0.5; p.play.pitchOffsetSemitones = -7.5; @@ -75,7 +76,18 @@ InstrumentParams dialed() { p.play.filter.velAmount = 0.6; p.play.filter.keyTrack = 1.5; p.play.filter.env.attackSeconds = 0.7; + p.play.filter.env.holdSeconds = 0.15; + p.play.filter.env.decaySeconds = 0.35; + p.play.filter.env.sustainLevel = 0.25; + p.play.filter.env.releaseSeconds = 0.55; + p.play.filter.env.attackCurve = 2.3; + p.play.filter.env.decayCurve = 0.5; + p.play.filter.env.releaseCurve = 2.8; + p.play.filter.trigEnv.attackSeconds = 0.12; p.play.filter.trigEnv.decaySeconds = 0.8; + p.play.filter.trigEnv.holdFraction = 0.44; + p.play.filter.trigEnv.attackCurve = 1.6; + p.play.filter.trigEnv.decayCurve = 0.6; p.play.filter.velocityCurve = VelocityCurve::linear(); p.play.ampSpline.mode = EnvMode::Spline; p.play.ampSpline.contour = VelocityCurve::linear(); @@ -92,8 +104,8 @@ InstrumentParams dialed() { // every other field — including play mode, which is a chosen neutral rather than a survivor. // Both run the same neutral sweep: if the reset ever inverted into "copy the dialed set, then // clear a blacklist", a non-survivor would come through and at most one fixture could still -// land on the defaults. A parameter added later is covered here by moving it in BOTH -// fixtures, which is the same work as adding it to the sweep. +// land on the defaults. The inherent limit this does NOT cover: nothing forces a newly added +// `InstrumentParams` field to be dialled in either fixture at all, let alone asserted. InstrumentParams dialedOther() { InstrumentParams p; p.rootOverride = 43; // survivor — same as dialed() @@ -123,6 +135,9 @@ InstrumentParams dialedOther() { p.play.pitchEnv.peakSemitones = 11.0; p.play.pitchEnv.shape.attackSeconds = 0.25; p.play.pitchEnv.shape.decaySeconds = 0.35; + p.play.pitchEnv.shape.holdFraction = 0.2; + p.play.pitchEnv.shape.attackCurve = 0.5; + p.play.pitchEnv.shape.decayCurve = 2.4; p.play.pitchVelocityCurve = VelocityCurve::rampDown(); p.play.playRate = 1.75; p.play.pitchOffsetSemitones = 3.25; @@ -133,7 +148,18 @@ InstrumentParams dialedOther() { p.play.filter.velAmount = -0.35; p.play.filter.keyTrack = -0.9; p.play.filter.env.attackSeconds = 1.7; + p.play.filter.env.holdSeconds = 0.95; + p.play.filter.env.decaySeconds = 0.75; + p.play.filter.env.sustainLevel = 0.85; + p.play.filter.env.releaseSeconds = 0.15; + p.play.filter.env.attackCurve = 0.4; + p.play.filter.env.decayCurve = 2.9; + p.play.filter.env.releaseCurve = 0.35; + p.play.filter.trigEnv.attackSeconds = 0.62; p.play.filter.trigEnv.decaySeconds = 1.8; + p.play.filter.trigEnv.holdFraction = 0.77; + p.play.filter.trigEnv.attackCurve = 0.3; + p.play.filter.trigEnv.decayCurve = 2.5; p.play.filter.velocityCurve = VelocityCurve::rampDown(); p.play.ampSpline.mode = EnvMode::Spline; p.play.ampSpline.contour = VelocityCurve::flat(); @@ -204,6 +230,9 @@ void checkNeutral(const BakeReset& reset) { CHECK(after.play.pitchEnv.peakSemitones == 0.0); CHECK(after.play.pitchEnv.shape.attackSeconds == freshPlay.pitchEnv.shape.attackSeconds); CHECK(after.play.pitchEnv.shape.decaySeconds == freshPlay.pitchEnv.shape.decaySeconds); + CHECK(after.play.pitchEnv.shape.holdFraction == freshPlay.pitchEnv.shape.holdFraction); + CHECK(after.play.pitchEnv.shape.attackCurve == freshPlay.pitchEnv.shape.attackCurve); + CHECK(after.play.pitchEnv.shape.decayCurve == freshPlay.pitchEnv.shape.decayCurve); // --- RESET: Rate and the baseline Pitch offset ----------------------------------- // Both are processing the bake already printed, so the whitelist leaves them at their @@ -225,7 +254,18 @@ void checkNeutral(const BakeReset& reset) { CHECK(after.play.filter.settings.driveNorm == freshPlay.filter.settings.driveNorm); CHECK(after.play.filter.settings.morphLaw == freshPlay.filter.settings.morphLaw); CHECK(after.play.filter.env.attackSeconds == freshPlay.filter.env.attackSeconds); + CHECK(after.play.filter.env.holdSeconds == freshPlay.filter.env.holdSeconds); + CHECK(after.play.filter.env.decaySeconds == freshPlay.filter.env.decaySeconds); + CHECK(after.play.filter.env.sustainLevel == freshPlay.filter.env.sustainLevel); + CHECK(after.play.filter.env.releaseSeconds == freshPlay.filter.env.releaseSeconds); + CHECK(after.play.filter.env.attackCurve == freshPlay.filter.env.attackCurve); + CHECK(after.play.filter.env.decayCurve == freshPlay.filter.env.decayCurve); + CHECK(after.play.filter.env.releaseCurve == freshPlay.filter.env.releaseCurve); + CHECK(after.play.filter.trigEnv.attackSeconds == freshPlay.filter.trigEnv.attackSeconds); CHECK(after.play.filter.trigEnv.decaySeconds == freshPlay.filter.trigEnv.decaySeconds); + CHECK(after.play.filter.trigEnv.holdFraction == freshPlay.filter.trigEnv.holdFraction); + CHECK(after.play.filter.trigEnv.attackCurve == freshPlay.filter.trigEnv.attackCurve); + CHECK(after.play.filter.trigEnv.decayCurve == freshPlay.filter.trigEnv.decayCurve); // --- RESET: the three spline contours AND their mode flags ---------------------- // The flag selects which shape ran, so the shape it selected is in the audio; with @@ -260,7 +300,9 @@ int main() { const InstrumentParams& after = reset.params; const InstrumentParams fresh; - // The fixtures must really move every field the sweep asserts, or the sweep is vacuous. + // A sample of the fields checkNeutral asserts, confirming the fixtures actually moved them + // off default — not the whole sweep, but enough spot checks that a fixture regressing to + // the defaults (making the sweep vacuous) would show here first. CHECK(fresh.keyTrack != before.keyTrack); CHECK(before.limiterEnabled != fresh.limiterEnabled); CHECK(before.bakeHold != fresh.bakeHold); @@ -300,8 +342,9 @@ int main() { // --- The loop enable and its points come back together --------------------------- // What the band shows after a bake is the reset override read against the newly banked - // entry, which carries NO loop intrinsic (the shell records that pairing where it builds - // the entry) — so the enable has nothing left to fall back to. + // entry's loop intrinsic — assumed std::nullopt below, which is what the shell lands + // today; if it ever populated one, the enable would have something to fall back to and + // this assumption, not just this test, would need revisiting. { constexpr std::int64_t kFrames = 1000; const LoopMarks dialedMarks =