strudel icon indicating copy to clipboard operation
strudel copied to clipboard

tonal: 2 small fixes

Open kasparsj opened this issue 1 year ago • 12 comments

  • fix scale() to allow both n() and note() at the same time
  • fix scale() to allow scale names without tonic and then default it to C

kasparsj avatar May 07 '24 22:05 kasparsj

Also I'm wondering should these 2 fail?

  it('scale is passed down to firstOf', () => {
    expect(
        n(0, 2)
            .scale('C:minor')
            .firstOf(3, x => x.note(2, 0))
            .firstCycleValues.map((h) => h.note),
    ).toEqual(['Eb3', 'C3']);
  });
// AssertionError: expected [ 2, +0 ] to deeply equal [ 'Eb3', 'C3' ]

  it('scale works with add inside firstOf', () => {
    expect(
        n(0, 2)
            .scale('C:minor')
            .firstOf(3, x => x.add(2))
            .firstCycleValues.map((h) => h.note),
    ).toEqual(['Eb3', 'G3']);
  });
// AssertionError: expected [ 'C3', 'Eb3' ] to deeply equal [ 'Eb3', 'G3' ]

kasparsj avatar May 08 '24 09:05 kasparsj

Looks good! I think that's expected behaviour for those tests to fail. The scale function changes the notes, so doesn't act as separate metadata. We could discuss alternatives though.

yaxu avatar May 12 '24 16:05 yaxu

fix scale() to allow scale names without tonic and then default it to C

agreed!

fix scale() to allow both n() and note() at the same time

I think note should not be used as an index type (like n). imo it creates a disjoint between note as a standalone value (roughly between 20 and 100) and as an index (mostly between 0 and 10). Also it gets much weirder if you start to think about what happens if note is a string.. note("f").scale("C:major") // = G10

iirc there was a discussion in discord about how to handle note in scale, and we came up with the idea to let scale quantise any note to fit the scale, e.g. `note("c eb g").scale("C:major") // = note("c e g")

felixroos avatar May 13 '24 04:05 felixroos

fix scale() to allow scale names without tonic and then default it to C

agreed!

fix scale() to allow both n() and note() at the same time

I think note should not be used as an index type (like n). imo it creates a disjoint between note as a standalone value (roughly between 20 and 100) and as an index (mostly between 0 and 10). Also it gets much weirder if you start to think about what happens if note is a string.. note("f").scale("C:major") // = G10

iirc there was a discussion in discord about how to handle note in scale, and we came up with the idea to let scale quantise any note to fit the scale, e.g. `note("c eb g").scale("C:major") // = note("c e g")

does that imply that note in strudel is more like midinote in tidal? is there another function that behaves like tidal's note in the sense of a numeric (zero based) scale degre? (probably defaulting to chromatic scale and c3 offset) ... that's how I've thought of tidal's note till now ...

kasparsj avatar May 13 '24 17:05 kasparsj

fix scale() to allow scale names without tonic and then default it to C

agreed!

fix scale() to allow both n() and note() at the same time

I think note should not be used as an index type (like n). imo it creates a disjoint between note as a standalone value (roughly between 20 and 100) and as an index (mostly between 0 and 10). Also it gets much weirder if you start to think about what happens if note is a string.. note("f").scale("C:major") // = G10 iirc there was a discussion in discord about how to handle note in scale, and we came up with the idea to let scale quantise any note to fit the scale, e.g. `note("c eb g").scale("C:major") // = note("c e g")

does that imply that note in strudel is more like midinote in tidal?

I'd say yes, but i haven't done much with tidal's midinote. Generally, note defaults to c3 / 36.

is there another function that behaves like tidal's note in the sense of a numeric (zero based) scale degre? (probably defaulting to chromatic scale and c3 offset) ... that's how I've thought of tidal's note till now ...

I don't think so, you'd have to take 36 as your default instead of 0. What do you think are the advantages of a 0 based version? Btw, if you're working with samples, you can still use n if your samples are arranged chromatically.

felixroos avatar May 20 '24 21:05 felixroos

I guess this is where the idea comes from: https://doc.sccode.org/Tutorials/A-Practical-Guide/PG_07_Value_Conversions.html

