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

Add onended callback to p5.SoundFile.prototype.play #78

Closed
blindfish3 opened this issue Nov 12, 2015 · 4 comments
Closed

Add onended callback to p5.SoundFile.prototype.play #78

blindfish3 opened this issue Nov 12, 2015 · 4 comments

Comments

@blindfish3
Copy link

@blindfish3 blindfish3 commented Nov 12, 2015

See this thread on the Processing forum. It seems like a reasonable suggestion to allow passing of a callback to the play method, that gets called when the sound file finishes playing.

I just tested the feasibility of this locally (simply replacing the separate arguments in p5.SoundFile.prototype.play with an object) and it seems relatively trivial. Of course you may well have deliberately chosen to separate the arguments. I don't know what's considered best practice with argument overloading, but presumably you could just check if the last arg is a function and assume a callback and then call that from this.bufferSourceNode.onended...

@therewasaguy

This comment has been minimized.

Copy link
Member

@therewasaguy therewasaguy commented Nov 13, 2015

Great suggestion, @blindfish3! Thank you for looking into it and bringing the issue here.

I'd lean towards adding a separate method to schedule this callback, like .onended(myCallback). I like the idea of replacing arguments with an arguments object, but I don't think we've done that in p5. A separate function is similar to the way p5.dom handles events.

Let me know if you'd like to work on this, or I can, and we can run it by the Processing forum for feedback

@blindfish3

This comment has been minimized.

Copy link
Author

@blindfish3 blindfish3 commented Nov 13, 2015

Using objects as arguments seems a fairly standard technique for dealing with large numbers of method arguments in JS - e.g. it makes it easy to override one default without having to provide all those that come before it in the argument list (and also gets around the fact that JS only supports overloading the number of args and not by arg type).

This is one reason I feel a little uneasy with p5js's apparent attempts to duplicate Processing syntax: it means you're missing opportunities to implement good practice afforded by JS's more flexible approach to Object creation. Anyway that's a separate topic for discussion (but one well worth having)...

If I had the time I'd commit to implementing this feature; but in practice I suspect it may happen more quickly if it's done at your end. My 2 year old daughter's demands mean that the time I have for personal dev work is sporadic and very limited :/

@therewasaguy

This comment has been minimized.

Copy link
Member

@therewasaguy therewasaguy commented Nov 14, 2015

ok! I've got it working, and this will be a very useful feature.

I think you're right about the options object, and other opportunities to take introduce p5 users to JS rather than just sticking to the Processing syntax. It would have a lot of benefits.

I'd want to do this as part of a larger effort so that it's consistent with other p5.sound methods, and ideally with p5.js in general.

Let's continue the discussion on the p5 github.

@jfred1979

This comment has been minimized.

Copy link

@jfred1979 jfred1979 commented Jun 30, 2016

We need a way to remove the callback. It is holding onto my sound object if I try to remove my sketch in instance mode before the sound has completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.