From cbfc13c4e0db103a3f5ea9a55138f2077f983872 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 24 Jul 2026 05:35:16 -0400 Subject: [PATCH 1/2] =?UTF-8?q?feat(banks):=20bindable=20multi-bank=20acti?= =?UTF-8?q?on=20family=20=E2=80=94=20create/rename/delete/evacuate/activat?= =?UTF-8?q?e/move/copy=20+=20full-height=20toggles=20(B3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/actions.cpp | 361 ++++++++++++++++++++++++++++++++++++++- src/actions.h | 27 +++ src/bank_book.cpp | 16 ++ src/bank_book.h | 13 ++ src/bank_panel.cpp | 31 ++++ src/bank_panel.h | 30 ++++ src/main.cpp | 10 ++ tests/test_bank_book.cpp | 59 +++++++ 8 files changed, 546 insertions(+), 1 deletion(-) diff --git a/src/actions.cpp b/src/actions.cpp index efb961f..2a36c0e 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -24,7 +24,9 @@ #include #include -#include "persist.h" // ReaSamplerSession (owns view() model) +#include "bank_book.h" // BankBook, nextBankId, TransferResult, kPoolBankId (B1) +#include "bank_panel.h" // selection seam + full-height toggles (B3/B4) +#include "persist.h" // ReaSamplerSession (owns book() + view() model) #include "track_guid.h" // shared MediaTrack* -> canonical GUID key #include "view.h" // applyMode (D2 shell) #include "view_mode_model.h" @@ -37,6 +39,10 @@ #define REAPERAPI_WANT_EnumProjects #define REAPERAPI_WANT_Main_SaveProject #define REAPERAPI_WANT_ShowConsoleMsg +#define REAPERAPI_WANT_GetUserInputs +#define REAPERAPI_WANT_ShowMessageBox +#define REAPERAPI_WANT_genGuid +#define REAPERAPI_WANT_guidToString #include "reaper_plugin_functions.h" namespace reasampler { @@ -271,4 +277,357 @@ void designViewUnregisterActions(reaper_plugin_info_t* rec) { g_session = nullptr; } +// =========================================================================== +// Multi-bank action family (Phase B3) +// =========================================================================== +// +// Each action drives the B1 model on g_session->book() and persists via +// g_session->saveToActiveProject() so the change travels with the .rpp — exactly as +// the capture path persists a new Sample (main.cpp RunCapture). The book's rules +// (pool privileges, collapse-by-hash, active-fallback-to-pool) all live in bank_book; +// these handlers only call the model and react to the boolean / TransferResult. +// +// REFERENCE-INVALIDATION GUARDRAIL (B2 review): book().activeIndex() / bank()->index +// return a reference INTO the book's internal vector, which a create/delete can +// reallocate. No handler here caches a BankIndex& (or a Bank*) across a structural +// mutation — each resolves ids to strings up front and re-resolves after any +// create/delete. Move/copy pass ids (not references) straight to moveSample/copySample. + +namespace { + +// FOREVER-STABLE multi-bank action-id strings. Same CEREBELLUM_REASAMPLER_ family +// prefix; each is minted into a persistent command id user keybindings key off — +// NEVER change these after ship. +constexpr const char* kIdBankCreate = "CEREBELLUM_REASAMPLER_BANK_CREATE"; +constexpr const char* kIdBankRename = "CEREBELLUM_REASAMPLER_BANK_RENAME"; +constexpr const char* kIdBankDelete = "CEREBELLUM_REASAMPLER_BANK_DELETE"; +constexpr const char* kIdBankEvacuate = "CEREBELLUM_REASAMPLER_BANK_EVACUATE"; +constexpr const char* kIdBankActivateNext = "CEREBELLUM_REASAMPLER_BANK_ACTIVATE_NEXT"; +constexpr const char* kIdBankActivatePool = "CEREBELLUM_REASAMPLER_BANK_ACTIVATE_POOL"; +constexpr const char* kIdBankMoveSel = "CEREBELLUM_REASAMPLER_BANK_MOVE_SELECTED"; +constexpr const char* kIdBankCopySel = "CEREBELLUM_REASAMPLER_BANK_COPY_SELECTED"; +constexpr const char* kIdBankPoolFull = "CEREBELLUM_REASAMPLER_BANK_POOL_FULLHEIGHT"; +constexpr const char* kIdBankBanksFull = "CEREBELLUM_REASAMPLER_BANK_BANKS_FULLHEIGHT"; + +int g_cmdBankCreate = 0; +int g_cmdBankRename = 0; +int g_cmdBankDelete = 0; +int g_cmdBankEvacuate = 0; +int g_cmdBankActivateNext = 0; +int g_cmdBankActivatePool = 0; +int g_cmdBankMoveSel = 0; +int g_cmdBankCopySel = 0; +int g_cmdBankPoolFull = 0; +int g_cmdBankBanksFull = 0; + +gaccel_register_t g_accelBankCreate{}; +gaccel_register_t g_accelBankRename{}; +gaccel_register_t g_accelBankDelete{}; +gaccel_register_t g_accelBankEvacuate{}; +gaccel_register_t g_accelBankActivateNext{}; +gaccel_register_t g_accelBankActivatePool{}; +gaccel_register_t g_accelBankMoveSel{}; +gaccel_register_t g_accelBankCopySel{}; +gaccel_register_t g_accelBankPoolFull{}; +gaccel_register_t g_accelBankBanksFull{}; + +// Persists the book after a bank mutation. Mirrors the capture path (main.cpp +// RunCapture): a bank change is held in-session and written to the active project's +// ext state so it travels with the .rpp. No Save-As prompt here — saveToActiveProject +// no-ops on an unsaved project (the change stays valid for the session and persists +// on the user's next save), matching how capture persists. +void persistBook() { g_session->saveToActiveProject(); } + +// Prompts the user for a single line of text via REAPER's stock input dialog. +// GetUserInputs(title, num_inputs=1, captions_csv, retvals_csv, sz) -> false on +// cancel (SDK ~3808). `initial` pre-fills the field. Returns false (leaving `out` +// untouched) on cancel or an empty entry. Self-contained bindable-action name entry; +// B4's panel affordances supersede this with in-panel editing. +bool promptText(const char* title, const char* caption, const std::string& initial, + std::string& out) { + std::vector buf(512, '\0'); + // Pre-fill: GetUserInputs seeds the field from the retvals buffer's initial value. + std::snprintf(buf.data(), buf.size(), "%s", initial.c_str()); + if (!GetUserInputs(title, 1, caption, buf.data(), static_cast(buf.size()))) + return false; // user cancelled + std::string s(buf.data()); + if (s.empty()) return false; // an empty name is not a valid bank name + out = std::move(s); + return true; +} + +// Mints a fresh, genuine REAPER GUID string as a stable bank id (the B2/model design: +// ids are caller-supplied and stable; the model stays pure and mints none). Distinct +// from a track GUID by origin only — both are canonical guidToString output. +std::string mintBankId() { + GUID g{}; + genGuid(&g); + char buf[64] = {0}; // guidToString needs >=64 chars (SDK contract) + guidToString(&g, buf); + return std::string(buf); +} + +// Resolves a user-typed bank reference (a display name) to a bank id, scanning the +// book's banks in ordinal order. Case-sensitive exact match on displayName; "Pool" +// resolves the pool. Returns "" when no bank carries that name. Kept in the action +// layer (not the model) — it is UI name-resolution, not a model rule. +std::string bankIdByDisplayName(const std::string& name) { + for (const Bank& b : g_session->book().banks()) + if (b.displayName == name) return b.id; + return {}; +} + +// -- Action bodies --------------------------------------------------------- + +// Create a named bank: prompt for a display name, mint a stable GUID id, create it in +// the model, persist. The new bank is NOT auto-activated (create and activate are +// distinct acts — mirrors capture/placement separation). A duplicate-name is allowed +// (display names are not unique in the model); the fresh GUID keeps the id unique. +void doBankCreate() { + std::string name; + if (!promptText("ReaSampler: create bank", "Bank name:", "", name)) return; + const std::string id = mintBankId(); + if (!g_session->book().createBank(id, name)) { + ShowConsoleMsg("ReaSampler: could not create bank (id collision — try again).\n"); + return; + } + persistBook(); + ShowConsoleMsg(("ReaSampler: created bank \"" + name + "\".\n").c_str()); +} + +// Rename a bank: prompt for which bank (by current display name) and the new name. +// The pool is un-renamable (the model rejects it). Two prompts keep the bindable form +// self-contained; B4's panel renames in place on a tab. +void doBankRename() { + std::string which; + if (!promptText("ReaSampler: rename bank", "Bank to rename (current name):", "", + which)) + return; + const std::string id = bankIdByDisplayName(which); + if (id.empty()) { + ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str()); + return; + } + std::string newName; + if (!promptText("ReaSampler: rename bank", "New name:", which, newName)) return; + if (!g_session->book().renameBank(id, newName)) { + ShowConsoleMsg("ReaSampler: cannot rename that bank (the pool is un-renamable).\n"); + return; + } + persistBook(); + ShowConsoleMsg(("ReaSampler: renamed \"" + which + "\" -> \"" + newName + "\".\n") + .c_str()); +} + +// Delete a named bank. Bindable safe-form of the confirm-on-non-empty guardrail: +// prompt for the bank; if it holds members, a YESNO ShowMessageBox names evacuate as +// the alternative before dropping them (a plain delete orphans those members' files +// until prune — CONTEXT.md §delete). An empty bank deletes with no prompt. The richer +// panel confirm (naming evacuate inline, with a one-click evacuate) arrives in B4. +void doBankDelete() { + std::string which; + if (!promptText("ReaSampler: delete bank", "Bank to delete:", "", which)) return; + const std::string id = bankIdByDisplayName(which); + if (id.empty()) { + ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str()); + return; + } + // Read member count BEFORE deleting (the Bank* is invalidated by deleteBank; we do + // not cache it — resolve size to an int up front). + const Bank* b = g_session->book().bank(id); + if (!b) return; // race-safe: id resolved above but re-check + const std::size_t members = b->index.size(); + if (members > 0) { + const std::string msg = + "\"" + which + "\" holds " + std::to_string(members) + + (members == 1 ? " sample" : " samples") + + ".\n\nDeleting drops them from every bank (their files are NOT deleted, " + "but no bank will reference them until prune).\n\nTo keep the samples, " + "cancel and Evacuate the bank to the pool first.\n\nDelete anyway?"; + const int r = ShowMessageBox(msg.c_str(), "ReaSampler: delete non-empty bank", 4); + if (r != 6) return; // 6 == YES; anything else cancels (SDK ~6544) + } + if (!g_session->book().deleteBank(id)) { + ShowConsoleMsg("ReaSampler: cannot delete that bank (the pool is un-deletable).\n"); + return; + } + persistBook(); + ShowConsoleMsg(("ReaSampler: deleted bank \"" + which + "\".\n").c_str()); +} + +// Evacuate a named bank: move every member back to the pool (index-only, collapse by +// hash), leaving the bank empty. The pool is un-evacuable (the model rejects it). The +// intended "keep the samples" companion to delete. +void doBankEvacuate() { + std::string which; + if (!promptText("ReaSampler: evacuate bank", "Bank to evacuate to the pool:", "", + which)) + return; + const std::string id = bankIdByDisplayName(which); + if (id.empty()) { + ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str()); + return; + } + if (!g_session->book().evacuate(id)) { + ShowConsoleMsg("ReaSampler: cannot evacuate that bank (the pool is the " + "destination, not a source).\n"); + return; + } + persistBook(); + ShowConsoleMsg(("ReaSampler: evacuated \"" + which + "\" to the pool.\n").c_str()); +} + +// Cycle the active bank forward in ordinal order (pool -> named -> ... -> pool), +// via the pure nextBankId helper. Activating a bank changes the CAPTURE TARGET (the +// next capture lands in the newly-active bank — B2's book().activeIndex() seam) and +// never touches the timeline. Persist so the active id travels with the .rpp. +void doBankActivateNext() { + std::vector ids; + ids.reserve(g_session->book().size()); + for (const Bank& b : g_session->book().banks()) ids.push_back(b.id); + const std::string target = nextBankId(ids, g_session->book().activeBankId()); + if (target.empty()) return; // degenerate (no banks) — cannot happen (pool seeded) + if (!g_session->book().setActiveBank(target)) return; + persistBook(); + const Bank* b = g_session->book().bank(target); + ShowConsoleMsg(("ReaSampler: active bank -> \"" + + (b ? b->displayName : target) + "\".\n") + .c_str()); +} + +// Activate the pool directly (the common "back to the default target" jump). Bindable +// direct-by-id form; a general activate-bank-by-name/menu is a B4 affordance. +void doBankActivatePool() { + if (!g_session->book().setActiveBank(kPoolBankId)) return; + persistBook(); + ShowConsoleMsg("ReaSampler: active bank -> \"Pool\".\n"); +} + +// Move or copy the panel's selected samples from the ACTIVE bank into a named +// destination bank (prompted by display name). The panel grid shows the active bank, +// so its selection ids are members of the active bank — that is the source. Both are +// index-only (files never relocate); move removes the source entry, copy retains it; +// both observe destination collapse-by-hash (bank_book). B4's "move to bank" menu will +// drive moveSample/copySample directly with a menu-chosen destination — this bindable +// form is the same operation with a text-prompt destination. +void doBankTransferSelected(bool copy) { + const std::vector selected = bankPanelSelectedSampleIds(); + if (selected.empty()) { + ShowConsoleMsg("ReaSampler: nothing selected in the bank panel to " + "move/copy.\n"); + return; + } + const char* verb = copy ? "copy" : "move"; + const std::string title = std::string("ReaSampler: ") + verb + " selected samples"; + std::string destName; + if (!promptText(title.c_str(), "Destination bank:", "", destName)) return; + const std::string destId = bankIdByDisplayName(destName); + if (destId.empty()) { + ShowConsoleMsg(("ReaSampler: no bank named \"" + destName + "\".\n").c_str()); + return; + } + // Source = the active bank (what the panel grid shows). Pass ids by value — no + // BankIndex& is cached across the loop's mutations. + const std::string srcId = g_session->book().activeBankId(); + if (srcId == destId) { + ShowConsoleMsg("ReaSampler: source and destination are the same bank.\n"); + return; + } + + int ok = 0, collapsed = 0, absent = 0; + for (const std::string& sampleId : selected) { + const TransferResult r = + copy ? g_session->book().copySample(sampleId, srcId, destId) + : g_session->book().moveSample(sampleId, srcId, destId); + switch (r) { + case TransferResult::Moved: + case TransferResult::Copied: ++ok; break; + case TransferResult::Collapsed: ++collapsed; break; + case TransferResult::RejectedSampleAbsent: ++absent; break; + // Unknown-bank / same-bank are pre-checked above; treat defensively as no-ops. + case TransferResult::RejectedUnknownBank: + case TransferResult::RejectedSameBank: break; + } + } + persistBook(); + std::string log = std::string("ReaSampler: ") + verb + " -> \"" + destName + + "\": " + std::to_string(ok) + " " + verb + "d"; + if (collapsed) log += ", " + std::to_string(collapsed) + " collapsed on hash"; + if (absent) log += ", " + std::to_string(absent) + " no longer present"; + log += ".\n"; + ShowConsoleMsg(log.c_str()); +} + +} // namespace + +void bankRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session) { + g_session = session; // shared with the Design View family; same live session + + g_cmdBankCreate = registerAction(rec, kIdBankCreate, g_accelBankCreate, + "ReaSampler: create bank"); + g_cmdBankRename = registerAction(rec, kIdBankRename, g_accelBankRename, + "ReaSampler: rename bank"); + g_cmdBankDelete = registerAction(rec, kIdBankDelete, g_accelBankDelete, + "ReaSampler: delete bank"); + g_cmdBankEvacuate = registerAction(rec, kIdBankEvacuate, g_accelBankEvacuate, + "ReaSampler: evacuate bank to pool"); + g_cmdBankActivateNext = registerAction(rec, kIdBankActivateNext, g_accelBankActivateNext, + "ReaSampler: activate next bank (cycle)"); + g_cmdBankActivatePool = registerAction(rec, kIdBankActivatePool, g_accelBankActivatePool, + "ReaSampler: activate pool"); + g_cmdBankMoveSel = registerAction(rec, kIdBankMoveSel, g_accelBankMoveSel, + "ReaSampler: move selected samples to bank"); + g_cmdBankCopySel = registerAction(rec, kIdBankCopySel, g_accelBankCopySel, + "ReaSampler: copy selected samples to bank"); + g_cmdBankPoolFull = registerAction(rec, kIdBankPoolFull, g_accelBankPoolFull, + "ReaSampler: toggle pool full-height"); + g_cmdBankBanksFull = registerAction(rec, kIdBankBanksFull, g_accelBankBanksFull, + "ReaSampler: toggle banks full-height"); +} + +bool bankHandleCommand(int command) { + if (command == 0 || !g_session) return false; + + if (command == g_cmdBankCreate) { doBankCreate(); return true; } + if (command == g_cmdBankRename) { doBankRename(); return true; } + if (command == g_cmdBankDelete) { doBankDelete(); return true; } + if (command == g_cmdBankEvacuate) { doBankEvacuate(); return true; } + if (command == g_cmdBankActivateNext) { doBankActivateNext(); return true; } + if (command == g_cmdBankActivatePool) { doBankActivatePool(); return true; } + if (command == g_cmdBankMoveSel) { doBankTransferSelected(false); return true; } + if (command == g_cmdBankCopySel) { doBankTransferSelected(true); return true; } + if (command == g_cmdBankPoolFull) { bankPanelToggledPoolFullHeight(); return true; } + if (command == g_cmdBankBanksFull) { bankPanelToggledBanksFullHeight(); return true; } + + return false; // not ours — caller's hookcommand keeps looking +} + +void bankUnregisterActions(reaper_plugin_info_t* rec) { + // Mirror-unregister with '-'-prefixed strings, reverse of registration order. + rec->Register("-gaccel", (void*)&g_accelBankBanksFull); + rec->Register("-command_id", (void*)kIdBankBanksFull); + rec->Register("-gaccel", (void*)&g_accelBankPoolFull); + rec->Register("-command_id", (void*)kIdBankPoolFull); + rec->Register("-gaccel", (void*)&g_accelBankCopySel); + rec->Register("-command_id", (void*)kIdBankCopySel); + rec->Register("-gaccel", (void*)&g_accelBankMoveSel); + rec->Register("-command_id", (void*)kIdBankMoveSel); + rec->Register("-gaccel", (void*)&g_accelBankActivatePool); + rec->Register("-command_id", (void*)kIdBankActivatePool); + rec->Register("-gaccel", (void*)&g_accelBankActivateNext); + rec->Register("-command_id", (void*)kIdBankActivateNext); + rec->Register("-gaccel", (void*)&g_accelBankEvacuate); + rec->Register("-command_id", (void*)kIdBankEvacuate); + rec->Register("-gaccel", (void*)&g_accelBankDelete); + rec->Register("-command_id", (void*)kIdBankDelete); + rec->Register("-gaccel", (void*)&g_accelBankRename); + rec->Register("-command_id", (void*)kIdBankRename); + rec->Register("-gaccel", (void*)&g_accelBankCreate); + rec->Register("-command_id", (void*)kIdBankCreate); + + // g_session is shared with the Design View family; designViewUnregisterActions + // also nulls it. Nulling twice is harmless. Leave it to whichever runs last. + g_session = nullptr; +} + } // namespace reasampler diff --git a/src/actions.h b/src/actions.h index 4b8d382..44bf442 100644 --- a/src/actions.h +++ b/src/actions.h @@ -39,4 +39,31 @@ bool designViewHandleCommand(int command); // '-'-prefixed strings (per the contract's unload rule). Call once on rec==nullptr. void designViewUnregisterActions(reaper_plugin_info_t* rec); +// --- Multi-bank action family (Phase B3) ----------------------------------- +// The bindable action set that drives the multi-bank workflow: create / rename / +// delete / evacuate a bank, activate a bank (direct pool/design-free + cycle), move / +// copy the panel's selected samples into a bank, and the two vertical-split +// full-height toggles. Every mutating action drives the B1 model on +// g_session.book() and persists via g_session.saveToActiveProject() so the change +// travels with the .rpp; the toggles flip the B4-rendered layout bit on the panel. +// +// Same registration/routing/unload contract as the Design View family above and the +// same shared g_session. Kept a distinct trio (not folded into the Design View one) +// because the two families are orthogonal pillars — but they share the single +// hookcommand main.cpp owns; each family's Handle claims only its own ids. + +// Registers the multi-bank family against `rec`. `session` is the live session (must +// outlive registration). Call exactly once at load. Shares g_session with the Design +// View family — pass the SAME session pointer. +void bankRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session); + +// Services one fired command for the multi-bank family. True iff it was one of this +// family's ids (and handled); false otherwise so the caller's hookcommand keeps +// looking. Safe for any command. +bool bankHandleCommand(int command); + +// Mirror-unregisters the multi-bank family with '-'-prefixed strings. Call once on +// rec==nullptr (before g_session is torn down). +void bankUnregisterActions(reaper_plugin_info_t* rec); + } // namespace reasampler diff --git a/src/bank_book.cpp b/src/bank_book.cpp index bec9a03..4fc4901 100644 --- a/src/bank_book.cpp +++ b/src/bank_book.cpp @@ -668,6 +668,22 @@ std::optional BankBook::deserialize(const std::string& json) { return book; } +// --------------------------------------------------------------------------- +// Active-bank cycle ordering (pure, free function — mirror of nextModeId) +// --------------------------------------------------------------------------- + +std::string nextBankId(const std::vector& orderedBankIds, + const std::string& currentBankId) { + if (orderedBankIds.empty()) return {}; // nothing to cycle to + for (std::size_t i = 0; i < orderedBankIds.size(); ++i) { + if (orderedBankIds[i] == currentBankId) + return orderedBankIds[(i + 1) % orderedBankIds.size()]; // wrap past the last + } + // Active id not in the list (stale/unknown) — jump to the first id as a sane + // home rather than returning "" (matches nextModeId's fallback). + return orderedBankIds.front(); +} + BankBook BankBook::loadFromPersisted(const std::string& banksJson, const std::string& legacyJson) { // Precedence 1: the authoritative `banks` blob. A present-but-malformed blob is diff --git a/src/bank_book.h b/src/bank_book.h index 7aeece9..5b9b8e7 100644 --- a/src/bank_book.h +++ b/src/bank_book.h @@ -221,4 +221,17 @@ private: void adoptBanks(std::vector&& banks, const std::string& activeBank); }; +// The next bank id to activate when cycling the active bank forward, in ordinal +// order (the ids arrive pool-first, named 1..N, matching banks()). Wraps: the id +// after the last returns the first (pool → named → … → pool). This is the pure +// decision behind the "cycle active bank" action — the shell reads the book's +// ordered bank ids + current active id, asks for the next, and activates it. +// * empty list -> "" (nothing to cycle to) +// * single id (pool-only) -> that id (a one-bank book stays put) +// * currentBankId not present -> the first id (a sane home to jump to) +// Exposed as a free function (not a BankBook member) so it is unit-testable against +// a bare id vector without a full book. Mirror of view_mode_model's nextModeId. +std::string nextBankId(const std::vector& orderedBankIds, + const std::string& currentBankId); + } // namespace reasampler diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 1b3a9e0..965c909 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -194,6 +194,13 @@ struct PanelState { // persistence is a noted follow-on. Mutated ONLY by a click in the footer strip. TailSetting tail; + // --- Vertical-split full-height layout (Phase B3) ------------------------- + // Which region(s) the vertical split shows: both (Split, default), pool only, + // or named-banks only. B3 actions flip it (bankPanelToggled*FullHeight); B4's + // panel renders from it. In-memory only (a UI-layout preference, not project + // state — it must not travel with the .rpp); resets to Split on unload. + BankPanelFullHeight fullHeight = BankPanelFullHeight::Split; + // --- Audition preview (Wave B) -------------------------------------------- // // The stock preview register we hand to PlayPreview/StopPreview. Its cs/mutex @@ -999,6 +1006,30 @@ TailSetting bankPanelTailSetting() { return s; } +BankPanelFullHeight bankPanelFullHeight() { + // In-memory for the extension's lifetime (g_panel is static), like the tail + // setting: survives panel open/close and bank changes, resets to Split on unload. + return g_panel.fullHeight; +} + +// Shared toggle body: enter `target` from any other state, or fall back to Split when +// already at `target` (a second press restores the split). Requests a repaint via the +// same InvalidateRect the refresh path uses, so an open panel reflects the change; a +// closed panel (hwnd null) simply stores the bit for B4 to render when it opens. +static void setFullHeight(BankPanelFullHeight target) { + g_panel.fullHeight = + (g_panel.fullHeight == target) ? BankPanelFullHeight::Split : target; + if (g_panel.hwnd) InvalidateRect(g_panel.hwnd, nullptr, FALSE); +} + +void bankPanelToggledPoolFullHeight() { + setFullHeight(BankPanelFullHeight::PoolOnly); +} + +void bankPanelToggledBanksFullHeight() { + setFullHeight(BankPanelFullHeight::BanksOnly); +} + void bankPanelShutdown() { closePanel(); // stops audition + destroys the window deinitPreview(); // destroy the preview lock (after the last stop) diff --git a/src/bank_panel.h b/src/bank_panel.h index 6e38528..95405a8 100644 --- a/src/bank_panel.h +++ b/src/bank_panel.h @@ -61,6 +61,36 @@ void bankPanelRefresh(); // state only; the toggle is mutated by a click inside the panel, never here. TailSetting bankPanelTailSetting(); +// The vertical-split full-height layout state (Phase B). The bank window splits +// vertically — pool on top, named-banks region below — and two toggles collapse the +// split: pool full-height (hide the named-banks region) and banks full-height (hide +// the pool). The two are mutually exclusive with the default (both regions shown), +// so one enum captures the whole state. +// +// This bit is B3-owned (the actions flip it); B4's panel RENDERS from it. It lives +// here beside the tail setting — the other session-level view-layout bit the panel +// reads — NOT in the persisted ReaSamplerSession: it is a UI-layout preference, not +// project state, so it must not travel with the .rpp. In-memory for the extension's +// lifetime; resets to Split on unload. +enum class BankPanelFullHeight { + Split, // default: pool region on top, named-banks region below + PoolOnly, // pool full-height — named-banks region hidden + BanksOnly, // banks full-height — pool region hidden +}; + +// The current full-height layout state (default Split). READ by B4's panel to decide +// which region(s) to draw. Safe before the panel has ever opened. +BankPanelFullHeight bankPanelFullHeight(); + +// Toggles pool full-height: Split <-> PoolOnly. From PoolOnly returns to Split; from +// either other state (Split or BanksOnly) enters PoolOnly. Bound to the "pool +// full-height" action. Requests a repaint so an open panel reflects the change. +void bankPanelToggledPoolFullHeight(); + +// Toggles banks full-height: Split <-> BanksOnly, symmetric to the pool toggle. +// Bound to the "banks full-height" action. Requests a repaint. +void bankPanelToggledBanksFullHeight(); + // Tears the panel down on extension unload: destroys the window and releases any // cached thumbnails / PCM handles. Mirror of bankPanelInit; safe if never opened. void bankPanelShutdown(); diff --git a/src/main.cpp b/src/main.cpp index 176d7e0..3e51bac 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -714,6 +714,8 @@ static bool OnHookCommand(int command, int /*flag*/) // Design View action family (D4). Claims only its own ids; returns false for the // rest so this hook keeps looking (per the contract). if (reasampler::designViewHandleCommand(command)) return true; + // Multi-bank action family (B3). Same contract: claims only its own ids. + if (reasampler::bankHandleCommand(command)) return true; return false; } @@ -761,6 +763,8 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT( // Tear down the Design View action family (D4) — mirror-unregisters each // gaccel + command_id with '-'-prefixed strings. After the hook is gone. reasampler::designViewUnregisterActions(g_rec); + // Tear down the multi-bank action family (B3) — same mirror-unregister. + reasampler::bankUnregisterActions(g_rec); g_rec->Register("-gaccel", (void*)&g_accelCancelRealtime); g_rec->Register("-command_id", (void*)(REASAMPLER_ACTION_PREFIX "CANCEL_REALTIME_CAPTURE")); @@ -910,6 +914,12 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT( // the hook so every id is minted first. reasampler::designViewRegisterActions(rec, &g_session); + // Register the multi-bank action family (B3): create/rename/delete/evacuate bank, + // activate (cycle + pool), move/copy selected samples to a bank, and the two + // full-height layout toggles. Shares g_session with the Design View family; routed + // by the same hookcommand via bankHandleCommand. Registered before the hook. + reasampler::bankRegisterActions(rec, &g_session); + // One hookcommand routes every ReaSampler action (spike + toggle + Design View). // Registered once, after all command ids are minted. rec->Register("hookcommand", (void*)&OnHookCommand); diff --git a/tests/test_bank_book.cpp b/tests/test_bank_book.cpp index f254da5..24e3788 100644 --- a/tests/test_bank_book.cpp +++ b/tests/test_bank_book.cpp @@ -405,6 +405,60 @@ static void testLoadMalformedBanksDegradesWithoutLegacyFallback() { CHECK(loaded.pool().index.query("id-stale") == nullptr); // legacy did not leak } +// --- B3: active-bank cycle ordering (pure free function) ------------------- +// nextBankId(orderedIds, current) is the pure decision behind the "cycle active +// bank" action: given the book's ordered bank ids (pool-first) + the current active +// id, return the next id in ordinal order, wrapping pool -> named -> ... -> pool. + +static void testCycleOrderingWrapAround() { + // pool -> drums -> hits -> (wrap) pool. Exercises every step + the wrap. + const std::vector ids = {kPoolBankId, "drums", "hits"}; + CHECK(nextBankId(ids, kPoolBankId) == "drums"); + CHECK(nextBankId(ids, "drums") == "hits"); + CHECK(nextBankId(ids, "hits") == std::string(kPoolBankId)); // wrap past the last +} + +static void testCyclePoolOnlyStaysPool() { + // A pool-only book (no named banks) cycles to itself — the single id wraps to + // itself. The action becomes a no-op activation, which is correct. + const std::vector ids = {kPoolBankId}; + CHECK(nextBankId(ids, kPoolBankId) == std::string(kPoolBankId)); +} + +static void testCycleUnknownActiveResolvesToFirst() { + // A stale/unknown active id (e.g. the active bank was just deleted and the + // ordered list already dropped it) resolves to the first id — a sane home to jump + // to rather than "" — matching nextModeId's fallback. + const std::vector ids = {kPoolBankId, "drums"}; + CHECK(nextBankId(ids, "ghost") == std::string(kPoolBankId)); +} + +static void testCycleEmptyListYieldsEmpty() { + // Degenerate guard: an empty list has nothing to cycle to. (A real BankBook always + // seeds the pool, so this cannot arise from the book — but the pure helper must not + // index into an empty vector.) + const std::vector ids; + CHECK(nextBankId(ids, kPoolBankId).empty()); +} + +static void testCycleMatchesBookOrdinalOrder() { + // Integration-flavoured but still pure: drive the cycle off a real book's banks() + // order and confirm one full loop lands back on the pool, activating each bank in + // ordinal order. This is exactly what the action does (build ids from banks(), + // call nextBankId, setActiveBank). + BankBook book; + CHECK(book.createBank("a", "A")); + CHECK(book.createBank("b", "B")); // ordinals: pool 0, a 1, b 2 + + std::vector ids; + for (const Bank& bk : book.banks()) ids.push_back(bk.id); + + std::string cur = book.activeBankId(); // pool + cur = nextBankId(ids, cur); CHECK(cur == "a"); + cur = nextBankId(ids, cur); CHECK(cur == "b"); + cur = nextBankId(ids, cur); CHECK(cur == std::string(kPoolBankId)); // full loop +} + static void testActiveBankResolveAfterCorruptPersistedId() { // A book blob whose activeBank names no bank resolves to the pool (defensive). const char* json = @@ -435,6 +489,11 @@ int main() { testLoadEmptyWhenNeither(); testLoadMalformedBanksDegradesWithoutLegacyFallback(); testActiveBankResolveAfterCorruptPersistedId(); + testCycleOrderingWrapAround(); + testCyclePoolOnlyStaysPool(); + testCycleUnknownActiveResolvesToFirst(); + testCycleEmptyListYieldsEmpty(); + testCycleMatchesBookOrdinalOrder(); if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; From ccf65415c94db3b774b065e58e80e63388e9ce1a Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 25 Jul 2026 01:29:47 -0400 Subject: [PATCH 2/2] fix(banks): enforce unique bank display names in-model + B3 review minors --- CONTEXT.md | 12 +++++++-- docs/product/multi-bank.md | 13 ++++++--- src/actions.cpp | 54 +++++++++++++++++++++++++++++--------- src/bank_book.cpp | 46 +++++++++++++++++++++++++++++++- src/bank_book.h | 13 +++++++-- tests/test_bank_book.cpp | 53 +++++++++++++++++++++++++++++++++++++ 6 files changed, 169 insertions(+), 22 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 465d863..c97e120 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -547,7 +547,13 @@ arrange; the only change is *which* index the entry lands in. display name, ordinal, BankIndex }`. **`BankIndex` is untouched** — the multi-bank layer wraps it, it does not modify it (additive; no `bank-id` field on `Sample`). Bank id is the stable key (GUID-style, minted on bank create); display name and - ordinal are mutable (rename / reorder). The pool is the first, seeded, fixed-id + ordinal are mutable (rename / reorder). **Display names are unique**, enforced in the + pure model on create and rename: `createBank` / `renameBank` reject a name that + duplicates an existing bank's (renaming a bank to its own current name is a no-op + success). The comparison is **trimmed + case-insensitive (ASCII)**, so "Drums", + "drums", and " Drums " cannot coexist; the pool's reserved name "Pool" is protected + by the same check. Uniqueness makes by-name resolution in the action shell + unambiguous by construction. The pool is the first, seeded, fixed-id member. `bank_book` is the mirror of `bank_model` and `view_mode_model`: pure, no REAPER types, unit-tested outside the DAW, JSON round-trip. - **Active bank lives in the model, routes through the capture path.** `bank_book` @@ -637,7 +643,9 @@ Pure (no REAPER types, unit-tested — the mirror of `bank_model` / `view_mode_m - `bank_book` — ordered bank registry (`{ bank id, display name, ordinal, BankIndex }`); pool seeded with fixed id + name; create / rename / reorder / delete named banks (pool-privilege rules enforced here: reject delete/rename of - pool; delete drops member index entries); **evacuate** a bank (move every member to + pool; delete drops member index entries; **display names unique** — create/rename + reject a name that duplicates another bank's, trimmed + case-insensitive, "Pool" + protected); **evacuate** a bank (move every member to the pool, index-only, destination-collapse observed; pool cannot be evacuated); active-bank id (get/set, defaults to pool); **move** and **copy** a sample between banks (index-only, destination-collapse observed); query a bank's index; JSON diff --git a/docs/product/multi-bank.md b/docs/product/multi-bank.md index 9f7bc63..e8702da 100644 --- a/docs/product/multi-bank.md +++ b/docs/product/multi-bank.md @@ -90,9 +90,12 @@ reasons: So: `bank_book` is an ordered registry of `{ bank id, display name, ordinal, BankIndex }`, pool seeded as bank-zero. Bank id is the stable key (minted GUID-style -on create); name and ordinal are mutable. `BankIndex` is untouched. This is the -defer-the-feature, design-the-seam principle: the seam is a container above the -tested core, not a modification of it. +on create); name and ordinal are mutable. Display names are **unique** — two banks +cannot share a name (compared trimmed + case-insensitively, so "Drums" and "drums" +are the same name), enforced in the model on create and rename; the pool's "Pool" is +reserved by the same rule. `BankIndex` is untouched. This is the defer-the-feature, +design-the-seam principle: the seam is a container above the tested core, not a +modification of it. --- @@ -349,7 +352,9 @@ Mirrors the capture and Design View pillars exactly. - Ordered bank registry: `{ bank id, display name, ordinal, BankIndex }`; pool seeded with fixed id + fixed name. - Create / rename / reorder / delete named banks; pool-privilege rules enforced - here (reject delete-pool, reject rename-pool, never zero banks). + here (reject delete-pool, reject rename-pool, never zero banks). Display names are + unique — create/rename reject a name already used by another bank (trimmed + + case-insensitive; the pool's "Pool" is protected). - Active-bank id (get/set, defaults to pool); resolve the active bank's `BankIndex`. - Move / copy a sample between banks — index-only, destination collapse-by-hash observed, move removes the source entry. diff --git a/src/actions.cpp b/src/actions.cpp index 2a36c0e..24bf7ab 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -331,11 +331,14 @@ gaccel_register_t g_accelBankCopySel{}; gaccel_register_t g_accelBankPoolFull{}; gaccel_register_t g_accelBankBanksFull{}; -// Persists the book after a bank mutation. Mirrors the capture path (main.cpp -// RunCapture): a bank change is held in-session and written to the active project's -// ext state so it travels with the .rpp. No Save-As prompt here — saveToActiveProject -// no-ops on an unsaved project (the change stays valid for the session and persists -// on the user's next save), matching how capture persists. +// Persists the book after a bank mutation. Mirrors the CAPTURE path (main.cpp +// RunCapture), NOT the Design-View path: a bank change is held in-session and written +// to the active project's ext state so it travels with the .rpp. Deliberately no +// Save-As prompt — saveToActiveProject no-ops on an unsaved project (the change stays +// valid for the session and persists on the user's next save), exactly as capture +// persists. This is an intentional divergence from persistViewState (above), which +// DOES prompt Save-As on an unsaved project; do not "align" the two — a bank mutation +// follows capture's quiet-persist idiom, a Design-View mutation follows the prompt idiom. void persistBook() { g_session->saveToActiveProject(); } // Prompts the user for a single line of text via REAPER's stock input dialog. @@ -343,12 +346,21 @@ void persistBook() { g_session->saveToActiveProject(); } // cancel (SDK ~3808). `initial` pre-fills the field. Returns false (leaving `out` // untouched) on cancel or an empty entry. Self-contained bindable-action name entry; // B4's panel affordances supersede this with in-panel editing. +// +// COMMA GUARD: GetUserInputs splits the returned values on a separator that defaults +// to ',', so a bank name containing a comma would be truncated at the comma. We +// override the return separator to \x1f (ASCII unit separator, un-typeable in the +// dialog) via the documented `separator=X` extra caption field (SDK ~3806), so any +// printable name — commas included — round-trips whole. The captions_csv itself stays +// comma-joined: the single field caption, then the `separator=` directive as a +// trailing pseudo-caption (the directive redefines only the RETURN separator). bool promptText(const char* title, const char* caption, const std::string& initial, std::string& out) { std::vector buf(512, '\0'); // Pre-fill: GetUserInputs seeds the field from the retvals buffer's initial value. std::snprintf(buf.data(), buf.size(), "%s", initial.c_str()); - if (!GetUserInputs(title, 1, caption, buf.data(), static_cast(buf.size()))) + const std::string captions = std::string(caption) + ",separator=\x1f"; + if (!GetUserInputs(title, 1, captions.c_str(), buf.data(), static_cast(buf.size()))) return false; // user cancelled std::string s(buf.data()); if (s.empty()) return false; // an empty name is not a valid bank name @@ -368,9 +380,11 @@ std::string mintBankId() { } // Resolves a user-typed bank reference (a display name) to a bank id, scanning the -// book's banks in ordinal order. Case-sensitive exact match on displayName; "Pool" -// resolves the pool. Returns "" when no bank carries that name. Kept in the action -// layer (not the model) — it is UI name-resolution, not a model rule. +// book's banks in ordinal order. Exact match on displayName; "Pool" resolves the pool. +// Returns "" when no bank carries that name. Kept in the action layer (not the model) +// — it is UI name-resolution, not a model rule. First-match is unambiguous BY +// CONSTRUCTION: the model enforces unique display names (trimmed + case-insensitive), +// so at most one bank can carry a given name — no duplicate can shadow another here. std::string bankIdByDisplayName(const std::string& name) { for (const Bank& b : g_session->book().banks()) if (b.displayName == name) return b.id; @@ -381,14 +395,18 @@ std::string bankIdByDisplayName(const std::string& name) { // Create a named bank: prompt for a display name, mint a stable GUID id, create it in // the model, persist. The new bank is NOT auto-activated (create and activate are -// distinct acts — mirrors capture/placement separation). A duplicate-name is allowed -// (display names are not unique in the model); the fresh GUID keeps the id unique. +// distinct acts — mirrors capture/placement separation). The model rejects a display +// name that duplicates an existing bank's (trimmed + case-insensitive, incl. "Pool"); +// the create then fails and the user is told the name is taken. void doBankCreate() { std::string name; if (!promptText("ReaSampler: create bank", "Bank name:", "", name)) return; const std::string id = mintBankId(); if (!g_session->book().createBank(id, name)) { - ShowConsoleMsg("ReaSampler: could not create bank (id collision — try again).\n"); + ShowConsoleMsg( + ("ReaSampler: could not create bank \"" + name + + "\" (a bank with that name already exists).\n") + .c_str()); return; } persistBook(); @@ -411,7 +429,10 @@ void doBankRename() { std::string newName; if (!promptText("ReaSampler: rename bank", "New name:", which, newName)) return; if (!g_session->book().renameBank(id, newName)) { - ShowConsoleMsg("ReaSampler: cannot rename that bank (the pool is un-renamable).\n"); + // renameBank rejects the pool (un-renamable) or a name already used by another + // bank (unique display names, trimmed + case-insensitive). + ShowConsoleMsg("ReaSampler: cannot rename that bank (the pool is un-renamable, " + "or another bank already uses that name).\n"); return; } persistBook(); @@ -432,6 +453,13 @@ void doBankDelete() { ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str()); return; } + // Pool early-out: the pool is un-deletable (the model rejects it). Catch it here, + // BEFORE the non-empty confirm, so typing "Pool" never shows a misleading + // "delete anyway?" prompt for an operation the model will refuse regardless. + if (id == kPoolBankId) { + ShowConsoleMsg("ReaSampler: the pool cannot be deleted.\n"); + return; + } // Read member count BEFORE deleting (the Bank* is invalidated by deleteBank; we do // not cache it — resolve size to an int up front). const Bank* b = g_session->book().bank(id); diff --git a/src/bank_book.cpp b/src/bank_book.cpp index 4fc4901..8f35d1d 100644 --- a/src/bank_book.cpp +++ b/src/bank_book.cpp @@ -77,6 +77,43 @@ void BankBook::normalizeOrdinals() { banks_[i].ordinal = static_cast(i); } +// --------------------------------------------------------------------------- +// Display-name uniqueness (trimmed + case-insensitive, ASCII) +// --------------------------------------------------------------------------- + +namespace { + +// Folds a display name to its uniqueness key: strip leading/trailing ASCII +// whitespace, lower-case ASCII letters. So "Drums", "drums", and " Drums " share one +// key and cannot coexist. ASCII-only by design — the pure core carries no locale +// facility and must not grow one; bank names are short user labels, not full Unicode +// case-folding candidates. +std::string nameKey(const std::string& s) { + std::size_t b = 0, e = s.size(); + auto isWs = [](char c) { return c == ' ' || c == '\t' || c == '\n' || c == '\r'; }; + while (b < e && isWs(s[b])) ++b; + while (e > b && isWs(s[e - 1])) --e; + std::string out; + out.reserve(e - b); + for (std::size_t i = b; i < e; ++i) { + char c = s[i]; + if (c >= 'A' && c <= 'Z') c = static_cast(c - 'A' + 'a'); + out += c; + } + return out; +} + +} // namespace + +// True if any bank OTHER than `exceptId` already carries `name`'s uniqueness key. The +// exception lets renameBank accept a bank keeping (or re-casing/-spacing) its own name. +bool BankBook::displayNameTaken(const std::string& name, const std::string& exceptId) const { + const std::string key = nameKey(name); + for (const auto& b : banks_) + if (b.id != exceptId && nameKey(b.displayName) == key) return true; + return false; +} + // --------------------------------------------------------------------------- // Bank lifecycle // --------------------------------------------------------------------------- @@ -84,7 +121,10 @@ void BankBook::normalizeOrdinals() { bool BankBook::createBank(const std::string& id, const std::string& displayName) { if (id.empty()) return false; // ids key the registry if (id == kPoolBankId) return false; // reserved pool id - if (bank(id) != nullptr) return false; // duplicate + if (bank(id) != nullptr) return false; // duplicate id + // Display names are unique (trimmed + case-insensitive); the pool's "Pool" is a + // reserved name and is caught here like any other collision. + if (displayNameTaken(displayName, /*exceptId=*/id)) return false; Bank b; b.id = id; @@ -99,6 +139,10 @@ bool BankBook::renameBank(const std::string& id, const std::string& displayName) if (id == kPoolBankId) return false; // pool is un-renamable Bank* b = bank(id); if (b == nullptr) return false; + // Reject a name already used by a DIFFERENT bank. Renaming a bank to its own + // current name (or a case/space variant of it) is a no-op success, not a + // rejection — exceptId=id excludes the bank itself from the collision scan. + if (displayNameTaken(displayName, /*exceptId=*/id)) return false; b->displayName = displayName; return true; } diff --git a/src/bank_book.h b/src/bank_book.h index 5b9b8e7..2a6084b 100644 --- a/src/bank_book.h +++ b/src/bank_book.h @@ -97,10 +97,14 @@ public: // Creates a named bank with the caller-supplied stable id and display name, // assigning the next ordinal. Rejects (returns false, no mutation) an empty id, - // a duplicate id, or the reserved pool id. Display name is not required unique. + // a duplicate id, the reserved pool id, or a display name that duplicates an + // existing bank's name (including the pool's "Pool"). Display-name uniqueness is + // trimmed + case-insensitive (ASCII): "Drums", "drums", and " Drums " collide. bool createBank(const std::string& id, const std::string& displayName); - // Renames a named bank. Rejects (false, no mutation) an unknown id or the pool. + // Renames a named bank. Rejects (false, no mutation) an unknown id, the pool, or a + // target name already used by a DIFFERENT bank (trimmed + case-insensitive, as + // createBank). Renaming a bank to its own current name is a no-op success. bool renameBank(const std::string& id, const std::string& displayName); // Deletes a NAMED bank, removing it (and its member index entries) from the @@ -210,6 +214,11 @@ private: std::vector banks_; // ordinal order; banks_[0] is always the pool std::string activeBankId_; // always names a live bank; defaults to pool + // True if a bank OTHER than `exceptId` already carries `name`'s uniqueness key + // (trimmed + case-insensitive, ASCII). Backs the create/rename uniqueness check; + // pass exceptId=id to let a bank keep (or re-case/-space) its own name. + bool displayNameTaken(const std::string& name, const std::string& exceptId) const; + // Re-sorts banks_ by ordinal (pool pinned first) and rewrites ordinals to a // contiguous 0..N-1 so the pool is 0 and named banks are 1..N. Called after any // structural change (create / delete / reorder). diff --git a/tests/test_bank_book.cpp b/tests/test_bank_book.cpp index 24e3788..5c96e22 100644 --- a/tests/test_bank_book.cpp +++ b/tests/test_bank_book.cpp @@ -92,6 +92,8 @@ static void testCreateRenameReorder() { CHECK(book.renameBank("b", "Beta-renamed")); CHECK(book.bank("b")->displayName == "Beta-renamed"); CHECK(!book.renameBank("missing", "x")); + // Renaming back to a non-colliding name keeps working. + CHECK(book.renameBank("b", "Beta")); // Reorder: move "c" to the front of the named region (ordinal 1). CHECK(book.reorderBank("c", 1)); @@ -113,6 +115,56 @@ static void testCreateRenameReorder() { CHECK(!book.reorderBank("missing", 1)); } +static void testDisplayNameUniqueness() { + BankBook book; + CHECK(book.createBank("a", "Drums")); + + // A unique name is accepted. + CHECK(book.createBank("b", "Bass")); + + // Exact duplicate rejected, no mutation (size unchanged, the collided id absent). + CHECK(!book.createBank("c", "Drums")); + CHECK(book.bank("c") == nullptr); + CHECK(book.size() == 3); // pool + a + b only + + // Trimmed + case-insensitive collisions: "drums", " Drums ", "DRUMS" all collide. + CHECK(!book.createBank("c", "drums")); + CHECK(!book.createBank("c", " Drums ")); + CHECK(!book.createBank("c", "DRUMS")); + CHECK(book.bank("c") == nullptr); + + // The pool's reserved name "Pool" (and its variants) cannot be taken by a new bank. + CHECK(!book.createBank("c", "Pool")); + CHECK(!book.createBank("c", " pool ")); + CHECK(book.bank("c") == nullptr); + + // -- renameBank uniqueness -------------------------------------------------- + // Rename to a name used by ANOTHER bank is rejected (no mutation). + CHECK(!book.renameBank("b", "Drums")); + CHECK(book.bank("b")->displayName == "Bass"); // unchanged + CHECK(!book.renameBank("b", "drums")); // case-insensitive collision too + CHECK(!book.renameBank("b", " Drums ")); // trimmed collision too + + // Renaming a bank to its OWN current name is a no-op success (not a rejection). + CHECK(book.renameBank("a", "Drums")); + CHECK(book.bank("a")->displayName == "Drums"); + // Re-casing/-spacing its own name is likewise allowed (it collides only with self). + CHECK(book.renameBank("a", " drums ")); + CHECK(book.bank("a")->displayName == " drums "); + + // Renaming to the pool's reserved name is rejected (pool is the "other" bank here). + CHECK(!book.renameBank("b", "Pool")); + CHECK(book.bank("b")->displayName == "Bass"); + + // A genuinely fresh unique name still renames fine. + CHECK(book.renameBank("b", "Low End")); + CHECK(book.bank("b")->displayName == "Low End"); + + // After the rejections, the previously-freed name is now reusable by a new bank. + CHECK(book.createBank("c", "Bass")); + CHECK(book.bank("c")->displayName == "Bass"); +} + static void testMoveSourceLosesDestGains() { BankBook book; CHECK(book.createBank("drums", "Drums")); @@ -473,6 +525,7 @@ int main() { testPoolSeededAndDefaults(); testPoolPrivileges(); testCreateRenameReorder(); + testDisplayNameUniqueness(); testMoveSourceLosesDestGains(); testCopySourceRetainedDestGains(); testMoveDestCollapse();