Merge pull request 'Bug Fix: Handle scale-for-notes when n also supplied' (#1625) from glossing/strudel:glossing/scale-with-note-and-n into main

Reviewed-on: https://codeberg.org/uzu/strudel/pulls/1625

packages/core/pattern.mjs

@@ -2574,8 +2574,8 @@ export const { chunkBack, chunkback } = register(
  * @returns Pattern
  * @example
  * "<0 8> 1 2 3 4 5 6 7"
- * .fastChunk(4, x => x.color('red')).slow(2)
  * .scale("C2:major").note()
+ * .fastChunk(4, x => x.color('red')).slow(2)
  */
 export const { fastchunk, fastChunk } = register(
   ['fastchunk', 'fastChunk'],

packages/core/util.mjs

@@ -7,8 +7,8 @@ This program is free software: you can redistribute it and/or modify it under th
 import { logger } from './logger.mjs';
 
 // returns true if the given string is a note
-export const isNoteWithOctave = (name) => /^[a-gA-G][#bs]*[0-9]$/.test(name);
+export const isNoteWithOctave = (name) => /^[a-gA-G][#bsf]*[0-9]*$/.test(name);
-export const isNote = (name) => /^[a-gA-G][#bsf]*-?[0-9]?$/.test(name);
+export const isNote = (name) => /^[a-gA-G][#bsf]*-?[0-9]*$/.test(name);
 export const tokenizeNote = (note) => {
   if (typeof note !== 'string') {
     return [];
@@ -23,6 +23,10 @@ export const tokenizeNote = (note) => {
 const chromas = { c: 0, d: 2, e: 4, f: 5, g: 7, a: 9, b: 11 };
 const accs = { '#': 1, b: -1, s: 1, f: -1 };
 
+export const getAccidentalsOffset = (accidentals) => {
+  return accidentals?.split('').reduce((o, char) => o + accs[char], 0) || 0;
+};
+
 // turns the given note into its midi number representation
 export const noteToMidi = (note, defaultOctave = 3) => {
   const [pc, acc, oct = defaultOctave] = tokenizeNote(note);
@@ -30,7 +34,7 @@ export const noteToMidi = (note, defaultOctave = 3) => {
     throw new Error('not a note: "' + note + '"');
   }
   const chroma = chromas[pc.toLowerCase()];
-  const offset = acc?.split('').reduce((o, char) => o + accs[char], 0) || 0;
+  const offset = getAccidentalsOffset(acc);
   return (Number(oct) + 1) * 12 + chroma + offset;
 };
 export const midiToFreq = (n) => {

packages/superdough/util.mjs

@@ -16,13 +16,17 @@ export const tokenizeNote = (note) => {
 const chromas = { c: 0, d: 2, e: 4, f: 5, g: 7, a: 9, b: 11 };
 const accs = { '#': 1, b: -1, s: 1, f: -1 };
 
+export const getAccidentalsOffset = (accidentals) => {
+  return accidentals?.split('').reduce((o, char) => o + accs[char], 0) || 0;
+};
+
 export const noteToMidi = (note, defaultOctave = 3) => {
   const [pc, acc, oct = defaultOctave] = tokenizeNote(note);
   if (!pc) {
     throw new Error('not a note: "' + note + '"');
   }
   const chroma = chromas[pc.toLowerCase()];
-  const offset = acc?.split('').reduce((o, char) => o + accs[char], 0) || 0;
+  const offset = getAccidentalsOffset(acc);
   return (Number(oct) + 1) * 12 + chroma + offset;
 };
 export const midiToFreq = (n) => {

packages/tonal/test/tonal.test.mjs

@@ -61,13 +61,6 @@ describe('tonal', () => {
           .firstCycleValues.map((h) => h.note),
       ).toEqual(['B2', 'Eb3', 'A2', 'G3', 'F3']);
     });
-    it('produces silence for mixed sharps and flats', () => {
-      expect(
-        n(seq('0b#', '1#b', '2#b#'))
-          .scale('C major')
-          .firstCycleValues.map((h) => h.note),
-      ).toEqual([]);
-    });
     it('snaps notes (upwards) to scale', () => {
       const inputNotes = ['Cb', 'Eb', 'G', 'A#', 'Bb'];
       const expectedNotes = ['B2', 'E3', 'G3', 'B3', 'B3'];

packages/tonal/tonal.mjs

@@ -5,9 +5,8 @@ This program is free software: you can redistribute it and/or modify it under th
 */
 
 import { Note, Interval, Scale } from '@tonaljs/tonal';
-import { register, _mod, silence, logger, pure, isNote } from '@strudel/core';
+import { register, _mod, logger, isNote, noteToMidi, removeUndefineds, getAccidentalsOffset } from '@strudel/core';
 import { stepInNamedScale, nearestNumberIndex } from './tonleiter.mjs';
-import { noteToMidi } from '../core/util.mjs';
 
 const octavesInterval = (octaves) => (octaves <= 0 ? -1 : 1) + octaves * 7 + 'P';
 
@@ -185,17 +184,15 @@ function _convertStepToNumberAndOffset(step) {
     step = String(step);
     // Check to see if the step matches the expected format:
     // - A number (possibly negative)
-    // - Some number of sharps or flats (but not both)
+    // - Some number of sharps or flats
-    const match = /^(-?\d+)(#+|b+)?$/.exec(step);
+    const match = /^(-?\d+)([#bsf]*)$/.exec(step);
 
     if (!match) {
       throw new Error(`invalid scale step "${step}", expected number or integer with optional # b suffixes`);
     }
     asNumber = Number(match[1]);
-    // These decorations will determine the semitone offset based on the number of
+    const accidentals = match[2] || '';
-    // sharps or flats
+    offset = getAccidentalsOffset(accidentals);
-    const decorations = match[2] || '';
-    offset = decorations[0] === '#' ? decorations.length : -decorations.length;
   }
   return [asNumber, offset];
 }
@@ -226,7 +223,7 @@ function _getNearestScaleNote(scaleName, note, preferHigher = true) {
  * Turns numbers into notes in the scale (zero indexed) or quantizes notes to a scale.
  *
  * When describing notes via numbers, note that negative numbers can be used to wrap backwards
- * in the scale as well as sharps or flats (but not both) to produce notes outside of the scale.
+ * in the scale as well as sharps or flats to produce notes outside of the scale.
  *
  * Also sets scale for other scale operations, like {@link Pattern#scaleTranspose}.
  *
@@ -254,7 +251,6 @@ function _getNearestScaleNote(scaleName, note, preferHigher = true) {
  * @example
  * note("C1*16").transpose(irand(36)).scale('Cb2 major').scaleTranspose(3)
  */
-
 export const scale = register(
   'scale',
   function (scale, pat) {
@@ -262,44 +258,47 @@ export const scale = register(
     if (Array.isArray(scale)) {
       scale = scale.flat().join(' ');
     }
-    return (
+    return pat.withHaps((haps) => {
-      pat
+      haps = haps.map((hap) => {
-        .fmap((value) => {
+        let hVal = hap.value;
-          const isObject = typeof value === 'object';
+        const isObject = typeof hVal === 'object';
-          // The case where the note has been defined via `n` or `pure`
+        // If hVal is a pure value, place it on `n` so that we interpret it as a scale degree
-          if (!isObject || (isObject && ('n' in value || 'value' in value))) {
+        hVal = isObject ? hVal : { n: hVal };
-            const step = isObject ? (value.n ?? value.value) : value;
+        const { note, n, value, ...otherValues } = hVal;
-            delete value.n; // remove n so it won't cause trouble
+        const noteOrStep = note ?? n ?? value;
-            if (isNote(step)) {
+        if (noteOrStep === undefined) {
-              // legacy..
+          logger(
-              return pure(step);
+            `[tonal] Invalid value format for 'scale'. Value must contain n, note, or value but received keys [${Object.keys(hVal).join(', ')}]`,
+            'error',
+          );
+          return hap; // pass the value through unchanged
+        }
+        let scaleNote;
+        if (isNote(noteOrStep)) {
+          // Note case (quantize to scale)
+          scaleNote = _getNearestScaleNote(scale, noteOrStep);
+          hap.value = { ...otherValues, note: scaleNote };
+        } else {
+          // Step case (convert to note in scale)
+          try {
+            const [number, offset] = _convertStepToNumberAndOffset(noteOrStep);
+            if (otherValues.anchor) {
+              scaleNote = stepInNamedScale(number, scale, otherValues.anchor);
+            } else {
+              scaleNote = scaleStep(number, scale);
             }
-            try {
+            if (offset != 0) scaleNote = Note.transpose(scaleNote, Interval.fromSemitones(offset));
-              const [number, offset] = _convertStepToNumberAndOffset(step);
+          } catch (err) {
-              let note;
+            logger(`[tonal] ${err.message}`, 'error');
-              if (isObject && value.anchor) {
+            return; // will be removed
-                note = stepInNamedScale(number, scale, value.anchor);
-              } else {
-                note = scaleStep(number, scale);
-              }
-              if (offset != 0) note = Note.transpose(note, Interval.fromSemitones(offset));
-              value = pure(isObject ? { ...value, note } : note);
-            } catch (err) {
-              logger(`[tonal] ${err.message}`, 'error');
-              return silence;
-            }
-            return value;
           }
-          // The case where the note has been defined via `note`
+        }
-          else {
+        hap.value = isObject ? { ...otherValues, note: scaleNote } : scaleNote;
-            const note = _getNearestScaleNote(scale, value.note);
+        // Tag with scale for downsteam scale-aware operations
-            return pure(isObject ? { ...value, note } : note);
+        return hap.setContext({ ...hap.context, scale });
-          }
+      });
-        })
+      return removeUndefineds(haps);
-        .outerJoin()
+    });
-        // legacy:
-        .withHap((hap) => hap.setContext({ ...hap.context, scale }))
-    );
   },
   true,
   true, // preserve step count

test/__snapshots__/examples.test.mjs.snap

@@ -3838,8 +3838,8 @@ exports[`runs examples > example "fast" example index 0 1`] = `
 
 exports[`runs examples > example "fastChunk" example index 0 1`] = `
 [
-  "[ 0/1 → 1/4 | color:red note:0 ]",
+  "[ 0/1 → 1/4 | note:C2 color:red ]",
-  "[ 1/4 → 1/2 | color:red note:1 ]",
+  "[ 1/4 → 1/2 | note:D2 color:red ]",
   "[ 1/2 → 3/4 | note:E2 ]",
   "[ 3/4 → 1/1 | note:F2 ]",
   "[ 1/1 → 5/4 | note:G2 ]",
@@ -3848,8 +3848,8 @@ exports[`runs examples > example "fastChunk" example index 0 1`] = `
   "[ 7/4 → 2/1 | note:C3 ]",
   "[ 2/1 → 9/4 | note:D3 ]",
   "[ 9/4 → 5/2 | note:D2 ]",
-  "[ 5/2 → 11/4 | color:red note:2 ]",
+  "[ 5/2 → 11/4 | note:E2 color:red ]",
-  "[ 11/4 → 3/1 | color:red note:3 ]",
+  "[ 11/4 → 3/1 | note:F2 color:red ]",
   "[ 3/1 → 13/4 | note:G2 ]",
   "[ 13/4 → 7/2 | note:A2 ]",
   "[ 7/2 → 15/4 | note:B2 ]",