Refactoring round 10: split SoundLibrary, fold sound duplicates (#85)

Refactoring round 10 covers everything since round 9 (808e1ee): 110 commits with about 47 000 new lines, nearly all of them sound, sound FX, instruments, patterns and songs. Behaviour is unchanged. Duplicate code (jscpd over src, 50 tokens, TypeScript only, measured on 808e1ee and here alike: 47 clones / 426 lines before, 33 / 245 after; round 9 itself ended at 22 / 160 by this count): - PreviewParams (new, editor/soundfx) is the header's parameter bar of the Sound FX and the Instrument tab: values, defaults, fields and the voices a moved field reaches. Both tabs had it line for line. - fxWorklet: an Effect base runs the bit crusher, phaser, flanger, limiter, LFO and bus shifter, a Source base the pitch player and the Chip. Start is the one wait for a voice's first sample, which four processors wrote out each. - nodes/shared has checkPort, detachedPort, fxParamsPort and letRingPort. The Play and Stop nodes of sounds, notes, patterns and songs spelled each port out, up to five times. - parameterValues builds both Sound FX parameters and Instrument parameters, which differed in name and text only. - SetLoopCommand sets the ruler loop of a pattern or a song. It replaces SetPatternLoopCommand and SetSongLoopCommand. - addLink in music/routing is the one rule for a link that both the song routing and the Master routing keep. - eqCurves works out each band and their sum for the EQ tab, which did that in its drawing code. It replaces eqCurveDb, which only a test called. - Smaller ones: the eight neighbour directions of a tile cell, and drawFloatingBox for the shadowed box of Popup and FloatWindow. - Left on purpose: Play pattern and Play song stay two parallel node definitions, and the smaller clones under ten lines stay. Architecture: - SoundLibrary.ts goes from 2897 to 2188 lines. What a stage of a sound FX becomes as Web Audio nodes, and how the knobs of a playing voice move, is now viewport/fxNodes.ts. None of it keeps state of the library. Dead code (TS language service, references per export and member): - soundRefIds, LFO_SHAPE_INDEX, TREMOLO_ROWS, VIBRATO_ROWS and REVERB_ROWS had no reader. The four routing node defs are registered without an unused exported name. - soundFileOf and portTypeOf were called by tests only. The tests now ask findSound and portOf, which the editor uses. DRY moves from src into the one test that uses it. - 51 values used only in their own file lose their `export`. Types and the node catalogues stay exported. - Kept on purpose: neighboursOf is the cell by cell reading the indexed neighboursIn is checked against, and PitchShifter.latency is the test's one view of how late the shifter is. Comments and docs: - Seven comments told history ("no longer", "any more", "as before", "of before #354"). Nine comment blocks ran far past the line width. Proof that this changed no code: the bundle was md5 identical, and the touched tests and e2e modules were identical after esbuild. - README: sound FX, instrument, pattern, song and bus in the asset list, the pages Instruments, Patterns and Songs, 1919 checks. - Wiki: see its own commit. Tests: Gate (melektaus-atx) 274 files, 4236 tests, CHECK EXIT 0 e2e 34, 64, 65 ALL PASSED, 180 checks Unit, touched areas 16 files, 804 tests No check was added, the e2e sum stays at 1919. Counter-checks, each put back by copy and compared with cmp: - addLink without the loop rule: 2 tests fail, one per routing. - eqCurves with the sum of the last band only: 1 test fails. - Start in the worklet never waiting: e2e 34 fails in exactly the four checks that hang on a voice's first sample (Flanger, Tremolo, Chip, a pitched note planned ahead). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-09 15:31:33 +02:00
1 parent 76f5097a38
commit 25084f8576
@@ -2,7 +2,7 @@ import { MIDDLE_C } from '../../../instrument/notes';
 import { heldTempo } from '../../../music/time';
 import type { NodeDef } from '../../NodeRegistry';
 import { asBoolean, asNumber, asString } from '../../values';
-import { exec, num, playedBusPort, soundFxPort, withDefault } from '../shared';
+import { detachedPort, exec, fxParamsPort, num, playedBusPort, soundFxPort, withDefault } from '../shared';
 import { fxParamsOf, tempoPort } from './PlaySound';

 export const PlayNote: NodeDef = {
@@ -35,17 +35,11 @@ export const PlayNote: NodeDef = {
    withDefault(num('velocity'), 1),
    withDefault(num('volume'), 1),
    withDefault(num('duration'), 0),
-    {
-      key: 'detached',
-      label: 'detached',
-      type: 'boolean',
-      default: false,
-      field: { key: 'detached', label: 'Detached (outlives this object)', kind: 'boolean', default: false },
-    },
-    { key: 'fxParams', label: 'params', type: 'fxparams' },
+    detachedPort(),
+    fxParamsPort('fxParams', 'params'),
    tempoPort(),
    { ...soundFxPort(), key: 'soundFx' },
-    { key: 'soundFxParams', label: 'fx params', type: 'fxparams' },
+    fxParamsPort('soundFxParams'),
    playedBusPort(),
  ],
  outputs: [exec(), num('voice')],
@@ -1,6 +1,15 @@
 import type { NodeDef, PortDef } from '../../NodeRegistry';
 import { asBoolean, asNumber, asString } from '../../values';
-import { exec, num, playedBusPort, soundFxPort, withDefault } from '../shared';
+import {
+  checkPort,
+  detachedPort,
+  exec,
+  fxParamsPort,
+  num,
+  playedBusPort,
+  soundFxPort,
+  withDefault,
+} from '../shared';
 import { fxParamsOf } from './PlaySound';

 export const patternPort = (): PortDef => ({
@@ -30,22 +39,10 @@ export const PlayPattern: NodeDef = {
    exec(),
    patternPort(),
    withDefault(num('volume'), 1),
-    {
-      key: 'loop',
-      label: 'loop',
-      type: 'boolean',
-      default: true,
-      field: { key: 'loop', label: 'Loop', kind: 'boolean', default: true },
-    },
-    {
-      key: 'detached',
-      label: 'detached',
-      type: 'boolean',
-      default: false,
-      field: { key: 'detached', label: 'Detached (outlives this object)', kind: 'boolean', default: false },
-    },
+    checkPort('loop', 'loop', 'Loop', true),
+    detachedPort(),
    soundFxPort(),
-    { key: 'fxParams', label: 'fx params', type: 'fxparams' },
+    fxParamsPort(),
    playedBusPort(),
  ],
  outputs: [exec(), num('playback')],
@@ -1,6 +1,15 @@
 import type { NodeDef, PortDef } from '../../NodeRegistry';
 import { asBoolean, asNumber, asString } from '../../values';
-import { exec, num, playedBusPort, soundFxPort, withDefault } from '../shared';
+import {
+  checkPort,
+  detachedPort,
+  exec,
+  fxParamsPort,
+  num,
+  playedBusPort,
+  soundFxPort,
+  withDefault,
+} from '../shared';
 import { fxParamsOf } from './PlaySound';

 export const songPort = (): PortDef => ({
@@ -31,22 +40,10 @@ export const PlaySong: NodeDef = {
    songPort(),
    withDefault(num('volume'), 1),
    withDefault(num('fadeIn', 'fade in'), 0),
-    {
-      key: 'loop',
-      label: 'loop',
-      type: 'boolean',
-      default: true,
-      field: { key: 'loop', label: 'Loop', kind: 'boolean', default: true },
-    },
-    {
-      key: 'detached',
-      label: 'detached',
-      type: 'boolean',
-      default: false,
-      field: { key: 'detached', label: 'Detached (outlives this object)', kind: 'boolean', default: false },
-    },
+    checkPort('loop', 'loop', 'Loop', true),
+    detachedPort(),
    soundFxPort(),
-    { key: 'fxParams', label: 'fx params', type: 'fxparams' },
+    fxParamsPort(),
    playedBusPort(),
  ],
  outputs: [exec(), num('playback')],
@@ -3,7 +3,16 @@ import { DEFAULT_TEMPO, heldTempo, MAX_TEMPO, MIN_TEMPO } from '../../../music/t
 import type { FxParams } from '../../../soundfx/nodes';
 import type { NodeDef, PortDef } from '../../NodeRegistry';
 import { asBoolean, asNumber, asString, type Value } from '../../values';
-import { exec, num, playedBusPort, soundFxPort, soundPort, withDefault } from '../shared';
+import {
+  detachedPort,
+  exec,
+  fxParamsPort,
+  num,
+  playedBusPort,
+  soundFxPort,
+  soundPort,
+  withDefault,
+} from '../shared';

 /**
 * Three answers, not two: a checkbox cannot say "leave it to the asset", and without that a sound
@@ -37,19 +46,6 @@ function loopChoice(value: Value | undefined): boolean | undefined {
  return undefined;
 }

-/**
- * The way out of the ownership rule, off by default. Needed for one shape that is common enough to
- * deserve it: an object that plays its own death sound and destroys itself in the same flow – the
- * voice would be cut off in the instant it started, because the behavior holding it is gone.
- */
-const detachedPort = (): PortDef => ({
-  key: 'detached',
-  label: 'detached',
-  type: 'boolean',
-  default: false,
-  field: { key: 'detached', label: 'Detached (outlives this object)', kind: 'boolean', default: false },
-});
-
 export const PlaySound: NodeDef = {
  type: 'PlaySound',
  label: 'Play sound',
@@ -76,7 +72,7 @@ export const PlaySound: NodeDef = {
    loopPort(),
    detachedPort(),
    soundFxPort(),
-    { key: 'fxParams', label: 'fx params', type: 'fxparams' },
+    fxParamsPort(),
    tempoPort(),
    playedBusPort(),
  ],
@@ -1,7 +1,7 @@
 import type { NodeDef } from '../../NodeRegistry';
 import { asNumber, asString } from '../../values';
 import { curvesOf, easingOfParams, easingParams, idleEasing } from '../easing';
-import { busPort, exec, num } from '../shared';
+import { busPort, exec, fxParamsPort, num } from '../shared';
 import { fxParamsOf } from './PlaySound';

 import { easeShareOf } from '../../../model/animation';
@@ -25,13 +25,7 @@ export const SetSoundFxParameters: NodeDef = {
    'time: seconds the values take to get there, along the chosen easing and on the clock of the ' +
    'sound itself, so a sweep sounds the same every time; 0 is a quick glide. Without a voice it changes ' +
    'the sound FX of "bus" instead, the master or a bus asset, for everything heard on it.',
-  inputs: [
-    exec(),
-    { ...num('voice'), inline: false },
-    { key: 'fxParams', label: 'fx params', type: 'fxparams' },
-    num('time'),
-    busPort('None', true),
-  ],
+  inputs: [exec(), { ...num('voice'), inline: false }, fxParamsPort(), num('time'), busPort('None', true)],
  outputs: [exec()],
  params: easingParams(),
  idleParams: idleEasing,
@@ -1,6 +1,6 @@
 import type { NodeDef } from '../../NodeRegistry';
 import { asBoolean, asNumber, asString } from '../../values';
-import { exec, num } from '../shared';
+import { exec, letRingPort, num } from '../shared';
 import { patternPort } from './PlayPattern';

 export const StopPattern: NodeDef = {
@@ -18,13 +18,7 @@ export const StopPattern: NodeDef = {
    patternPort(),
    // Not typed in: unconnected it is null, and only then does the pattern decide.
    { ...num('playback'), inline: false },
-    {
-      key: 'letRing',
-      label: 'let ring',
-      type: 'boolean',
-      default: false,
-      field: { key: 'letRing', label: 'Let ring', kind: 'boolean', default: false },
-    },
+    letRingPort(),
  ],
  outputs: [exec()],
  params: [],
@@ -1,6 +1,6 @@
 import type { NodeDef } from '../../NodeRegistry';
 import { asBoolean, asNumber, asString } from '../../values';
-import { exec, num, soundPort } from '../shared';
+import { exec, letRingPort, num, soundPort } from '../shared';

 export const StopSound: NodeDef = {
  type: 'StopSound',
@@ -18,13 +18,7 @@ export const StopSound: NodeDef = {
    soundPort(),
    // Not typed in: unconnected it is null, and only then does the sound decide.
    { ...num('voice'), inline: false },
-    {
-      key: 'letRing',
-      label: 'let ring',
-      type: 'boolean',
-      default: false,
-      field: { key: 'letRing', label: 'Let ring', kind: 'boolean', default: false },
-    },
+    letRingPort(),
  ],
  outputs: [exec()],
  params: [],
@@ -1,37 +1,15 @@
-import type { StructValue } from '../../../model/Properties';
-import { INSTRUMENT_PARAMETER_VALUES, INSTRUMENT_SETTING, soundFxSourceOf } from '../../nested';
+import { INSTRUMENT_PARAMETER_VALUES, INSTRUMENT_SETTING } from '../../nested';
 import type { NodeDef } from '../../NodeRegistry';
-import { asNumber } from '../../values';
+import { parameterValues } from './SoundFxParameters';

 /**
- * Values for the parameters of one instrument (#331), picked like those of a sound FX: each one picked
- * is a number input starting at the instrument's default. The bundle goes into the `params` input of
- * a `Play note`, or the `fx params` of a `Set sound FX parameters` that changes a playing note.
+ * Values for the parameters of one instrument (#331), picked like those of a sound FX. The bundle
+ * goes into the `params` input of a `Play note`, or the `fx params` of a `Set sound FX parameters`
+ * that changes a playing note.
 */
-export const InstrumentParameterValues: NodeDef = {
-  type: INSTRUMENT_PARAMETER_VALUES,
-  label: 'Instrument parameters',
-  category: 'data',
-  description:
-    'Values for the parameters of an instrument, for the "params" input of a Play note or the "fx params" of a Set sound FX parameters. Pick the parameters to set; one not picked keeps the default its Parameters node gives it, or on a playing note the value it has.',
-  inputs: [],
-  outputs: [{ key: 'params', label: 'params', type: 'fxparams' }],
-  params: [INSTRUMENT_SETTING],
-  declaresPorts: 'inputs',
-  portKinds: ['number'],
-  declaredPort: (port, node, _graph, lookup) => {
-    const declared = lookup.soundFx(soundFxSourceOf(node.params))?.params.find((p) => p.key === port.key);
-    const value = declared?.default ?? 0;
-    return {
-      key: port.key,
-      label: port.key,
-      type: 'number',
-      default: value,
-      field: { key: port.key, label: port.key, kind: 'number', default: value },
-    };
-  },
-  run: (_ctx, inputs, _params, def) => {
-    const values: StructValue = Object.fromEntries(def.inputs.map((p) => [p.key, asNumber(inputs[p.key])]));
-    return { outputs: { params: values } };
-  },
-};
+export const InstrumentParameterValues: NodeDef = parameterValues(
+  INSTRUMENT_PARAMETER_VALUES,
+  'Instrument parameters',
+  'Values for the parameters of an instrument, for the "params" input of a Play note or the "fx params" of a Set sound FX parameters. Pick the parameters to set; one not picked keeps the default its Parameters node gives it, or on a playing note the value it has.',
+  INSTRUMENT_SETTING,
+);
@@ -1,37 +1,55 @@
 import type { StructValue } from '../../../model/Properties';
+import type { PropertyDef } from '../../../properties/PropertySchema';
 import { SOUND_FX_PARAMETERS, SOUND_FX_SETTING, soundFxSourceOf } from '../../nested';
 import type { NodeDef } from '../../NodeRegistry';
 import { asNumber } from '../../values';

 /**
- * Values for the parameters of one sound FX (#311), picked from those its Input declares the way a
- * Variables node picks variables. Each picked one is a number input starting at the FX's default;
- * the bundle goes into the `fx params` input of a `Play sound` or a `Set sound FX parameters`.
+ * A node that bundles values for the parameters of one sound FX or instrument, picked from those it
+ * declares the way a Variables node picks variables. Each picked one is a number input starting at
+ * the asset's default; the bundle goes into an `fx params` or `params` input.
 */
-export const SoundFxParameters: NodeDef = {
-  type: SOUND_FX_PARAMETERS,
-  label: 'Sound FX parameters',
-  category: 'data',
-  description:
-    'Values for the parameters of a sound FX, for the "fx params" input of a Play sound, Play note, Play pattern, Play song or a Set sound FX parameters. Pick the parameters to set; one not picked keeps the default its Input gives it, or on a playing sound the value it has.',
-  inputs: [],
-  outputs: [{ key: 'params', label: 'params', type: 'fxparams' }],
-  params: [SOUND_FX_SETTING],
-  declaresPorts: 'inputs',
-  portKinds: ['number'],
-  declaredPort: (port, node, _graph, lookup) => {
-    const declared = lookup.soundFx(soundFxSourceOf(node.params))?.params.find((p) => p.key === port.key);
-    const value = declared?.default ?? 0;
-    return {
-      key: port.key,
-      label: port.key,
-      type: 'number',
-      default: value,
-      field: { key: port.key, label: port.key, kind: 'number', default: value },
-    };
-  },
-  run: (_ctx, inputs, _params, def) => {
-    const values: StructValue = Object.fromEntries(def.inputs.map((p) => [p.key, asNumber(inputs[p.key])]));
-    return { outputs: { params: values } };
-  },
-};
+export function parameterValues(
+  type: string,
+  label: string,
+  description: string,
+  setting: PropertyDef,
+): NodeDef {
+  return {
+    type,
+    label,
+    category: 'data',
+    description,
+    inputs: [],
+    outputs: [{ key: 'params', label: 'params', type: 'fxparams' }],
+    params: [setting],
+    declaresPorts: 'inputs',
+    portKinds: ['number'],
+    declaredPort: (port, node, _graph, lookup) => {
+      const declared = lookup.soundFx(soundFxSourceOf(node.params))?.params.find((p) => p.key === port.key);
+      const value = declared?.default ?? 0;
+      return {
+        key: port.key,
+        label: port.key,
+        type: 'number',
+        default: value,
+        field: { key: port.key, label: port.key, kind: 'number', default: value },
+      };
+    },
+    run: (_ctx, inputs, _params, def) => {
+      const values: StructValue = Object.fromEntries(def.inputs.map((p) => [p.key, asNumber(inputs[p.key])]));
+      return { outputs: { params: values } };
+    },
+  };
+}
+
+/**
+ * Values for the parameters of one sound FX (#311), for the `fx params` input of a `Play sound` or a
+ * `Set sound FX parameters`.
+ */
+export const SoundFxParameters: NodeDef = parameterValues(
+  SOUND_FX_PARAMETERS,
+  'Sound FX parameters',
+  'Values for the parameters of a sound FX, for the "fx params" input of a Play sound, Play note, Play pattern, Play song or a Set sound FX parameters. Pick the parameters to set; one not picked keeps the default its Input gives it, or on a playing sound the value it has.',
+  SOUND_FX_SETTING,
+);
@@ -14,6 +14,15 @@ export const any = (key: string, label = key): PortDef => ({ key, label, type: '

 export const withDefault = (port: PortDef, value: PropertyValue): PortDef => ({ ...port, default: value });

+/** A boolean input that shows a check box while it is not connected. */
+export const checkPort = (key: string, label: string, fieldLabel: string, value = false): PortDef => ({
+  key,
+  label,
+  type: 'boolean',
+  default: value,
+  field: { key, label: fieldLabel, kind: 'boolean', default: value },
+});
+
 /** A whole-number input (an index, a count): its field shows no decimals, the node floors it anyway. */
 export const int = (key: string, value = 0, label = key): PortDef => ({
  key,
@@ -58,13 +67,7 @@ export const PER_CELL_INPUT = 'perCell';
 * Which pairs a collision event hears of a map's trigger tiles (#160): off, one per area of a tile –
 * a pond is entered once –, on, one per cell.
 */
-export const perCellPort = (): PortDef => ({
-  key: PER_CELL_INPUT,
-  label: 'per cell',
-  type: 'boolean',
-  default: false,
-  field: { key: PER_CELL_INPUT, label: 'Per cell (trigger tiles)', kind: 'boolean', default: false },
-});
+export const perCellPort = (): PortDef => checkPort(PER_CELL_INPUT, 'per cell', 'Per cell (trigger tiles)');

 /**
 * The trigger tile a collider lies on (#160) and its cell in the map, which is then `other`. No
@@ -127,6 +130,24 @@ export const busPort = (none: string, master: boolean): PortDef => ({
 /** The `bus` input of a Play node: empty is the bus its asset names. */
 export const playedBusPort = (): PortDef => busPort('From the asset', true);

+/**
+ * The way out of the ownership rule, off by default. Needed for one shape that is common enough to
+ * deserve it: an object that plays its own death sound and destroys itself in the same flow – the
+ * voice would be cut off in the instant it started, because the behavior holding it is gone.
+ */
+export const detachedPort = (): PortDef =>
+  checkPort('detached', 'detached', 'Detached (outlives this object)');
+
+/** Values for the parameters of a sound FX or an instrument, bundled by their node (#311, #331). */
+export const fxParamsPort = (key = 'fxParams', label = 'fx params'): PortDef => ({
+  key,
+  label,
+  type: 'fxparams',
+});
+
+/** Whether a Stop node lets what it stops fall away in its release instead of cutting it. */
+export const letRingPort = (): PortDef => checkPort('letRing', 'let ring', 'Let ring');
+
 export function binaryMath(type: string, description: string, fn: (a: number, b: number) => number): NodeDef {
  return {
    type,