tidal skips the "degree" part which is good, because it makes the formula much simpler: midinote = root + octave*12 + scale[note]

The pros:

  • the separation of "octave" and "degree" into 2 pattern-able entities
  • idea that scales are made up of degrees rather then notes
  • not having to think in terms of midi numbers or note names (non-musician/mathematical thinking...)

when using synths or 1 sample sounds, sometimes, I think of n as a shortcut for note, but that might be wrong (perhaps that's the problem with my unit tests - should have used note instead?). if there are 2 or more samples, I think it always should mean just the sample file index.

kasparsj avatar May 21 '24 09:05 kasparsj

I guess this is where the idea comes from: https://doc.sccode.org/Tutorials/A-Practical-Guide/PG_07_Value_Conversions.html

tidal skips the "degree" part which is good, because it makes the formula much simpler: midinote = root + octave*12 + scale[note]

The pros:

  • the separation of "octave" and "degree" into 2 pattern-able entities
  • idea that scales are made up of degrees rather then notes
  • not having to think in terms of midi numbers or note names (non-musician/mathematical thinking...)

when using synths or 1 sample sounds, sometimes, I think of n as a shortcut for note, but that might be wrong (perhaps that's the problem with my unit tests - should have used note instead?). if there are 2 or more samples, I think it always should mean just the sample file index.

I guess the way to emulate that would be to do:

n("0 1 2 3 4").scale("chromatic")

I still find it a bit weird that n can both be a scale degree and a sample index. It could be more logical if there was something like degree that does the same as n but only for scale...

felixroos avatar May 23 '24 07:05 felixroos

@kasparsj should we then only add the defaulting to C in this PR? I'd prefer if note could be used for scale quantization later..

felixroos avatar May 29 '24 11:05 felixroos

Hi Felix! But does it break anything? I still think it's needed to use together: n for selecting a sample and note + scale for selecting a note. I think we sort of sidestepped a bit and discussed other aspects that were kicked off by this comment, but the actual code in this PR for the 2 fixes is still solid?

kasparsj avatar Jun 02 '24 10:06 kasparsj

Hi Felix! But does it break anything? I still think it's needed to use n for selecting a sample and note + scale for selecting a note. I think we sort of sidestepped a bit and discussed other aspects that were kicked off by this comment, but the actual code in this PR for the 2 fixes is still solid?

I still think note should not be used as an index/offset type (see comment) + adding this behaviour would block the possibility to let scale quantize notes to the scale later, which I think is more useful.

The problem you've tried to solve, this one:

n(0, 1, 2) // <- assuming this is the sample number
.note(3, 4, 0) // <- assuming this is the scale index
.scale('C major')

.. can be solved already like this:

n(3, 4, 0)
.scale('C major')
.set.out(n(0, 1, 2)) // <-- set.out takes structure from here

additionally, scale will "swallow" any n, just returning the note, so n will be undefined after scale:

n(3, 4, 0)
.scale('C major')
.s("sawtooth") // <- n in first line won't interfere

felixroos avatar Jun 02 '24 16:06 felixroos

I still find it a bit weird that n can both be a scale degree and a sample index. It could be more logical if there was something like degree that does the same as n but only for scale...

I still think it would be useful if there was another control that is specifically for setting a scale step, something like:

n(0, 1, 2) // <- assuming this is the sample number
.degree(3, 4, 0) // <- assuming this is the scale index
.scale('C major')

I just think it should not be note.. I also think the above code is a rare edge case, as it is not very common to let the sample index provide the structure when you already have a pattern for notes. edit: the more common thing to write would be:

n(3, 4, 0) // <- assuming this is the scale index
.scale('C major')
.n(0, 1, 2) // <- assuming this is the sample number

felixroos avatar Jun 02 '24 16:06 felixroos

  • i've just noticed we have this legacy behavior: https://github.com/tidalcycles/strudel/pull/1092/files#diff-494fcbad55a1e686f45e8d0211c7dfa10a80e8b0325ad8f09964d568eaeeced2L207 which causes your change (to use note for step) to have no effect

felixroos avatar Jun 02 '24 16:06 felixroos