Should SoundClip.reset() call SoundClip.stop()?

[zepumph]

I was surprised to not find this implemented in SoundClip. @jbphet, do you think that this is a valuable addition for completeness? The main use case I can think of is with the reset all button. It seems like it would be buggy if sounds bleed into after that button is pressed. For example, something like the lightning sound in the demo.

I'm thought about this as a result of subtyping a SoundClip (see https://github.com/phetsims/ratio-and-proportion/issues/197), and implementing a reset function for added properties. Even though the sound is short, I was surprised I didn't find a supertype that I should be calling.

What do you think?

[jbphet]

I was surprised to not find this implemented in SoundClip.

If I understand you correctly, by "this", I think you're saying that you're surprised that these is no reset function in SoundClip. There hasn't been a need so far, and while I'd be fine with just calling stop in a reset function for long sounds (which are fairly rare), I'm reluctant to have a reset function that just calls stop and does nothing else. If we were to add a reset function here, it should match the spirit of other reset functions and restore any state that has changed since the instance was created, which for SoundClip would be output level, playback rate, and a handful of other parameters. Again, this hasn't been something that I've encountered a need for so far.

In the example that you describe, I think I'd recommend just adding a call to stop in the subclass's reset function and call it good.

[zepumph]

What about adding a simple reset to SoundClip that just calls stop, and then if in the future we need to add to it to fill it out more (like you were discussing), we can. My only hesitation is that calling stop in a subtype reset is less future proof. When I go back to that line of code in 2 years when we have a built-out reset function for SoundClip, I have to know if I specifically meant to only stop it, or if I wanted to reset it. Thoughts?

[jbphet]

I'm afraid I disagree that this approach would be more future proof. If we create a reset method that only calls stop, and then fill it out more in the future, it could break existing usages. I also don't think it's that unusual to call something like stop in a reset function if that's what you need to do. I would argue that it makes the code more clear, not less. I just did a quick scan of the PhET code base for reset methods that do things other than calling reset on other things, and very quickly found this in CurvePoint:

  reset() {
    this.y = this.initialY;
    this.savedYValues = [];
  }

I don't find anything wrong with this. So, my opinion is that we either create a full-fledged reset method for SoundClip, which would probably imply also adding one to its base class (SoundGenerator), or you just call stop in this particular situation.