-
Notifications
You must be signed in to change notification settings - Fork 1
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
✨ real music #16
✨ real music #16
Conversation
public/audiosynth.js
Outdated
@@ -0,0 +1,350 @@ | |||
var Synth, AudioSynth, AudioSynthInstrument; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you search for this lib on npm ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this lib is not present on npm, and i don't find equivalent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should fork it (https://github.com/keithwhor/audiosynth) and publish it.
So we can import it trough webpack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for that ? it's one file, and i think, we don't maintain this lib ?
src/components/rythmbox/rythmbox.js
Outdated
392, // G | ||
587, // D | ||
const notes = [ | ||
'C', // E |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete commentaries
src/components/rythmbox/rythmbox.js
Outdated
'bass', | ||
'ocarina', | ||
'harp', | ||
const characters = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should pass this from props to avoid duplication
src/drivers/music.js
Outdated
next: (note) => { | ||
const { type = 'sine', frequency, time = 200 } = note | ||
next: (music) => { | ||
const { instrument, note, time = 2 } = music |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should delete default value as we argue before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, some minors commentaries
src/components/speaker/speaker.js
Outdated
.flatten() | ||
.map(music => Object.assign({}, music, { stop: true })) | ||
|
||
const music$ = xs.merge(musicStart$, musicStop$) | ||
|
||
const vdom$ = music$ | ||
.startWith({ stop: true }) | ||
.startWith({ stop: true }).debug() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug
src/components/rythmbox/rythmbox.js
Outdated
) | ||
export default ({ DOM$, props$ }) => { | ||
const ranges$ = props$ | ||
.map(characters => characters.map(c => isolate(Range, c.name)({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to map in two instructions, I find it easier to read and explain
src/drivers/music.js
Outdated
osc.connect(context.destination) | ||
osc.start() | ||
osc.stop(context.currentTime + (time / 1000)) | ||
// eslint-disable-next-line no-undef |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cand delete the eslint-disable, I forgot it
src/components/rythmbox/rythmbox.js
Outdated
'ocarina', | ||
'harp', | ||
] | ||
const notes = ['A', 'B', 'C', 'D', 'E', 'F', 'G'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge the other PR then move this into the config file ?
Add library to play real music => https://github.com/keithwhor/audiosynth
rythmbox Playing by character and not by instrument, because, the instrument changed by the character.