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

fullyEnabledProperty isn't affected by parent connection's fullyEnabledProperties #120

Open
zepumph opened this issue Aug 31, 2020 · 0 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Aug 31, 2020

After a lengthy conversation where I mostly floundered in explaining a recent gotcha I was experiencing, it was discussed that a "child" sound generator may have a fullyEnabledProperty that is "true" but still won't play because a parent connection will not be fullyEnabled. For this issue, the "parent/child" relationship is based on SoundGenerator.connect calls.

My case in which I ran into this was for ratio and proportion where InProportionSoundGenerator was a parent to successSoundClip (see https://github.com/phetsims/ratio-and-proportion/blob/78cf62e7844c9d9d0ac71d27c7d2496c3b2f8e07/js/common/view/sound/InProportionSoundGenerator.js#L40. I passed an enabled control Property option to InProportionSoundGenerator, but found that the logic in SoundClip wasn't running to stop the sound when not fully enabled (which would only apply to the child's fullyEnabledProperty):

this.fullyEnabledProperty.lazyLink( fullyEnabled => {
if ( !this.loop && !fullyEnabled ) {
this.stop();
}
} );

Note that for RaP specifically: This issue doesn't block RaP because I will just end up using inheritance instead of composition

I am mainly feeling that this is a potential "gotcha" that could be confusing. In part it is because of the name "fullyEnabledProperty", which feels like "fully" might represent if the generator is connected to the base AudioContext. @jbphet accurately mentioned that an easy solution for this problem is to just pass through the enableControlProperties to the sub-component. It is a bit sloppy for large/general cases, but would probably work in most.

@jbphet and I discussed a few things to potentially do here:

  1. Increase documentation about how fullyEnabledProperty works
  2. Rename fullyEnabledProperty so that it is more clear that it only effects the enabledProperties for that exact SoundGenerator. He liked using something like "locallyEnabledProperty". I like that too!
  3. Develop a more holistic Property that takes into consideration all parents' locallyEnabledProperties to give a summary of the "rendered" enabled, or in other words if that sound generator will be "heard" (that may be a misnomer because it won't likely take gain into consideration, just connection). This could be modeled off of scenery's RendererSummary which stores info about the whole linked list to the root for any given Node in the tree.

Let me know if I can be of any more assistance here. Thanks for a good conversation!

@jbphet jbphet changed the title fullyEnabledProperty isn't effected by parent connection's fullyEnabledProperties fullyEnabledProperty isn't affected by parent connection's fullyEnabledProperties Aug 3, 2023
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

No branches or pull requests

2 participants