Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nexus.tune.createScale() does not produce expected scale #138

Closed
botway opened this issue Dec 19, 2018 · 3 comments
Closed

Nexus.tune.createScale() does not produce expected scale #138

botway opened this issue Dec 19, 2018 · 3 comments
Labels

Comments

@botway
Copy link

botway commented Dec 19, 2018

Looks like the scale is not played back properly:

// Create a minor scale
Nexus.tune.createScale(0,2,3,5,7,8,10);

// Playing back steps (0,1,2,3,4,5,6,7,8,9,10,11,12)
console.log(step , Tone.Frequency(Nexus.note(step)).toNote());

Produces this result:
0 "C4"
1 "D4"
2 "D#4"
3 "F4"
4 "G4"
5 "G#4"
6 "C5"
7 "D5"
8 "D#5"
9 "F5"
10 "G5"
11 "G#5"
12 "C6"

Looks like it jumps to the beginning of the scale after the 6th step.

Logging Nexus.tune (minor scale above):
scale: Array(6)
0: 1
1: 1.122462048309373
2: 1.1892071150027212
3: 1.3348398541700344
4: 1.4983070768766815
5: 1.5874010519681996
length: 6

Array.length is just 6, that's why it jumps to the beginning.
I am not sure about this, but in tuning.js

 createScale() {
    let newScale = [];
    for (let i=0;i<arguments.length;i++) {
      newScale.push( math.mtof( 60 + arguments[i] ) );
    }
    this.loadScaleFromFrequencies(newScale);
  }
// maybe it should be 
 for (let i=0;i<=arguments.length;i++) {...}
@botway botway changed the title Nexus.tune.createScale() and Nexus.note() do not produce expected scale Nexus.tune.createScale() does not produce expected scale Dec 19, 2018
@taylorbf
Copy link
Contributor

taylorbf commented Mar 6, 2019

Hi. thanks and sorry it took a while to get back to you.

Yes this is a bug. It is actually in the following function, a few lines down from the code you were looking at:

loadScaleFromFrequencies(freqs) {
    this.scale = [];
    for (let i=0;i<freqs.length-1;i++) {
      this.scale.push(freqs[i]/freqs[0]);
    }
  }

The loop counts up to freqs.length-1, and it should count to freqs.length . This error is due to this function being legacy code from Tune.js, which uses scale data that includes the octave note duplicated at the end of the scale (i.e. scale data that would look like: C D E F G A B C ).

I will aim to revise this in the next round of updates.

@vladansaracpv
Copy link
Contributor

Created pr with changes @taylorbf suggested

tatecarson added a commit that referenced this issue Mar 31, 2020
@tatecarson
Copy link
Collaborator

Fixed with #179 .

tatecarson added a commit that referenced this issue Aug 29, 2020
* updated discord message

* feat: support for vertical orientation (#115)

* fix: Nexus.tune.createScale() does not produce expected scale (#138)

* fix: numberOfSliders does not change number of sliders for Multislider (#161)

* feat: support for vertical orientation (#115)

* feat: add padding to sequencer (#150) #178

* feat: add padding to sequencer (#150) #178

* removed commented out line

* chore(release): 2.1.0

* chore(release): 2.1.1

* chore(release): 2.1.2

* add build

* fix issue #161

* modify regex to support input of negative numbers to number component

* update dist after regex change

* chore(release): 2.1.3

* 🐛 Fix Nexus.Interval import regression

* Fix Nexus.Interval import regression #184

* chore(release): 2.1.4

* Implement multi-touch piano

* chore(release): 2.1.5

* Remove document.write

* Rebuild dist files

* Fix broken CSS in examples

* Bump websocket-extensions from 0.1.3 to 0.1.4 (#185)

Bumps [websocket-extensions](https://github.com/faye/websocket-extensions-node) from 0.1.3 to 0.1.4.
- [Release notes](https://github.com/faye/websocket-extensions-node/releases)
- [Changelog](https://github.com/faye/websocket-extensions-node/blob/master/CHANGELOG.md)
- [Commits](faye/websocket-extensions-node@0.1.3...0.1.4)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump standard-version from 7.1.0 to 8.0.1 (#187)

Bumps [standard-version](https://github.com/conventional-changelog/standard-version) from 7.1.0 to 8.0.1.
- [Release notes](https://github.com/conventional-changelog/standard-version/releases)
- [Changelog](https://github.com/conventional-changelog/standard-version/blob/master/CHANGELOG.md)
- [Commits](conventional-changelog/standard-version@v7.1.0...v8.0.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Reduce dependency on web audio (#190)

* Only create AudioContext when necessary

* Tweak npm version

* Rebuild dist files

* update documention cdn to use latest build

* fixed mistake

* fixed main and build

* "chore(release): 2.1.6"

* Update index.html

trying to get it to rebuild

Co-authored-by: Vladan Sarac <vladan.sarac.pv@gmail.com>
Co-authored-by: jamesstaub <james.staub@gmail.com>
Co-authored-by: Theis Bazin <theis.bazin@outlook.com>
Co-authored-by: Andy Harman <andyh.at.pendragon@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Andy Harman <pendragon-andyh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants