Skip to content

soundManager and SoundGenerator have different definitions of enabled #197

@zepumph

Description

@zepumph

We have duplicated code that determines if sound gets played in the sim:

tambo/js/soundManager.ts

Lines 218 to 226 in b3b1ef8

Multilink.multilink(
[
this.enabledProperty,
audioEnabledProperty,
simConstructionCompleteProperty,
simVisibleProperty,
simActiveProperty,
simSettingPhetioStateProperty
],

this.fullyEnabledProperty = new DerivedProperty(
[
soundManager.enabledProperty,
this.enabledProperty,
isResettingAllProperty,
viewNodeDisplayedProperty,
soundManager.extraSoundEnabledProperty,
isSettingPhetioStateProperty
],

These are similar, but a bit different, and it bit us over in https://github.com/phetsims/phet-studio/issues/15 because we would expect sounds to be disabled when the activeProperty is false, but SoundGenerator doesn't listen to that value. We were able to find a workaround there, but there are real concerns with trying to use the sim for PhET Studio and PhET-iO more generally that we don't have a complete shutoff valve for the sim sound.

@jbphet could we try to align these?

SoundGenerator is missing:
sim's activeProperty
audioEnabledProperty
simConstructionCompleteProperty,
simVisibleProperty,

SoundManager is missing:
isResettingAllProperty

I know these are joist/sim specific Properties that I'm asking to be a part of tambo code, so of course we need a more clever strategy than relying on joist directly, but I do feel like this discrepancy is awkward, and that it is only a matter of time because we come across a sound that really can only be silenced by its instance, and not globally (like a looping sound or something).

Also note that we have another list of potentially out of sync Properties we are listening to in audioManager for "speechAllowedProperty"

https://github.com/phetsims/joist/blob/3beb5f019280f88ae01e16e46bfbf75d18eaefff/js/audioManager.ts#L110-L115

My questions for @jbphet (sorry implicate you):

  1. Do these discrepancies bother you as they do me?
  2. What do you think about that above list of missing Properties above?
  3. Should we factor out a collection of these into a single DerivedProperty that can be used in all cases?

Let's hear back from @jbphet, and then determine priority and champion, and scheduling.

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions