Skip to content

5/5 — cleanup: delete the event plumbing the mixer made redundant #6

Description

@KevinGruber2001

Part 5 of #1. Depends on #5. Not part of the MVP — after #5 the refactor is functionally done. This is the tidy-up that stops the old design's leftovers from confusing you in six months.

Problem

internal/state implements a typed event system: EventFXParamChanged, EventClipGainChanged, and so on, each with a matching mutation and an engine-side handler that pokes the live miniaudio graph.

That design existed because the graph was stateful and lived in C — the engine had to be told what changed so it could call the right setter on the right node.

After #4, the mixer reads the immutable project snapshot at the top of every block. It doesn't need to be told anything; it just sees the new value on the next block, ~21 ms later. So most of that machinery is now dead weight:

internal/state/events.go      161
internal/state/mutations.go   320

Leaving it in place is worse than deleting it: it's the kind of code you read later, assume is load-bearing, and design around.

Before you start

  • Check what the extension actually uses. state.Apply and the mutations may be on the path the VS Code extension uses to write TOML edits. Grep vscode-extension/src and internal/serve before deleting anything — this issue is about removing what became redundant, not about removing the write path.
  • Re-read docs/ipc.md. The two-plane rule (files carry what you'd commit; the channel carries what you'd never commit) is still correct and still worth keeping. This issue doesn't change it.
  • Subscribers may still be wanted for non-audio reasons — the watcher logging what changed, or the extension being told a reload happened. Keep the notification, drop the per-parameter detail if nothing consumes it.

Proposed solution

flowchart TD
  subgraph BEFORE
    W1[watcher] --> A1["Apply(mutation)"] --> E1["Event<br/>FXParamChanged"] --> H1["engine.handle()"] --> N1["ma_node setter"]
  end
  subgraph AFTER
    W2[watcher] --> A2["Store.Set(project)"] --> P2[("atomic.Pointer")]
    M2["MixBlock()"] -- "Load() each block" --> P2
  end
Loading

Three steps, each independently revertable:

1. Store.Get() → atomic.Pointer[project.Project]. The store is already copy-on-write (see the comment at the top of internal/project/clone.go — every mutation runs on a fresh clone and swaps it in), so this is a mechanical change, not a redesign:

type Store struct {
    project atomic.Pointer[project.Project]
    // mu and the RWMutex go away
    subsMu  sync.RWMutex
    subs    []chan Event
}

func (s *Store) Get() *project.Project { return s.project.Load() }

Honestly: with the ring buffer from #5 the mixer isn't on the device thread, so the RWMutex was never actually dangerous. Do this because it's simpler — one line, no lock discipline to explain — not because it fixes a bug.

2. Reduce events to what's left. Probably one:

// Event says the project changed. What changed is not described, because
// nothing needs to know: the mixer reads the current snapshot every block.
type Event struct {
    Project *project.Project
    Err     error // a reload that failed to parse
}

3. Delete the dead handlers in internal/engine — handle(), listen(), and the per-event branches.

Why this solution

  • It deletes a whole category of future work. Under the old design, every new parameter needed an event type, a mutation and a handler. Under the new one it needs a struct field. Removing the old code is what makes that true in practice rather than just in principle.
  • ~480 lines gone from a codebase you want to hold in your head entirely.
  • It removes a genuinely confusing thing: two mechanisms for "the project changed", one of which is vestigial.

Alternatives considered

Leave it. It works, and it's tested. Reasonable if you're bored of refactoring — nothing is broken. The cost is purely cognitive, and it's paid every time someone (including you, later) reads state/ and tries to work out which path is live. Given the explicit goal of mentally owning the whole project, that cost is the point.

Keep fine-grained events for the UI. Worth checking rather than assuming. If the extension wants to know which parameter changed to avoid re-rendering a whole panel — keep exactly those events and delete the rest. Let the consumer decide; just don't keep events nothing consumes.

Delete state entirely and have the watcher hand projects straight to the engine. Tempting, but the store is a genuinely useful seam: one place that owns "what is the current project", with subscribers. Keep the seam, shrink the contents.

Done when

  • Store uses atomic.Pointer; no mutex around the project
  • Event types reduced to what something actually consumes (verified by grep, not by assumption)
  • Dead handlers in internal/engine deleted
  • internal/serve and the extension behave identically — hot-reload, transport, all of it
  • Existing store tests still pass, trimmed to the surviving API

Afterwards

With this done the engine is: project → state → mix (+ sample, dsp, plugin) → audio. Seven small packages, no C, and every one of them readable in a sitting.

Then the fun starts — recording (#1 lists it), meters over serve, 24/32-bit render, waveforms in the arrangement view.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions