tidalcycles / strudel

Web-based environment for live coding algorithmic patterns, incorporating a faithful port of TidalCycles to JavaScript
https://strudel.cc/
GNU Affero General Public License v3.0
579 stars 105 forks source link

tonal: 2 small fixes #1092

Open kasparsj opened 1 month ago

kasparsj commented 1 month ago
kasparsj commented 1 month ago

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' ]
yaxu commented 1 month ago

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.

felixroos commented 1 month ago

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")

kasparsj commented 1 month ago

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 ...

felixroos commented 1 month ago

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.

kasparsj commented 1 month ago

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:

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.

felixroos commented 1 month ago

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 commented 1 month ago

@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..

kasparsj commented 3 weeks ago

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?

felixroos commented 3 weeks ago

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 commented 3 weeks ago

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 commented 3 weeks ago