diff --git a/CLAUDE.md b/CLAUDE.md index a810155fb..b1aab050b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,6 +57,26 @@ You are an expert in JUCE and in desktop audio application UI. Element is a JUCE - Shared singletons are reached through `context()` — e.g. `context().session()`, `context().mapping()`, `context().midi()`. - Marshal state changes off the audio/MIDI thread to the message thread with `juce::AsyncUpdater`. Plugin parameter changes must be wrapped in `beginChangeGesture()` / `setValueNotifyingHost()` / `endChangeGesture()`. +## Lua Bindings (`src/el/`) + +- **One entry point.** Lua reaches the application only through `el.Context.instance()` + (backed by `_G["el.context"]`, planted by `Lua::setGlobals`). Never plant additional + globals to reach other app objects. Expose them as methods on the owning usertype instead, + e.g. `Context:commands()` resolves per call via `ctx.services().find()`. +- **Bind on the owner, resolve per call.** A binding returns a reference into the live + object graph; it does not cache it. Lua modules must not cache what they get back either + (a session is replaced on load), so resolve lazily inside functions. +- **Register what you return.** If a method returns a usertype, `require` that module + in the `luaopen_*` of the module that returns it (see `luaopen_el_Context`). +- **Bind less C++, write more Lua.** C++ binds a minimal handle; ergonomics go in a native + Lua module under `src/el/*.lua` (`command.lua`, `object.lua`, `script.lua`). +- **Register modules in both places.** A new `el.*` module goes into + `searchInternalModules` in `src/scripting/bindings.cpp` and, for Lua sources, is picked + up by the `src/el/CMakeLists.txt` glob. +- **No shortcuts.** If reaching an object needs plumbing (an accessor, a service lookup), + add the plumbing. Working around it with a global, a static or a cached pointer is a + defect, not a fix. + ## Testing - Tests use **Boost.Test** and live in `test/` (built into the `test_element` console app). `test/CMakeLists.txt` globs all `*.cpp`, but each suite must ALSO be registered with an explicit `add_test(NAME "MySuite" COMMAND test_element --run_test=MySuite)` line — forgetting this is the usual reason a new test "doesn't run." diff --git a/docs/config.ld b/docs/config.ld index 2f7cee9b4..d8b40c27f 100644 --- a/docs/config.ld +++ b/docs/config.ld @@ -8,7 +8,6 @@ file = { exclude = { '../src/el/midi_buffer.hpp', '../src/el/sol_helpers.hpp', - '../src/el/vector.c', '../src/el/color.lua', '../src/el/session.lua', '../src/el/Parameter.cpp' diff --git a/docs/plans/extensions.md b/docs/plans/extensions.md index c7d0b7476..675c7717a 100644 --- a/docs/plans/extensions.md +++ b/docs/plans/extensions.md @@ -1,14 +1,17 @@ # `.element` Extension Format + App-Side Lua Runtime -> **Phase 0 prerequisite:** [session-scripts.md](session-scripts.md) — session-embedded -> hook scripts. `HookBus`, `el.hooks`, the restricted-environment/capability model, and -> `ScriptingService` are built there first; Phase 3 below then reduces to wiring -> `app.*` events and extension-owned registration. See also [luajit.md](luajit.md) for -> the LuaJIT assessment. +> **Phase 0 prerequisites:** [session-proxy.md](session-proxy.md) — the `GraphController` +> through which every topology mutation (UI, undo, Lua, session load) flows and from which +> hook events are dispatched — and [session-scripts.md](session-scripts.md) — console, +> `HookBus`, `el.hooks`, mutation verbs on `el.Session`/`el.Graph`, the +> restricted-environment/capability model, and `ScriptingService`. Those are built first; +> Phase 2 below reduces to graph providers over the `el.Session`/`el.Graph` bindings and Phase 3 to +> extension-owned registration. [scripting-audit.md](scripting-audit.md) records the +> starting state. See also [luajit.md](luajit.md) for the LuaJIT assessment. ## Context -Element has a mature Lua substrate (embedded Lua 5.4, sol2 bindings under `src/el/`, a shared `ScriptingEngine` on `Context`) but the app-side surface is thin: no startup script execution (`ScriptingEngine::execute` is declared but never defined), no Lua graph-mutation API, no lifecycle hooks, and GUI scripting is limited to embedded View scripts. The goal is a new **`*.element` extension format** — a directory (like `.lv2`/`.vst3`) acting as a package/extension that can carry Lua modules, hook scripts, views/panels, DSP scripts, graphs (serialized data **or** Lua builder scripts — manifest decides), presets, resources, and bundled plugins (CLAP first) — plus the app-side Lua runtime it requires: graph building, lifecycle hooks, and GUI extensibility. +Element has a mature Lua substrate (embedded Lua 5.4, sol2 bindings under `src/el/`, a shared `ScriptingEngine` on `Context`) but the app-side surface is thin (see [scripting-audit.md](scripting-audit.md)). Phase 0 supplies startup script execution (`ScriptingEngine::execute`, user `init.lua`), the Lua graph-mutation API (`el.Session`/`el.Graph` over `GraphController`) and lifecycle hooks; GUI scripting remains limited to embedded View scripts until Phase 4. The goal is a new **`*.element` extension format** — a directory (like `.lv2`/`.vst3`) acting as a package/extension that can carry Lua modules, hook scripts, views/panels, DSP scripts, graphs (serialized data **or** Lua builder scripts — manifest decides), presets, resources, and bundled plugins (CLAP first) — plus the app-side Lua runtime it requires: graph building, lifecycle hooks, and GUI extensibility. **Decisions made with user:** - Terminology: **Extension** everywhere (classes, service, Lua modules). Directory suffix stays `.element`. @@ -63,53 +66,52 @@ C++ side: `struct ExtensionManifest` — plain struct parsed once from the sol t |---|---| | `ExtensionManifest`, `Extension` | `src/scripting/extension.hpp/.cpp` | | `ExtensionManager` (scan/load/unload registry) | `src/scripting/extensionmanager.hpp/.cpp` | -| `ScriptingService` (new `Service`) | `src/services/scriptingservice.hpp/.cpp` | -| `HookBus` (C++ event dispatcher) | `src/scripting/hookbus.hpp/.cpp` | -| `el.engine`, `el.hooks`, `el.ui` Lua modules | `src/el/Engine.cpp`, `Hooks.cpp`, `UI.cpp` | +| `ScriptingService` (`Service`, **Phase 0**) | `src/services/scriptingservice.hpp/.cpp` | +| `HookBus` (C++ event dispatcher, **Phase 0**, owned by `Context`) | `include/element/hooks.hpp`, `src/hooks.cpp` | +| `GraphController` (**Phase 0**, see session-proxy.md) | `src/engine/graphcontroller.hpp/.cpp` | +| `el.hooks` (**Phase 0**), `el.ui` Lua modules | `src/el/Hooks.cpp` + `hooks.lua`, `src/el/UI.cpp` | | `ViewFactory` (slug → ContentView registry) | `src/ui/viewfactory.hpp/.cpp`, owned by `GuiService` | | `ScriptContentView` + `MissingExtensionView` | `src/ui/scriptcontentview.hpp/.cpp` | Key existing seams to reuse (verified): -- `ScriptingEngine::addPackage(name, loader)` runtime package registry ([scripting.cpp:118](src/scripting.cpp#L118)) — extension Lua modules register here (namespaced; no global `package.path` pollution). -- `EngineService` mutation vocabulary ([engine.hpp](include/element/engine.hpp)): `addGraph/addNode/addPlugin/addConnection/connectChannels/connect(PortType,...)/removeNode/disconnectNode` — Lua rides the existing message/undo/engine-sync path. -- `StandardContent::createContentView(const String&)` virtual, consulted first by `setMainView`/`setSecondaryView` ([standard.hpp:88](include/element/ui/standard.hpp#L88), [standard.cpp:541,633](src/ui/standard.cpp#L541)). -- `NavigationConcertinaPanel::addPanel(desc, factory, header)` public ([navigation.hpp:73](include/element/ui/navigation.hpp#L73)); panel state keys by name — extension slugs must be stable. -- `ScriptView` per-view `sol::environment` + descriptor pattern ([scriptview.cpp](src/ui/scriptview.cpp)) — the sandbox precedent for all extension script execution. -- `SessionService::sigSessionLoaded/sigWillSave` ([sessionservice.hpp:37-38](src/services/sessionservice.hpp#L37-L38)), `EngineService::sigNodeRemoved`. -- `Node::parse` tolerant multi-format graph reader ([node.cpp](src/node.cpp)) for `type="data"` providers. +- `ScriptingEngine::addPackage(name, loader)` runtime package registry ([scripting.cpp:118](../../src/scripting.cpp#L118)) — extension Lua modules register here (namespaced; no global `package.path` pollution). +- `el.Session`/`el.Graph` mutation bindings (`addGraph/removeGraph/moveGraph/setActiveGraph`, `addNode/addPlugin/removeNode/connect/disconnect/connectChannels`) over `GraphController` ([session-proxy.md](session-proxy.md)), resolved per call through the services, landed in Phase 0. The model itself carries no engine verbs. `EngineService` keeps its public signatures as thin forwards for UI/undo callers. +- `StandardContent::createContentView(const String&)` virtual, consulted first by `setMainView`/`setSecondaryView` ([standard.hpp:88](../../include/element/ui/standard.hpp#L88), [standard.cpp:541,633](../../src/ui/standard.cpp#L541)). +- `NavigationConcertinaPanel::addPanel(desc, factory, header)` public ([navigation.hpp:73](../../include/element/ui/navigation.hpp#L73)); panel state keys by name — extension slugs must be stable. +- `ScriptView` per-view `sol::environment` + descriptor pattern ([scriptview.cpp](../../src/ui/scriptview.cpp)) — the sandbox precedent for all extension script execution. +- `HookBus` events (`session.loaded`, `session.saving`, `session.closed`, `graph.*`, `node.*`, `connection.*`, `app.started`, `app.shutdown`) — all dispatched in Phase 0; `EngineService::sigNodeRemoved` is gone. +- `Node::parse` tolerant multi-format graph reader ([node.cpp](../../src/node.cpp)) for `type="data"` providers. -Ownership/lifetime rule: `ExtensionManager` is owned by `ScriptingEngine::Impl` (inside the Lua state's lifetime — sol::environment destruction order). `ScriptingService::deactivate()` unloads extensions **before** state teardown. `ScriptingService` registers in `Services` ctor ([services.cpp](src/services.cpp)) after `EngineService`, before `SessionService`, so extension views/graphs exist before the startup session restores. +Ownership/lifetime rule: `ExtensionManager` is owned by `ScriptingEngine::Impl` (inside the Lua state's lifetime — sol::environment destruction order). `HookBus` is owned by `Context` and freed after `services` but before the Lua state ([session-scripts.md](session-scripts.md) § HookBus). `ScriptingService::deactivate()` unloads extensions **before** state teardown. `ScriptingService` already exists from Phase 0 and registers in `Services` ctor ([services.cpp](../../src/services.cpp)) after `EngineService`, before `SessionService`, so extension views/graphs exist before the startup session restores. ## Phases ### Phase 1 — Extension core: format, discovery, module/script loading - New: `extension.hpp/.cpp`, `extensionmanager.hpp/.cpp`, `scriptingservice.hpp/.cpp`, `test/scripting/extensiontests.cpp`, fixture `test/scripting/fixtures/TestPack.element/`. -- Modified: `src/scripting.hpp/.cpp` (implement dead `ScriptingEngine::execute` as protected `lua.script()` in fresh env returning `Result`; expose `extensions()`), `src/datapath.cpp` + `include/element/datapath.hpp` (`defaultExtensionsDir()` = `~/Music/Element/Extensions`, create in `initializeUserLibrary`; also scan `applicationDataDir()/Extensions`; dev env var `ELEMENT_EXTENSIONS_PATH` mirroring `ELEMENT_SCRIPTS_PATH` handling in [bindings.cpp](src/scripting/bindings.cpp)), `src/services.cpp`, `src/scripting/scriptmanager.cpp/.hpp` (**additive** scan — current `scanDirectory` replaces the registry), `include/element/tags.hpp` (`EL_TAG(Extension)`, `tags::extensionId`, `tags::extensionVersion`, `tags::requires`), `src/CMakeLists.txt`, `test/CMakeLists.txt` (+ `add_test`). +- Modified: `src/scripting.hpp/.cpp` (`ScriptingEngine::execute (code, env)` is provided by Phase 0 — see [session-scripts.md](session-scripts.md) § Console foundation — and is reused here with a fresh env per entry script; expose `extensions()`), `src/datapath.cpp` + `include/element/datapath.hpp` (`defaultExtensionsDir()` = `~/Music/Element/Extensions`, create in `initializeUserLibrary`; also scan `applicationDataDir()/Extensions`; dev env var `ELEMENT_EXTENSIONS_PATH` mirroring `ELEMENT_SCRIPTS_PATH` handling in [bindings.cpp](../../src/scripting/bindings.cpp)), `src/services.cpp`, `src/scripting/scriptmanager.cpp/.hpp` (**additive** scan — current `scanDirectory` replaces the registry), `include/element/tags.hpp` (`EL_TAG(Extension)`, `tags::extensionId`, `tags::extensionVersion`, `tags::requires`), `src/CMakeLists.txt`, `test/CMakeLists.txt` (+ `add_test`). - Load sequence: scan (parse manifests only, no code) → for enabled extensions: register modules via `addPackage`, register DSP/View scripts with `ScriptManager`, run entry script in `sol::environment(lua, sol::create, lua.globals())`; every failure → `logError`, status `error`, never throws out. Enable/disable persisted in `Settings` (`extensionsDisabled` list). `Commands::reloadExtensions` for dev iteration. - Unload = disconnect hooks, drop env, clear `package.loaded` + registered packages, remove ScriptManager entries and view/panel registrations. Full hot-unload of usertypes is explicitly out of scope (documented). -### Phase 2 — Graph-building Lua API + graph providers -- New: `src/el/Engine.cpp` (`luaopen_el_Engine`), `test/scripting/enginescripttests.cpp`. -- Modified: `src/scripting/bindings.cpp` (register module), `src/el/CMakeLists.txt`, `src/services/scriptingservice.cpp` (provider instantiation), `src/ui/mainmenu.cpp` + commands (File → New Graph From Extension ▸ submenu). -- Facade binds a thin wrapper resolving `EngineService` per call (never bind the service class raw); assert message thread; Lua two-value `nil, "message"` error convention: +### Phase 2 — Graph providers (the Lua graph API comes from Phase 0) +- New: `test/scripting/graphprovidertests.cpp`. +- Modified: `src/services/scriptingservice.cpp` (provider instantiation), `src/ui/mainmenu.cpp` + commands (File → New Graph From Extension ▸ submenu). +- Builder scripts use the Phase 0 `el.Session`/`el.Graph` bindings; no `el.engine` module exists. Lua two-value `nil, "message"` error convention: ```lua - local engine = require ("el.engine") - local g = engine.addGraph ("My Rig") - local n = engine.addNode (g, "element.volume") - local p = engine.addPlugin (g, { format = "CLAP", id = "org.surge..." }) - engine.connect (g, p, 0, n, 0) -- optional 5th arg "midi" for PortType - engine.remove (g, n); engine.saveGraph (g, path) + local s = require ('el.Context').instance():session() + local g = s:addGraph ("My Rig") + local n = g:addNode ("element.volume") + local p = g:addPlugin { format = "CLAP", id = "org.surge..." } + g:connectChannels (p, 0, n, 0) -- optional 5th arg "midi" for PortType + g:removeNode (n); g:writeFile (path) ``` - `addPlugin` looks up `context().plugins().getKnownPlugins()` by format + identifier/uid/name. + `addPlugin` looks up `context().plugins().getKnownPlugins()` by format + identifier/uid/name (bound in Phase 0). - Providers: `type="data"` → `Node::parse` → re-UUID (same as `.elg` import) → `EngineService::addGraph(node, true)`. `type="script"` → protected run in fresh env, wrapped in a **single UndoManager transaction** (fallback: per-op undo, documented, if bracketing proves infeasible). - Tests headless with existing `Context` + engine fixtures: builder script topology assertions, data-provider instantiation, plugin-miss returns nil+msg. -### Phase 3 — Hook system -- New: `src/scripting/hookbus.hpp/.cpp`, `src/el/Hooks.cpp`, `test/scripting/hooktests.cpp`. -- Modified: `include/element/engine.hpp` + `src/services/engineservice.cpp` — add `sigNodeAdded`, `sigGraphAdded`, `sigGraphRemoved` at the same sites that fire `sigNodeRemoved`; `scriptingservice.cpp` wires all signals → `HookBus`. -- Events: `app.started`, `app.shutdown`, `session.loaded`, `session.saving`, `graph.added`, `graph.removed`, `graph.changed`, `node.added`, `node.removed`; extensions can `hooks.emit` custom events. -- `HookBus`: message-thread only; handlers are `sol::protected_function` tagged with owner extension id (ExtensionManager sets a "current extension" scope during entry scripts; console registrations = `"user"`); dispatch iterates a copy; reentrancy guard (`dispatching` flag + pending queue); auto-disable a handler after 3 consecutive errors; `removeOwner(id)` on unload. -- Lua: `hooks.on(event, fn) → handle`, `hooks.off(handle)`, `hooks.emit(event, ...)`. +### Phase 3 — Extension-owned hooks (the bus and events come from Phase 0) +- Modified: `src/scripting/extensionmanager.cpp` — set the `HookBus` "current owner" to the extension id while running its entry script so `hooks.action` registrations are tagged automatically; `removeOwner (id)` on unload. `test/scripting/extensiontests.cpp` gains load→unload→load without duplicate handlers. +- Extension default grant is friendlier than session scripts (trusted-but-isolated): `session`, `ui` granted without a prompt; `io` still declared. +- Events and the `el.hooks` API (`hooks.action/filter/off/emit/owner`) are as defined in [session-scripts.md](session-scripts.md); extensions can `hooks.emit` custom events. ### Phase 4 — GUI extensibility - New: `viewfactory.hpp/.cpp`, `scriptcontentview.hpp/.cpp`, `src/el/UI.cpp`. @@ -131,7 +133,7 @@ Ownership/lifetime rule: `ExtensionManager` is owned by `ScriptingEngine::Impl` ## Verification -- Per-phase Boost.Test suites in `test/scripting/` (each registered in `test/CMakeLists.txt` with `add_test`): manifest parse/reject, discovery, `require` resolution, entry-script error isolation (Phase 1); builder-script graph topology + undo-transaction rollback (Phase 2); hook dispatch/auto-disable/removeOwner/reentrancy (Phase 3); ViewFactory register/lookup/missing-placeholder + headless descriptor instantiation (Phase 4); search-path merge + preset discovery from fixture (Phase 5); requires-tree round-trip + missing-extension session load degradation + load→unload→load cycle without duplicate handlers (Phase 6). +- Per-phase Boost.Test suites in `test/scripting/` (each registered in `test/CMakeLists.txt` with `add_test`): manifest parse/reject, discovery, `require` resolution, entry-script error isolation (Phase 1); builder-script graph topology + undo-transaction rollback (Phase 2); extension owner tagging + unload/reload without duplicate handlers (Phase 3, the bus itself is tested in Phase 0); ViewFactory register/lookup/missing-placeholder + headless descriptor instantiation (Phase 4); search-path merge + preset discovery from fixture (Phase 5); requires-tree round-trip + missing-extension session load degradation + load→unload→load cycle without duplicate handlers (Phase 6). - `cmake --build build && ctest --test-dir build --output-on-failure -R Extension...` per suite. - Manual end-to-end after Phase 4: drop `TestPack.element` into `~/Music/Element/Extensions`, launch app, confirm entry script runs, `New Graph From Extension` builds a graph, hooks fire in the Lua console, and the registered view opens via `presentView`. diff --git a/docs/plans/kushview-licensing-authorization.md b/docs/plans/kushview-licensing-authorization.md deleted file mode 100644 index ca145f0ee..000000000 --- a/docs/plans/kushview-licensing-authorization.md +++ /dev/null @@ -1,311 +0,0 @@ -# Kushview Licensing & Authorization Architecture - -**Status:** Draft / design -**Scope:** Kushview-owned plugins only. Client-side design complete; server-side deferred (stubbed contracts only). -**Goal:** A subscriber (or standalone purchaser) authenticates once, and every Kushview plugin they're entitled to unlocks silently — no per-plugin user/pass prompt. Cancelling a subscription revokes access on the next refresh. - ---- - -## 1. Summary - -Introduce a dedicated **Kushview Account Manager** application as the single place that logs in and manages entitlements. It writes a **shared per-user store** of auth tokens and signed unlock keys. Element and every Kushview plugin link a small **shared read-side SDK** that reads that store and applies keys through a **common JUCE product-unlocking implementation** — never running an interactive login themselves. - -This mirrors the industry-standard pattern (iLok License Manager, NI Native Access, Waves Central, Steinberg Download Assistant): one privileged writer, many trivial readers. - -### Why this shape - -- **Plugins stay trivial.** No OAuth, no browser, no URL-scheme handling shipped in each plugin — just "read store → apply key → else fall back to the existing manual prompt." Less code, less attack surface, replicated across every product. -- **Auth changes ship once**, in the Manager, decoupled from plugin release cycles. -- **Background subscription refresh.** Subscription keys are short-lived by design; a plugin can only refresh while loaded. The Manager runs as a login-item/agent and keeps every owned plugin's key fresh regardless of whether anything is open — the one place this can be solved cleanly. -- **Graceful degradation.** No token / offline / non-subscriber → plugin shows today's manual unlock UI. Nothing breaks. - ---- - -## 2. Background: JUCE unlocking primer (for the target project) - -Kushview plugins are JUCE products. JUCE's licensing primitives: - -- **RSA key pair.** Generated once per product with `juce::RSAKey::createKeyPair()`. - - **Private key** signs unlock "key files". **Lives only on the server.** Never shipped, never in any client. - - **Public key** is compiled into the product and only *verifies* a signed key file. It cannot forge licenses. It is safe to embed — and is already inside every shipped plugin binary. -- **Key file.** A signed blob (produced server-side by `juce::KeyGeneration::generateKeyFile(...)`) containing the user email, product ID, **machine IDs**, and an **expiry**. Applied client-side with `OnlineUnlockStatus::applyKeyFile(...)`. -- **`juce::OnlineUnlockStatus`.** The client-side state object. We use it in "key file" mode (`applyKeyFile` / `isUnlocked` / `getExpiryTime`) and **bypass its built-in web-authentication flow** entirely — keys come from the shared store, not from OnlineUnlockStatus's own HTTP calls. - -Two protections do the real work: **machine-locking** (a key file only unlocks on the machines whose IDs it was signed for) and **expiry** (subscription semantics). OnlineUnlockStatus's own state obfuscation is weak and is *not* relied on for security. - -### Current state in Element (starting point) - -Element already has the auth foundation this builds on: - -- OAuth2 Authorization-Code + PKCE against WordPress `kv-auth/v1` (`src/auth.cpp`, `src/auth.hpp`, namespace `element::auth`). -- Token response already carries an `entitlements` object (today just `preview_updates`) plus user email/display name. -- An established precedent for "authenticated request → short-lived server-signed artifact": `/appcast-url?plat=` returns a signed, expiring Sparkle feed URL that the client caches and refreshes when expired (`GuiService::checkUpdates`, `auth::isAppcastUrlExpired`). -- **No** `OnlineUnlockStatus`/`RSAKey`/`KeyGeneration` anywhere yet — the unlock layer is net-new. -- Plugin scanning/hosting has no entitlement gate — nothing to unwind. - -The signed-key endpoint is the same shape as the existing appcast-url endpoint. The OAuth/PKCE client is the reusable core that gets lifted into the shared SDK. - ---- - -## 3. Component overview - -``` -┌───────────────────────────────────────────────┐ -│ Kushview Account Manager │ the ONLY app that logs in -│ • interactive OAuth2 + PKCE (browser) │ -│ • URL-scheme callback (kushview://) │ -│ • entitlement sync (tier → product list) │ -│ • key fetch + refresh daemon │ -│ • (future) product download / install │ -└───────────────────────┬───────────────────────┘ - │ writes (atomic) - ▼ - ╔═══════════════════════════════════════╗ - ║ Shared per-user Kushview store ║ - ║ • tokens (OS secure storage) ║ - ║ • keys/.key (machine- ║ - ║ locked, expiring signed blobs) ║ - ║ • entitlements.json ║ - ╚═══════════════════════════════════════╝ - ▲ ▲ - read │ │ read - ┌───────────────┴──┐ ┌────┴──────────────────┐ - │ Element │ │ Kushview plugins │ - │ (host + reader) │ │ ToneGenerator, … │ - └──────────────────┘ └───────────────────────┘ - both link the shared READ-SIDE SDK: - KushviewUnlockStatus + StoreReader + fallback -``` - -### 3.1 Kushview Account Manager (new application) - -The single privileged writer. - -Responsibilities: -- Interactive **OAuth2 + PKCE** login (lifted from `element::auth`), including browser launch and `kushview://auth/callback` URL-scheme handling. -- Fetch and persist **tokens** into the shared store (refresh-token rotation, silent refresh). -- Fetch **entitlements** (tier → owned/subscribed product list) and persist to `entitlements.json`. -- For each entitled product, request a **signed, machine-locked, expiring key file** and write it to `keys/.key`. -- **Refresh daemon:** run on login-item/agent schedule; re-request any key nearing expiry so subscription keys never lapse silently. On the server saying "no longer entitled," delete that product's key. -- Account UI: signed-in identity, tier, owned products, per-product authorization status, manual "refresh all." -- (Future) download & install products and updates; can absorb Element's appcast entitlement flow. - -Cross-platform: one codebase (JUCE app or native), signed/notarized, self-updating. - -### 3.2 Shared read-side SDK (linked by Element + every plugin) - -A small static/shared library. Contains **no interactive login** — read + apply + fall back only. - -Public surface: - -```cpp -namespace kv::lic { - -// Identity a product provides about itself. -struct ProductInfo { - juce::String productId; // stable slug, e.g. "kv.tonegenerator" - juce::String publicKey; // this product's embedded RSA public key - juce::String displayName; // for any UI/prompt fallback -}; - -// Locates and reads the shared per-user store. Read-only. -class StoreReader { -public: - static juce::File storeDir(); // platform-specific (see §5) - juce::File keyFileFor (const juce::String& productId) const; - bool hasTokens() const; // is anyone logged in? - // NOTE: reads tokens for refresh-on-read only if we choose to allow it (see §7, open decision). -}; - -// Common JUCE product-unlocking implementation. One per plugin instance. -class KushviewUnlockStatus : public juce::OnlineUnlockStatus { -public: - explicit KushviewUnlockStatus (ProductInfo); - - // The unlock ladder (see §4.2). Cheap; safe to call on load. - bool authorize(); - - juce::String getProductID() override { return info.productId; } - juce::RSAKey getPublicKey() override { return juce::RSAKey (info.publicKey); } - juce::String getState() override; // persisted applied-key state - void saveState (const juce::String&) override; - // Built-in web auth is intentionally unused: - juce::URL getServerAuthenticationURL() override { return {}; } - juce::String readReplyFromWebserver (const juce::String&, const juce::String&) override { return {}; } - -private: - ProductInfo info; - StoreReader store; -}; - -} // namespace kv::lic -``` - -### 3.3 Common key-writing pattern (the shared contract) - -This is the "common pattern for key writing" — one implementation, honored identically by the Manager (writer) and every plugin (reader). - -- **File per product:** `keys/.key`, containing the raw JUCE key-file text produced by `KeyGeneration::generateKeyFile` server-side. -- **Atomic writes** (Manager): write to `keys/.key.tmp`, `fsync`, rename over the target. Readers never observe a half-written file. -- **Applied-state cache** (plugin): after `applyKeyFile` succeeds, the plugin persists the applied state via `saveState()` into its *own* settings. So an already-authorized plugin keeps working even if the store file is later removed — until the embedded **expiry** lapses. -- **Machine-locked:** every key file carries this machine's IDs; copying it to another machine fails `applyKeyFile`. This is what makes the shared store safe to sync/back-up. -- **Expiry-bearing:** subscription keys are short-lived (e.g. 30 days). The Manager refreshes ahead of expiry; a lapsed entitlement simply stops being refreshed and the plugin falls back to prompting after expiry. - -### 3.4 Per-plugin integration (minimal) - -Each plugin adds only: - -```cpp -static const kv::lic::ProductInfo kToneGenProduct { - "kv.tonegenerator", - EMBEDDED_TONEGEN_PUBLIC_KEY, // build-time constant, safe to embed - "Kushview Tone Generator" -}; - -// On construction / first UI show: -kv::lic::KushviewUnlockStatus unlock (kToneGenProduct); -if (! unlock.authorize()) - showManualUnlockPrompt(); // existing per-plugin fallback UI -``` - -Nothing else. No OAuth, no browser, no URL scheme. - -### 3.5 Element's role - -Element is just another reader of the shared store for the *plugin* keys. Its existing OAuth login for **update entitlements** continues to work as-is short-term. - -- **Short-term:** Element keeps its own login (writes the same shared token store the Manager uses) and additionally reads plugin keys via the SDK. Both apps interoperate through one store. -- **Long-term:** the Manager becomes the canonical account/entitlement hub; Element migrates to a pure reader and drops its embedded login UI. *(Decision — see §7.)* - ---- - -## 4. Data flows - -### 4.1 Login (Manager only) - -``` -User clicks Sign In (Manager) - → PKCE verifier/challenge generated, state stored - → browser opens kushview.net/auth/authorize - → user authenticates on the store - → redirect kushview://auth/callback?code=…&state=… - → OS routes to Manager URL-scheme handler - → validate state, exchange code at kv-auth/v1/token - → receive JWT access token + refresh token + entitlements{tier, products[]} - → write tokens (OS secure storage) + entitlements.json - → for each entitled product: fetch signed key → write keys/.key -``` - -### 4.2 Unlock ladder (Element + plugins, on load) - -``` -authorize(): - 1. Local applied state valid AND not expired? → unlocked, ZERO network - 2. keys/.key present? - applyKeyFile(blob) - machine matches AND not expired? → unlocked, cache state - 3. Otherwise → return false - → caller shows existing manual unlock prompt -``` - -Step 1 makes the common case free (no I/O beyond a settings read). Step 3 guarantees graceful degradation. - -### 4.3 Background subscription refresh (Manager daemon) - -``` -On schedule / login-item wake: - ensure access token fresh (silent refresh; rotate refresh token) - GET entitlements - for each product: - if entitled: - if key missing OR expiry within threshold (e.g. < 7 days): - fetch new signed, machine-locked key → atomic write - else: - delete keys/.key # revoked / downgraded -``` - -This is the mechanism that gives real subscription semantics: cancel → next refresh stops re-signing → key expires → plugin falls back to prompting. - ---- - -## 5. Shared store layout & locations - -``` -/ - tokens → refresh/access tokens (prefer OS secure storage, see below) - entitlements.json - keys/ - kv.tonegenerator.key - kv..key -``` - -Platform base directory: -- **macOS:** `~/Library/Application Support/Kushview/Account/` -- **Windows:** `%APPDATA%\Kushview\Account\` -- **Linux:** `$XDG_CONFIG_HOME/kushview/account/` (fallback `~/.config/kushview/account/`) - -**Token at-rest protection:** store the refresh token in OS secure storage where available — macOS **Keychain**, Windows **Credential Manager (DPAPI)**, Linux **libsecret** — falling back to a restricted-permission file (0600) only if unavailable. Key files themselves are machine-locked, so they are low-sensitivity and can live as plain files. - ---- - -## 6. Security model & invariants - -Non-negotiable: - -1. **Private signing key never leaves the server.** All key-file generation is server-side. No client (Manager, Element, plugin) can mint or re-sign keys. -2. **Public keys are embedded and harmless.** They only verify. Already present in shipped binaries. -3. **Entitlement decisions are server-side.** The client asks; the server returns a signed key or a denial. A patched client cannot grant itself products — it can only replay what the server signed, which is machine-locked and time-limited. -4. **Machine-locked keys.** Every key file is bound to `getLocalMachineIDs()`; it cannot be lifted to another machine. -5. **Expiring keys.** Subscriptions rely on short TTL + refresh. No perpetual key for subscription tiers. -6. **TLS everywhere**; tokens in OS secure storage. -7. **Fail closed to the *prompt*, not to unlocked.** Any failure in the ladder ends at the manual unlock UI, never at an unearned unlock. - -Threats explicitly out of scope of "safe embedding": binary patching to skip the `isUnlocked()` check is possible for any offline-verifiable scheme and is not made worse by this design; machine-locking + expiry + server-side entitlement are the mitigations. - ---- - -## 7. Open decisions - -| # | Decision | Options | Lean | -|---|----------|---------|------| -| 1 | Element long-term auth | (a) keep own login + read keys; (b) delegate all account to Manager, Element becomes pure reader | (a) now → (b) later | -| 2 | Can plugins refresh a token themselves? | Read-only (Manager is sole refresher) vs. plugins allowed silent token refresh when store token is stale | Read-only first; simplest, smallest attack surface | -| 3 | Key TTL & refresh threshold | e.g. 30-day TTL, refresh < 7 days | confirm with subscription cadence | -| 4 | Store token format | OS secure storage vs. encrypted file | OS secure storage, file fallback | -| 5 | Standalone-plugin-without-Manager UX | require Manager once vs. per-plugin lightweight login | require Manager (matches Native Access) | - ---- - -## 8. Server-side (DEFERRED — contract stubs only) - -Not designed here; captured so the client contracts are unambiguous. Extends the existing `kv-auth/v1` namespace. - -- `GET /kv-auth/v1/entitlements` (Bearer) → `{ tier, products: [productId…] }` -- `GET /kv-auth/v1/plugin-key?product=&machine=` (Bearer) - → signed JUCE key file (machine-locked, expiring) or `403` if not entitled. - Same shape as the existing `/appcast-url` precedent. -- Reuse existing `/token`, `/token/refresh`, `/token/revoke`. - -Signing service holds the per-product **private** keys and calls `KeyGeneration::generateKeyFile`. - ---- - -## 9. Phased roadmap - -1. **Shared read-side SDK** — `StoreReader`, `KushviewUnlockStatus`, key-file read/apply/cache, store layout & locations. Unit-testable with a fixture store and a locally generated key pair. -2. **Per-plugin integration in one product** (Tone Generator) — embed public key, wire the ladder + fallback prompt. Prove silent unlock from a hand-placed key file. -3. **Account Manager MVP** — lift `element::auth` OAuth/PKCE into it, URL-scheme handling, write tokens + entitlements + keys to the store. -4. **Refresh daemon** — login-item/agent, expiry-driven re-fetch, revocation delete. -5. **Element as reader** — link the SDK, read plugin keys (keep existing update login). -6. **Roll SDK across remaining plugins.** -7. **Server-side** — entitlements + plugin-key signing endpoints (separate plan). -8. *(Future)* download/install + migrate update entitlement into Manager. - ---- - -## 10. Portability notes (for the other project) - -- The SDK and `KushviewUnlockStatus` are pure JUCE + a store path — no Element dependencies. They drop into any Kushview JUCE product. -- The only per-product inputs are the **productId**, the **embedded public key**, and a **display name**. -- The store layout in §5 and the key-writing pattern in §3.3 are the interop contract between the Manager and all readers — keep them identical on both sides. -- Nothing here requires the server work to exist first: with a locally generated test key pair and a hand-written `keys/.key`, the entire read/apply/ladder path is buildable and testable today. diff --git a/docs/plans/scripting-audit.md b/docs/plans/scripting-audit.md new file mode 100644 index 000000000..5312b3128 --- /dev/null +++ b/docs/plans/scripting-audit.md @@ -0,0 +1,199 @@ +# App-Side Scripting — Audit (September 2026) + +State of Element's Lua scripting *outside* the ScriptNode, taken before building the +console REPL and the hook system. Companion docs: [session-proxy.md](session-proxy.md) +(the mutation route), [session-scripts.md](session-scripts.md) (Phase 0: hooks, console, +session scripts), [extensions.md](extensions.md), [luajit.md](luajit.md). + +ScriptNode (`src/nodes/scriptnode*`) has its own `sol::state` and its own lifecycle and is +out of scope here. + +## Verdict + +| Layer | State | Evidence | +|---|---|---| +| Lua core: vendored 5.4 + sol2, `el.*` module searcher, `ScriptLoader`, `DSPScript` | **Solid** | Tested in `test/scripting/*`; `ScriptLoader` and `DSPScript` are exercised by ScriptNode in production. | +| Model bindings `el.Session` / `el.Node` / `el.Graph` | **Solid, read-mostly** | Userdata wraps the *live* tree (`std::make_shared (tree, false)`, `SessionPtr`), not copies. Can set name/tempo and save/restore state, nothing else. | +| App-side scripting: `ScriptingEngine`, `ScriptManager`, console, `el.command`, `el.Content` | **Flaky — abandoned mid-build** | Console bootstrap fails silently in dev builds, `el.command` calls a method that does not exist, several declared-but-undefined or permanently dead members (below). | +| Sandboxing | **None** | `Lua::initializeState` opens every stdlib. `ScriptView` creates a `sol::environment` and never applies it, so View scripts embedded in a session run in raw globals with `io`/`os`/`debug`. | +| Event / hook layer | **Absent** | No registry, no emit, no priorities. The signals that exist are asymmetric and incomplete. | + +## 1. The Lua state + +- One root `sol::state` per `Context`, owned by the pimpl `ScriptingEngine::State` + ([scripting.cpp:17-49](../../src/scripting.cpp#L17)). `Context::Impl::init()` creates + the engine and calls `initialize (owner)` ([context.cpp:80-81](../../src/context.cpp#L80)). + Reached via `context().scripting().getLuaState()`. +- Init is two-phase and partly duplicated: the `State` ctor opens `base`+`string` and + installs a searcher `resolve_internal_package` whose `builtins`/`packages` maps are + **never filled** (`fill_builtins` is commented out, `addPackage` has no callers). Then + `Lua::initializeState` ([bindings.cpp:413-435](../../src/scripting/bindings.cpp#L413)) + opens *all* libraries, inserts the real searcher `searchInternalModules` (a 160-line + if/else chain kept in sync by hand with ~30 `extern "C"` declarations) at position 2 + of `package.searchers`, and sets `package.path`, `package.cpath` (always empty) and the + non-standard `package.spath`. Both searchers coexist: `resolve_internal_package` is + pushed to position 3, so anything registered through `addPackage` *does* resolve — the + map is simply empty because nothing calls it. +- `_G["el.context"]` is a raw `std::ref` ([bindings.cpp:402-405](../../src/scripting/bindings.cpp#L402)), + nilled in `~ScriptingEngine`. Nothing guards Lua values that captured it earlier. +- No thread assertions anywhere in `src/scripting/`, `src/el/`, `src/scripting.cpp`. + Everything app-side runs on the message thread by convention only. + +## 2. The console today + +Classes: `Console` ([console.hpp](../../src/ui/console.hpp)) → `LuaConsole` +([luaconsole.hpp](../../src/ui/luaconsole.hpp)); `LuaConsoleView` +([luaconsoleview.hpp](../../src/ui/luaconsoleview.hpp)) is the `ContentView` that owns a +`LuaConsole` and listens to `Log`. Shown only as the secondary (bottom) pane via +`Commands::showConsole` ([standard.cpp:1154-1163](../../src/ui/standard.cpp#L1154)), +instantiated at [standard.cpp:644-647](../../src/ui/standard.cpp#L644). No feature flag. + +Eval chain: `ConsolePrompt::onReturnKey` → `Console::handleTextEntry` → +`LuaConsole::textEntered` ([luaconsole.cpp:25-73](../../src/ui/luaconsole.cpp#L25)): +compile-probe `"return ;"`, else run the raw text, through +`sol::state_view::script (code, env, "console=")`. The env is created once per view: +`sol::environment (view, sol::create, view.globals())` +([luaconsoleview.cpp:21-26](../../src/ui/luaconsoleview.cpp#L21)) — reads fall through +to `_G`, writes stay local. `print` is an env-local lambda pushing to a `StringArray` +drained by a `Timer`. Up/Down history (100 entries, not persisted). + +Reaching the session works right now: + +```lua +local ctx = require ('el.Context').instance() -- reads _G["el.context"] +local s = ctx:session(); print (#s, s[1].name) +``` + +Defects, in priority order: + +1. **Prelude fails silently.** `setEnvironment` runs + `require('el.script').exec('console', _ENV)`. `el.script.exec` *returns* the error + string instead of raising ([script.lua:30-34](../../src/el/script.lua#L30)), so the + `valid()` check never fires. `scripts/console.lua` is looked up on `package.spath` + (`~/Music/Element/Scripts`, app-data `Scripts`, + `Element.app/Contents/Resources/Scripts`); none exist in a dev build and + `scripts/CMakeLists.txt` only installs to `share/element/scripts`. The scripts *are* + already compiled into binary data (`scripts::console_lua` in `luascripts.hpp`) but + only ScriptNode uses that. +2. **`el.command` is dead.** [command.lua:10-13](../../src/el/command.lua#L10) calls + `Context.instance():commands()`; `el.Context` binds no such method + ([Context.cpp:39-58](../../src/el/Context.cpp#L39)) and `Context` has no accessor — + `Commands` lives on `GuiService::Impl`. Every `command.invoke` asserts. + `scripts/commands.lua` requires a non-existent `el.CommandManager`. +3. **No topology mutation API.** Nothing in `src/el/` calls + `addNode/addPlugin/addConnection/removeNode/addGraph`. +4. **State dies on toggle.** `showConsole` deletes the view; env, history and buffer are + rebuilt each time. +5. **Threading.** `LuaConsoleView::messageLogged` is called from `Log::logMessage` on the + *caller's* thread inside `Log`'s lock and writes a `TextEditor`. `printMessages` is a + plain `StringArray` shared between the Lua `print` lambda and the timer. +6. `_G.print` is swapped for the duration of every eval + ([luaconsole.cpp:33,71](../../src/ui/luaconsole.cpp#L33)); re-entrancy (modal, + `os.exit`) leaves it swapped. `lastError` is never assigned and `errorHandler` is + declared but never defined. The `else` error branch is unreachable (sol throws by + default). `buffer.length()` (characters) is passed as the byte length to + `load_buffer`. No `debug.traceback`. `EL_VIEW_CONSOLE` is `"LuaConsoleViw"` and is + persisted to settings. No default keypress. `console.log` in the prelude writes to + stdout, not the console. + +## 3. `ScriptingEngine` / `ScriptManager` dead weight + +| Item | Where | Fact | +|---|---|---| +| `ScriptingEngine::execute (const String&)` | [scripting.hpp:28](../../src/scripting.hpp#L28) | Declared, never defined. | +| `lua_State* L` | [scripting.hpp:40](../../src/scripting.hpp#L40) | Never assigned. | +| `State::builtins`, `fill_builtins` | [scripting.cpp:84-126](../../src/scripting.cpp#L84) | Map never filled; the fill call is commented out. Dead. | +| `State::resolve_internal_package`, `packages`, `addPackage` | [scripting.cpp:64-126](../../src/scripting.cpp#L64) | **Live**, not dead: the searcher stays at position 3 of `package.searchers`. No callers yet; [extensions.md](extensions.md) Phase 1 registers extension modules through it. | +| `EL_LUA_SPATH` | [scripting.cpp:10](../../src/scripting.cpp#L10) | Defined, never referenced. | +| `ScriptManager` | [scriptmanager.cpp](../../src/scripting/scriptmanager.cpp) | Never scans in the app: `Application::setupScripting` is `ignoreUnused (scripts)`; `Impl::scanDefaultLoctaion` (sic) has no callers. Test-only. | +| `ScriptInstance::object` (keep the class) | [scriptinstance.hpp](../../src/scripting/scriptinstance.hpp) | Private, no setter, so `cleanup()` is unreachable. | +| `DSPUIScript`, `ScriptSource` | `src/scripting/` | No users *yet*: intentional scaffolding for the script-type hierarchy and for where script code is sourced from (`ValueTreeScriptSource` → session scripts). Keep. | +| `DSPScript::validate` | [dspscript.cpp:434-438](../../src/scripting/dspscript.cpp#L434) | Returns `ok()` for any non-empty string; real body is `#if 0`. | +| `el.vector` | `src/el/vector.c` | Compiled, not registered in `searchInternalModules`. | +| `widget.hpp` `__props` | `src/el/widget.hpp` | Missing comma fuses `"visible" "opaque"` into `"visibleopaque"`. | +| `el/session.lua` | [session.lua:12](../../src/el/session.lua#L12) | Caches the `Session` userdata at require time; stale after reload. | +| `ScriptView::Impl::env` | [scriptview.cpp:24,71-78](../../src/ui/scriptview.cpp#L24) | Created, never passed to `loader.call()`; View scripts run in raw globals. | +| `getLuaCPath`, `getLocalScriptsDir`, `getLocalLuaDirs` | [bindings.cpp](../../src/scripting/bindings.cpp#L79) | Stubs returning empty. `EL_LUADIR`/`EL_SCRIPTSDIR`/`LUA_PATH_DEFAULT` are branched on but defined by no CMake file. | + +## 4. `el.*` bindings inventory + +Lua-source modules (compiled via `src/el/CMakeLists.txt` into `luamods.hpp`): +`el.AudioBuffer`, `el.object` (the proxy/OO system used by widgets), `el.script` +(loader + type constants), `el.session` (two helpers), `el.command` (broken), +`el.strings`, `el.color` (empty). + +C/C++ modules: `el.Context`, `el.Session`, `el.Node`, `el.Graph`, `el.Commands`, +`el.Content`, `el.View`, `el.GraphEditor`, `el.MidiPipe` (defined in +`src/engine/midipipe.cpp`), `el.Widget`, `el.TextButton`, `el.Slider`, `el.Desktop`, +`el.Graphics`, `el.MouseEvent`, `el.Bounds`/`Rectangle`/`Point`/`Range`, +`el.AudioBuffer32/64`, `el.MidiBuffer`, `el.MidiMessage`, `el.audio`, `el.midi`, +`el.bytes`, `el.round`; experimental `el.DocumentWindow`, `el.File`. + +What the model bindings can do today: + +- `el.Session` ([Session.cpp](../../src/el/Session.cpp)): `#s`, `s[i]` (live child + wrapped as `Node`), `name` and `tempo` get/set, `toXmlString`, `saveState`, + `restoreState`. +- `el.Node` / `el.Graph` ([nodetype.hpp](../../src/el/nodetype.hpp)): `name` get/set, + `valid`, `displayName`, `pluginName`, `uuidString`, `nodeId`, `nodeType`, `isGraph`, + `isRoot`, `toXmlString`, `saveState`/`restoreState`, `missing`, `enabled`, + `bypassed`, `muted` (read-only), `writeFile`, `resetPorts`, `hasEditor`, + `hasViewScript`, `viewScript`. The index metamethod returns every child as a plain + `Node`, even graphs. +- `el.Commands`: `invokeDirectly`, static `standard()`, `toString()`. No way to obtain + an instance. +- `el.Content`: `showToolbar`, `presentView (name)`; `presentViewObject` is an empty stub. + +## 5. Notification plumbing + +`include/element/signals.hpp` is three `boost::signals2` aliases. Signals that describe +a model or engine change: + +| Signal | Emitted from | Gap | +|---|---|---| +| `SessionService::sigSessionLoaded` | `refreshOtherControllers()` ([sessionservice.cpp:321-328](../../src/services/sessionservice.cpp#L321)) **and** directly from [pluginprocessor.cpp:800](../../src/pluginprocessor.cpp#L800) | The plugin path skips `EngineService::sessionReloaded()` and `MappingService::refresh()`. | +| `SessionService::sigWillSave` | `saveSession` | — | +| `EngineService::sigNodeRemoved` | `removeNode (const Node&)`, `removeGraph` | Not fired by `removeNode (uint32)` or by `GraphManager::removeNode`. | +| `GuiService::nodeSelected`, `sigRefreshed` | UI | `sigRefreshed` has no subscribers. | +| `DeviceService::sigMidiDevicesChanged`, `sigAudioDeviceStatus` | devices | — | + +Missing entirely: node added, graph added/removed/moved/activated, connection +added/removed, session closed/new. + +- `Session` is a `ChangeBroadcaster`; its four `ValueTree::Listener` overrides all + collapse into one undifferentiated `notifyChanged()` + ([session.cpp:255-294](../../src/session.cpp#L255)). Its only listener is + `SessionDocument` (dirty flag). +- `GraphManager::changed()` is a payload-less `sendChangeMessage()`. +- `AppMessage` types ([messages.hpp](../../src/messages.hpp)) are dispatched by a + `dynamic_cast` chain in `Services::handleMessage`; only `GuiService` overrides + `handleMessage`. +- Commands are integer ids and a `switch` in `GuiService::perform`; no runtime + registration. `Commands::toString` is incomplete, which silently truncates the + constants `command.lua` generates. +- Threading: all `Service` methods and all signals above run on the message thread. + Engine-side state changes reach the message thread through `AsyncUpdater` + (`RootGraphRender`, `GraphNode`, `Processor::EnablementUpdater`, + `AudioProcessorParameterCapture`, `MappingEngine`). + +## 6. Tests + +`test/scripting/`: `ScriptLoaderTest`, `DSPScriptTest`, `PresetScriptsTest`, +`MidiScriptTests`, `BytesTest`, `ScriptInfoTest`, `ScriptManagerTest`, `ScriptPlayground`. + +- `LuaFixture` ([luatest.hpp](../../test/scripting/luatest.hpp)) stands up a bare + `sol::state` with the *no-Context* `initializeState`, so `el.Context`, `el.Session`, + `el.command` and anything needing the app are untestable through it. `resetPaths()` + is CWD-relative while `getSnippetFile()` uses `EL_TEST_SOURCE_ROOT`. +- `test/TestMain.cpp` provides `element::test::context()`, which constructs a `Context` + and activates services — the right fixture for proxy/hook tests. +- `ScriptManagerTest` asserts a hard-coded `getNumScripts() == 13`. +- `test/snippets/sol3_parent.lua` and `stream_from_c.lua` are orphaned. + +## Not verified in this audit + +- Whether any shipped installer places `scripts/*.lua` in the macOS bundle + (`Contents/Resources/Scripts`); only the CMake install rule and the dev bundle were + checked. +- Behaviour of `DSPScript::process`'s unprotected `lua_call` on error with Lua built as + C (`longjmp` vs C++ exceptions) — noted, not exercised. diff --git a/docs/plans/session-proxy.md b/docs/plans/session-proxy.md new file mode 100644 index 000000000..785bf3bb0 --- /dev/null +++ b/docs/plans/session-proxy.md @@ -0,0 +1,335 @@ +# Graph Controller — the engine-side mutation path + +Architectural decision behind [session-scripts.md](session-scripts.md): where topology +changes to the live session are performed, and where hook events are dispatched from. +See [scripting-audit.md](scripting-audit.md) for the state of the code this builds on. + +Formerly "SessionProxy". Renamed because the object is not a proxy for the session: it +is the engine-side controller of the session's graphs. The file name is kept so links +elsewhere stay valid. + +## Motivation + +Today `EngineService` is the only coordination point between model and engine, and it +keeps them aligned by ordering and by index: + +- Root graphs: `EngineService::addGraph (const Node&, bool)` + ([engineservice.cpp:347-386](../../src/services/engineservice.cpp#L347)) creates a + `RootGraphHolder`, `attach (engine)`, *then* `session->addGraph (node, makeActive)`; + `Session::addGraph` ([session.cpp:109](../../src/session.cpp#L109)) is a pure tree op. +- Nodes: `GraphManager::addNode` ([graphmanager.cpp:376-413](../../src/engine/graphmanager.cpp#L376)) + is engine-first: create the `Processor`, copy the tree, set `id`/`object`/`type`, + `nodes.addChild`. +- `EngineService::moveGraph` calls `AudioEngine::moveGraph` then `Session::moveGraph`. + That "pure model primitive + pure engine primitive, coordinated by the service" split + (#1184) was done deliberately so a single coordinator could call both. + +Two things are wanted from the change: + +1. **One mutation path.** Every topology change — menu, undoable action, Lua, session + load — goes through one object, so `graph.added` / `node.removed` / + `connection.added` fire exactly once regardless of source. That object is the single + `HookBus` dispatch point for those events. +2. **Separation of concerns.** The model (`Session`, `Graph`, `Node` in + `include/element`) stays a data layer: value types over a `ValueTree`, no engine + headers, callable from anywhere. The engine layer owns engine changes. The binding + layer (`src/el/`), which already knows both, wires Lua to the controller. Dependencies + point one way: engine → model, never model → engine. + +## Options considered + +**A — controller object stored in the tree** (a ref-counted object under a +`tags::proxy` property, the `tags::updater` idiom). Rejected on lifetime grounds: +`Context::Impl::freeAll()` frees `services` first and `session` later +([context.cpp:94-106](../../src/context.cpp#L94)); `PluginProcessor::~PluginProcessor` +deactivates services and *then* calls `session->clear()`; `Session::clear()` and +`loadData()` strip or replace the tree; a `var`-held object serializes as +`"Object 0x…"` unless stripped. + +**B — leave `EngineService` as the facade.** Least change, but `RootGraphs` stays inside +the service, the hook dispatch is smeared across it, and it stays the god object. + +**C — controller registered on the `Session`, discoverable from any `Node` copy via a +`Session::Handle` planted on the root tree, with engine verbs on the model.** Rejected +after review: + +- It makes `include/element` depend on `src/engine`, makes value types + message-thread-only, and lets a `Graph` copy instantiate plugins. +- The "detached copies are inert" claim does not hold. [session.cpp:170](../../src/session.cpp#L170), + [:338](../../src/session.cpp#L338) and [:373](../../src/session.cpp#L373) copy the + *root* tree, and a root copy carries the handle; `findFor` on that copy would resolve + the live session and mutate the running engine. +- `Session::addGraph` behaving differently depending on whether a controller happens to + be installed is a hidden mode. + +**D — `EngineService`-owned controller, reached through the services.** Chosen. Same +object as C (it absorbs `RootGraphs` and is the hook dispatch point), but the model is +not involved: callers are `EngineService` forwards, the undo actions (unchanged, they +call `EngineService`), and the Lua bindings, which resolve it per call through +`Context` → services → `EngineService`, per the *Lua Bindings* rules in CLAUDE.md. + +## Design + +### Ownership and lifetime + +- `GraphController` (`src/engine/graphcontroller.hpp/.cpp`) becomes the home of + `RootGraphHolder` and `RootGraphs`, moved verbatim from + [engineservice.cpp:78-305](../../src/services/engineservice.cpp#L78). The constructor + takes `Context&`, `AudioEnginePtr`, `SessionPtr` and `HookBus&` (injected, never + looked up). The destructor never calls `context()` (services may already be gone). +- `EngineService` owns `std::unique_ptr controller`: created in + `activate()` after `engine->activate()` and before `sessionReloaded()`; reset in + `deactivate()` after `session->saveGraphState()` (replaces `graphs->clear()`). + `GraphController* EngineService::controller()` returns null when inactive. +- The controller survives session reload for free: `loadData()`/`clear()` touch the + tree, not the controller. Holders are rebuilt by `sessionReloaded()` → `reload()`. +- The model is untouched: no `Session::Handle`, no `findFor`, no weak pointer in + `Session::Impl`, no engine verbs on `Session`/`Graph`. `Session::addGraph`, + `moveGraph`, `setActiveGraph` stay what they are today: pure tree operations. + +### `GraphController` API + +All methods return `bool`/`Node`, never show UI, and assert the message thread. + +```cpp +// root graphs +bool addGraph (const Node& graph, bool makeActive); +Node addGraph (const juce::String& name); // default ports from engine->getNumChannels +bool removeGraph (int index); +bool moveGraph (const Node& graph, int newIndex); +bool setActiveGraph (int index); // today's setRootNode + Session::setActiveGraph +void reload(); // today's sessionReloaded, under HookBus::ScopedSuspend +void clear(); void syncModels(); +// nodes +Node addNode (const Node& graph, const Node& nodeTemplate, const ConnectionBuilder& = {}); +Node addPlugin (const Node& graph, const juce::PluginDescription&, double rx = .5, double ry = .5); +bool removeNode (const Node&); // incl. the IO-port workaround at engineservice.cpp:717-738 +Node replace (const Node&, const juce::PluginDescription&); +bool changeBusesLayout (const Node&, const juce::AudioProcessor::BusesLayout&); +// connections +bool addConnection (const Node& graph, uint32 s, uint32 sp, uint32 d, uint32 dp); +bool removeConnection (const Node& graph, uint32 s, uint32 sp, uint32 d, uint32 dp); +bool connectChannels (const Node& graph, const Node& src, int sc, const Node& dst, int dc, + PortType = PortType::Audio, int count = 1); +struct DisconnectOptions { bool inputs = true, outputs = true, audio = true, midi = true; }; +void disconnectNode (const Node&, DisconnectOptions = {}); // per-arc removeConnection +// lookup +GraphManager* findGraphManagerFor (const Node&) const; // by graph tree identity +RootGraphManager* findActiveRootGraphManager() const; +``` + +- `addNode`/`addPlugin` pre-validate through `PluginManager::findDescriptionFor` / + `getKnownPlugins()` so a bad identifier from Lua returns an invalid `Node`. +- `DisconnectOptions` replaces today's four positional booleans; no new method on the + controller takes more than one `bool`. +- `findGraphManagerFor` matches on the graph's underlying tree, so any `Node` copy of a + live graph resolves; a copy of a detached subtree resolves to null and the call fails. + +### `GraphManager` never shows UI + +`GraphManager` shows modal alerts at +[graphmanager.cpp:29-31](../../src/engine/graphmanager.cpp#L29), [:380](../../src/engine/graphmanager.cpp#L380), +[:408](../../src/engine/graphmanager.cpp#L408) and [:418](../../src/engine/graphmanager.cpp#L418). +Pre-validation in the controller catches "unknown plugin" but not "found but failed to +instantiate", and a modal in that path hangs a headless test. `GraphManager::addNode` +and friends return an invalid `Node`/`false` and log; the alert moves to +`EngineService`, which is where UI policy lives after this change. This is a +prerequisite for the controller and lands as its own step (see *Order of work* in +session-scripts.md). + +### `EngineService` afterwards + +Every public signature in [engine.hpp](../../include/element/engine.hpp) stays, as a +thin forward to the controller plus the UI/plugin-list policy that does not belong in +the controller: the alerts (including those moved out of `GraphManager`), +`presentPluginWindow` (only on the entry points that show it today, so undo/redo never +pops windows), `detail::verifyPlugin` / `saveUserPlugins` / `addToKnownPlugins`, +`addMidiDeviceNode` (rewritten over `controller->addPlugin` + `Node::getObject()`), +`stabilizeViews` after `replace`/`changeBusesLayout`. Group the alert and plugin-list +policy in a `detail::` helper inside `engineservice.cpp` so the service itself is +forwards plus policy calls. + +Callers that keep compiling unchanged: `src/messages.cpp` undo actions, +`src/services.cpp:224-320`, `guiservice.cpp:1070-1073`, +`sessiontreepanel.cpp:592/693/1079`, `sessionservice.cpp:104`. + +The active-graph-only overloads (`addConnection (s, sp, d, dp)`, `removeNode (uint32)`) +resolve `session->getActiveGraph()` and forward — this also fixes `removeNode (uint32)` +([engineservice.cpp:755-763](../../src/services/engineservice.cpp#L755)) bypassing +notification today. + +Moves out of `EngineService` entirely: `RootGraphHolder`, `RootGraphs`, the bodies of +`addGraph (Node, bool)`, `removeGraph`, `moveGraph`, `setRootNode`, `sessionReloaded`, +`syncModels`, `addNode (Node, Node, Builder)`, private `addPlugin (GraphManager&, …)`, +`removeNode (Node)`, the graph overloads of `addConnection`/`removeConnection`, +`connectChannels`/`connect`, `disconnectNode`, `replace`, `changeBusesLayout`, `clear`. + +`sigNodeRemoved` is deleted. Its consumer +([grapheditorview.cpp:80,158](../../src/ui/grapheditorview.cpp#L80)) subscribes to +`graph.removed` on `context().hooks()` instead. + +### Lua reaches the controller through the services + +Per CLAUDE.md *Lua Bindings*: one entry point, resolve per call, no globals. The +`el.Session`/`el.Graph` methods in [session-scripts.md](session-scripts.md) § 4 are +implemented in `src/el/Session.cpp` / `Graph.cpp` as: + +```cpp +static GraphController* controllerFor (lua_State* L) +{ + auto& ctx = element::lua::contextFrom (L); // reads _G["el.context"] like el.Context.instance() + auto* engine = ctx.services().find(); + return engine != nullptr ? engine->controller() : nullptr; +} +// el.Graph:addNode +"addNode", [] (Graph& self, const std::string& id, sol::optional format, sol::this_state L) + -> std::tuple { + auto* c = controllerFor (L); + if (c == nullptr) return { nil, "engine not running" }; + auto node = c->addNode (self, Node::makeTemplate (id, format...)); + return node.isValid() ? { make_object (node), nil } : { nil, "could not add " + id }; +} +``` + +Nothing is cached: no controller pointer in a userdata, no session cached at require +time. A `Graph` userdata is only a tree handle; the controller decides whether it +refers to a live graph. + +### Undo + +No structural change. `AddPluginAction`, `RemoveNodeAction`, `AddConnectionAction`, +`RemoveConnectionAction` ([messages.cpp](../../src/messages.cpp)) keep calling +`EngineService`; each `perform()`/`undo()` is exactly one controller mutation and +therefore exactly one hook. Lua-initiated mutations are **not** undoable in this phase: +they bypass `GuiService::handleMessage` +([guiservice.cpp:1235-1245](../../src/services/guiservice.cpp#L1235)). Making them +undoable later means posting `AppMessage`s, which is asynchronous and cannot return the +created node — a deliberate non-goal for now. + +### GUI reacts through hooks + +- `GuiService::activate()` registers handlers (ids stored, removed in `deactivate()`): + `node.removing` → `closePluginWindowsFor (node, true)` + deselect (moved from + [engineservice.cpp:706-712](../../src/services/engineservice.cpp#L706)); + `graph.removed` → `stabilizeContent()` (replaces the `// FIXME: dont notify the UI + top-down` at `:489`); `graph.activated` → close/show plugin windows (moved from + [sessiontreepanel.cpp:618-627](../../src/ui/sessiontreepanel.cpp#L618) and + [guiservice.cpp:171-175](../../src/services/guiservice.cpp#L171)). +- Direct writes of `tags::active` in `sessiontreepanel.cpp:624-626`, `:758` and + `guiservice.cpp:173-174` become `EngineService::setActiveGraph (index)`; otherwise + Lua-initiated activation and hooks diverge from the UI path. + +### Hook dispatch points + +Only `GraphController` fires topology actions: `graph.added`, `graph.removing`, +`graph.removed`, `graph.moved`, `graph.activated`, `node.added`, `node.removing`, +`node.removed`, `connection.added`, `connection.removed`. Pre-hooks (`*.removing`) exist +so the GUI can close plugin windows *before* the processor dies regardless of who +initiated the removal. + +- `reload()` runs under `HookBus::ScopedSuspend`, so session load fires nothing per node + (`GraphManager::setNodeModel` and `IONodeEnforcer` operate below the controller + anyway) and the internal `setActiveGraph` during load does not leak `graph.activated`. +- `session.loaded` is fired once from a new `SessionService::notifySessionLoaded()`, + which also emits `sigSessionLoaded()`; called from `refreshOtherControllers()` and + from [pluginprocessor.cpp:800](../../src/pluginprocessor.cpp#L800) in place of the + direct emit. Not fired from `reload()`: `EngineService::sessionReloaded()` is also + called by `PluginProcessor::reloadEngine()` on every host re-prepare. +- `session.saving` next to `sigWillSave`; `session.closed` in `closeSession()`. +- `Session::ScopedFrozenLock` is left alone: it guards the document-dirty + `ChangeBroadcaster`, a different concern, and `Services::run()` keeps the whole + default-session open frozen, which would wrongly swallow a legitimate `graph.added` + from an `.elg` import. + +**Coverage gaps, decided:** + +- `ConnectionBuilder` connections made while adding a node currently happen inside + `GraphManager::addNode` ([graphmanager.cpp:801](../../src/engine/graphmanager.cpp#L801)), + below the controller. The controller applies the builder itself through its own + `addConnection` after the node exists, so each auto-connection fires + `connection.added`; `GraphManager::addNode` loses its builder parameter. +- `Processor` re-applies channel connections after a bus-layout change at + [processor.cpp:972-977](../../src/engine/processor.cpp#L972), also below the + controller. Phase 0 fires no `connection.*` for these; `changeBusesLayout` is + documented as "ports and their connections may change" and a `node.portsChanged` + action is a candidate follow-up. +- Engine-initiated active-graph changes (MIDI program change, + [audioengine.cpp:657-665](../../src/engine/audioengine.cpp#L657)) bypass the + controller; `graph.activated` excludes them in Phase 0. Tracked as + session-scripts.md open question 5. + +## Future: model-initiated mutation, if ever wanted + +If a later phase wants `Session::addGraph (...)` to drive the engine, the path is a +delegate interface **declared by the model and implemented by the controller** — not a +handle in the tree, and not engine headers in `include/element`: + +```cpp +// include/element/session.hpp +class Session { +public: + struct Delegate { + virtual ~Delegate() = default; + virtual bool addGraph (Session&, const Node& graph, bool makeActive) = 0; + virtual bool removeGraph (Session&, int index) = 0; + // one method per controller verb that should be model-initiated + }; + void setDelegate (Delegate*); // EngineService installs in activate(), clears in deactivate() +}; +``` + +`Session::addGraph` calls the delegate synchronously: the controller attaches the +engine side first, then performs the tree op, then fires the hook, and the result is +returned to the caller. That is the ordering `EngineService::addGraph` already uses +today. Graph-level verbs take the graph as an argument on the session +(`session->addNode (graph, template)`) so `Graph`/`Node` copies stay inert. A +"model first, engine follows via `ValueTree::Listener`" design is explicitly not the +path: the caller cannot learn about engine failure, and reload/undo/`loadData` all have +to suspend the listener. + +## Tests + +`test/GraphControllerTests.cpp`, suite `GraphControllerTests`, using +`element::test::context()` ([TestMain.cpp:15-23](../../test/TestMain.cpp#L15), which +activates services so the controller exists) and a test node provider extracted from +`CountingNodeProvider` ([SessionLoadBenchTests.cpp:58-82](../../test/SessionLoadBenchTests.cpp#L58)) +into `test/fixture/TestNodeProvider.h`: + +- `ControllerInstalledAndCleared` — own `Context`, `services().activate()/deactivate()`, + `controller()` non-null only while active. +- `AddGraphAttachesEngine` — `controller->addGraph` → `getObject()` non-null, + `graph.added` once; the session tree gained the child. +- `AddNodeCreatesProcessor`, `ConnectDisconnect` (arc present in `arcs`, hooks fired). +- `BuilderConnectionsFireHooks` — `addNode` with a `ConnectionBuilder` → one + `connection.added` per built arc. +- `RemoveNodeOrdering` — `node.removing` before `node.removed`. +- `RemoveGraphFixesActive`, `SetActiveGraphSyncsEngine` (`engine->getActiveGraphIndex()`). +- `ReloadFiresNoPerNodeHooks` — `loadData` + `sessionReloaded()` → zero `node.added`. +- `InstantiationFailureIsSilent` — a node type whose provider returns null → invalid + `Node`, no modal, test does not hang (proves the `GraphManager` alert move). +- `DetachedGraphIsRejected` — `addNode` on a `Graph` copied out of the session → invalid + `Node`, no hook. + +Register with `add_test (NAME "GraphControllerTests" COMMAND test_element --run_test=GraphControllerTests)`. +Existing `SessionTests` keep covering the pure-tree path unchanged. Remember: a +`GraphNode` driven by `GraphManager` must be heap-allocated (`ProcessorPtr keep (new GraphNode (ctx))`). + +## Risks / not verified + +- Headless attach with real graphs: `SessionTests::MoveEngineGraph` proves + `AudioEngine::addGraph` works without a device and `test::context()` runs + `sessionReloaded()` with zero graphs, but the full `attach` + `setRootNode` + (`setPlayConfigFor (devices)`) path with real graphs has no current test; sample + rate/block size may be 0 headless. Write this test before moving code. +- `PluginProcessor::reloadEngine` → `reload()` must keep the full detach/re-attach + behaviour; `prepareExternalPlayback` re-preparation was not traced. +- Re-entrant hooks (a handler adding a node from `node.added`) work through the depth + guard but interleave `GraphManager::changed()` broadcasts. Untested territory. +- `RootGraphHolder` touches `Processor` internals as `friend class EngineService` + ([processor.hpp:539](../../include/element/processor.hpp#L539)); `GraphController` + needs adding to the friend list. +- `new_nodetype`'s index metamethod already returns non-graph children as + `Graph` today; the `Graph (const Node&)` constructor asserts type == Graph in debug + builds. The `sol::make_object` split in session-scripts.md is required, not optional. +- The #1184 issue text was not read; the split-primitive intent is inferred from the + `Session::moveGraph` doc comment and `EngineService::moveGraph`. diff --git a/docs/plans/session-scripts.md b/docs/plans/session-scripts.md index 3328f6e70..fc2c5d06f 100644 --- a/docs/plans/session-scripts.md +++ b/docs/plans/session-scripts.md @@ -1,9 +1,19 @@ -# Phase 0 — Session Scripts & the Hook System +# Phase 0 — Console, Hooks & Session Scripts -Predecessor to [extensions.md](extensions.md). Front-loads the pieces the extension format -needs anyway — the hook bus, the script descriptor conventions, and the restricted -environment / capability model — but delivers them first for scripts **embedded in the -session file**, where DSP/DSPUI scripts already live today. +Predecessor to [extensions.md](extensions.md). Front-loads what the extension format +needs anyway — the hook bus, the script descriptor conventions, the restricted +environment / capability model — and delivers them first for the Lua **console** and for +scripts **embedded in the session file**, where DSP/DSPUI scripts already live today. + +Prerequisites and companions: + +- [scripting-audit.md](scripting-audit.md) — what exists today and what is broken. +- [session-proxy.md](session-proxy.md) — the `GraphController`: the engine-side + mutation path and the single hook dispatch point. **Read it first**; this document + assumes it. Consequences here: hooks fire from the controller; `el.Session`/`el.Graph` + reach it per call through `Context` → services → `EngineService` (CLAUDE.md *Lua + Bindings*); the model stays a data layer with no engine verbs. The `el.engine` facade + module previously planned is dropped. ## Context @@ -12,58 +22,250 @@ gzip'd code), with `Graph::findViewScript()` resolving the `View` script and `Sc embedding DSP source in its state blob. The Session root does **not** have a scripts tree (`src/session.cpp` — children are `graphs`, `controllers`, `maps`, `midiMappings`, `ui`). -Phase 0 extends the same pattern to the session: sessions can carry scripts that *run* at -defined lifecycle points. Scripts follow the **same descriptor format planned for -extensions**, so a script is portable between "written into the session" and "shipped in -a `.element` extension" without edits. +The console exists (`src/ui/luaconsole.cpp`) and evaluates in the shared app state, but +its prelude never loads in dev builds, `el.command` is broken, and there is no way to +change graph topology from Lua. There is no hook/event registry of any kind. ## Design principles 1. **WordPress-inspired hooks, not a clone.** Two dispatch kinds: - **actions** — fire-and-forget notifications (`node.added`, `session.loaded`, ...) - **filters** — each handler receives a value and returns a (possibly modified) value; - the host uses the final result (e.g. filter a display name, a save payload, a menu). - Handlers have an optional integer priority (default 10, lower runs first) and an owner - tag for bulk teardown. No WordPress-style global mutable everything — events are - explicit, dispatch is message-thread only, and the C++ side owns the registry. + the host uses the final result. + Handlers have an integer priority (default 10, lower runs first) and an owner tag for + bulk teardown. Events are explicit, dispatch is message-thread only, and the C++ side + owns the registry. 2. **Untrusted by default — capabilities are earned.** A session file is a *document*; - documents that carry executable code must not silently get the keys to the app. Session - scripts run in a restricted `sol::environment`: - - Base env: safe stdlib subset (no `io`, no `os` beyond `time`/`clock`, no `require` - of arbitrary modules), plus pure-data `el.*` modules (bytes, midi, colors, strings). - - Anything powerful — `el.engine`, `el.Context`, `el.ui`, file access — must be - declared in the script descriptor (`requires = { "engine", "ui" }`) and granted by - the host. Grant policy v1: per-session trust prompt on first run ("This session - contains scripts that want: engine access. Run / Run always for this session / - Don't run"), persisted in settings keyed by a session content hash or path. - - Extensions (user-installed) get a more permissive default later; the *mechanism* - (declared requires → injected capabilities) is identical. - -3. **Bind less C++, write more Lua.** The rule going forward: C++ binds a minimal opaque - userdata handle ("impl"), and the ergonomic API is a native Lua module holding that - handle as a private member. Precedent already in-tree: `src/el/object.lua`, - `session.lua`, `command.lua`, `script.lua` wrap C bindings in Lua tables. New surface - (`el.hooks`, later `el.engine` sugar) should be Lua-first with a thin C core — this - avoids sol2 usertype boilerplate for every class and keeps the public script API - decoupled from C++ headers. + documents that carry executable code must not silently get the keys to the app. + Session scripts run in a restricted `sol::environment`; anything powerful must be + declared in the descriptor (`requires = { 'session' }`) and granted by the host. + +3. **Bind less C++, write more Lua.** C++ binds a minimal opaque handle; the ergonomic + API is a native Lua module holding it. Precedent: `src/el/object.lua`, `command.lua`, + `script.lua`. New surface (`el.hooks`) is Lua-first with a thin C core. + +4. **One mutation path.** Every topology change — UI, undo, Lua, session load — goes + through `GraphController`. Hooks are dispatched there and nowhere else for those + events. ## What gets built -### 1. Model: session-level scripts +### 1. `HookBus` (`include/element/hooks.hpp`, `src/hooks.cpp`) + +A plain registry; no `boost::signals2` groups needed. + +```cpp +struct HookEvent { + juce::String action; + Node node; // the node/graph the event is about (invalid if n/a) + Node graph; // its parent graph, or the root graph for graph.* events + Arc arc; // connection.* events + int index = -1; // graph.moved / graph.activated +}; + +class HookBus { +public: + using Action = std::function; + using Filter = std::function; + + int addAction (const juce::String& name, Action, int priority = 10, const juce::String& owner = {}); + int addFilter (const juce::String& name, Filter, int priority = 10, const juce::String& owner = {}); + void remove (int id); + void removeOwner (const juce::String& owner); + + void doAction (const juce::String& name, const HookEvent& = {}); + juce::var applyFilters (const juce::String& name, juce::var value, const HookEvent& = {}); + bool hasActions (const juce::String& name) const; + + struct ScopedSuspend { explicit ScopedSuspend (HookBus&); ~ScopedSuspend(); }; + +private: + struct Handler { int id; int priority; juce::String owner; Action action; Filter filter; int errors = 0; bool enabled = true; }; + std::map> actions, filters; + int suspendCount = 0, depth = 0; + std::deque> pending; +}; + +namespace hooks { +constexpr const char* appStarted = "app.started"; +constexpr const char* appShutdown = "app.shutdown"; +constexpr const char* sessionLoaded = "session.loaded"; +constexpr const char* sessionSaving = "session.saving"; +constexpr const char* sessionClosed = "session.closed"; +constexpr const char* graphAdded = "graph.added"; +constexpr const char* graphRemoving = "graph.removing"; +constexpr const char* graphRemoved = "graph.removed"; +constexpr const char* graphMoved = "graph.moved"; +constexpr const char* graphActivated = "graph.activated"; +constexpr const char* nodeAdded = "node.added"; +constexpr const char* nodeRemoving = "node.removing"; +constexpr const char* nodeRemoved = "node.removed"; +constexpr const char* connectionAdded = "connection.added"; +constexpr const char* connectionRemoved = "connection.removed"; +constexpr const char* graphDefaultName = "graph.defaultName"; // filter +} +``` + +Rules: + +- `doAction`/`applyFilters` assert the message thread. While `suspendCount > 0` they are + no-ops. +- Dispatch iterates a *copy* of the handler list sorted by priority (handlers may add or + remove during dispatch). +- Nested `doAction` (a handler mutating topology) is queued in `pending` and drained after + the outer dispatch returns; `depth` is asserted `< 8`. +- Every handler call is wrapped; an exception increments `errors`, three consecutive + errors disable the handler; a success resets the count. Errors go to + `Context::logger()` (and, for Lua handlers, `ScriptingEngine::logError`). Nothing + propagates. +- Filters thread the `juce::var`; a handler returning `void`/`nil` leaves the value + unchanged. + +Ownership: `Context::Impl` owns `std::unique_ptr hooks` with accessor +`HookBus& Context::hooks()`. Created in `init()` **before** `services` (so services +subscribe in `activate()`). In `freeAll()` +([context.cpp:94-106](../../src/context.cpp#L94)) reset **right after `services`** and +before `lua`: Lua-backed handlers hold `sol::protected_function`s and must die while the +state is alive. + +Proof-of-shape filter: `graph.defaultName`, applied in `GraphController::addGraph (name)` +to the generated `"Graph N"` string. Single call site, no UI paint-path cost. Filters +like `node.displayName` are deferred until a call site that is not in a paint loop is +chosen. + +### 2. Dispatch sites + +See [session-proxy.md](session-proxy.md) § *Hook dispatch points*. Summary: every +`GraphController` mutation fires its action; `reload()` runs under `ScopedSuspend`; +`SessionService::notifySessionLoaded()` fires `session.loaded` (replacing the direct +`sigSessionLoaded()` at `pluginprocessor.cpp:800` too); `session.saving` beside +`sigWillSave`; `session.closed` in `closeSession()`; `app.started` at the end of +`Services::launch`; `app.shutdown` in `ScriptingService::deactivate()`. + +`EngineService::sigNodeRemoved` is deleted; `GuiService` and `GraphEditorView` subscribe +to hooks instead (details in the controller doc). + +### 3. Console foundation (`src/ui/luaconsole.*`, `luaconsoleview.*`, `src/scripting.*`) + +- **Evaluator in the engine.** Implement the declared-but-missing + `ScriptingEngine::execute` as + `juce::Result execute (const juce::String& code, sol::environment env)`: the + `"return ;"` compile probe, `sol::protected_function` with a `debug.traceback` + handler, byte length from `toRawUTF8()`. Result message carries the traceback. + Asserts the message thread (`JUCE_ASSERT_MESSAGE_THREAD`); the shared state has no + other guard, and this is the entry point every later caller (console, hook scripts, + extension entry scripts) goes through. + `LuaConsole::textEntered` becomes a thin caller; drop the `_G.print` swap, the dead + `lastError`/`errorHandler`, and the unreachable error branch. +- **Persistent environment.** `sol::environment& ScriptingEngine::consoleEnvironment()`, + lazily created with globals fallback (`sol::environment (state, sol::create, + state.globals())`). `LuaConsoleView::initializeView` uses it, so variables, `print` + override and registered hooks survive view toggles. History moves alongside it + (`ScriptingEngine::consoleHistory()` or keep it in the env as a Lua table). +- **Prelude from binary data.** `LuaConsole::setEnvironment` runs `scripts::console_lua` + (`luascripts.hpp`, already compiled by `scripts/CMakeLists.txt`) in the env with + `sol::script_pass_on_error` and reports failures to the console. `el.script.exec` + is changed to `error()` when `load` fails instead of returning the message. + `scripts/console.lua` gains: + + ```lua + hooks = require ('el.hooks') + Context = require ('el.Context') + session = function() return Context.instance():session() end -- a function: never cache the session + console.log = function (...) print (...) end -- to the console, not stdout + ``` + +- **Threading.** `LuaConsoleView::messageLogged` posts to the message thread with + `juce::MessageManager::callAsync` and a `Component::SafePointer`; `printMessages` is + guarded by a `juce::CriticalSection` (or replaced with an `AsyncUpdater`). +- **`el.command`.** `el.Context` gains `commands()`, resolved per call through + `ctx.services().find()->commands()`, so `command.lua`'s existing + `Context.instance():commands()` is correct as written. No new globals: `el.context` is + the single entry point into the app. `scripts/commands.lua` is deleted. +- Cosmetic: `EL_VIEW_CONSOLE` becomes `"LuaConsoleView"` (accept the old + `"LuaConsoleViw"` when restoring `ContentContainer_lastSecondaryView`); `showConsole` + gets a default keypress alongside `showPatchBay`/`showGraphEditor`. + +### 4. Lua surface for mutation (`src/el/Session.cpp`, `Graph.cpp`, `nodetype.hpp`) + +All of these resolve the `GraphController` per call through +`Context` → services → `EngineService` (see the controller doc § *Lua reaches the +controller through the services*) and therefore fire hooks. With no active engine they +return `nil, "engine not running"`; nothing falls back to silently editing the tree. + +`el.Session`: + +```lua +local s = session() +g = s:addGraph ("Rig" [, active]) -- string or el.Graph template; returns el.Graph +s:removeGraph (index) s:moveGraph (from, to) s:setActiveGraph (index) +s:activeGraph() s.activeGraphIndex s:findNodeById (uuid) +s[i] -- now returns el.Graph, not el.Node +``` + +`el.Graph`: + +```lua +n = g:addNode ("element.volume" [, format]) -- returns el.Node or nil, "message" +p = g:addPlugin { format = "VST3", name = "..." | id = "..." } +g:removeNode (n) +g:connect (srcId, srcPort, dstId, dstPort) g:disconnect (...) +g:connectChannels (src, sc, dst, dc [, "midi"]) +for node in g:nodes() do ... end for arc in g:connections() do ... end +``` + +- `luaopen_el_Session` requires `el.Graph` (as `Context.cpp:60-63` requires `Node`/`Session`). +- In `nodetype.hpp` the index metamethod returns a `Graph` userdata when + `child.isGraph()` via `sol::make_object`, else a `Node`, so the `Graph (const Node&)` + type assert never fires for plain nodes. +- Lua two-value error convention: `nil, "message"`; never throws into the console. + +### 5. `el.hooks` — Lua-first + +Thin C core `src/el/Hooks.cpp` (`luaopen_el_Hooks`), registered in +`searchInternalModules` and `src/el/CMakeLists.txt`: + +``` +register (kind, name, fn, priority, owner) -> id -- kind = "action" | "filter" +unregister (id) +unregister_owner (owner) +emit (name, event_table) +``` + +Wrapped by `src/el/hooks.lua`: + +```lua +local hooks = require ('el.hooks') +local id = hooks.action ('node.added', function (e) print (e.node.name, e.graph.name) end [, priority]) +hooks.filter ('graph.defaultName', function (name, e) return name .. ' *' end) +hooks.off (id) +hooks.emit ('my.event', { ... }) -- custom events +local mine = hooks.owner ('my-script') -- scoped table: same API, owner pre-filled +mine.action (...); mine.clear() +``` + +Handlers receive `{ action=, node=, graph=, arc=, index= }`. +Filters receive `(value, event)` and return the value. The C side keeps a per-state map +`id → sol::protected_function` so `unregister` and state teardown can drop references. +Console registrations default to owner `"console"`; `ScriptingService` sets a "current +owner" while running a script so `hooks.action` picks it up without the caller passing +it. + +### 6. Model: session-level scripts - `Session` gains a `scripts` child tree, identical shape to `Node::getScriptsValueTree()` - (`include/element/node.hpp:434`). Add `Session::scripts()` / `addScript()` / - `removeScript()` accessors mirroring `Node::addScript` (`src/node.cpp:476`). -- New script type tag: `EL_TAG(Hook)` in `include/element/tags.hpp` (joins `DSP`, `View`, - `GraphView`, `Anonymous`). `Script::make` (`src/script.cpp:183`) accepts `types::Hook` - and seeds a template. + ([node.hpp:442](../../include/element/node.hpp#L442)). Add `Session::scripts()`, + `addScript()`, `removeScript()` mirroring `Node::addScript` + ([node.cpp:476](../../src/node.cpp#L476)). +- New script type tag `EL_TAG (Hook)` in `include/element/tags.hpp` (joins `DSP`, + `View`, `GraphView`, `Anonymous`). `Script::make` ([script.cpp:183](../../src/script.cpp#L183)) + accepts `types::Hook` and seeds the template below. - Additive child tree — no `EL_SESSION_VERSION` bump; add a no-op guard in `Session::migrate`. -- Persistence is free: `Script` code is already gzip'd into the tree, so `.els` stays a - single file. +- Persistence is free: `Script` code is already gzip'd into the tree. -### 2. Hook script descriptor (portable session ↔ extension) +### 7. Hook script descriptor (portable session ↔ extension) ```lua --- Session hooks example. @@ -71,90 +273,195 @@ a `.element` extension" without edits. -- @type Hook return { type = 'Hook', - requires = { 'engine' }, -- capabilities this script needs - attach = function (hooks, ctx) -- called once when the script is activated + requires = { 'session' }, -- capabilities this script needs + attach = function (hooks, ctx) -- called once when the script is activated hooks.action ('session.loaded', function() ... end) - hooks.action ('node.added', function (node) ... end, 20) -- priority - hooks.filter ('node.displayName', function (name, node) + hooks.action ('node.added', function (e) ... end, 20) -- priority + hooks.filter ('graph.defaultName', function (name, e) return name .. ' *' end) + if ctx.session then ctx.session():addGraph ('Auto') end end, - detach = function() ... end, -- optional cleanup + detach = function() ... end, -- optional cleanup } ``` -`ctx` is the capability table: only granted entries are present (`ctx.engine`, -`ctx.context`, ...). Scripts that got nothing still get `hooks` — pure observers that, -e.g., filter cosmetic values, need no grants at all. +`ctx` is the capability table: only granted entries are present. Scripts that got nothing +still get `hooks` — pure observers and cosmetic filters need no grants. + +### 8. Restricted environment and capabilities + +`ScriptingEngine::createRestrictedEnvironment()` returns a `sol::environment` with **no** +globals fallback, populated with: + +- Base: `assert error ipairs next pairs pcall select tonumber tostring type unpack + xpcall rawequal rawget rawset setmetatable getmetatable` and the tables `string`, + `table`, `math`, `utf8`; `os` limited to `time clock date difftime`. No `io`, no + `debug`, no `load`/`loadstring`/`dofile`/`loadfile`. +- `require` replaced by a function that only resolves an allowlist: + `el.bytes`, `el.midi`, `el.strings`, `el.color`, `el.object`, `el.hooks`, + `el.MidiMessage`, `el.MidiBuffer`, `el.AudioBuffer`. + +Capabilities → `ctx` keys, granted per script from its `requires`: + +| `requires` entry | `ctx` key | What it injects | +|---|---|---| +| `session` | `ctx.session` | function returning the live `el.Session` (carries mutation) | +| `engine` | `ctx.engine` | alias of `session` for readability; reserved for future engine-only calls | +| `ui` | `ctx.ui` | `el.Content` + `el.Commands.instance()` | +| `io` | `ctx.io` | the real `io` table | + +The same restricted env (with the widget modules `el.View`, `el.Widget`, `el.Slider`, +`el.TextButton`, `el.Graphics`, `el.Bounds`… added to the allowlist) is applied in +`ScriptView::setScript` via `loader.call (env)`, closing the "View scripts run in raw +globals" hole. + +**Compatibility.** View scripts embedded in existing user sessions run with full globals +today, so enforcing the allowlist is a behaviour change that can break a saved session +silently. Before flipping it: audit what the shipped and example View scripts +(`scripts/*.lua`, `docs/`, test snippets) actually `require` and which globals they +touch; then ship one release where `ScriptView` runs the script in the restricted env +with a *reporting* `require`/`__index` that logs each disallowed access to +`ScriptingEngine::logError` but still resolves it; enforce only after that log is quiet. +Anything a View script legitimately needs (e.g. `el.Context` read access) is added to +the allowlist rather than worked around. + +### 9. Trust + +- `Settings` gains `sessionScriptTrust`: map of *scripts-subtree SHA1* → `allow | deny`. + Hashing only the `scripts` subtree invalidates trust on any edit and ignores unrelated + session changes. +- On `session.loaded`, if any `Hook` script has a non-empty `requires` and the hash is + unknown: asynchronous `AlertWindow` — "This session contains scripts that want: session + access. **Run** / **Always for this session** / **Don't run**". *Run* grants for this + process only; *Always* persists `allow`; *Don't run* persists `deny`. Scripts with + empty `requires` attach immediately with no prompt. +- A "Re-ask trust" action clears the entry for the current hash. + +### 10. Runner: `ScriptingService` (`src/services/scriptingservice.hpp/.cpp`) + +New `Service`, registered in the `Services` ctor after `EngineService` and before +`SessionService`. Per the teardown rule, `activate()` captures `HookBus*`, +`ScriptingEngine*`, `SessionService*`, `Settings*` and never calls `context()` in the +destructor. -### 3. `HookBus` (C++, `src/scripting/hookbus.hpp/.cpp`) +- `activate()`: subscribe `session.loaded` → `attachAll()`; `session.closed` → `detachAll()`; + run `~/Music/Element/Scripts/init.lua` if present (owner `"user"`, globals-fallback env, + no trust prompt — user-installed); fire `app.started` when `Services::launch` completes. +- `attachAll()`: for each `Hook` script in `session->scripts()`: restricted env → + protected run → descriptor table → check `requires` against grants (prompt if needed) + → `attach (hooks_for_owner, ctx)`. Owner id = the script's tree UUID. +- `detachAll()` / reload: call `detach` (protected), `HookBus::removeOwner (uuid)`, drop + the env. +- `reattach (Script)`: called by `ScriptEditorView` after saving a hook script (detach → + re-run), so edits take effect without reloading the session. +- `deactivate()`: `detachAll()`, fire `app.shutdown`, unsubscribe. -Same class the extensions plan specifies (its Phase 3 shrinks to "wire more signals"): +### 11. UI (minimal) -- `addAction (event, sol::protected_function, priority, owner)` / `addFilter (...)` → - `Handle`; `remove (Handle)`; `removeOwner (ownerId)`. -- `dispatchAction (event, pusher)` and `applyFilters (event, initialValue, pusher)`. -- Message-thread only (assert); dispatch iterates a copy (handlers may add/remove); - reentrancy guard — nested dispatch queues and drains after; a handler is auto-disabled - after 3 consecutive errors; every call is protected, errors go to - `ScriptingEngine::logError`, never propagate. +- Session panel (`src/ui/sessiontreepanel.cpp`, where node scripts are listed around + `:507` and the `TODO enable script types at the graph level` at `:822`): list session + scripts, add (`Script::make (types::Hook)`) / remove, open in the existing + `ScriptEditorView`. +- Status line "scripts: N attached, M blocked" with the "Re-ask trust" action. -Initial events (actions): `session.loaded`, `session.saving`, `graph.added`, -`graph.removed`, `graph.changed`, `node.added`, `node.removed`. Initial filters: start -with one or two proving the shape (e.g. `node.displayName`) — filters are the part to -grow cautiously. Signal sources: `SessionService::sigSessionLoaded` / `sigWillSave` -(`src/services/sessionservice.hpp:37-38`), `EngineService::sigNodeRemoved`, plus new -`sigNodeAdded` / `sigGraphAdded` / `sigGraphRemoved` emitted at the same sites in -`src/services/engineservice.cpp`. +### 12. Dead-code cleanup (same PR series) -### 4. `el.hooks` Lua module — Lua-first +Delete: `ScriptingEngine::L`; `State::builtins` and `EL_LUA_SPATH`; +`Impl::scanDefaultLoctaion`; `scripts/commands.lua`; +the orphaned `test/snippets/sol3_parent.lua` and `stream_from_c.lua`. **Keep** +`addPackage`, `State::packages` and `resolve_internal_package`: the searcher is live +(position 3 of `package.searchers`, see the audit § 1) and [extensions.md](extensions.md) +Phase 1 registers extension modules through it; delete only `State::builtins` and the +commented-out `fill_builtins`. Register `el.vector` or delete `vector.c`. Fix the missing +comma in `widget.hpp` `__props`. Make `el/session.lua` resolve the session per call. +Restore the real body of `DSPScript::validate` (currently `#if 0`, returns `ok()` for +any non-empty string) or delete the method and its callers. Leave `ScriptManager` with a +comment that it is test-only until extensions land. Replace the hard-coded `== 13` in +`ScriptManagerTest` with a lower bound. **Keep** `ScriptInstance` and `DSPUIScript` (base and +placeholder for the script-type hierarchy: DSP, DSPUI, View, Hook…) and +`ScriptSource`/`ValueTreeScriptSource` (where a script's code comes from; +`ValueTreeScriptSource` is what session `Hook` scripts in §6 read their code through). -Per principle 3: a small C binding exposing an opaque bus handle + `register/unregister/ -emit` primitives (`src/el/Hooks.cpp`), wrapped by `src/el/hooks.lua` providing the -friendly `action`/`filter`/`off` API, priority defaults, and owner scoping. The `hooks` -object passed to `attach` is a Lua table closing over the owner id — no usertype needed. +## Order of work -### 5. Runner & lifecycle (`ScriptingService`, new — shared with extensions plan) +Each step builds and passes `ctest` before the next. Steps 1 and 2 depend on nothing in +[session-proxy.md](session-proxy.md) and land first: they fix what the audit found broken +and give a working REPL for exercising every later step by hand. -- On `sigSessionLoaded`: read the session `scripts` tree, for each `Hook` script: - restricted env → run → descriptor table → check `requires` vs grants → prompt if - needed → call `attach (hooks, ctx)`. Owner id = script's tree UUID. -- On session unload/reload/close: call `detach` (protected), `HookBus::removeOwner`, - drop envs. Also handles in-session edits: saving a hook script in the editor - re-attaches it (detach → re-run). -- `app.*` events and extension ownership come later (extensions plan); the bus API is - already owner-based so nothing changes shape. +1. Dead-code cleanup (§12) + console foundation (§3) + `el.command` fix; `ConsoleTests`. + Touches `src/scripting*`, `src/ui/luaconsole*`, `src/el/`, `scripts/` only — not + `EngineService`. +2. `HookBus` + `HookBusTests` (no callers yet). +3. `GraphManager` stops showing alerts: failures return an invalid `Node`/`false`, the + alerts move to `EngineService`. Prerequisite for headless controller tests. +4. The headless "real graphs through `attach` + `setRootNode`" test first (the untested + path in [session-proxy.md](session-proxy.md) § Risks — a modal alert on that path + hangs the runner, so prove it before moving code). Then `GraphController` + + `EngineService` forwards in one PR so all callers keep compiling; + `GuiService`/`GraphEditorView` hook handlers; `session.*` events; + `GraphControllerTests`. +5. Lua surface: `el.Session`/`el.Graph` verbs, `el.hooks`; `SessionLuaTests`. +6. View-script `require` audit and the reporting pass (§8 *Compatibility*), then session + `scripts` tree, `types::Hook`, restricted env, trust, `ScriptingService`, panel UI; + `HookScriptTests`. -### 6. UI (minimal) +Format with `util/format.py`. Headers under `src/` first; `include/element/` only for +the public model API (`hooks.hpp`, `session.hpp`, `graph.hpp`, `tags.hpp`). -- Session properties / session panel: list session scripts, add/remove, open in the - existing `ScriptEditorView` (`src/ui/scripteditorview.cpp`). -- A "scripts blocked / granted" indicator with a way to re-open the trust prompt. +## Testing (Boost.Test) -## Testing (Boost.Test, `test/scripting/`) +Register every suite in `test/CMakeLists.txt` with +`add_test (NAME "" COMMAND test_element --run_test=)`. -- Session scripts tree round-trip (add → save XML → load → scripts intact). -- HookBus: priority order, filter value threading, auto-disable after errors, - removeOwner, reentrancy (action handler emitting another action queues, no recursion). -- Runner: descriptor with `requires` not granted → `attach` never called, no error spam; - granted → ctx contains exactly the granted capabilities. -- Restricted env: `io`/`os.execute`/raw `require` unavailable inside a hook script. -- Register suites in `test/CMakeLists.txt` with `add_test`. +- `HookBusTests` (`test/HookBusTests.cpp`): priority order; filter value threading; + `remove`/`removeOwner`; auto-disable after 3 errors and re-enable after success; + nested `doAction` queued not recursed; `ScopedSuspend`; off-thread `doAction` asserts. +- `GraphControllerTests` — see [session-proxy.md](session-proxy.md). +- `ConsoleTests` (`test/scripting/consoletests.cpp`): `execute ("1+1")` prints `2`; + `execute ("x = 5")` then `execute ("x")` prints `5` in the persistent env; a syntax + error yields a failed `Result` whose message contains a traceback line; prelude loads + from binary data and defines `hooks`, `session`, `console`. +- `SessionLuaTests` (`test/scripting/sessionluatest.cpp`, snippet + `test/snippets/session_mutate.lua`): state initialised with `test::context()`; + `session():addGraph ('lua')`, `g:addNode ('element.volume')`, `hooks.action + ('node.removed', …)`, `g:removeNode (n)` → handler ran once; `g:addNode ('bogus')` + returns `nil, msg`; with services deactivated, `session():addGraph ('x')` returns + `nil, "engine not running"` and the tree is unchanged. +- `HookScriptTests` (`test/scripting/hookscripttests.cpp`): session `scripts` tree + round-trips through XML; descriptor with `requires = {'session'}` not granted → + `attach` never called and no error spam; granted → `ctx.session` present and nothing + else; restricted env has no `io`, `os.execute`, `load`, or `require ('el.Context')`; + `detach` + `removeOwner` leave no handlers; `reattach` after an edit replaces them. +- Manual: View → Console; `session():addGraph ("Test")`; + `hooks.action ('node.added', function (e) print (e.node.name) end)` then add a plugin + from the UI → name printed; undo/redo the add → exactly one `node.removed` then one + `node.added`; add a `Hook` script to the session, save, reopen → trust prompt, then + `attach` runs. ## Relationship to the extensions plan -- HookBus, `el.hooks`, restricted-env + capability machinery, and `ScriptingService` - move **here** (Phase 0). [extensions.md](extensions.md) Phase 3 reduces to wiring - `app.started`/`app.shutdown` and extension-owned registration; extension entry scripts - reuse the same descriptor/capability conventions with a friendlier default grant. +- `HookBus`, `el.hooks`, the restricted-env + capability machinery, `ScriptingService` + and the mutation verbs all land here. [extensions.md](extensions.md) Phase 2 becomes + "reuse the `el.Session`/`el.Graph` bindings"; Phase 3 reduces to wiring extension ownership and a + friendlier default grant; extension entry scripts reuse the same descriptor and + capability conventions. - The capability model is also the answer to "extensions are trusted-but-isolated" — one mechanism, two default policies. ## Open questions -1. Trust persistence key: content hash (safer, invalidates on edit) vs file path - (friendlier). Suggest hash of the scripts subtree only. -2. Do hook scripts on *nodes/graphs* (not just the session root) run too? Suggest: yes - eventually (graph-scoped hooks fire only while that graph is active), but session-root - only in Phase 0. -3. Filter catalog — which host values are worth filtering first. +1. Trust key: content hash of the `scripts` subtree (chosen) vs file path. Revisit if the + prompt is too frequent in practice. +2. Hook scripts on *graphs* (not just the session root): yes eventually, graph-scoped + hooks firing only while that graph is active; session-root only in Phase 0. +3. Filter catalogue beyond `graph.defaultName`: candidates are `session.saving` payload + and `node.displayName`, the latter only once a non-paint call site is identified. +4. Undoable Lua mutations: needs `AppMessage`-based routing or an explicit + `UndoManager` transaction API on the controller. Out of scope for Phase 0. +5. Engine-initiated active-graph changes (MIDI program change, + [audioengine.cpp:657-665](../../src/engine/audioengine.cpp#L657), writes + `tags::active` from an async update) bypass the controller, so `graph.activated` does + **not** fire for them in Phase 0. Document it as such in the hook catalogue. Fix is a + `ValueTree::Listener` on the `graphs` child inside `GraphController` with a self-change + flag; follow-up after step 4. diff --git a/scripts/commands.lua b/scripts/commands.lua deleted file mode 100644 index e59d6bbea..000000000 --- a/scripts/commands.lua +++ /dev/null @@ -1,51 +0,0 @@ ---- List commands registered in the command manager. --- @script commands --- @type anonymous --- @license GPLv3 --- @author Michael Fisher --- @usage --- > script.exec ('commands') --- SHOW_ABOUT = 256 --- SHOW_ALL_PLUGIN_WINDOWS = 265 --- SHOW_SESSION_CONFIG = 260 --- RECENTS_CLEAR = 4096 --- GRAPH_OPEN = 1793 --- GRAPH_SAVE_AS = 1795 --- UNDO = 4104 --- ... - -local CMD = require ('el.CommandManager') -local strings = require ('el.strings') -local format = select (1, ...) or 'constants' - -if not strings.isslug (format) then - error ("Invalid format: " .. tostring (format)) -end - -local list = { - -- List as constants used with the el.command module - constants = function (cmds) - for k,v in pairs (cmds) do - local out = string.upper (strings.tosnake (k)) - out = out .. " = " .. tostring(v) - print (out) - end - end -} - -local cmds = {} - --- add standard commands -for _, cmd in ipairs (CMD.standard()) do - local str = CMD.tostring (cmd) - if string.len(str) > 0 then - cmds[str] = cmd - end -end - -if type(list[format]) == 'function' then - list[format] (cmds) -end - --- SPDX-FileCopyrightText: Copyright (C) Kushview, LLC. --- SPDX-License-Identifier: GPL-3.0-or-later diff --git a/scripts/console.lua b/scripts/console.lua index 5c98a6fcb..e5340a3aa 100644 --- a/scripts/console.lua +++ b/scripts/console.lua @@ -1,29 +1,32 @@ --- Console init. --- Runs when the console is loaded in the GUI. This anonymous script sets global --- variables, so be careful if you use it directly. +-- Runs once in the persistent console environment when the console is first +-- opened. It sets globals in that environment, so be careful if you use it +-- directly. -- @script console -- @pragma nostrip -- @type Anonymous -io = require ('io') object = require ('el.object') command = require ('el.command') script = require ('el.script') +Context = require ('el.Context') + +--- Returns the active session. +-- A function rather than a value: the session object is replaced when a new +-- session is loaded, so never cache the result. +-- @function session +-- @treturn el.Session +function session() + return Context.instance():session() +end console = { - --- Log to stdout. - -- Calls `tostring` on each argument then prints the combined result to - -- stdout + --- Log to the console. + -- Calls `tostring` on each argument then prints the combined result. -- @function console.log -- @param ... Things to log log = function (...) - local out = "" - for i = 1, select ('#', ...) do - out = out .. tostring (select (i, ...)) .. "\t" - end - if string.len(out) > 0 then - io.stdout:write (out .. "\n") - end + print (...) end } diff --git a/src/el/Context.cpp b/src/el/Context.cpp index 4b472636d..e6457cb7e 100644 --- a/src/el/Context.cpp +++ b/src/el/Context.cpp @@ -8,6 +8,8 @@ #include #include +#include +#include #include #include #include @@ -49,6 +51,14 @@ EL_PLUGIN_EXPORT int luaopen_el_Context (lua_State* L) // @within Instance Methods "session", &Context::session, + /// Returns the application's command manager. + // @function Context:commands + // @treturn el.Commands + // @within Instance Methods + "commands", [] (Context& ctx) -> element::Commands& { + return ctx.services().find()->commands(); + }, + "audio", &Context::audio, "devices", &Context::devices, "mapping", &Context::mapping, @@ -58,6 +68,7 @@ EL_PLUGIN_EXPORT int luaopen_el_Context (lua_State* L) "settings", &Context::settings); lua.script (R"( + require ('el.Commands') require ('el.Node') require ('el.Session') )"); diff --git a/src/el/command.lua b/src/el/command.lua index ea1e9827d..871558855 100644 --- a/src/el/command.lua +++ b/src/el/command.lua @@ -64,7 +64,7 @@ end for _,cmd in ipairs (Commands.standard()) do local strings = require ('el.strings') local s = Commands.toString (cmd) - if string.len(s) > 0 and strings.valid(s) then + if string.len(s) > 0 and strings.issymbol(s) then local k = strings.tosnake (s) M[string.upper (k)] = cmd end diff --git a/src/el/script.lua b/src/el/script.lua index 9d1706c27..f5a0b3e25 100644 --- a/src/el/script.lua +++ b/src/el/script.lua @@ -26,10 +26,11 @@ end -- @tparam table env The environment to use or _ENV -- @tparam any ... Arguments passed to script -- @treturn any Return value from script or no value +-- @raise When the script cannot be found or compiled -- @usage script.exec ('scriptname') function M.exec (path, env, ...) local invoke, err = M.load (path, env) - if err then return err end + if not invoke then error (err, 2) end return invoke (...) end diff --git a/src/el/session.lua b/src/el/session.lua index 4752bad86..82f2d88e1 100644 --- a/src/el/session.lua +++ b/src/el/session.lua @@ -9,10 +9,12 @@ local Node = require ('el.Node') local Graph = require ('el.Graph') local M = {} -local session = Context.instance():session() -function M.toxmlstring() return session:toXmlString() end -function M.name() return session.name end +-- Never cache the session: it is replaced when a new session is loaded. +local function session() return Context.instance():session() end + +function M.toxmlstring() return session():toXmlString() end +function M.name() return session().name end return M diff --git a/src/el/vector.c b/src/el/vector.c deleted file mode 100644 index dde0f0528..000000000 --- a/src/el/vector.c +++ /dev/null @@ -1,198 +0,0 @@ -// Copyright 2023 Kushview, LLC -// SPDX-License-Identifier: GPL-3.0-or-later - -#if 0 - -/// A vector of `kv_sample_t`'s suitable for realtime -// @module el.vector -#include -#include -#include "luainc.h" -#include "vector.h" -#include "util.h" -#include "element/element.h" - -struct kv_vector_impl_t { - kv_sample_t* values; - lua_Integer size; - lua_Integer used; -}; - -typedef struct kv_vector_impl_t Vector; - -//============================================================================= -kv_vector_t* kv_vector_new (lua_State* L, int size) { - Vector* vec = lua_newuserdata (L, sizeof (Vector)); - luaL_setmetatable (L, EL_MT_VECTOR); - - if (size > 0) { - vec->size = vec->used = size; - vec->values = malloc (sizeof (kv_sample_t) * size); - memset (vec->values, 0, sizeof (kv_sample_t) * size); - } else { - vec->size = vec->used = 0; - vec->values = NULL; - } - - return vec; -} - -static void kv_vector_free_values (kv_vector_t* vec) { - if (vec->values != NULL) { - free (vec->values); - vec->values = NULL; - } - vec->size = vec->used = 0; -} - -size_t kv_vector_size (kv_vector_t* vec) { - return (size_t) vec->used; -} - -size_t kv_vector_capacity (kv_vector_t* vec) { - return (size_t) vec->size; -} - -kv_sample_t* kv_vector_values (kv_vector_t* vec) { - return vec->values; -} - -kv_sample_t kv_vector_get (kv_vector_t* vec, int index) { - return vec->values [index]; -} - -void kv_vector_set (kv_vector_t* vec, int index, kv_sample_t value) { - vec->values [index] = value; -} - -void kv_vector_clear (kv_vector_t* vec) { - if (vec->used > 0) { - memset (vec->values, 0, sizeof(kv_sample_t) * vec->used); - } -} - -void kv_vector_resize (kv_vector_t* vec, int size) { - size = MAX (0, size); - if (size <= vec->size) { - vec->used = size; - } else { - vec->used = vec->size = size; - vec->values = realloc (vec->values, sizeof(kv_sample_t) * vec->size); - } -} - -static int vector_gc (lua_State* L) { - kv_vector_free_values (lua_touserdata (L, 1)); - return 0; -} - -static int vector_len (lua_State* L) { - Vector* vec = lua_touserdata (L, 1); - lua_pushinteger (L, vec->size); - return 1; -} - -static int vector_index (lua_State* L) { - Vector* vec = lua_touserdata (L, 1); - lua_Integer i = lua_tointeger (L, 2) - 1; - if (i >= 0 && i < vec->used) { - lua_pushnumber (L, vec->values[i]); - } else { - lua_pushnil (L); - } - return 1; -} - -static int vector_newindex (lua_State* L) { - Vector* vec = lua_touserdata (L, 1); - lua_Integer i = lua_tointeger (L, 2) - 1; - lua_Number v = lua_tonumber (L, 3); - if (i >= 0 && i < vec->used) { - vec->values[i] = v; - } - return 0; -} - -static int vector_tostring (lua_State* L) { - Vector* vec = lua_touserdata (L, 1); - lua_pushfstring (L, "Vector: size=%d capacity=%d", vec->used, vec->size); - return 1; -} - -static const luaL_Reg vector_m[] = { - { "__gc", vector_gc }, - { "__len", vector_len }, - { "__index", vector_index }, - { "__newindex", vector_newindex }, - { "__tostring", vector_tostring }, - { NULL, NULL } -}; - -//============================================================================= - -/// Creates a new vector -// @param size Number of elements -// @function new -static int f_new (lua_State* L) { - if (NULL == kv_vector_new (L, MAX (0, lua_tointeger (L, 1)))) - lua_pushnil (L); - return 1; -} - -/// Clears a vector -// @param vec The vector to clear -// @function clear -static int f_clear (lua_State* L) { - kv_vector_t* vec = luaL_checkudata (L, 1, EL_MT_VECTOR); - if (vec->used <= 0 || vec->values == NULL) { - return 0; - } - - switch (lua_gettop (L)) { - default: { - memset (vec->values, 0, sizeof(kv_sample_t) * vec->used); - break; - } - } - - return 0; -} - -/// Reserve a number of elements -// @param vec The vector to operate on -// @param size Number of elements to allocate memory for -// @function reserve -static int f_reserve (lua_State* L) { - return 0; -} - -/// Resize a vector -// @param vec The vector to clear -// @param size The new number of elements to allocate -// @function resize -static int f_resize (lua_State* L) { - return 0; -} - -static const luaL_Reg vector_f[] = { - { "new", f_new }, - { "clear", f_clear }, - { "reserve", f_reserve }, - { "resize", f_resize }, - { NULL, NULL } -}; - -void kv_vector_metatable (lua_State* L) { - if (0 != luaL_newmetatable (L, EL_MT_VECTOR)) { - luaL_setfuncs (L, vector_m, 0); - } -} - -EL_PLUGIN_EXPORT -int luaopen_el_vector (lua_State* L) { - kv_vector_metatable (L); - luaL_newlib (L, vector_f); - return 1; -} - -#endif diff --git a/src/el/widget.hpp b/src/el/widget.hpp index 19a2587a2..c51fce83b 100644 --- a/src/el/widget.hpp +++ b/src/el/widget.hpp @@ -366,7 +366,7 @@ inline static sol::table defineWidget (lua_State* L, const char* name, Args&&... "y", "width", "height", - "visible" + "visible", "opaque"); T_mt["__methods"] = view.create_table().add ( diff --git a/src/scripting.cpp b/src/scripting.cpp index 6fdac8221..d7bca6b61 100644 --- a/src/scripting.cpp +++ b/src/scripting.cpp @@ -7,10 +7,6 @@ #include "scripting/bindings.hpp" #include -#ifndef EL_LUA_SPATH -#define EL_LUA_SPATH "" -#endif - namespace element { //============================================================================= @@ -29,6 +25,7 @@ class ScriptingEngine::State ~State() { + consoleEnv = sol::environment(); auto& g = state.globals(); g.set (refkey, sol::lua_nil); } @@ -47,8 +44,10 @@ class ScriptingEngine::State friend class ScriptingEngine; ScriptingEngine& owner; sol::state state; - element::lua::PackageLoaderMap builtins; element::lua::PackageLoaderMap packages; + // Declared after `state` so it is released before the state is closed. + sol::environment consoleEnv; + juce::StringArray consoleHistory; /** global table key to state reference */ static constexpr const char* refkey = "__state"; @@ -58,15 +57,13 @@ class ScriptingEngine::State return view.globals()[refkey]; } + /** Inserts the runtime package searcher after Lua's preload searcher. + `Lua::initializeState` later inserts the internal-module searcher ahead + of it, so both stay active. */ static void init_packages (lua_State* L) { sol::state_view view (L); view.open_libraries (sol::lib::package); -#if 0 - // doing this destroys preloading capabilities in lua. - view.clear_package_loaders(); - view.add_package_loader (resolve_internal_package); -#else auto package = view["package"]; const char* skey = LUA_VERSION_NUM < 502 ? "loaders" : "searchers"; @@ -77,29 +74,17 @@ class ScriptingEngine::State for (size_t i = 2; i <= orig_searchers.size(); ++i) // add everything after (file searchers) new_searchers.add (package[skey][i]); // .. package[skey] = new_searchers; // replace them -#endif } - /** Custom lua searcher handler */ + /** Searcher for packages registered at runtime via `addPackage`. */ static int resolve_internal_package (lua_State* L) { sol::state_view view (L); auto& state = getref (view); - // if (state.builtins.empty()) - // element::lua::fill_builtins (state.builtins); - const auto mid = sol::stack::get (L); - auto it = state.builtins.find (mid); - auto end = state.builtins.end(); - std::string msgkey = "builtins"; - if (it == end) - { - it = state.packages.find (mid); - end = state.packages.end(); - msgkey = "packages"; - } - if (it != end) + auto it = state.packages.find (mid); + if (it != state.packages.end()) { if (nullptr != it->second) sol::stack::push (L, it->second); @@ -108,7 +93,7 @@ class ScriptingEngine::State } else { - lua_pushfstring (L, "\n\tno field %s['%s']", msgkey.c_str(), mid.c_str()); + lua_pushfstring (L, "\n\tno field packages['%s']", mid.c_str()); } return 1; @@ -128,8 +113,6 @@ void ScriptingEngine::addPackage (const std::string& name, element::lua::CFuncti std::vector ScriptingEngine::getPackageNames() const noexcept { std::vector v; - for (const auto& pkg : state->builtins) - v.push_back (pkg.first); for (const auto& pkg : state->packages) v.push_back (pkg.first); std::sort (v.begin(), v.end()); @@ -149,11 +132,6 @@ class ScriptingEngine::Impl ~Impl() {} - void scanDefaultLoctaion() - { - manager.scanDefaultLocation(); - } - ScriptManager& getManager() { return manager; } private: @@ -174,6 +152,7 @@ ScriptingEngine::~ScriptingEngine() { if (state != nullptr) { + state->consoleEnv = sol::environment(); Lua::clearGlobals (state->state); state->collectGarbage(); state.reset(); @@ -197,4 +176,88 @@ ScriptManager& ScriptingEngine::getScriptManager() return impl->manager; } +//============================================================================= +sol::environment& ScriptingEngine::consoleEnvironment() +{ + auto& env = state->consoleEnv; + if (! env.valid()) + { + sol::state_view lua (state->state); + env = sol::environment (lua, sol::create, lua.globals()); + } + return env; +} + +juce::StringArray& ScriptingEngine::consoleHistory() +{ + return state->consoleHistory; +} + +juce::Result ScriptingEngine::execute (const juce::String& code, + sol::environment env, + const juce::String& chunkName) +{ + JUCE_ASSERT_MESSAGE_THREAD + sol::state_view lua (state->state); + const auto chunk = "=" + chunkName.toStdString(); + + try + { + bool haveReturn = true; + juce::String buffer; + buffer << "return " << code << ";"; + auto loaded = lua.load_buffer (buffer.toRawUTF8(), buffer.getNumBytesAsUTF8(), chunk, sol::load_mode::text); + + if (! loaded.valid()) + { + haveReturn = false; + loaded = lua.load_buffer (code.toRawUTF8(), code.getNumBytesAsUTF8(), chunk, sol::load_mode::text); + } + + if (! loaded.valid()) + { + sol::error error = loaded; + return juce::Result::fail (error.what()); + } + + sol::protected_function fn = loaded; + if (env.valid()) + sol::set_environment (env, fn); + + sol::protected_function traceback = lua["debug"]["traceback"]; + if (traceback.valid()) + fn.set_error_handler (traceback); + + auto result = fn(); + if (! result.valid()) + { + sol::error error = result; + return juce::Result::fail (error.what()); + } + + if (haveReturn && result.return_count() > 0) + { + sol::protected_function print; + if (env.valid()) + print = env["print"]; + else + print = lua["print"]; + if (print.valid()) + { + auto printed = print (result); + if (! printed.valid()) + { + sol::error error = printed; + return juce::Result::fail (error.what()); + } + } + } + } catch (const std::exception& e) + { + return juce::Result::fail (e.what()); + } + + return juce::Result::ok(); +} + } // namespace element diff --git a/src/scripting.hpp b/src/scripting.hpp index cc79277d0..ecae2b562 100644 --- a/src/scripting.hpp +++ b/src/scripting.hpp @@ -25,7 +25,38 @@ class ScriptingEngine lua_State* getLuaState() const; //========================================================================== - juce::Result execute (const String& code); + /** Compiles and runs a chunk of Lua in the given environment. + + The code is first compiled as an expression (`return ;`). When + that succeeds and the chunk yields values, they are passed to the + environment's `print` function, so a console can echo results. When it + does not compile as an expression the code is run as a plain chunk. + + Runtime errors are caught with a `debug.traceback` handler and never + propagate. Must be called on the message thread. + + @param code The Lua source to run. + @param env The environment to run in. An invalid environment + falls back to the global table. + @param chunkName Name used in error messages and tracebacks. + @return ok on success, otherwise the error message (with traceback + for runtime errors). + */ + juce::Result execute (const juce::String& code, + sol::environment env = {}, + const juce::String& chunkName = "console"); + + /** Returns the persistent console environment. + + Created on first use with a fallback to the globals table, so reads + see everything in `_G` while writes stay local to the console. It + lives as long as the engine, so console state survives the console + view being closed and reopened. + */ + sol::environment& consoleEnvironment(); + + /** Returns the console's command history (persists with the engine). */ + juce::StringArray& consoleHistory(); std::vector getPackageNames() const noexcept; void addPackage (const std::string& name, lua::CFunction loader); @@ -37,7 +68,6 @@ class ScriptingEngine class Impl; std::unique_ptr impl; Context* world = nullptr; - lua_State* L = nullptr; class State; std::unique_ptr state; diff --git a/src/scripting/dspscript.cpp b/src/scripting/dspscript.cpp index 42a6f8e34..236a29bbb 100644 --- a/src/scripting/dspscript.cpp +++ b/src/scripting/dspscript.cpp @@ -1,6 +1,8 @@ // Copyright 2023 Kushview, LLC // SPDX-License-Identifier: GPL-3.0-or-later +#include + #include "el/factories.hpp" #include #include "scripting/dspscript.hpp" @@ -431,86 +433,107 @@ DSPScript::~DSPScript() deref(); } +template +bool DSPScript::callOptional (const char* name, Args&&... args) +{ + sol::protected_function f = DSP[name]; + if (! f.valid()) + return true; + + auto result = f (std::forward (args)...); + if (result.valid()) + return true; + + sol::error error = result; + lastError = error.what(); + std::clog << "[dsp script] " << name << ": " << lastError.toStdString() << std::endl; + return false; +} + +bool DSPScript::init() { return callOptional ("init"); } +bool DSPScript::prepare (double rate, int block) { return callOptional ("prepare", rate, block); } +bool DSPScript::release() { return callOptional ("release"); } + Result DSPScript::validate (const String& script) { if (script.isEmpty()) return Result::fail ("script contains no code"); - return Result::ok(); -#if 0 + sol::state state; element::Lua::initializeState (state); ScriptLoader loader (state.lua_state(), script); if (loader.hasError()) return Result::fail (loader.getErrorMessage()); - - auto ctx = std::make_unique (loader.call()); - if (! ctx->isValid()) - return Result::fail ("could not parse script"); - juce::Result result (juce::Result::fail ("Unknown script problem")); - + auto descriptor = loader.call(); + if (loader.hasError()) + return Result::fail (loader.getErrorMessage()); + if (! descriptor.valid() || descriptor.get_type() != sol::type::table) + return Result::fail ("script did not return a table"); + + auto dsp = std::make_unique (descriptor.as()); + if (! dsp->isValid()) + return Result::fail ("could not instantiate script"); + try { - const int block = 1024; const double rate = 44100.0; + const int block = 512; + const int cycles = 4; + + PortList ports; + dsp->getPorts (ports); + const int nchans = jmax (1, ports.size (PortType::Audio, true), ports.size (PortType::Audio, false)); + const int nmidi = jmax (ports.size (PortType::Midi, true), ports.size (PortType::Midi, false)); + + AudioSampleBuffer audio (nchans, block); + OwnedArray midiBuffers; + Array midiChannels; + for (int i = 0; i < nmidi; ++i) + { + midiBuffers.add (new MidiBuffer()); + midiChannels.add (i); + } + MidiPipe midi (midiBuffers, midiChannels); + + if (! dsp->init()) + return Result::fail ("init: " + dsp->getLastError()); + if (! dsp->prepare (rate, block)) + return Result::fail ("prepare: " + dsp->getLastError()); + + for (int cycle = 0; cycle < cycles; ++cycle) + { + // A quiet test tone so gain-style scripts have something to shape. + for (int c = 0; c < nchans; ++c) + for (int f = 0; f < block; ++f) + audio.setSample (c, f, 0.25f * std::sin (float (f) * 0.05f)); + + for (auto* buffer : midiBuffers) + { + buffer->clear(); + buffer->addEvent (MidiMessage::noteOn (1, 60, 0.8f), 0); + buffer->addEvent (MidiMessage::noteOff (1, 60), block / 2); + } + + dsp->process (audio, midi); + if (! dsp->isValid()) + return Result::fail ("process: " + dsp->getLastError()); + + for (int c = 0; c < audio.getNumChannels(); ++c) + for (int f = 0; f < block; ++f) + if (! std::isfinite (audio.getSample (c, f))) + return Result::fail ("process produced non-finite audio samples"); + } - using PT = PortType; - - // call node_io_ports() and node_params() - PortList validatePorts; - ctx->getPorts (validatePorts); - - // create a dummy audio buffer and midipipe - auto nchans = jmax (validatePorts.size (PT::Audio, true), - validatePorts.size (PT::Audio, false)); - auto nmidi = jmax (validatePorts.size (PT::Midi, true), - validatePorts.size (PT::Midi, false)); - - ctx->prepare (rate, block); - state["__ln_validate_rate"] = rate; - state["__ln_validate_nmidi"] = nmidi; - state["__ln_validate_nchans"] = nchans; - state["__ln_validate_nframes"] = block; - state.script (R"( - function __ln_validate_render() - local AudioBuffer = require ('el.AudioBuffer') - local MidiPipe = require ('el.MidiPipe') - - local a = AudioBuffer (__ln_validate_nchans, __ln_validate_nframes) - local m = MidiPipe (__ln_validate_nmidi) - - for _ = 1,4 do - for i = 0,m:size() - 1 do - local b = m:get(i) - b:insert (0, midi.noteon (1, 60, math.random (1, 127))) - b:insert (10, midi.noteoff (1, 60, 0)) - end - node_render (a, m) - a:clear() - m:clear() - end - - a = nil - m = nil - collectgarbage() - end - - __ln_validate_render() - __ln_validate_render = nil - collectgarbage() - )"); - - ctx->release(); - ctx.reset(); - result = Result::ok(); - } - catch (const std::exception& e) - { - result = Result::fail (e.what()); - } - return result; -#endif + if (! dsp->release()) + return Result::fail ("release: " + dsp->getLastError()); + } catch (const std::exception& e) + { + return Result::fail (e.what()); + } + + return Result::ok(); } void DSPScript::getPorts (PortList& out) @@ -524,48 +547,45 @@ void DSPScript::process (AudioSampleBuffer& a, MidiPipe& m) if (! loaded) return; - if (lua_rawgeti (L, LUA_REGISTRYINDEX, processRef) == LUA_TFUNCTION) + const int top = lua_gettop (L); + + if (lua_rawgeti (L, LUA_REGISTRYINDEX, processRef) == LUA_TFUNCTION + && lua_rawgeti (L, LUA_REGISTRYINDEX, audioRef) == LUA_TUSERDATA + && lua_rawgeti (L, LUA_REGISTRYINDEX, midiRef) == LUA_TUSERDATA + && lua_rawgeti (L, LUA_REGISTRYINDEX, paramsUserData.registry_index()) == LUA_TUSERDATA + && lua_rawgeti (L, LUA_REGISTRYINDEX, controlsUserData.registry_index()) == LUA_TUSERDATA + && lua_rawgeti (L, LUA_REGISTRYINDEX, positionRef) == LUA_TUSERDATA) { - if (lua_rawgeti (L, LUA_REGISTRYINDEX, audioRef) == LUA_TUSERDATA) + (*audio)->setDataToReferTo (a.getArrayOfWritePointers(), + a.getNumChannels(), + a.getNumSamples()); + (*midi)->swapWith (m); + + if (playhead != nullptr) + (*position)->update (playhead->getPosition()); + + // Lua is built as C: an error inside an unprotected lua_call has no + // handler to unwind to and aborts the process. A protected call keeps + // the audio thread alive; the script is disabled instead. The String + // assignment only happens on that error path. + if (lua_pcall (L, 5, 0, 0) != LUA_OK) { - if (lua_rawgeti (L, LUA_REGISTRYINDEX, midiRef) == LUA_TUSERDATA) - { - if (lua_rawgeti (L, LUA_REGISTRYINDEX, paramsUserData.registry_index()) == LUA_TUSERDATA) - { - if (lua_rawgeti (L, LUA_REGISTRYINDEX, controlsUserData.registry_index()) == LUA_TUSERDATA) - { - if (lua_rawgeti (L, LUA_REGISTRYINDEX, positionRef) == LUA_TUSERDATA) - { - (*audio)->setDataToReferTo (a.getArrayOfWritePointers(), - a.getNumChannels(), - a.getNumSamples()); - (*midi)->swapWith (m); - - if (playhead != nullptr) - (*position)->update (playhead->getPosition()); - - try - { - lua_call (L, 5, 0); - } catch (const sol::error& e) - { - std::clog << e.what() << std::endl; - loaded = false; - } - (*midi)->swapWith (m); - - for (int ci = outParams.size(); --ci >= 0;) - outParams.getUnchecked (ci)->update (controlData[ci]); - } - } - } - } + lastError = lua_tostring (L, -1); + std::clog << "[dsp script] process: " << lastError.toStdString() << std::endl; + loaded = false; } + + (*midi)->swapWith (m); + + for (int ci = outParams.size(); --ci >= 0;) + outParams.getUnchecked (ci)->update (controlData[ci]); } else { DBG ("didn't get render function in callback"); } + + lua_settop (L, top); } void DSPScript::save (MemoryBlock& out) diff --git a/src/scripting/dspscript.hpp b/src/scripting/dspscript.hpp index ee7c70d94..f31fc81a6 100644 --- a/src/scripting/dspscript.hpp +++ b/src/scripting/dspscript.hpp @@ -23,31 +23,38 @@ class DSPScript : public ScriptInstance DSPScript (sol::table tbl); ~DSPScript(); - void init() - { - if (sol::function f = DSP["init"]) - f(); - } - + /** Calls the script's optional `init` function. + @return false if the script raised an error (see getLastError) + */ + bool init(); + + /** Compiles and instantiates a DSP script in a scratch Lua state, then + runs a few cycles of prepare/process/release to catch runtime errors. + + @param script The Lua source of the script. + @return ok when the script loads, returns a descriptor table, and + renders without raising or producing non-finite audio. + */ static juce::Result validate (const juce::String& script); void setPlayHead (juce::AudioPlayHead* ph) noexcept { playhead = ph; } - /** Returns true if the script loaded ok */ + /** Returns true if the script loaded ok and has not failed in process */ bool isValid() const noexcept { return loaded; } + /** Returns the message of the last error raised by the script, if any. */ + juce::String getLastError() const { return lastError; } + //========================================================================== - void prepare (double rate, int block) - { - if (sol::function f = DSP["prepare"]) - f (rate, block); - } + /** Calls the script's optional `prepare` function. + @return false if the script raised an error (see getLastError) + */ + bool prepare (double rate, int block); - void release() - { - if (sol::function f = DSP["release"]) - f(); - } + /** Calls the script's optional `release` function. + @return false if the script raised an error (see getLastError) + */ + bool release(); //========================================================================== void process (juce::AudioSampleBuffer& a, element::MidiPipe& m); @@ -83,6 +90,7 @@ class DSPScript : public ScriptInstance lua_State* L = nullptr; bool loaded = false; + juce::String lastError; int numParams = 0, // input params numControls = 0; // output params enum @@ -109,6 +117,9 @@ class DSPScript : public ScriptInstance void addParameterPorts(); void unlinkParams(); void setParameter (int, float, bool); + + template + bool callOptional (const char* name, Args&&... args); }; } // namespace element diff --git a/src/scripting/scriptmanager.hpp b/src/scripting/scriptmanager.hpp index 93854cd14..cccc7360c 100644 --- a/src/scripting/scriptmanager.hpp +++ b/src/scripting/scriptmanager.hpp @@ -9,6 +9,11 @@ namespace element { using ScriptArray = juce::Array; +/** Registry of scripts found on disk. + + Not used by the application yet: nothing scans at startup, so it is only + exercised by tests. Extensions will drive it (see docs/plans/extensions.md). +*/ class ScriptManager final { public: diff --git a/src/ui/console.cpp b/src/ui/console.cpp index d952aa241..652b1859a 100644 --- a/src/ui/console.cpp +++ b/src/ui/console.cpp @@ -127,6 +127,14 @@ class Console::Content : public Component historyPos = history.size(); } + const StringArray& getHistory() const { return history; } + + void setHistory (const StringArray& newHistory) + { + history = newHistory; + historyPos = history.size(); + } + void addText (const String& text, bool prefix) { String line = prefix ? prefixText : String(); @@ -238,6 +246,16 @@ void Console::addText (const String& text, bool prefix) content->addText (text, prefix); } +const StringArray& Console::getHistory() const +{ + return content->getHistory(); +} + +void Console::setHistory (const StringArray& history) +{ + content->setHistory (history); +} + void Console::textEntered (const String& text) { addText (text, true); diff --git a/src/ui/console.hpp b/src/ui/console.hpp index bf86a89d2..65f737751 100644 --- a/src/ui/console.hpp +++ b/src/ui/console.hpp @@ -28,6 +28,12 @@ class Console : public juce::Component /** Show or hide the text prompt */ void setPromptVisible (bool visible); + /** Returns the command history, oldest first. */ + const StringArray& getHistory() const; + + /** Replaces the command history. */ + void setHistory (const StringArray& history); + /** Override this to handle when text is entered on the prompt. The default implementation just adds entered text to the display buffer */ virtual void textEntered (const String& text); diff --git a/src/ui/luaconsole.cpp b/src/ui/luaconsole.cpp index fc5d87782..e9da0b34c 100644 --- a/src/ui/luaconsole.cpp +++ b/src/ui/luaconsole.cpp @@ -8,6 +8,7 @@ #include #include "sol/sol.hpp" +#include "luascripts.hpp" namespace element { using namespace juce; @@ -20,73 +21,63 @@ LuaConsole::LuaConsole() startTimer (200); } -LuaConsole::~LuaConsole() {} - -void LuaConsole::textEntered (const String& text) +LuaConsole::~LuaConsole() { - if (text.isEmpty() || ! env.valid()) - return; - Console::textEntered (text); - auto& e = env; - sol::state_view lua (e.lua_state()); - - auto gprint = lua["print"]; - lua["print"] = e["print"]; + if (engine != nullptr) + engine->consoleHistory() = getHistory(); - try + // Drop anything in the environment that captured `this`. The environment + // outlives this component; reads fall back to the globals. + if (env.valid()) { - bool haveReturn = true; - String buffer = "return "; - buffer << text << ";"; - { - auto loadResult = lua.load_buffer (buffer.toRawUTF8(), buffer.length()); - if (! loadResult.valid() || loadResult.status() != sol::load_status::ok) - { - haveReturn = false; - buffer = text; - } - } - - auto result = lua.script (buffer.toRawUTF8(), e, "console=", sol::load_mode::text); + env["print"] = sol::lua_nil; + env["clear"] = sol::lua_nil; + env["os"] = sol::lua_nil; + } +} - if (result.valid()) - { - if (haveReturn) - e["print"](result); - } - else - { - sol::error error = result; - for (const auto& line : StringArray::fromLines (error.what())) - addText (line); - } +void LuaConsole::initialize (ScriptingEngine& e) +{ + engine = &e; + env = engine->consoleEnvironment(); + setHistory (engine->consoleHistory()); + installEnvironment(); + runPrelude(); +} - if (lastError.isNotEmpty()) - addText (lastError); - } catch (const sol::error& e) - { - addText (e.what()); - } +void LuaConsole::textEntered (const String& text) +{ + if (text.isEmpty() || engine == nullptr || ! env.valid()) + return; + Console::textEntered (text); + addResultText (engine->execute (text, env)); +} - lua["print"] = gprint; - lastError.clear(); +void LuaConsole::addResultText (const juce::Result& result) +{ + if (result.wasOk()) + return; + for (const auto& line : StringArray::fromLines (result.getErrorMessage())) + addText (line); } -void LuaConsole::setEnvironment (const sol::environment& _env) +void LuaConsole::installEnvironment() { - env = _env; - auto& e = env; - jassert (e.valid()); - sol::state_view lua (e.lua_state()); + jassert (env.valid()); + sol::state_view lua (env.lua_state()); - e["os"]["exit"] = sol::overload ( + // A console-local `os` so overriding `exit` does not touch the global table. + sol::table os = lua.create_table(); + os[sol::metatable_key] = lua.create_table_with ("__index", lua.globals()["os"]); + os["exit"] = sol::overload ( [this]() { ViewHelpers::invokeDirectly (this, Commands::quit, true); }, [this] (int code) { JUCEApplication::getInstance()->setApplicationReturnValue (code); ViewHelpers::invokeDirectly (this, Commands::quit, true); }); + env["os"] = os; - e["clear"] = [this] (sol::variadic_args va) { + env["clear"] = [this] (sol::variadic_args va) { if (va.size() == 1 && va.get_type (0) == sol::type::boolean) { clear (va.get (0)); @@ -101,8 +92,8 @@ void LuaConsole::setEnvironment (const sol::environment& _env) } }; - e.set_function ("print", [this] (sol::variadic_args va) { - auto& e = env; + env.set_function ("print", [this] (sol::variadic_args va) { + sol::state_view state (va.lua_state()); String msg; for (auto v : va) { @@ -112,7 +103,7 @@ void LuaConsole::setEnvironment (const sol::environment& _env) continue; } - sol::function ts = e["tostring"]; + sol::function ts = state["tostring"]; if (ts.valid()) { sol::object str = ts ((sol::object) v); @@ -124,38 +115,39 @@ void LuaConsole::setEnvironment (const sol::environment& _env) if (msg.isNotEmpty()) { + const ScopedLock sl (printLock); printMessages.add (msg.trimEnd()); } }); +} - try - { - auto result = lua.safe_script ("require('el.script').exec('console', _ENV)", e); - if (! result.valid()) - { - sol::error error = result; - for (const auto& line : StringArray::fromLines (error.what())) - addText (line); - } - } catch (const sol::error& e) - { - addText (e.what()); - } +void LuaConsole::runPrelude() +{ + // The prelude defines `console`; its presence means it already ran in + // this (persistent) environment. + if (env["console"].valid()) + return; + const auto code = String::fromUTF8 (scripts::console_lua, scripts::console_luaSize); + addResultText (engine->execute (code, env, "console.lua")); } void LuaConsole::timerCallback() { - if (! printMessages.isEmpty()) + StringArray pending; { - const int block = jmax (1, printMessages.size() / 4); - const int count = jmin (block, printMessages.size()); - - if (count > 0) + const ScopedLock sl (printLock); + if (! printMessages.isEmpty()) { - addText (printMessages.joinIntoString ("\n", 0, count)); + const int block = jmax (1, printMessages.size() / 4); + const int count = jmin (block, printMessages.size()); + pending.addArray (printMessages, 0, count); printMessages.removeRange (0, count); } + } + if (! pending.isEmpty()) + { + addText (pending.joinIntoString ("\n")); startTimerHz (50); } else diff --git a/src/ui/luaconsole.hpp b/src/ui/luaconsole.hpp index dbc836f79..10a44713d 100644 --- a/src/ui/luaconsole.hpp +++ b/src/ui/luaconsole.hpp @@ -10,6 +10,9 @@ namespace element { +/** Interactive console that evaluates Lua in the engine's persistent + console environment. +*/ class LuaConsole : public Console, private juce::Timer { @@ -17,15 +20,25 @@ class LuaConsole : public Console, LuaConsole(); virtual ~LuaConsole(); + /** Binds the console to a scripting engine. + + Installs the console's `print`, `clear` and `os.exit` into the engine's + console environment and runs the console prelude the first time the + environment is used. Command history is restored from the engine. + */ + void initialize (ScriptingEngine& engine); + void textEntered (const String&) override; - void setEnvironment (const sol::environment& e); private: - using LuaResult = sol::protected_function_result; + ScriptingEngine* engine = nullptr; sol::environment env; - String lastError; + juce::CriticalSection printLock; StringArray printMessages; - LuaResult errorHandler (lua_State* L, LuaResult pfr); + + void installEnvironment(); + void runPrelude(); + void addResultText (const juce::Result&); friend class juce::Timer; void timerCallback() override; diff --git a/src/ui/luaconsoleview.cpp b/src/ui/luaconsoleview.cpp index 4779a1512..ac86e4e22 100644 --- a/src/ui/luaconsoleview.cpp +++ b/src/ui/luaconsoleview.cpp @@ -20,10 +20,7 @@ LuaConsoleView::~LuaConsoleView() void LuaConsoleView::initializeView (Services& app) { - auto& se = app.context().scripting(); - sol::state_view view (se.getLuaState()); - console.setEnvironment ( - sol::environment (view, sol::create, view.globals())); + console.initialize (app.context().scripting()); log = &app.context().logger(); log->addListener (this); @@ -34,4 +31,12 @@ void LuaConsoleView::initializeView (Services& app) console.addText (buffer.trimEnd(), false); } +void LuaConsoleView::messageLogged (const String& msg) +{ + juce::MessageManager::callAsync ([safe = juce::Component::SafePointer (this), msg]() { + if (safe != nullptr) + safe->console.addText (msg, false); + }); +} + } // namespace element diff --git a/src/ui/luaconsoleview.hpp b/src/ui/luaconsoleview.hpp index b37ad271c..d9147ddb0 100644 --- a/src/ui/luaconsoleview.hpp +++ b/src/ui/luaconsoleview.hpp @@ -7,7 +7,9 @@ #include "ui/luaconsole.hpp" #include "log.hpp" -#define EL_VIEW_CONSOLE "LuaConsoleViw" +#define EL_VIEW_CONSOLE "LuaConsoleView" +/** Misspelled view name persisted to settings by older builds. */ +#define EL_VIEW_CONSOLE_LEGACY "LuaConsoleViw" namespace element { @@ -37,10 +39,8 @@ class LuaConsoleView : public ContentView, console.setBounds (getLocalBounds().reduced (2)); } - void messageLogged (const String& msg) override - { - console.addText (msg, false); - } + /** Called from whichever thread logs the message, inside the Log's lock. */ + void messageLogged (const String& msg) override; private: LuaConsole console; diff --git a/src/ui/standard.cpp b/src/ui/standard.cpp index 1526aabec..e9d73bfa3 100644 --- a/src/ui/standard.cpp +++ b/src/ui/standard.cpp @@ -248,6 +248,8 @@ class ContentContainer : public Component lastSecondaryHeight = jmax (50, lastSecondaryHeight); showAccessoryView = props->getIntValue ("ContentContainer_showAccessoryView", showAccessoryView); auto lastSecondaryName = props->getValue ("ContentContainer_lastSecondaryView"); + if (lastSecondaryName.trim() == EL_VIEW_CONSOLE_LEGACY) + lastSecondaryName = EL_VIEW_CONSOLE; if (showAccessoryView) { owner.setSecondaryView (lastSecondaryName.trim()); @@ -1065,6 +1067,7 @@ void StandardContent::getCommandInfo (CommandID commandID, ApplicationCommandInf int flags = (showAccessoryView() && getAccessoryViewName() == EL_VIEW_CONSOLE) ? Info::isTicked : 0; + result.addDefaultKeypress (KeyPress::F3Key, 0); result.setInfo ("Console", "Show the scripting console", "UI", flags); break; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8655acaba..b87afcade 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -63,6 +63,7 @@ add_test(NAME "AudioRoutingTests" COMMAND test_element --run_test=AudioRoutingTe add_test(NAME "AuthTests" COMMAND test_element --run_test=AuthTests) add_test(NAME "BytesTest" COMMAND test_element --run_test=BytesTest) add_test(NAME "BusesLayoutTests" COMMAND test_element --run_test=BusesLayoutTests) +add_test(NAME "ConsoleTests" COMMAND test_element --run_test=ConsoleTests) add_test(NAME "JuceIntegrationTests" COMMAND test_element --run_test=JuceIntegrationTests) add_test(NAME "DataPathTests" COMMAND test_element --run_test=DataPathTests) add_test(NAME "DeviceMonitorTests" COMMAND test_element --run_test=DeviceMonitorTests) diff --git a/test/scripting/consoletests.cpp b/test/scripting/consoletests.cpp new file mode 100644 index 000000000..cfedd8a9f --- /dev/null +++ b/test/scripting/consoletests.cpp @@ -0,0 +1,149 @@ +// Copyright 2026 Kushview, LLC +// SPDX-License-Identifier: GPL-3.0-or-later + +#include + +#include + +#include "scripting.hpp" +#include "sol/sol.hpp" +#include "luascripts.hpp" +#include "testutil.hpp" + +using namespace element; +using namespace juce; +namespace et = element::test; + +namespace { + +/** A console environment whose `print` collects into a StringArray. */ +struct ConsoleFixture { + ConsoleFixture() + : engine (et::context()->scripting()), + env (engine.consoleEnvironment()) + { + env.set_function ("print", [this] (sol::variadic_args va) { + sol::state_view lua (va.lua_state()); + sol::function tostring = lua["tostring"]; + String line; + for (auto v : va) + line << tostring (v).get() << " "; + printed.add (line.trimEnd()); + }); + } + + ~ConsoleFixture() + { + env["print"] = sol::lua_nil; + } + + ScriptingEngine& engine; + sol::environment env; + StringArray printed; +}; + +} // namespace + +BOOST_AUTO_TEST_SUITE (ConsoleTests) + +BOOST_AUTO_TEST_CASE (ExpressionIsPrinted) +{ + ConsoleFixture fix; + auto result = fix.engine.execute ("1 + 1", fix.env); + BOOST_REQUIRE_MESSAGE (result.wasOk(), result.getErrorMessage().toStdString()); + BOOST_REQUIRE_EQUAL (fix.printed.size(), 1); + BOOST_REQUIRE_EQUAL (fix.printed[0].toStdString(), "2"); +} + +BOOST_AUTO_TEST_CASE (StatementIsNotPrinted) +{ + ConsoleFixture fix; + auto result = fix.engine.execute ("local a = 1", fix.env); + BOOST_REQUIRE (result.wasOk()); + BOOST_REQUIRE (fix.printed.isEmpty()); +} + +BOOST_AUTO_TEST_CASE (EnvironmentPersists) +{ + { + ConsoleFixture fix; + BOOST_REQUIRE (fix.engine.execute ("consoletests_x = 5", fix.env).wasOk()); + } + { + ConsoleFixture fix; + BOOST_REQUIRE (fix.engine.execute ("consoletests_x", fix.env).wasOk()); + BOOST_REQUIRE_EQUAL (fix.printed.size(), 1); + BOOST_REQUIRE_EQUAL (fix.printed[0].toStdString(), "5"); + } + + // Writes stay in the environment, not in the globals. + sol::state_view lua (et::context()->scripting().getLuaState()); + BOOST_REQUIRE (! lua["consoletests_x"].valid()); +} + +BOOST_AUTO_TEST_CASE (SyntaxErrorFails) +{ + ConsoleFixture fix; + auto result = fix.engine.execute ("local = ", fix.env); + BOOST_REQUIRE (result.failed()); + BOOST_REQUIRE (result.getErrorMessage().contains ("console")); + BOOST_REQUIRE (fix.printed.isEmpty()); +} + +BOOST_AUTO_TEST_CASE (RuntimeErrorHasTraceback) +{ + ConsoleFixture fix; + auto result = fix.engine.execute ("local function boom() error ('kaboom') end boom()", fix.env); + BOOST_REQUIRE (result.failed()); + const auto msg = result.getErrorMessage(); + BOOST_REQUIRE_MESSAGE (msg.contains ("kaboom"), msg.toStdString()); + BOOST_REQUIRE_MESSAGE (msg.contains ("stack traceback"), msg.toStdString()); +} + +BOOST_AUTO_TEST_CASE (ReturnCodeUsesEnvironmentPrint) +{ + ConsoleFixture fix; + BOOST_REQUIRE (fix.engine.execute ("print ('hello', 2)", fix.env).wasOk()); + BOOST_REQUIRE_EQUAL (fix.printed.size(), 1); + BOOST_REQUIRE_EQUAL (fix.printed[0].toStdString(), "hello 2"); +} + +BOOST_AUTO_TEST_CASE (PreludeLoadsFromBinaryData) +{ + ConsoleFixture fix; + const auto prelude = String::fromUTF8 (scripts::console_lua, scripts::console_luaSize); + auto result = fix.engine.execute (prelude, fix.env, "console.lua"); + BOOST_REQUIRE_MESSAGE (result.wasOk(), result.getErrorMessage().toStdString()); + + BOOST_REQUIRE (fix.env["console"].get_type() == sol::type::table); + BOOST_REQUIRE (fix.env["session"].get_type() == sol::type::function); + BOOST_REQUIRE (fix.env["Context"].get_type() == sol::type::table); + BOOST_REQUIRE (fix.env["command"].get_type() == sol::type::table); + // el.command generates SHOW_ABOUT etc. from Commands::toString. + BOOST_REQUIRE (fix.env["command"]["SHOW_ABOUT"].get_type() == sol::type::number); + + // The command manager is reached through el.Context, not a global. + BOOST_REQUIRE (fix.engine.execute ("Context.instance():commands() ~= nil", fix.env).wasOk()); + BOOST_REQUIRE_EQUAL (fix.printed.size(), 1); + BOOST_REQUIRE_EQUAL (fix.printed[0].toStdString(), "true"); + fix.printed.clear(); + + // console.log goes to the environment's print, not stdout. + BOOST_REQUIRE (fix.engine.execute ("console.log ('via console')", fix.env).wasOk()); + BOOST_REQUIRE_EQUAL (fix.printed.size(), 1); + BOOST_REQUIRE_EQUAL (fix.printed[0].toStdString(), "via console"); + + // session() resolves the live session through el.Context. + BOOST_REQUIRE (fix.engine.execute ("session().name", fix.env).wasOk()); + BOOST_REQUIRE_EQUAL (fix.printed.size(), 2); +} + +BOOST_AUTO_TEST_CASE (ScriptExecRaisesOnMissingScript) +{ + ConsoleFixture fix; + auto result = fix.engine.execute ("require ('el.script').exec ('does-not-exist', _ENV)", fix.env); + BOOST_REQUIRE (result.failed()); + BOOST_REQUIRE (fix.printed.isEmpty()); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/test/scripting/dspscripttest.cpp b/test/scripting/dspscripttest.cpp index 26e422a04..04a708e17 100644 --- a/test/scripting/dspscripttest.cpp +++ b/test/scripting/dspscripttest.cpp @@ -4,6 +4,7 @@ #include #include "luatest.hpp" +#include "luascripts.hpp" #include "scripting/dspscript.hpp" #include "scripting/scriptloader.hpp" #include "testutil.hpp" @@ -81,4 +82,87 @@ BOOST_AUTO_TEST_CASE (Basics) expect (Amp.get_or ("released", false) == true); } +static juce::String dspScriptWithProcess (const char* body) +{ + return juce::String (R"( +--- Validate fixture. +-- @script validate_fixture +-- @type DSP +local function layout() return { audio = { 2, 2 }, midi = { 1, 1 } } end +local function process (a, m, p, c, t) +)") + body + + R"( +end +return { type = 'DSP', layout = layout, process = process } +)"; +} + +BOOST_AUTO_TEST_CASE (ValidateShippedAmp) +{ + const auto amp = String::fromUTF8 (scripts::amp_lua, scripts::amp_luaSize); + auto result = DSPScript::validate (amp); + BOOST_REQUIRE_MESSAGE (result.wasOk(), result.getErrorMessage().toStdString()); +} + +BOOST_AUTO_TEST_CASE (ValidateRejectsEmpty) +{ + BOOST_REQUIRE (DSPScript::validate ("").failed()); +} + +BOOST_AUTO_TEST_CASE (ValidateRejectsSyntaxError) +{ + BOOST_REQUIRE (DSPScript::validate ("return {").failed()); +} + +BOOST_AUTO_TEST_CASE (ValidateRejectsNonTable) +{ + BOOST_REQUIRE (DSPScript::validate ("return 42").failed()); +} + +BOOST_AUTO_TEST_CASE (ValidateRendersMidiAndAudio) +{ + auto result = DSPScript::validate (dspScriptWithProcess (R"( + local buf = m:get (1) + assert (buf:size() > 0, 'expected midi events') + a:fade (1.0, 0.5) + )")); + BOOST_REQUIRE_MESSAGE (result.wasOk(), result.getErrorMessage().toStdString()); +} + +BOOST_AUTO_TEST_CASE (ValidateRejectsProcessError) +{ + auto result = DSPScript::validate (dspScriptWithProcess ("error ('process exploded')")); + BOOST_REQUIRE (result.failed()); + BOOST_REQUIRE_MESSAGE (result.getErrorMessage().contains ("process exploded"), + result.getErrorMessage().toStdString()); +} + +BOOST_AUTO_TEST_CASE (ValidateRejectsNonFiniteOutput) +{ + auto result = DSPScript::validate (dspScriptWithProcess ("a:fade (math.huge, math.huge)")); + BOOST_REQUIRE (result.failed()); + BOOST_REQUIRE_MESSAGE (result.getErrorMessage().contains ("non-finite"), + result.getErrorMessage().toStdString()); +} + +BOOST_AUTO_TEST_CASE (ProcessErrorDisablesScriptInsteadOfAborting) +{ + LuaFixture fix; + sol::state_view lua (fix.luaState()); + ScriptLoader loader (lua, dspScriptWithProcess ("error ('boom')")); + BOOST_REQUIRE_MESSAGE (! loader.hasError(), loader.getErrorMessage().toStdString()); + sol::table descriptor = loader.call(); + DSPScript dsp (descriptor); + BOOST_REQUIRE (dsp.isValid()); + + AudioSampleBuffer audio (2, 64); + MidiPipe midi; + dsp.process (audio, midi); + BOOST_REQUIRE (! dsp.isValid()); + BOOST_REQUIRE (dsp.getLastError().contains ("boom")); + + // A disabled script is a no-op afterwards, not a crash. + dsp.process (audio, midi); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/scripting/scriptmanagertest.cpp b/test/scripting/scriptmanagertest.cpp index 81c5917db..f6c9452e0 100644 --- a/test/scripting/scriptmanagertest.cpp +++ b/test/scripting/scriptmanagertest.cpp @@ -17,8 +17,9 @@ BOOST_AUTO_TEST_CASE (ScanDirectory) scripts.scanDirectory (d); // Counts every script that parses with a non-empty @type (ScriptInfo::valid), - // regardless of type — not just dsp/dspui. - BOOST_REQUIRE_EQUAL (scripts.getNumScripts(), 13); + // regardless of type — not just dsp/dspui. A lower bound so adding or + // removing a shipped script does not break the test. + BOOST_REQUIRE_GE (scripts.getNumScripts(), 10); } BOOST_AUTO_TEST_SUITE_END() diff --git a/test/snippets/sol3_parent.lua b/test/snippets/sol3_parent.lua deleted file mode 100644 index 8f9e70293..000000000 --- a/test/snippets/sol3_parent.lua +++ /dev/null @@ -1,72 +0,0 @@ --- SPDX-FileCopyrightText: Copyright (C) Kushview, LLC. --- SPDX-License-Identifier: GPL-3.0-or-later - -local object = require ('el.object') - -local function dumpt (obj) - for k,v in pairs (obj) do print (k,v) end -end - -local function dumpmt (obj) - dumpt (getmetatable (obj)) -end - -local function runtest() - begintest ("object.new (Parent)") - local parent = object.new (Parent) - expect (parent.name == "Parent Object") - parent.name = "Parent" - expect (parent.name == "Parent") - - local Derived = object (Parent, { - name = { - get = function (self) - return rawget (self, '_name') or "Derived" - end - } - }) - - function Derived:init() - Parent.init (self) - end - - function Derived:get() - local impl = getmetatable(self).__impl - return impl:get() * 2.0 - end - - local d = object.new (Derived) - local impl = getmetatable(d).__impl - - begintest ("Derived: rawset") - d.name2 = "Alternate Name" - expect (d.name2 == "Alternate Name") - expect (d.name == "Derived", tostring (d.name)) - - begintest ("readonly att") - local r,e = pcall (function() - d.name = d.name2 - end) - expect (r == false) - expect (d.name ~= d.name2, d.name) - - begintest ("Derived:get") - - expect (type(d.get) == 'function', "wrong type: " .. type(d.get)) - expect (impl:get() == 100, impl:get()) - expect (d:get() == 200, d:get()) - - begintest ("changed") - expect (impl.changed == nil) - d.changed = function() - print ("value changed: " .. tostring (impl:get())) - end - - -- expect (impl.changed ~= nil) - begintest ("Derived:set") - d:set (50) - expect (d:get() == 100, d:get()) -end - -runtest() -collectgarbage() diff --git a/test/snippets/stream_from_c.lua b/test/snippets/stream_from_c.lua deleted file mode 100644 index d4599fc87..000000000 --- a/test/snippets/stream_from_c.lua +++ /dev/null @@ -1,18 +0,0 @@ --- SPDX-FileCopyrightText: Copyright (C) Kushview, LLC. --- SPDX-License-Identifier: GPL-3.0-or-later - -result = "" -local function read_and_print_input() - result = io.read ("a") - print ("data read: " .. result) -end - -currentpos = stream:seek ("cur") -local oi = io.input() -io.input (stream) - -read_and_print_input() - -io.input (oi) -stream:close() -stream = nil