github src vs. Tone.js src? And FR: add 'randomWalkNoRepeat' option

[erichlof]

Hello! First, thank you for this amazing library! I was wanting to see how you implemented some of the pattern generators, 'randomWalk' in particular. But I looked all over this repo's typescript source code and cannot find any mention of 'randomWalk' nor its implementation. Yet, when I type 'randomWalk' in my project, it magically works! So, it must be somewhere, lol. I finally found it buried in the Tone.js download dist but it is minified so it is quite hard to read and understand.

Am I missing something - why can't I find 'randomWalk' anywhere on the Tone.js repo? Is there another repo other than GitHub where you have newer features for the latest version of the library?

Also, a feature request: I find myself wanting a combo of both randomOnce and randomWalk. In other words, a melody would randomly walk from note to note, but would not repeat the same note from note to note. This leads to more 'human' melody making on the piano for instance, A, B, C, B, C, D, E, F, E, D, etc. etc. The current randomWalk comes close, but repeats, so I end up with A, B, B, B, C, B, C, C, D, D, D, D, C, etc. btw I am a pro pianist/music teacher as well as a hobbyist coder (killer combo, huh? lol)

Maybe a 'randomWalkNoRepeat' would be useful to others as well?

Again, thanks for your awesome library! -Erich

[tambien]

Honestly i didn't know if anyone was using the CtrlPattern code on it's own, so during the refactor, i moved it to PatternGenerator since it seemed like the primary use was within Tone.Pattern.

Also seems like i may have removed the randomWalk pattern. If you're up for it and this is a useful feature for you, i'd happily review a PR for randomWalk and randomWalkNoRepeat.

[erichlof]

@tambien Thank you for the response! Yes I agree with the move to PatternGenerator - it makes most sense to live in the Tone.Pattern space. The nice thing about randomWalk and the other CtrlPatterns is that you can let them operate on not just melody pitches, but other data that is inside a pattern array as well, like different rhythms, chords, etc.. The randomWalk in particular is great for linear melody making as it follows the human voice and other voice-like instruments which typically move smoothly from note-choice to note-choice, rather than randomly jumping to wide unrelated intervals. As you may have guessed from my posts, I am investigating computer-generated music. I would argue that randomWalk as well as all the other CtrlPatterns should be left in the permanent Tone.js codebase, as these are really useful.

I was starting to roll my own randomWalkNoRepeat by adding some conditional statements on top of your existing randomWalk feature, but I wanted to see how you implemented randomWalk - which is how we got to this issue, lol! I can't quite glean from the Tone.js file how you did randomWalk. Is there somewhere else that you have the original source code for the CtrlPattern's that is not minimized? For instance I can follow the other CtrlPatterns source code as they are in typescript here on the Tone.js GitHub src. But if I could see randomWalk, then yes I would be up for contributing randomWalkNoRepeat and doing a PR.

Let me know how you want to move forward. Thanks again!

[erichlof]

I was able to locate the source code for randomWalk inside the Tone.js src from many versions back over on npm. Here's a snippet:

else if (type === Tone.CtrlPattern.Type.RandomWalk) { if (Math.random() < 0.5) { this.index--; this.index = Math.max(this.index, 0); } else { this.index++; this.index = Math.min(this.index, this.values.length - 1); }

Ok I can see now that you never intended it to repeat, but this algo can sort of 'get stuck' at the boundaries of the array. Rather than randomWalk and randomWalkNoRepeat, how about we just add 2 more conditional lines to the beginning of the randomWalk algo, like:

if (this.index == 0) { this.index++; } else if (this.index == (this.values.length - 1)) { this.index--; } else if (Math.random() < 0.5) { // .... the rest of your old code

What this essentially does is 'unstick' the walker from the bottom and top edges of the array. If it starts at the bottom, it walks up: it has to. If it reaches the top, it must walk down. This is why I was getting repeated notes with the old randomWalk - it got stuck on the edges, and you can't reliably predict when that might happen.

I would prefer this simple fix for the existing (in older versions) randomWalk algo rather than adding new consts and code, as the improved 'randomWalk' should behave as most people would expect. Please let me know what you think, thanks!

[tambien]

I think that all sounds good. The original code is here. Feel free to submit a PR on the master branch if modifying things in PatternGenerator seems too daunting. but i won't be merging that in since i don't take PRs on the master branch, but i can use your code as reference and add it on the 'dev' branch in typescript.

[erichlof]

@tambien Thanks for the reply and the old code link! I think I might try to submit the new function in Typescript, so you can just integrate it without too much hassle. I haven't really worked in Typescript before but I have a good background in C and Javascript so hopefully I can figure it out. Plus I can see how you ported some of the old JS code to Typescript like RandomOnce, which is still in the current versions. That will help me with some of the syntax and style conventions that you prefer.

I'll try to get a PR up in the next few days. Talk to you soon! -Erich