diff --git a/docs/product/bank-package.md b/docs/product/bank-package.md index fca7e79..d011804 100644 --- a/docs/product/bank-package.md +++ b/docs/product/bank-package.md @@ -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 `/reasampler_bank/` (`core/capture/capture_paths.h:16`, +the audio sits in `/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:263-269`). 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:263-269` 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 `` 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:121`, 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. diff --git a/docs/product/code-organization.md b/docs/product/code-organization.md index c0b9357..65b7d3b 100644 --- a/docs/product/code-organization.md +++ b/docs/product/code-organization.md @@ -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) diff --git a/docs/product/linux-readiness.md b/docs/product/linux-readiness.md index c9e983f..b8afb29 100644 --- a/docs/product/linux-readiness.md +++ b/docs/product/linux-readiness.md @@ -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-t-` — Λ-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 `` 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 diff --git a/docs/product/midi-playback.md b/docs/product/midi-playback.md index 97e58c6..add589d 100644 --- a/docs/product/midi-playback.md +++ b/docs/product/midi-playback.md @@ -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 diff --git a/docs/product/render-in-place.md b/docs/product/render-in-place.md index f3afac9..da1873c 100644 --- a/docs/product/render-in-place.md +++ b/docs/product/render-in-place.md @@ -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 diff --git a/docs/product/versioning-and-release.md b/docs/product/versioning-and-release.md index a48b7c4..b3464cc 100644 --- a/docs/product/versioning-and-release.md +++ b/docs/product/versioning-and-release.md @@ -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. diff --git a/src/core/package/CLAUDE.md b/src/core/package/CLAUDE.md index 4733c76..c5600b0 100644 --- a/src/core/package/CLAUDE.md +++ b/src/core/package/CLAUDE.md @@ -187,7 +187,7 @@ append-only rule and the per-fixture inventory. hostile/older package declaring one is not this track's concern). - **`import_plan`'s `spelledLikeABankFile` mints a fresh name even with NO collision, and that third condition is a deliberate decision, not spec-derived.** - `docs/product/bank-package.md:447` ties the auto-rename mint to a *collision* + `docs/product/bank-package.md` §"Identity and collision on import" (collision rule 2) ties the auto-rename mint to a *collision* only; `spelledLikeABankFile` additionally mints whenever the package's own name isn't spelled the way `deriveBankPaths` spells one (extension, sanitized stem). Kept for two reasons: uniform folder spelling for every landed file regardless of @@ -195,4 +195,4 @@ append-only rule and the per-fixture inventory. filename or carries an unexpected extension (e.g. `evil.exe`) lands sanitized (`evil_.wav`) rather than verbatim. `ImportPlan` counts this separately from a genuine folder-name collision (`sanitizeRenameCount` vs `collisionRenameCount`) so - the summary line means what `bank-package.md:447` says it means. + the summary line means what `bank-package.md` §"Identity and collision on import" (collision rule 2) says it means.