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

Added 'end' event for groups. Fixes #69 #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sergioabreu-g
Copy link

Fixes #69 , I have added the 'end' event functionality for groups, just as it works for sounds. I've also included its correspondent test.

The 'end' event in Pizzicato is quite different from other events, since it's called by the Web Audio API itself, not by the user. Being so, only the sound receives the 'end' event from the API, using a callback to its node. I found 2 main ways to spread this event from the Pz.Sound itself through the Pizzicato library.

  • Modify the callback passed to the API to include other trigger calls aside from the one of Pz.Sound. I found this solution awful. No control/visibility at all on what your callback is doing if you modify it multiple times. Very hard to manage different groups needing each one a custom call to its trigger function. Also, this callback should be managed only by the sound itself, modifying it externally was just terrible.

  • Use the observer pattern to watch the last sound of a group, receiving the events it triggers as parameters. This way the group can easily watch its last element, when it triggers an 'end' event, the group also triggers its 'end' event. I found this to be the most pleasant way to do it. It doesn't modify or affect the current software architecture. The code is short, straightforward and easy to test/maintain. And finally, the observer pattern gives the sound 'class' capabilities that could be needed for other tasks.

Thanks for your time!

@sergioabreu-g sergioabreu-g changed the title Added 'end' event for groups Added 'end' event for groups. Fixes #69 Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant