Merge dev into phase-g: Phase Ε/Ρ and the 1.5.0 bump meet Phase Gamma's instrument work; 120/120 green
The per-directory CLAUDE.md count is re-derived at twenty-seven rather than carried from either side. The "Decouple the instrument reload from VST3 activation" TODO entry does not survive: Γ-W3-T1 landed it, and COMPLETED.md carries the discharge.
This commit is contained in:
@@ -22,9 +22,9 @@ stated; contradict it in review with an argument, not a preference.
|
||||
|
||||
**A bank package is one file that carries one bank — its audio and its index —
|
||||
out of a project and into another.** Today a bank is per-project by construction:
|
||||
the audio sits in `<projectDir>/reasampler_bank/` (`core/capture/capture_paths.h:16`,
|
||||
the audio sits in `<projectDir>/reasampler_bank/` (`core/capture/capture_paths.h`'s
|
||||
`kBankSubfolder`) and the index that gives that audio meaning lives in the `.rpp`'s
|
||||
project ext state under the `"reasampler"` namespace (`src/ext_keys.h:25`,
|
||||
project ext state under the `"reasampler"` namespace (`src/ext_keys.h`'s
|
||||
`kProjExtBanksKey`). The two travel together with the project and nowhere else.
|
||||
Export writes both halves into a single `.rsbank` file; import lands them into
|
||||
another project's bank folder and index.
|
||||
@@ -116,7 +116,7 @@ cases, and still no compression.
|
||||
**The two costs, accepted with the ruling.** (1) The package is **opaque without our
|
||||
tool** — no unzip-and-look support path. (2) We own the hostile-input hardening of our
|
||||
own parser, to the discipline `bank_model::deserialize` and `parseLedger` already
|
||||
carry — *error signaled, never UB* (`bank_model.h:204-206`). Both are priced in; a
|
||||
carry — *error signaled, never UB* (`bank_model.h`'s `BankModel::deserialize`). Both are priced in; a
|
||||
later "let's make it inspectable" impulse is a new phase's argument, not this one's.
|
||||
|
||||
**This was a one-way door and it is now shut** — packages are in users' hands the day
|
||||
@@ -133,28 +133,29 @@ moves from here.
|
||||
The repo already carries **two** versioning mechanisms, and they answer different
|
||||
questions:
|
||||
|
||||
1. **A blob-schema ladder.** `src/core/tracking/origin_ledger.cpp:8-30` states the
|
||||
ladder for the `owned_files` blob in a header comment (v1 legacy path-only, v2
|
||||
1. **A blob-schema ladder.** `src/core/tracking/origin_ledger.cpp`'s version-ladder
|
||||
header comment states the
|
||||
ladder for the `owned_files` blob (v1 legacy path-only, v2
|
||||
current), pins `constexpr int kLedgerVersion = 2`, and — the load-bearing part —
|
||||
**reads and validates `"v"`, not merely writes it**: "A version above
|
||||
`kLedgerVersion` is therefore its own degraded status, never a Loaded ledger"
|
||||
(`:20-21`). The parse outcome is a three-way `Ok` / `Malformed` / `FutureVersion`
|
||||
(`:171`, `:185`), deliberately distinguished so the operator gets the right
|
||||
(the same comment). The parse outcome is a three-way `Ok` / `Malformed` / `FutureVersion`
|
||||
(`origin_ledger.cpp`'s `ParseOutcome` enum and `parseStored`), deliberately distinguished so the operator gets the right
|
||||
recovery advice. A *field-vocabulary* gap behaves oppositely: an unrecognized
|
||||
`OriginKind` integer degrades to `Unknown` rather than failing the parse
|
||||
(`:32-40`), because "a vocabulary gap must not halt the prune"
|
||||
(`src/core/tracking/CLAUDE.md:82-86`).
|
||||
2. **An app writing-version stamp.** `src/core/version/app_version.h:165-186` —
|
||||
(`origin_ledger.cpp`'s `kindFromInt`), because "a vocabulary gap must not halt the prune"
|
||||
(`src/core/tracking/CLAUDE.md` §"Gotchas").
|
||||
2. **An app writing-version stamp.** `src/core/version/app_version.h`'s
|
||||
`WritingVersion` with `PreVersioning` / `Unknown` / `Stamped`, classified by
|
||||
`classifyWritingVersion`, stamped into project ext state by
|
||||
`src/shell/persist/ext_state_io.cpp:172-176` using `stampVersion()` (the numeric
|
||||
`ReaSamplerSession::saveToActiveProject` (`src/shell/persist/ext_state_io.cpp`) using `stampVersion()` (the numeric
|
||||
triple only, no channel suffix). It is informational: an absent stamp is "not an
|
||||
error and not a warning" (`app_version.h:166-168`).
|
||||
error and not a warning" (`app_version.h`'s `WritingVersion` comment, the `PreVersioning` case).
|
||||
|
||||
**An observation worth recording, not a defect to fix here:** `BankBook` writes
|
||||
`"version": 1` into the banks blob (`src/core/model/bank_book_json.cpp:37`) but its
|
||||
`"version": 1` into the banks blob (`src/core/model/bank_book_json.cpp`'s `BankBook::serialize`) but its
|
||||
parser skips the key along with every other unknown one
|
||||
(`bank_book_json.cpp:182` — `if (!r.skipValue()) return false; // version, or unknown`).
|
||||
(`bank_book_json.cpp`'s `parseBook` — `if (!r.skipValue()) return false; // version, or unknown`).
|
||||
The book's version field is therefore **decorative today** — written, never read,
|
||||
never gating. The ledger's is the precedent to extend; the book's is the precedent
|
||||
not to repeat.
|
||||
@@ -175,10 +176,10 @@ advise (an integer tells a user nothing about which build to install).
|
||||
|
||||
A single ladder has one bad property: **every change strands every older reader,
|
||||
even a purely additive one.** That is not hypothetical here — look at what `Sample`
|
||||
has actually accumulated: `rootNote` and `loop` (`bank_model.h:112-122`,
|
||||
has actually accumulated: `rootNote` and `loop` (`bank_model.h`'s `Sample::rootNote` / `Sample::loop`,
|
||||
"additive like `provenance`. Both default cleanly empty"), `captureTimeSigNum` /
|
||||
`captureTimeSigDenom` (`:103-108`, "0/0 means UNSTAMPED"), `channelCount`
|
||||
(`:91-96`, "0 = unknown — a pre-field entry"). Every one of those was additive with
|
||||
`captureTimeSigDenom` (`Sample::captureTimeSigNum` / `Sample::captureTimeSigDenom`, "0/0 means UNSTAMPED"), `channelCount`
|
||||
(`Sample::channelCount`, "0 = unknown — a pre-field entry"). Every one of those was additive with
|
||||
a defined absent-value. Under a single ladder, each would have blocked older readers
|
||||
for no reason.
|
||||
|
||||
@@ -197,17 +198,17 @@ message text and the log.
|
||||
|
||||
**One change class that looks additive and is not: a new enum value.**
|
||||
`BankModel::deserialize` *rejects* an out-of-range `SourceMode` or `Tier` rather than
|
||||
degrading it (`bank_model.cpp:232-239`, `:339-346`), and every enum a package carries
|
||||
degrading it (`bank_model.cpp`'s `parseSample` — the `sourceMode` and `tier` branches), and every enum a package carries
|
||||
rides inside the nested `BankModel` blob. So growing either vocabulary is
|
||||
**structural** and bumps `minReaderVersion` too. This is wider than packages and
|
||||
predates them: `BankModel::deserialize` is also the live project ext-state parser
|
||||
(`bank_book_json.cpp:99`), so appending a `SourceMode` value already strands an older
|
||||
(`bank_book_json.cpp`'s `parseBank`), so appending a `SourceMode` value already strands an older
|
||||
build opening a newer project's `.rpp`. Phase Ε inherits that property; it did not
|
||||
cause it, and changing it — degrade-to-`Unknown` at those two sites, the way
|
||||
`BakeStatus` already does — is a change to the model layer, not a package concern. It
|
||||
leaves the argument above untouched: the four fields that motivated the two-integer
|
||||
design are *fields*, and `parseSample`'s `skipValue()` fallback
|
||||
(`bank_model.cpp:370-372`), plus the manifest parsers' equivalent at each level, still
|
||||
(`bank_model.cpp`), plus the manifest parsers' equivalent at each level, still
|
||||
carries them forward.
|
||||
|
||||
This is a borrowed pattern, not an invention: Matroska's `EBMLVersion` /
|
||||
@@ -216,7 +217,7 @@ This is a borrowed pattern, not an invention: Matroska's `EBMLVersion` /
|
||||
must understand to read me." It costs one extra integer and one writer discipline —
|
||||
*decide honestly whether your change is additive* — and that discipline is exactly
|
||||
the one `origin_ledger` already enforces on `OriginKind`
|
||||
(`src/core/tracking/CLAUDE.md:82-83`: "PERSISTED INTEGERS — never renumber, only
|
||||
(`src/core/tracking/CLAUDE.md` §"Gotchas": "PERSISTED INTEGERS — never renumber, only
|
||||
append").
|
||||
|
||||
### Both directions, concretely
|
||||
@@ -225,12 +226,12 @@ append").
|
||||
Every reader reads every `minReaderVersion <= kPackageFormatVersion`. Absent manifest
|
||||
keys take their defined defaults, exactly as `Sample`'s additive fields already do,
|
||||
and exactly as `origin_ledger` lifts a v1 path-only blob into v2 records with kind
|
||||
`Unknown` and empty ids (`origin_ledger.cpp:14-16`). Unrecognized manifest keys are
|
||||
`Unknown` and empty ids (`origin_ledger.cpp`'s version-ladder header comment). Unrecognized manifest keys are
|
||||
skipped, which is already how every parser in this repo behaves
|
||||
(`bank_book_json.cpp:182`). Unrecognized enum integers (the manifest's own —
|
||||
(`bank_book_json.cpp`'s `parseBook`). Unrecognized enum integers (the manifest's own —
|
||||
`BankModel`'s nested ones reject) degrade to their defined `Unknown`-equivalent,
|
||||
never to the numeric default and never to a parse failure —
|
||||
`bake_wire`'s rule verbatim (`src/core/wire/CLAUDE.md:83`: "an unrecognized value
|
||||
`bake_wire`'s rule verbatim (`src/core/wire/CLAUDE.md` §"Modules", the `bake_wire` bullet: "an unrecognized value
|
||||
decodes as `Failed` rather than as the numeric default `Ok`").
|
||||
**The user sees:** a normal import summary. Optionally a single console line naming
|
||||
the older writer version. No dialog, no warning, no ceremony — a supported case is
|
||||
@@ -240,7 +241,7 @@ not an incident.
|
||||
written.** `minReaderVersion > kPackageFormatVersion` is a hard stop, before a single
|
||||
byte is written to the bank folder and before the index is touched. This is exactly
|
||||
`LedgerStatus::FutureVersion`'s treatment, and for the same reason stated at
|
||||
`origin_ledger.cpp:18-21`: parsing an unknown shape by old rules "would yield a
|
||||
`origin_ledger.cpp`'s version-ladder header comment: parsing an unknown shape by old rules "would yield a
|
||||
plausible-but-partial" result, and a partial bank is worse than no bank.
|
||||
**The user sees** a message box (`ShowMessageBox`, verified —
|
||||
`vendor/reaper-sdk/sdk/reaper_plugin_functions.h:6546`,
|
||||
@@ -273,13 +274,13 @@ will not find out which twelve of forty samples were dropped until they need one
|
||||
`bank_book_json` precedent applied outward: the book writer "emits the bank
|
||||
envelope … plus a raw `index` member whose value is the `BankModel` blob verbatim,
|
||||
so per-bank sample serialization stays owned by `bank_model` and is not duplicated
|
||||
here" (`bank_book_json.cpp:15-20`). The package does the same, so a future `Sample`
|
||||
here" (`bank_book_json.cpp`'s file-header comment). The package does the same, so a future `Sample`
|
||||
field reaches packages for free and the shape has exactly one owner.
|
||||
- **Per entry, additionally:** the payload's **bare file name** inside the package,
|
||||
its byte length, and a whole-file `hashBytes` digest
|
||||
(`core/capture/wav_codec.h:143` — FNV-1a 64-bit over raw bytes, 16-char lowercase
|
||||
(`core/capture/wav_codec.h`'s `hashBytes` — FNV-1a 64-bit over raw bytes, 16-char lowercase
|
||||
hex). Note carefully: `hashBytes`, **not** `hashWavContent`. The latter deliberately
|
||||
skips non-`fmt `/`data` chunks (`wav_codec.h:145-151`), which is right for dedup
|
||||
skips non-`fmt `/`data` chunks (`wav_codec.h`'s `hashWavContent`), which is right for dedup
|
||||
identity and wrong for "did these bytes survive the trip." Both hashes are already
|
||||
in the codebase; the package needs the raw one for integrity and carries the
|
||||
`Sample`'s existing `contentHash` for dedup, and they are different fields
|
||||
@@ -306,14 +307,14 @@ guarantee for this feature; overselling it would be the error.
|
||||
one genuinely security-shaped surface this feature has.
|
||||
- **The origin ledger.** The ledger is *this project's* record of files *it*
|
||||
created, and it is the authority prune's protected set is computed from
|
||||
(`src/core/tracking/CLAUDE.md:5-13`). Importing foreign ownership records would
|
||||
(`src/core/tracking/CLAUDE.md` §"Scope"). Importing foreign ownership records would
|
||||
assert this project's authority over another project's history. Instead the
|
||||
importer writes **its own** birth records for the files it lands, at the moment it
|
||||
lands them, through the one writer (`ReaSamplerSession::recordCreated`,
|
||||
`src/shell/persist/session.h:95` — it already takes an `OriginKind`). Without that,
|
||||
`src/shell/persist/session.h` — it already takes an `OriginKind`). Without that,
|
||||
every imported file would be "foreign, therefore never reclaimed"
|
||||
(`core/tracking/CLAUDE.md:24-31`) and a user's bank folder would grow forever.
|
||||
- **Live-instance usage records** (`rsusage_*`, `src/ext_keys.h:65`). Per-instance
|
||||
(`core/tracking/CLAUDE.md` §"Invariants", the "No silent gaps" bullet) and a user's bank folder would grow forever.
|
||||
- **Live-instance usage records** (`rsusage_*`, `src/ext_keys.h`'s `kProjExtUsageKeyPrefix`). Per-instance
|
||||
runtime state of a specific project's specific FX instances. Meaningless elsewhere.
|
||||
- **Project state that is not bank state:** which bank was active, the Design View
|
||||
mode model (`view_state`), the tail setting, the project GUID, the bank-generation
|
||||
@@ -342,11 +343,11 @@ Four distinct collisions hide under the word "collision," and they need four
|
||||
different answers.
|
||||
|
||||
1. **Sample id.** Ids are minted as `"cap-" + uniqueTag + "-" + fileName`
|
||||
(`src/shell/capture/capture.cpp:567`) and `"imp-" + …`
|
||||
(`src/shell/actions/ingest.cpp:269`) — unique within a project, **not** globally.
|
||||
(`src/shell/capture/capture.cpp`'s `OfflineRenderBackend::capture`) and `"imp-" + …`
|
||||
(`src/shell/actions/ingest.cpp`'s `importFileIntoActiveBank`) — unique within a project, **not** globally.
|
||||
Re-importing a package into the project it came from would collide.
|
||||
**Answer: remint every sample id on import**, under its own prefix, and remap
|
||||
`Provenance::parentSampleId` (`bank_model.h:45-50`) through the same map — to the
|
||||
`Provenance::parentSampleId` (`bank_model.h`'s `Provenance` struct) through the same map — to the
|
||||
reminted parent if that parent came in the same package, cleared otherwise. A
|
||||
foreign id never enters the destination index. This also makes "import the same
|
||||
package twice" a clean, duplicative, correct operation rather than an undefined
|
||||
@@ -354,18 +355,18 @@ different answers.
|
||||
2. **File name in the destination bank folder.** **Never overwrite.** Overwriting
|
||||
would destroy an existing capture, and only prune touches existing bank bytes.
|
||||
Mint a fresh unique name through the existing `deriveBankPaths` +
|
||||
unique-tag machinery (`core/capture/capture_paths.h:42`), automatically, no
|
||||
unique-tag machinery (`core/capture/capture_paths.h`'s `deriveBankPaths`), automatically, no
|
||||
prompt, and report the count in the summary.
|
||||
3. **Content hash.** `BankModel::add` collapses an equal-`contentHash` add onto the
|
||||
existing entry (`bank_model.h:144-147`, `AddResult::Collapsed`). Desirable — but
|
||||
existing entry (`bank_model.h`'s `AddResult::Collapsed`). Desirable — but
|
||||
if the file was already written to disk before the collapse, it becomes an
|
||||
instant orphan. **Answer: check the destination bank's `findByHash` BEFORE writing
|
||||
the payload**; on a hit, skip the write entirely and report "N already present."
|
||||
This is the one place the import must consult the model before touching the
|
||||
filesystem, and it is a concrete acceptance criterion rather than an optimization.
|
||||
4. **Bank display name.** `bank_book` enforces unique display names, trimmed and
|
||||
case-insensitive ASCII (`src/core/model/CLAUDE.md:22-26`; `createBank`'s own
|
||||
contract at `bank_book.h:92-96` — *"Drums"/"drums"/" Drums " collide, including
|
||||
case-insensitive ASCII (`src/core/model/CLAUDE.md` §"Invariants", the "Bank identity, movement, dedup" bullet; `createBank`'s own
|
||||
contract at `bank_book.h` — *"Drums"/"drums"/" Drums " collide, including
|
||||
against the pool's "Pool"*), so `createBank("Drums")` into a project that already
|
||||
has "Drums" returns `false` with no mutation. **Answer: an automatic numeric
|
||||
suffix, specified below.** No prompt, no overwrite, no refusal.
|
||||
@@ -381,7 +382,7 @@ trim, the seed is the literal `Imported bank`.
|
||||
|
||||
**The probe.** Let `seed` be that string and `fold(x)` be `BankBook`'s own uniqueness
|
||||
key — strip leading/trailing ASCII whitespace, lower-case ASCII letters
|
||||
(`bank_book.h:252-258`). Take the **first** name in this sequence whose fold is not
|
||||
(`bank_book.h`'s `BankBook::nameKey`). Take the **first** name in this sequence whose fold is not
|
||||
already carried by a bank in the destination book:
|
||||
|
||||
seed, seed + " 2", seed + " 3", seed + " 4", …
|
||||
@@ -410,7 +411,7 @@ implementations diverge:**
|
||||
`B + 1` candidates is free by pigeonhole, so no cap is needed and none should be
|
||||
added.
|
||||
4. **The fold has exactly one home.** `import_plan` must **not** re-implement
|
||||
`nameKey` — `bank_book.h:252-258` says in as many words that a drifted second copy
|
||||
`nameKey` — `bank_book.h`'s `BankBook::nameKey` says in as many words that a drifted second copy
|
||||
would let the uniqueness invariant be violated. The probe therefore runs behind
|
||||
`BankBook`'s own folding, which means Ε-W2-T2 adds **one additive public `const`
|
||||
member** to `BankBook` (recommended: `std::string uniqueDisplayName(const
|
||||
@@ -422,7 +423,7 @@ name. Sample ids are reminted by collision rule 1 regardless of whether a name
|
||||
collision occurred, and the two mechanisms are independent. **`Sample` display names
|
||||
are never suffixed** — two banks may legitimately hold a sample called `"Kick"`, and
|
||||
`resample_name`'s own contract already states that sample display names are not unique
|
||||
(`resample_name.h:13-16`). Bank-folder file names are handled by collision rule 2 and
|
||||
(`resample_name.h`'s `nextIterationName`). Bank-folder file names are handled by collision rule 2 and
|
||||
are unaffected by the bank's name. `slot_map` positions ride along unchanged.
|
||||
|
||||
**The pool case is guaranteed, not hypothetical.** Exporting the pool is in scope (the
|
||||
@@ -456,7 +457,7 @@ settled on: report before acting, and never leave a half-state that looks whole.
|
||||
| Destination package file exists | export | Platform save dialog's own overwrite confirm | Native dialog |
|
||||
| Write fails partway | export | Temp file in the destination directory, atomic rename only on complete success | Console error; no `.rsbank` left behind. A truncated package must never exist |
|
||||
| `minReaderVersion` above this build | import | Refuse whole. Nothing written, index untouched | The three-part message box above (package needs / this build reads / what to install) |
|
||||
| Malformed or truncated container | import | Refuse whole. Reported **distinctly from** the version case | "This file is not a readable bank package (corrupt or truncated)." The distinction matters: the two have opposite recoveries — one is "install a newer build," the other is "get an intact copy." `origin_ledger.cpp:178-185` makes exactly this distinction for exactly this reason |
|
||||
| Malformed or truncated container | import | Refuse whole. Reported **distinctly from** the version case | "This file is not a readable bank package (corrupt or truncated)." The distinction matters: the two have opposite recoveries — one is "install a newer build," the other is "get an intact copy." `origin_ledger.cpp`'s `parseStored` makes exactly this distinction for exactly this reason |
|
||||
| Entry name contains a path separator, `..`, or is absolute | import | Refuse whole, before any write | "This package is not well-formed." Hostile input, not user error — no need to elaborate |
|
||||
| Payload hash mismatch on any entry | import | Refuse whole, before landing anything | "This bank package is damaged (entry `<name>` failed its integrity check). Nothing was imported." |
|
||||
| A write fails mid-import (disk full, permission) | import | Roll back: delete the files **this import wrote** and abandon the index mutation | "Import failed and was rolled back. Nothing was added." |
|
||||
@@ -467,7 +468,7 @@ settled on: report before acting, and never leave a half-state that looks whole.
|
||||
|
||||
**On the rollback, and why it is not an invariant breach.** Prune is the single
|
||||
exclusive file-deletion authority, with exactly one carve-out, stated in one place —
|
||||
`src/shell/persist/prune_fs.cpp:5-11`: "a shell removing a file it wrote itself
|
||||
`src/shell/persist/prune_fs.cpp`'s file-header comment: "a shell removing a file it wrote itself
|
||||
moments earlier and that no index ever referenced is self-cleanup, not authority
|
||||
over user data … the discriminator is 'did this call create it, and did anything ever
|
||||
reference it', not where it sits." An import rollback fits that discriminator
|
||||
@@ -477,11 +478,11 @@ restate it, or a reviewer will correctly read the rollback as a breach.
|
||||
|
||||
**On undo.** The index side of an import is one Ctrl-Z, through the same
|
||||
`persistBankOp` undo batching every bank verb already uses
|
||||
(`src/shell/bank_ops/CLAUDE.md:29-31`; `Undo_BeginBlock2` / `Undo_EndBlock2` verified
|
||||
(`src/shell/bank_ops/CLAUDE.md` §"Invariants", the "One bank operation is one Ctrl-Z" bullet; `Undo_BeginBlock2` / `Undo_EndBlock2` verified
|
||||
at `reaper_plugin_functions.h:7758` and `:7806`). **Undo does not un-write the
|
||||
files** — they remain on disk, referenced by no index, until a prune reclaims them.
|
||||
That is the same designed orphaned-until-prune window a non-empty bank delete already
|
||||
produces (`src/core/model/CLAUDE.md:38-40`). Say it out loud in the spec; do not let
|
||||
produces (`src/core/model/CLAUDE.md` §"Invariants", the "Bank identity, movement, dedup" bullet). Say it out loud in the spec; do not let
|
||||
a user infer that Ctrl-Z cleans the folder.
|
||||
|
||||
### Import under a degraded tracking ledger (Ε-F3, RULED: refuse)
|
||||
@@ -494,7 +495,7 @@ reasoning that carried it is recorded below rather than re-argued.
|
||||
|
||||
**The trigger, exactly.** The guard fires when `tracking::ledgerDegraded(status)` holds
|
||||
for the project's loaded ledger status — that is, `LedgerStatus::Unreadable` or
|
||||
`LedgerStatus::FutureVersion` (`src/core/tracking/origin_ledger.h:94`, `:100-101`).
|
||||
`LedgerStatus::FutureVersion` (`src/core/tracking/origin_ledger.h`'s `LedgerStatus` and `ledgerDegraded`).
|
||||
`Fresh` (absent key — a legitimate new project) and `Loaded` both proceed normally.
|
||||
|
||||
**Two things the guard is deliberately NOT keyed on:**
|
||||
@@ -513,7 +514,7 @@ package is read, before any allocation. Making the user find and pick a file we
|
||||
already decided to refuse is the wrong order.
|
||||
|
||||
**What the user sees.** A console block through `ShowConsoleMsg`, mirroring prune's
|
||||
abort (`src/shell/actions/prune_action.cpp:30-69`) in structure and in tone, because a
|
||||
abort (`src/shell/actions/prune_action.cpp`'s `doBankPruneFolder` — the `blockedByTracking` console block) in structure and in tone, because a
|
||||
user who has hit prune's block should recognise this one. Every recovery line names
|
||||
**this build's** ext-state namespace via `version::extStateNamespace()` — the
|
||||
beta/stable trap prune already documents, where a beta user handed the stable spelling
|
||||
@@ -545,7 +546,7 @@ clears the wrong key and is still blocked. Two cases, exactly one of which fires
|
||||
> them could be given a birth record, and every one would be permanently unreclaimable.
|
||||
|
||||
**The recovery path.** The status is written only by `loadFromProject`, so it is sticky
|
||||
for the session (`src/shell/persist/CLAUDE.md:39-46`): repair or clear the key
|
||||
for the session (`src/shell/persist/CLAUDE.md` §"Invariants", the "A ledger this build cannot read is degraded" bullet): repair or clear the key
|
||||
(malformed case only), or install the newer build (future-version case), **reopen the
|
||||
project**, then import again. The package needs no re-export, and nothing about the
|
||||
destination project was changed by the refusal.
|
||||
@@ -557,7 +558,7 @@ likely to want to. Only the landing side refuses.
|
||||
|
||||
**Why the ruling went this way.** The rejected option — allow the import behind an
|
||||
up-front confirm — matched the accepted residual already stated at
|
||||
`core/tracking/CLAUDE.md:24-31`, where a capture made during a degraded session is
|
||||
`core/tracking/CLAUDE.md` §"Invariants" (the "No silent gaps" bullet), where a capture made during a degraded session is
|
||||
recorded in memory but not persisted and degrades to foreign. The argument that carried
|
||||
is **scale**: that residual contemplates *one* untracked capture, and a bulk import can
|
||||
strand two hundred files in a single gesture. Same mechanism, different animal. A
|
||||
@@ -611,7 +612,7 @@ Two new directories, following the split the whole repo turns on.
|
||||
and `classifyPackageVersion(formatVersion, minReader) -> Readable | TooNew |
|
||||
Malformed`. The ladder lives with the framing because the ladder *is* the framing's
|
||||
contract, and it gets a header-comment ladder written the way
|
||||
`origin_ledger.cpp:8-21` writes one.
|
||||
`origin_ledger.cpp`'s version-ladder header comment writes one.
|
||||
- `package_manifest` — the manifest model and its JSON codec, nesting `BankModel`'s
|
||||
own blob verbatim.
|
||||
- `bank_package` — header encode / prefix decode / entry layout, composing the two
|
||||
@@ -643,21 +644,21 @@ responsibility seam, which is what the structural heuristic asks for.
|
||||
`mode=1` an existing one (import's source). `extension_list` takes the
|
||||
`'ReaSampler banks|*.rsbank|All files|*.*'` form. `GetUserFileNameForRead` is
|
||||
explicitly "Superseded, see GetUserFileName" (`:3796`) and is not used. No fallback
|
||||
is needed: `src/app/main.cpp:15` defines `REAPERAPI_IMPLEMENT` without
|
||||
is needed: `src/app/main.cpp`'s `#define REAPERAPI_IMPLEMENT` appears without
|
||||
`REAPERAPI_MINIMAL`, so the resolver walks the full table (`GetUserFileName` at
|
||||
`:9084`), and `main.cpp:292-293` refuses to load the extension if any one function
|
||||
`:9084`), and `REAPER_PLUGIN_ENTRYPOINT`'s `REAPERAPI_LoadAPI` check refuses to load the extension if any one function
|
||||
fails to resolve — so no REAPER build that loads us can lack it.
|
||||
- `export_bank` / `import_bank` — the promptless verbs, mirroring
|
||||
`src/shell/bank_ops/`'s pattern exactly: take a `ReaSamplerSession&`, do the work,
|
||||
return an outcome, **no prompts and no message boxes**. The bindable action and the
|
||||
panel menu item are then thin skins over one verb apiece, so the logic has one home
|
||||
(`src/shell/bank_ops/CLAUDE.md:1-12`).
|
||||
(`src/shell/bank_ops/CLAUDE.md` §"Scope").
|
||||
|
||||
**The dependency-shape criterion, stated because the brief demands it.** The pure
|
||||
planners take **explicit value inputs** — the decoded manifest, the destination
|
||||
`BankBook`, the set of file names present in the bank folder — never a session handle,
|
||||
never a service container, never a "pass me the thing that has everything." The shell
|
||||
*gathers*; the core *decides*. That is the same shape `src/shell/persist/CLAUDE.md:11`
|
||||
*gathers*; the core *decides*. That is the same shape `src/shell/persist/CLAUDE.md` §"Scope"
|
||||
already states ("it gathers rather than decides"). If a circular dependency shows up
|
||||
during the build, the fix is a service split or a thin interface at the seam — never
|
||||
threading an extra parameter through a chain of constructors, and never handing a
|
||||
@@ -690,12 +691,12 @@ constructors.
|
||||
freshly-generated pair.
|
||||
- **Bank generation.** Import mutates bank content that live ReaSampler 9000
|
||||
instances may play, so it must `bumpBankGeneration()`
|
||||
(`src/shell/persist/session.h:108`, whose own comment says call sites "err toward
|
||||
(`src/shell/persist/session.h`'s `ReaSamplerSession::bumpBankGeneration`, whose own comment says call sites "err toward
|
||||
bumping"). Export mutates nothing and must bump nothing, write no ext state, and
|
||||
open no undo point.
|
||||
- **Beta/stable channel isolation.** Packages are channel-**agnostic** and this is
|
||||
deliberate. Channel isolation exists so a beta cannot rewrite a stable project's
|
||||
ext state (`app_version.h:73-76`); a package is a file the user moves by hand, not
|
||||
ext state (`app_version.h`'s `extStateNamespace` — the ISOLATION comment); a package is a file the user moves by hand, not
|
||||
ambient project state, so there is no isolation property to preserve. A beta build
|
||||
and a stable build at the same package format read each other's packages, and that
|
||||
is the useful behaviour. The version ladder — not the channel — is what gates.
|
||||
@@ -725,6 +726,30 @@ contemplates one untracked capture, an import strands hundreds).
|
||||
|
||||
---
|
||||
|
||||
## Implementation decisions — Ε-W2-T1
|
||||
|
||||
Not [Daniel]-class forks — both were `[propose at review]` calls in `docs/PLAN.md`'s
|
||||
Ε-W2-T1 track, answered at implementation review rather than by Daniel, and recorded
|
||||
here per this phase's own convention for keeping such answers where the design lives
|
||||
rather than only in the track's own now-stale open-questions line.
|
||||
|
||||
- **Affordance: both the bindable action and the panel row.** The action targets the
|
||||
**active** bank and is the only spelling that can reach the **pool** (the panel's
|
||||
`showTabMenu` returns early on `isPool()` — a named-bank-tab context menu has no tab
|
||||
to right-click for the pool), while the exported unit's own definition above includes
|
||||
the pool. The panel row is the direct gesture on a specific named bank. Neither
|
||||
subsumes the other.
|
||||
- **Default file name: the bank's display name**, sanitized through
|
||||
`capture_paths::sanitizeStem`, seeded into `<projectDir>/<stem>.rsbank`. A
|
||||
project-derived name was the rejected alternative: three banks exported from one
|
||||
project must produce three distinguishable files, and a project-derived name
|
||||
collides on the second export. Known wart, worth recording rather than hiding:
|
||||
`sanitizeStem` collapses an all-non-ASCII display name to the literal `capture`, so
|
||||
two such banks still collide — the existing rename verb is the recovery, same as the
|
||||
import-side auto-suffix collisions above.
|
||||
|
||||
---
|
||||
|
||||
## Non-goals and guardrails
|
||||
|
||||
- **No auto-insertion of imported audio into the arrange.** Same rule as capture.
|
||||
|
||||
@@ -53,12 +53,20 @@ snapshot/restore, forces dither and all normalize-postprocessing off, and render
|
||||
32-bit float. The tail wires into that existing path — no new render trigger, no
|
||||
new backend.
|
||||
|
||||
### Bounds are always custom — so the tail bit is always `&1`
|
||||
### Bounds are always the time selection — so the tail bit is always `&4`
|
||||
|
||||
The backend renders with `RENDER_BOUNDSFLAG = 0` (custom time bounds) for **every**
|
||||
scope and every range type: it sets `RENDER_STARTPOS` / `RENDER_ENDPOS` explicitly
|
||||
from the request's exact seconds (`capture.cpp` ~L352–354). It does **not** use the
|
||||
time-selection / selected-items / regions bounds modes.
|
||||
The backend renders with `RENDER_BOUNDSFLAG = 2` (time selection) for **every**
|
||||
scope and every range type: it writes the request's exact seconds into the
|
||||
project's own time selection via `GetSet_LoopTimeRange` (`capture.cpp` ~L470–477;
|
||||
`RENDER_STARTPOS`/`RENDER_ENDPOS` are also written, as a defensive no-op for a
|
||||
mode-0-only field, but the window itself travels in the time selection). It does
|
||||
**not** use the custom-time-bounds mode (`RENDER_BOUNDSFLAG = 0`) — that mode was
|
||||
tried and retired: DAW observation showed REAPER resolving a custom-bounds window
|
||||
on a whole-millisecond grid AT RENDER TIME, flooring the end and rendering exactly
|
||||
the floored frame count, which silently broke the exact-bounds precision
|
||||
invariant. The time-selection mode does not floor the window. (The one narrative
|
||||
home for that finding is `render_settings.h`'s `kRenderBoundsTimeSelection`; this
|
||||
doc points there rather than retelling it.)
|
||||
|
||||
`RENDER_TAILFLAG` is a bitmask keyed to the **bounds mode**, not the capture range
|
||||
type (header line 3047):
|
||||
@@ -69,18 +77,20 @@ RENDER_TAILFLAG : &1=custom time bounds, &2=entire project, &4=time selection,
|
||||
&32=selected project markers/regions
|
||||
```
|
||||
|
||||
Because we always render in custom-time-bounds mode, **the only tail bit that ever
|
||||
applies is `&1`**. There is no per-range-type tail-flag decision to make — a razor
|
||||
capture, a time-selection capture, and an item capture are all custom-bounds
|
||||
renders under the hood, so all three take `RENDER_TAILFLAG = 1`.
|
||||
Because we always render in time-selection mode, **the only tail bit that ever
|
||||
applies is `&4`**. There is no per-range-type tail-flag decision to make — a razor
|
||||
capture, a time-selection capture, and an item capture are all time-selection-bounds
|
||||
renders under the hood, so all three take `RENDER_TAILFLAG = 4`.
|
||||
|
||||
> **Correction to the framing brief.** The brief asked us to pick a
|
||||
> `RENDER_TAILFLAG` bit *per capture range type* (time selection vs. razor vs. item)
|
||||
> and flagged `&32` as "markers/regions." The header (line 3047) says `&32` =
|
||||
> *selected project regions* and `&8` = *all markers/regions* — but neither matters:
|
||||
> our renders are all `RENDER_BOUNDSFLAG = 0`, so the tail bit is `&1` unconditionally.
|
||||
> The existing `kTailFlagCustomBounds = 1.0` constant in `capture.cpp` (~L80) is
|
||||
> already correct; the field wiring is what's missing.
|
||||
> our renders are all `RENDER_BOUNDSFLAG = 2`, so the tail bit is `&4` unconditionally.
|
||||
> The existing `kTailFlagTimeSelection = 4` constant in
|
||||
> `src/core/capture/render_settings.h` (the bounds mode's own bit, per bounds mode —
|
||||
> header line 3047) is already correct — it was right from the start; the wording
|
||||
> above it (which had assumed a custom-bounds render) was what was wrong.
|
||||
|
||||
### Mode 1 — Automatic (default): generous tail + auto-trim to -72 dB
|
||||
|
||||
@@ -88,7 +98,7 @@ Set, in addition to the exact `STARTPOS`/`ENDPOS` already driven:
|
||||
|
||||
| Setting | Value | Meaning / header ref |
|
||||
|---|---|---|
|
||||
| `RENDER_TAILFLAG` | `1` | apply tail for custom time bounds (line 3047, `&1`) |
|
||||
| `RENDER_TAILFLAG` | `4` | apply tail for time selection (line 3047, `&4`) |
|
||||
| `RENDER_TAILMS` | `8000` | the 8 s cap, in ms (line 3048) |
|
||||
| `RENDER_NORMALIZE` | `32768` | **only** the trim-ending-silence bit (line 3051, `&32768`) |
|
||||
| `RENDER_TRIMEND` | `≈ 0.000251` | -72 dB threshold (line 3062; scaling below) |
|
||||
@@ -156,7 +166,7 @@ The existing (currently unwired) `CaptureRequest.renderTail` / `tailMs` fields
|
||||
|
||||
| Setting | Value |
|
||||
|---|---|
|
||||
| `RENDER_TAILFLAG` | `1` |
|
||||
| `RENDER_TAILFLAG` | `4` |
|
||||
| `RENDER_TAILMS` | `request.tailMs` (clamped to the 8 s cap — see below) |
|
||||
| `RENDER_NORMALIZE` | `262144` (`kNormalizeDisableAll`, unchanged) |
|
||||
| `RENDER_TRIMEND` | not set / irrelevant (trim bit is clear) |
|
||||
@@ -177,9 +187,9 @@ adds a third state, so the wiring is a small enum, not a bool:
|
||||
- **None** (default for null-test / verify captures, and the current two-scope
|
||||
action defaults): `RENDER_TAILFLAG = 0`, `RENDER_TAILMS = 0`, normalize =
|
||||
disable-all. Exact bounds. Byte-identical to today.
|
||||
- **Auto** (the new user-facing default for tail-on captures): tailFlag `1`,
|
||||
- **Auto** (the new user-facing default for tail-on captures): tailFlag `4`,
|
||||
tailMs `8000`, normalize `32768` (surgical trim), trimEnd `0.00025119`.
|
||||
- **Manual(ms)**: tailFlag `1`, tailMs `clamp(ms, 8000)`, normalize `262144`
|
||||
- **Manual(ms)**: tailFlag `4`, tailMs `clamp(ms, 8000)`, normalize `262144`
|
||||
(disable-all), no trim.
|
||||
|
||||
Recommended shape: replace `bool renderTail` with a `TailMode { None, Auto,
|
||||
|
||||
@@ -226,25 +226,25 @@ them through the reorg, not to change them:
|
||||
These are the naming equivalent of the JSON-`Parser` DRY violation — concrete hazards, not taste:
|
||||
|
||||
1. **Four hand-rolled `Parser` classes, one name.** `class Parser` is defined **four times** —
|
||||
`bank_model.cpp:306`, `bank_book.cpp:663`, `owned_manifest.cpp:107`, `view_mode_model.cpp:654`.
|
||||
`bank_model.cpp`, `bank_book.cpp`, `owned_manifest.cpp`, `view_mode_model.cpp`.
|
||||
Q-W1 already deletes three of them by extracting `core/json`; the naming rule is that the
|
||||
survivor is **`json::Parser`** (or a more specific `json::Reader`/`json::Writer` pair — see
|
||||
Q-8), never a bare `Parser` in flat scope.
|
||||
2. **`FooterRect` and `ButtonRect` are shared across pure UI modules — and the codebase already
|
||||
*knows* it.** `struct FooterRect` and `struct ButtonRect` are defined in `prune_button.h`
|
||||
(lines 32, 46) and **reused** by `footer_bar.h`, which carries an explicit in-file "NAME NOTE"
|
||||
(`footer_bar.h:27–34`) documenting that `ButtonRect / FooterRect / SegmentRect / ActionBarRect /
|
||||
and **reused** by `footer_bar.h`, which carries an explicit in-file "NAME NOTE"
|
||||
(`footer_bar.h`) documenting that `ButtonRect / FooterRect / SegmentRect / ActionBarRect /
|
||||
KitBox / KitButtonBox` are "already owned in this namespace" and that new types must carry a
|
||||
`FooterBar*` prefix to avoid collision. That comment is a smell made visible: the flat
|
||||
`reasampler::` namespace forces every pure-UI author to hand-check for name collisions before
|
||||
minting a type. This is the single strongest in-codebase argument for the Q-4 sub-namespaces —
|
||||
under `reasampler::ui` these shared rect types get one clear owner and the hand-checking stops.
|
||||
3. **`Sample` (`bank_model.h:69`, the bank metadata struct) vs `AudioSample` (the `peaks` float
|
||||
3. **`Sample` (`bank_model.h`'s `Sample` struct, the bank metadata struct) vs `AudioSample` (the `peaks` float
|
||||
alias).** Already flagged in §2.4/Q-4; verified — `Sample` is the model record, `AudioSample`
|
||||
is a raw PCM float. Under `model::Sample` vs `audio::AudioSample` the collision risk is gone,
|
||||
but the *names* still read oddly side by side (a `Sample` that is metadata, an `AudioSample`
|
||||
that is one float). Noted; the namespace split is the required fix, a rename is optional (Q-8).
|
||||
4. **`Selection` (`bank_grid.h:112`) and `CellRect` (`bank_grid.h:23`) are generic names in a
|
||||
4. **`Selection` (`bank_grid.h`'s `Selection` struct) and `CellRect` (`bank_grid.h`'s `CellRect`) are generic names in a
|
||||
flat namespace.** `Selection` in particular is the kind of name a newcomer cannot place without
|
||||
opening the file. `ui::Selection` / `ui::CellRect` resolve it structurally; no rename needed
|
||||
beyond the namespace.
|
||||
@@ -255,23 +255,23 @@ Here the names are legal and non-colliding but do not read on one principle —
|
||||
at" gap:
|
||||
|
||||
1. **The model-family suffixes disagree: `_model` vs `_book` vs `Index`.** Verified: the pure model
|
||||
modules are `bank_model.{h,cpp}` (owning `class BankIndex`, `bank_model.h:132`), `bank_book.{h,cpp}`
|
||||
(owning `class BankBook`, `bank_book.h:208`), `view_mode_model.{h,cpp}` (owning `class ViewModeModel`,
|
||||
`view_mode_model.h:376`), `owned_manifest.{h,cpp}` (owning `class OwnedFileManifest`,
|
||||
`owned_manifest.h:52`). Four modules, four different file↔class naming relationships:
|
||||
modules are `bank_model.{h,cpp}` (owning `class BankIndex`, `bank_model.h`), `bank_book.{h,cpp}`
|
||||
(owning `class BankBook`, `bank_book.h`'s `BankBook`), `view_mode_model.{h,cpp}` (owning `class ViewModeModel`,
|
||||
`view_mode_model.h`'s `ViewModeModel`), `owned_manifest.{h,cpp}` (owning `class OwnedFileManifest`,
|
||||
`owned_manifest.h`). Four modules, four different file↔class naming relationships:
|
||||
`bank_model`→`BankIndex` (file says "model," class says "index"), `bank_book`→`BankBook`
|
||||
(file = class), `view_mode_model`→`ViewModeModel` (file = class), `owned_manifest`→`OwnedFileManifest`
|
||||
(file ≈ class, but the class adds "File"). The `bank_model`/`BankIndex` mismatch is the worst:
|
||||
the file name and its primary class name share no word. This is a genuine legibility wart — the
|
||||
fix is a *rename decision* (Q-8), not something the directory move alone resolves.
|
||||
2. **The `bank_book` "wraps `bank_model`" relationship is invisible in the names.** `BankBook`
|
||||
(`bank_book.h:208`) is a registry of `Bank` (`bank_book.h:147`), each wrapping a `BankIndex`
|
||||
(`bank_model.h:132`). The names `Book` → `Bank` → `Index` do not read as a containment hierarchy;
|
||||
(`bank_book.h`'s `BankBook`) is a registry of `Bank` (`bank_book.h`'s `Bank` struct), each wrapping a `BankIndex`
|
||||
(`bank_model.h`). The names `Book` → `Bank` → `Index` do not read as a containment hierarchy;
|
||||
a reader has to learn it. (Not necessarily worth a rename — "book of banks" is evocative — but
|
||||
it is the kind of call Q-8 should make deliberately, not by accident.)
|
||||
3. **`realtime_record.h` (pure) vs `capture_realtime.cpp` (shell) — the word order flips.** Verified:
|
||||
the pure realtime module is `realtime_record.{h}` (owning `RecordModePlan`/`RecordPhase`/
|
||||
`RecordTickInputs`, `realtime_record.h:57–173`) while its shell is `capture_realtime.cpp`. So the
|
||||
`RecordTickInputs`, `core/capture/capture_realtime.h`) while its shell is `capture_realtime.cpp`. So the
|
||||
pure core is `realtime_record` but the shell is `capture_realtime` — the two halves of one feature
|
||||
are named on inverted word order (`realtime_record` vs `capture_realtime`). Compare the *clean*
|
||||
shell-pair convention elsewhere: `drag_out` (pure) ↔ `drag_out_win` (shell) — same stem, suffix
|
||||
@@ -279,7 +279,7 @@ at" gap:
|
||||
naming-drift instance in the tree (Q-9).
|
||||
4. **`capture.{h,cpp}` is the *offline* backend shell, but the name claims all of capture.**
|
||||
Verified: `capture.h` declares `ICaptureBackend`, `OfflineRenderBackend`, **and**
|
||||
`RealtimeRecordBackend` (`capture.h:112,124,201`), while the realtime *implementation* lives in
|
||||
`RealtimeRecordBackend` (`capture.h`'s `OfflineRenderBackend`), while the realtime *implementation* lives in
|
||||
`capture_realtime.cpp` and its pure planner in `realtime_record.h`. So `capture` is really
|
||||
"capture interface + offline backend," a fat header (the §2.3 Interface-Segregation concern) whose
|
||||
name oversells its scope. Its Q-W3 hoist (`capture_orchestrator`/`scope_resolve`) is the moment
|
||||
@@ -290,11 +290,11 @@ at" gap:
|
||||
Swept for names a newcomer couldn't decode; the tree is mostly clean here (a credit to it). Two
|
||||
minor notes:
|
||||
|
||||
- **`guid_diff` / `GuidBaseline` (`guid_diff.h:40`)** — "GUID diff" is decodable in context (it
|
||||
- **`guid_diff` / `GuidBaseline` (`guid_diff.h`'s `GuidBaseline`)** — "GUID diff" is decodable in context (it
|
||||
diffs the live track/item GUID set between polls) but `GuidBaseline` reads more clearly as "the
|
||||
previous-poll snapshot" than the module name suggests. Low priority; leave unless its `core/view`
|
||||
relocation invites it.
|
||||
- **`MinMax` (`peaks.h:30`), `KitBox` (`component_geometry.h:28`)** — terse but correct and local;
|
||||
- **`MinMax` (`peaks.h`'s `MinMax`), `KitBox` (`component_geometry.h`'s `KitBox`)** — terse but correct and local;
|
||||
no change. Named here only to record they were swept and cleared.
|
||||
|
||||
### 2b.5 What the naming audit does NOT touch (hard boundary)
|
||||
|
||||
@@ -25,7 +25,8 @@ with its reasoning stated; contradict it in review with an argument, not a prefe
|
||||
|
||||
**Slug convention.** `Λ → l`, so tracks dispatch into `pl-w<wave>-t<track>-<slug>` —
|
||||
Λ-W2-T1 into `pl-w2-t1-linux-compile-blockers`. The two audits already ran under
|
||||
`pl-w1-t1-build-toolchain-audit` and `pl-w1-t2-source-runtime-audit`. **`docs/PLAN.md:33–36`
|
||||
`pl-w1-t1-build-toolchain-audit` and `pl-w1-t2-source-runtime-audit`. **`docs/PLAN.md`'s
|
||||
"Worktree slug convention" paragraph
|
||||
lists the transliterations for Θ / Ξ / Γ / Ψ / Ε / Ρ and does not yet carry Λ — adding it
|
||||
is a plan-doc edit this phase's PLAN.md entry must make.**
|
||||
|
||||
@@ -77,21 +78,21 @@ this doc re-read against the tree while writing.
|
||||
**The extension is close — two one-line compile blockers stand between the tree and a
|
||||
GCC/Clang build.**
|
||||
|
||||
- `src/shell/panel/draw_kit.cpp:73` passes `DEFAULT_PITCH | FF_DONTCARE` to `CreateFont`.
|
||||
- `src/shell/panel/draw_kit.cpp`'s `loadFont` passes `DEFAULT_PITCH | FF_DONTCARE` to `CreateFont`.
|
||||
`FF_DONTCARE` has zero occurrences anywhere in `vendor/WDL/` (L2-01); the file is not
|
||||
platform-guarded, only its include is (`:70–77`). `draw_kit` links into both modules, so
|
||||
platform-guarded, only its include is (`loadFont`'s whole body). `draw_kit` links into both modules, so
|
||||
nothing builds.
|
||||
- `src/shell/actions/instrument_drop_win.cpp:59` calls `GetCurrentProcessId()` inside
|
||||
`writeTempPreset` with no platform branch anywhere in the TU. SWELL exports
|
||||
- `src/shell/actions/instrument_drop_win.cpp`'s `writeTempPreset` calls `GetCurrentProcessId()`
|
||||
with no platform branch anywhere in the TU. SWELL exports
|
||||
`GetCurrentThreadId` and not this (L2-02). The PID exists only to keep two concurrent
|
||||
REAPER instances from colliding in the shared temp dir; the atomic counter at `:52`
|
||||
REAPER instances from colliding in the shared temp dir; the atomic counter in the same function
|
||||
already carries the intra-process half.
|
||||
|
||||
**`core/` is genuinely pure, and it was verified rather than assumed.** Every `#include`
|
||||
under `src/core/**` is a `core/` sibling, one of 26 standard headers, or the generated
|
||||
`version_generated.h` — zero REAPER, SWELL, WDL, LICE, VST3 or `windows.h` (T2 §1.1). The
|
||||
whole directory contains nine preprocessor conditional lines, exactly one of which is a
|
||||
platform fork, and that one (`capture_paths.cpp:18–20`, the Windows case-fold) is *correct*
|
||||
platform fork, and that one (`capture_paths.cpp`'s `normalizeSlashes`, the Windows case-fold) is *correct*
|
||||
for Linux with both branches already asserted by `tests/test_capture_paths.cpp`. All 91
|
||||
test TUs under `tests/` are platform-neutral.
|
||||
|
||||
@@ -100,24 +101,25 @@ double-buffered LICE `WM_PAINT`, mouse/wheel/capture, `WM_CAPTURECHANGED` rollba
|
||||
seven cursors, menus, the keyboard accelerator path, modifier reads, tooltips, drag-out and
|
||||
`DragQueryFile`/`DragFinish` were each checked by name against `swell-functions.h` /
|
||||
`swell-types.h` and are present (T2 §1.5). The Windows-only escapes are three:
|
||||
`DragAcceptFiles` (`panel_window.cpp:148–150`, `#ifdef _WIN32`), `SHFileOperationW`
|
||||
`DragAcceptFiles` (`panel_window.cpp`'s `openPanel`, under `#ifdef _WIN32`), `SHFileOperationW`
|
||||
(prune), and OLE `DoDragDrop` (drag-out) — each already carrying a non-Windows branch or a
|
||||
documented reason it does not. **This is the audits' single most load-bearing finding.**
|
||||
|
||||
**The dock panel will not appear until the dialog-resource question is answered.**
|
||||
`panel_window.cpp:135` is `CreateDialogParam(g_hInst, MAKEINTRESOURCE(IDD_BANK_PANEL), …)`,
|
||||
`panel_window.cpp`'s `openPanel` calls `CreateDialogParam(g_hInst, MAKEINTRESOURCE(IDD_BANK_PANEL), …)`,
|
||||
which SWELL resolves out of a per-module registry populated by a **resgen-generated source
|
||||
file that is not in the Linux target**: `src/app/CMakeLists.txt:97` has the `target_sources`
|
||||
line commented out (and `:86` for macOS). The registry head stays null, `SWELL_CreateDialog`
|
||||
returns null, `panel_window.cpp:137` returns, and the toggle action is a silent no-op with
|
||||
file that is not in the Linux target**: `src/app/CMakeLists.txt`'s `else()` (Linux) branch has the `target_sources`
|
||||
line commented out (and the `elseif(APPLE)` branch's own copy for macOS). The registry head stays null, `SWELL_CreateDialog`
|
||||
returns null, `openPanel`'s `if (!g_panel.hwnd) return;` guard returns, and the toggle action is a silent no-op with
|
||||
no console line and no Actions-list checkmark (Λ-01, L2-06). Three defects stack in the
|
||||
commented-out instructions themselves: the script named at `:96` (`mac_resgen.php`) does
|
||||
commented-out instructions themselves: the script named in the `elseif(APPLE)`/`else()` branches' comment lines (`mac_resgen.php`) does
|
||||
not exist, the output filename is wrong, and the output is an `#include`-only artifact that
|
||||
cannot be a `target_sources` entry at all (Λ-01). Λ-F2 decides the route.
|
||||
|
||||
**The instrument's Linux editor is a from-scratch X11 job, and today the target does not
|
||||
configure at all.** `src/shell/instrument/CMakeLists.txt:9` is
|
||||
`if(WIN32 AND EXISTS "${VST3_SDK}/…/pluginfactory.cpp")` — a conjunction, so a Linux
|
||||
configure at all.** `src/shell/instrument/CMakeLists.txt`'s
|
||||
`if(WIN32 AND EXISTS "${VST3_SDK}/…/pluginfactory.cpp")` gate is
|
||||
a conjunction, so a Linux
|
||||
configure silently omits `reasampler_vst` even with the submodule slice fully initialised.
|
||||
Beyond the gate: the wrong module entry point is compiled, the artifact is a file where
|
||||
Linux wants a directory bundle, nothing hands a VST3 plugin the SWELL function table, and
|
||||
@@ -127,7 +129,7 @@ drawing survives a window-system change intact. It is the window and event plumb
|
||||
is entirely absent.
|
||||
|
||||
**Nothing about the build is optimized, and the documented ship command is a no-op on
|
||||
Linux.** Root `CMakeLists.txt:28–30` is the complete list of language settings — there is
|
||||
Linux.** Root `CMakeLists.txt`'s `set(CMAKE_CXX_STANDARD ...)`/`set(CMAKE_CXX_STANDARD_REQUIRED ...)`/`set(CMAKE_POSITION_INDEPENDENT_CODE ...)` block is the complete list of language settings — there is
|
||||
no `CMAKE_BUILD_TYPE`, no `CMAKE_CXX_FLAGS`, no IPO/LTO, and no `target_compile_options`
|
||||
anywhere in the tree. `--config Release` is accepted and ignored by Ninja and Make, so the
|
||||
README's ship incantation produces a binary with no `-O` flag at all, on a tree whose
|
||||
@@ -214,7 +216,7 @@ meaning the same edit covers both. **Those edits still get made in their shared
|
||||
noted as shared** — a `#ifdef _WIN32` / `#else` that is right for both costs nothing extra
|
||||
and does not require a mac. What is out is: macOS as a phase deliverable, any macOS
|
||||
verification, the `swell-modstub.mm`-under-a-CXX-only-`project()` question (root
|
||||
`CMakeLists.txt:26` is `LANGUAGES CXX`), signing and notarization, and the macOS-only
|
||||
`CMakeLists.txt`'s `project(...)` call is `LANGUAGES CXX`), signing and notarization, and the macOS-only
|
||||
half of L2-11 (`normalizeSlashes` under-folds on case-insensitive APFS — a real pre-existing
|
||||
defect this phase surfaces and does not own).
|
||||
|
||||
@@ -283,7 +285,7 @@ Route B3a (Λ-D2) is the ruled route. It must be reached without the stub's own
|
||||
|
||||
1. **Never define `SWELL_LOAD_SWELL_DYLIB`.** Compile
|
||||
`swell-modstub-generic.cpp` in its default branch — the same branch the extension already
|
||||
uses (`src/app/CMakeLists.txt:91–92`) — which exports `SWELL_dllMain(hInst, callMode,
|
||||
uses (`src/app/CMakeLists.txt`'s `else()` (Linux) branch's `target_sources`/`target_compile_definitions` pair) — which exports `SWELL_dllMain(hInst, callMode,
|
||||
GetFunc)` (`:135`) and calls `doinit` on the pointer it is handed. The whole file is
|
||||
inside `#ifdef SWELL_PROVIDED_BY_APP` (`:21`), so the VST3 target must define that
|
||||
symbol too; today it does not.
|
||||
@@ -422,8 +424,8 @@ audits and verified in Λ-W3; T4 is verifiable on the current box.
|
||||
**Goal.** The extension compiles and links under GCC/Clang, and when it refuses to load it
|
||||
says why instead of vanishing.
|
||||
|
||||
**Surface boundary — owns:** `src/shell/panel/draw_kit.cpp` (`loadFont`, `:70–77`),
|
||||
`src/shell/actions/instrument_drop_win.cpp` (`writeTempPreset`, `:50–61`), `src/app/main.cpp`
|
||||
**Surface boundary — owns:** `src/shell/panel/draw_kit.cpp` (`loadFont`),
|
||||
`src/shell/actions/instrument_drop_win.cpp` (`writeTempPreset`), `src/app/main.cpp`
|
||||
(the `REAPERAPI_LoadAPI` failure branch only). **Does not own:** any `CMakeLists.txt`,
|
||||
`panel_window.cpp`, or any `core/` file.
|
||||
|
||||
@@ -432,7 +434,7 @@ says why instead of vanishing.
|
||||
**Do not add `windows.h`** (L2-01's stated direction). The family bits are advisory to
|
||||
Windows' font mapper and meaningless to fontconfig.
|
||||
- Replace `GetCurrentProcessId()` with a platform-neutral uniqueness source behind a guard;
|
||||
the atomic counter at `:52` already carries the intra-process half (L2-02).
|
||||
the atomic counter in `writeTempPreset` already carries the intra-process half (L2-02).
|
||||
- On the load-failure branch, either switch `main.cpp` to `REAPERAPI_MINIMAL` plus an
|
||||
explicit `WANT` list — the pattern `panel_window.cpp` and `panel_audition.cpp` already
|
||||
use — or keep the full load and print the failure count via
|
||||
@@ -507,8 +509,8 @@ only; the `WIN32` gate is Λ-W6-T1's), `README.md` and root `CLAUDE.md` §"Build
|
||||
**Goal.** The docked bank panel opens on Linux, and if it ever fails to, it says so.
|
||||
|
||||
**Surface boundary — owns:** `src/shell/panel/panel_window.cpp` (the `CreateDialogParam`
|
||||
call at `:135–137`, the dialog proc's platform contract, the drop-accept opt-in at
|
||||
`:145–150`), `src/resource.rc`, `src/resource.h`, and — **under the resgen route only** —
|
||||
call and its `if (!g_panel.hwnd) return;` guard, both in `openPanel`, the dialog proc's platform contract, the drop-accept opt-in
|
||||
also in `openPanel`), `src/resource.rc`, `src/resource.h`, and — **under the resgen route only** —
|
||||
one `target_sources` line in `src/app/CMakeLists.txt`'s `else()` branch plus a new
|
||||
include-shim TU. **Does not own:** any other panel TU, `draw_kit`, or any CMake target
|
||||
property.
|
||||
@@ -685,7 +687,7 @@ as the Linux defaults, following SWELL's own no-fontconfig fallback list
|
||||
(LiberationSans/DejaVuSans, LiberationMono/DejaVuSansMono) as precedent. **One code path**,
|
||||
shared with macOS's eventual San Francisco/Menlo (Λ-D4: made in shared form, not verified).
|
||||
|
||||
The subtlety worth carrying into the work: `draw_kit.cpp:74`'s `if (!hf) return` guard does
|
||||
The subtlety worth carrying into the work: `draw_kit.cpp`'s `loadFont`'s `if (!hf) return` guard does
|
||||
**not** catch the failure mode here. SWELL's `CreateFont` always returns a non-null handle
|
||||
even when the face never resolved — the failure is recorded internally as a null
|
||||
`typedata`, not as a null return. So a wrong or missing face is not observable at the call
|
||||
@@ -706,7 +708,7 @@ cosmetic or a readability regression. **Discharges:** L2-09.
|
||||
**Goal.** The build's source list stops relying on every TU's own `#ifdef` discipline, and
|
||||
the invariants Linux weakens are stated where a reviewer will read them.
|
||||
|
||||
**Surface boundary — owns:** the `target_sources` list in `src/app/CMakeLists.txt:8–51`
|
||||
**Surface boundary — owns:** the `add_library(reaper_reasampler MODULE ...)` source list in `src/app/CMakeLists.txt`
|
||||
(the *list*; the property blocks are Λ-W2-T2's), any new platform-sibling TU the sweep
|
||||
showed was needed, `src/shell/actions/drag_out_win.h`'s invariant comment, and the
|
||||
corresponding `src/shell/**/CLAUDE.md` invariant passages. **Does not own:** any behaviour
|
||||
@@ -718,7 +720,7 @@ change in a shipped code path.
|
||||
`arrange_drop_win.cpp` and `instrument_drop_win.cpp` are `_win`-suffixed for the surface
|
||||
they serve, not for a platform dependency, and the audits found them portable by
|
||||
inspection — confirm against the actual compile rather than re-inspecting.
|
||||
- **L2-10** — make `drag_out_win.h:7–11` the doc a Linux reviewer is pointed at, and treat
|
||||
- **L2-10** — make `drag_out_win.h`'s file-header invariant comment the doc a Linux reviewer is pointed at, and treat
|
||||
"MOVE is structurally impossible" as a Windows-scoped claim. **The wording is Λ-F3's
|
||||
ruling**; the edit is this track's regardless of which way it goes.
|
||||
- **L2-11** — no Linux action. If the predicate is touched at all it becomes
|
||||
@@ -808,7 +810,7 @@ modstub TU), and the three D5 passages in `src/core/instrument/CLAUDE.md`,
|
||||
includes `<windows.h>` with no `SMTG_OS_*` guard; `linuxmain.cpp` exports `ModuleEntry`
|
||||
and `ModuleExit`, **both mandatory** — the SDK's own loader refuses the module without
|
||||
either. Both files are already vendored; this is a source swap plus a platform `if()`.
|
||||
- **Split the `WIN32 AND EXISTS` conjunction** at `src/shell/instrument/CMakeLists.txt:9`.
|
||||
- **Split the `WIN32 AND EXISTS` conjunction** in `src/shell/instrument/CMakeLists.txt`'s `if(WIN32 AND EXISTS ...)` gate.
|
||||
The `EXISTS` half stays (a fresh clone with no VST3 slice must still configure); the
|
||||
`WIN32` half becomes a Windows-or-Linux predicate.
|
||||
- **B2** — the artifact becomes a directory:
|
||||
@@ -1001,7 +1003,7 @@ the strict reading:
|
||||
| File | Tracks | Nature |
|
||||
|---|---|---|
|
||||
| `src/app/CMakeLists.txt` | Λ-W2-T2 (property + platform blocks), Λ-W2-T3 (one `target_sources` line, **resgen route only**), Λ-W4-T3 (the source list), Λ-W5-T1 (the `install()` rule) | Four disjoint regions of one file. Λ-W2-T2 and Λ-W2-T3 are the only pair in the same wave; one line each. |
|
||||
| `src/shell/panel/draw_kit.cpp` | Λ-W2-T1 (`loadFont`'s `CreateFont` args, `:73`), Λ-W4-T2 (the five call sites, `:154–158`) | Different waves. |
|
||||
| `src/shell/panel/draw_kit.cpp` | Λ-W2-T1 (`loadFont`'s `CreateFont` args), Λ-W4-T2 (`kitFontsInit`'s five `loadFont` call sites) | Different waves. |
|
||||
| `src/shell/instrument/CMakeLists.txt` | Λ-W2-T2 (thread linkage), Λ-W6-T1 (gate, entry point, bundle, install), Λ-W7-T1 (one added TU) | Different waves. |
|
||||
| `src/shell/panel/panel_window.cpp` | Λ-W2-T3 alone | **Deliberately not split.** The L2-06 diagnostic and the Λ-01 resource route are the same function; under the resource-id-0 route they are the same *line*. Splitting them would be semantic contention. |
|
||||
| `src/shell/instrument/editor_platform.cpp` | Λ-W6-T2 (the refusal branch), Λ-W8-T1 (the real branch) | Different waves; the second replaces the first's computation without touching its call sites. |
|
||||
@@ -1125,7 +1127,7 @@ carried. Nothing is gated on it before Λ-W5.
|
||||
0, which creates an opaque child window, provided a `WNDPROC` returning `LRESULT` (cast to
|
||||
`DLGPROC`) is passed instead of a real `DLGPROC`. The implementation confirms both halves —
|
||||
`swell-dlg-generic.cpp` skips the resource lookup entirely when `resid` is 0. And
|
||||
`IDD_BANK_PANEL` is precisely the case it was written for: `src/resource.rc:18–22` is a
|
||||
`IDD_BANK_PANEL` is precisely the case it was written for: `src/resource.rc`'s `IDD_BANK_PANEL` dialog block is a
|
||||
`WS_CHILD` dialog with an empty `BEGIN`/`END` body and zero controls, whose own header
|
||||
comment says "the bank_panel shell owns every pixel and draws the sample grid with LICE in
|
||||
`WM_PAINT`". **Taking this route deletes the entire resgen pipeline from the non-Windows
|
||||
|
||||
@@ -1117,7 +1117,7 @@ Addendum is the *why*; those are the *what/how*.
|
||||
|
||||
**Framing.** Folds one more control into the S-VIEW redesign: a **visual velocity → amp
|
||||
transfer-curve editor**. Today the engine maps velocity to gain *linearly* (`velocityGain_ =
|
||||
velocity / 127.0`, `sampler_core.cpp:261`), applied once at note-on in `Voice::start()`. Daniel
|
||||
velocity / 127.0`, `Voice::start()`), applied once at note-on in `Voice::start()`. Daniel
|
||||
wants that mapping to become an **editable transfer curve** — a bezier from a default flat line to
|
||||
an arbitrary multi-point curve — so velocity dynamics are fully shapeable per sound.
|
||||
|
||||
@@ -1181,7 +1181,7 @@ a LICE shell that draws handles and routes the mouse).
|
||||
evaluation is called at note-on, not per frame (see call 4).
|
||||
|
||||
4. **Voice-engine application point → `Voice::start()`, replacing the linear `velocity/127`.**
|
||||
Confirmed from source: `sampler_core.cpp:261` computes `velocityGain_ = velocity / 127.0` **once
|
||||
Confirmed from source: `Voice::start()` computes `velocityGain_ = velocity / 127.0` **once
|
||||
at note-on** inside `Voice::start()`; the per-frame render path (`advanceFrame`, line 408:
|
||||
`gain = amp * velocityGain_`) then just multiplies the cached scalar. So the transfer curve
|
||||
slots in at exactly one line: `velocityGain_ = curve.eval(velocity)` at note-on — **off the
|
||||
|
||||
@@ -199,7 +199,7 @@ it — three small pure additions and one bounded seam:
|
||||
|
||||
1. **A render destination that is not the bank.** `OfflineRenderBackend::capture`
|
||||
derives its output path from `deriveBankPaths(projectDir, …)` unconditionally
|
||||
(`capture.cpp:417`) and points `RENDER_FILE` at the bank folder. Nothing about
|
||||
(`capture.cpp`'s `OfflineRenderBackend::capture`) and points `RENDER_FILE` at the bank folder. Nothing about
|
||||
that is parameterized. The alternative — render into the bank and then move the
|
||||
file out — was rejected: it puts a transient, unindexed, unowned file inside the
|
||||
folder prune enumerates, which is exactly the file class the ownership rule exists
|
||||
|
||||
@@ -45,7 +45,7 @@ below:
|
||||
|
||||
Two sharp edges follow directly and recur throughout this note:
|
||||
|
||||
- **The `STABLE_FOREVER_STRING` command-id contract** (CLAUDE.md; `main.cpp:41`,
|
||||
- **The `STABLE_FOREVER_STRING` command-id contract** (CLAUDE.md; `app_version.h`'s `commandIdPrefix()`,
|
||||
prefix `CEREBELLUM_REASAMPLER_`). Command-id strings are minted once and **never
|
||||
changed after shipping** — user keybindings key off them. Two coexisting binaries
|
||||
that register the *same* id strings collide in REAPER's Actions list.
|
||||
@@ -67,9 +67,9 @@ allowed to touch.
|
||||
|
||||
## What we have today
|
||||
|
||||
- No version anywhere. `CMakeLists.txt:2` is `project(reaper_reasampler LANGUAGES
|
||||
- No version anywhere. `CMakeLists.txt` is `project(reaper_reasampler LANGUAGES
|
||||
CXX)` — no `VERSION`. The binary announces itself only as `"ReaSampler loaded.\n"`
|
||||
to the console (`main.cpp:960`). There is no number a user, a bug report, or a
|
||||
to the console (`main.cpp`). There is no number a user, a bug report, or a
|
||||
future migration can key off.
|
||||
- The natural user-visible readout already exists: the docked LICE bank panel, and
|
||||
the console (`ShowConsoleMsg`). A version has cheap homes; none is wired.
|
||||
|
||||
Reference in New Issue
Block a user