danigb/soundfont-player

[API] Take options on play() instead of instrument()

Closed this issue · 5 comments

Thanks for the great library!

One small API problem I ran into for my project: right now instrument() is what takes the options parameter with the gain and destination information. I feel it would be more flexible to have options be an argument to play() instead:

var inst = soundfont.instrument('guitar_harmonics');
inst.play('c2', 1, 0.5, { gain: 0.2, destination: my_dest });

I need this functionality to play notes softer or louder depending on how hard a user strikes my virtual instrument's strings.

I'll submit a PR shortly and you can see if you agree with the API change.

Hi Matt!,

thanks for the issue and the PR. I like your proposal, except for a little thing: I don't think is practical to give the destination each time you play an instrument. Instead of move options out of instrument to play I would keep instrument options and add play options as override of that. So you can:

var inst = soundfont.instrument('guitar_harmonics', { gain: 0.5, destination: ac.destination });
inst.play('c2', 1, 0.5); // use defaults
inst.play('c2', 1, 0.5, { gain: 0.2, destination: my_dest }); // override defaults

What do you think? Do you mind to change your PR to adapt those changes?

No, that sounds like a much better (and less api breaking) change. I've update the PR and squashed the changes to take a defaultOptions on instrument and then an override options parameter on play.

Great, thanks!!

Nice PR! Thanks a lot.

The changes are published on npm as 0.8.0