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

When sound for the Fourier series is turned on, duck all user-interface sounds. #55

Closed
pixelzoom opened this issue Apr 15, 2021 · 10 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 15, 2021

From #54 (comment):

@kathy-phet suggested "ducking" UI sounds when Fourier sounds checkbox is checked, as in Waves Intro.
@pixelzoom is wondering if this is something supported by tambo, or is it something specific to Waves Intro.
@arouinfar says to look at https://github.com/phetsims/wave-interference/blob/master/js/common/view/WaveMeterNode.js#L225-L227.

@pixelzoom pixelzoom self-assigned this Apr 15, 2021
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Apr 15, 2021

I looked at the ducking solution in WaveMeterNode.js. It is very sim-specific. When either of the meter's probes is inside the box, "the water drops, speaker or light sound" are ducked:

    // Turn down the water drops, speaker or light sound when the wave meter is being used.
    this.duckingProperty = new DerivedProperty( [ series1PlayingProperty, series2PlayingProperty ], ( a, b ) => {
      if ( a || b ) {
        return 0.3;
      }
      else {
        return 1;
      }
    } );

It was a little difficult to hear the effects of this, so I changed 0.3 to 0 in the above code, and confirmed that those sounds are totally turned off.

So this is ducking a few sounds for which sim is fully responsible -- it owns their sound clips, can directly set their output levels, etc. There is nothing here that can be used to duck all "user-interface" category sounds. What we need is an output level control for each category of sounds. Hopefully something like that exists. I'll discuss with @jbphet.

@pixelzoom pixelzoom changed the title When sound for the Fourier series is turned on, duck all other sounds. When sound for the Fourier series is turned on, duck all user-interface sounds. Apr 15, 2021
@jbphet
Copy link
Contributor

jbphet commented Apr 15, 2021

Use soundManager.setOutputLevelForCategory and, if needed, soundManager.getOutputLevelForCategory. Heads up though: this hasn't been used much - it was created in anticipation of this sort of thing, but this is the first time it is being incorporated into a sim.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Apr 15, 2021

Ducking for 'user-interface' sounds has been implemented. When fourierSeriesSoundEnabledProperty is true, all 'user-interface' sounds are reduced to half of their perceived volume (which is 0.1 of the default output level).

We can fiddle with balancing output levels later in #54.

Closing.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jul 12, 2021

Reopening, there's a problem here. The Fourier series sound is attenuating sound on other screens. I think I've figured out how to address this, but I need a sanity check.

When the Fourier series sound is turned on, all sounds for the 'user-interface' category are attenuated. In DiscreteScreenView.js, we observe model.fourierSeriesSoundEnabledProperty:

// When sound is enabled for the Fourier series, duck all user-interface sounds.
const userInterfaceDefaultOutputLevel = soundManager.getOutputLevelForCategory( 'user-interface' );
  model.fourierSeriesSoundEnabledProperty.link( enabled => {
  const outputLevel = enabled ? 0.1 * userInterfaceDefaultOutputLevel : userInterfaceDefaultOutputLevel;
  soundManager.setOutputLevelForCategory( 'user-interface', outputLevel );
} );

In #101, we associated FourierSoundGenerator with the Discrete screen, so that it will be silenced when switching screens. But model.fourierSeriesSoundEnabledProperty does not change when switching screens. So if you switch screens while Fourier Series sound is on, the sounds on other screens will remain attenuated.

To demonstrate the problem:

  1. Start the sim.
  2. Go to the Discrete screen.
  3. Press some UI components and note sound level.
  4. Turn on sound for the Fourier series (checkbox with music notes).
  5. Press some UI components and note the decreased sound level.
  6. Go to one of the other screens. Note that Fourier series sound stops.
  7. Click some UI components and note that their sound level is still decreased, when they should be normal volume.

Observing FourierSoundGenerator's fullyEnabledProperty produces the desired result:

    // When the FourierSoundGenerator is producing sound, duck all user-interface sounds.
    const userInterfaceDefaultOutputLevel = soundManager.getOutputLevelForCategory( 'user-interface' );
    fourierSoundGenerator.fullyEnabledProperty.link( fullyEnabled => {
      const outputLevel = fullyEnabled ? 0.1 * userInterfaceDefaultOutputLevel : userInterfaceDefaultOutputLevel;
      soundManager.setOutputLevelForCategory( 'user-interface', outputLevel );
    } );

I've read the doc, and I don't really understand fullyEnabledProperty, or it's partner locallyEnabledProperty. So I'm not sure if this is the preferred pattern, or if there are situations were observing fullyEnabledProperty might cause problems.

@jbphet please advise.

@jbphet
Copy link
Contributor

jbphet commented Jul 20, 2021

I took a shot at incorporating the visibility of the screen view into the logic for when the UI sounds are ducked. Here is what I came up with. I tested it, and it seemed to work. Will this do the trick?

    // When the FourierSoundGenerator is producing sound, duck all user-interface sounds.
    const userInterfaceDefaultOutputLevel = soundManager.getOutputLevelForCategory( 'user-interface' );
    const viewDisplayedProperty = new DisplayedProperty( this );
    Property.multilink(
      [ viewDisplayedProperty, model.fourierSeriesSoundEnabledProperty ],
      ( viewDisplayed, soundEnabled ) => {
        const uiComponentSoundOutputLevel = viewDisplayed && soundEnabled ?
                                            0.1 * userInterfaceDefaultOutputLevel :
                                            userInterfaceDefaultOutputLevel;
        soundManager.setOutputLevelForCategory( 'user-interface', uiComponentSoundOutputLevel );
      }
    )

@jbphet jbphet assigned pixelzoom and unassigned jbphet Jul 20, 2021
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jul 20, 2021

I guess that would work. But was there a problem with how I solved this, by observing fourierSoundGenerator.fullyEnabledProperty ? It seems more straightforward to say "duck other sounds only when fourierSoundGenerator is actually producing sound".

I'd also like to understand SoundGenerator fullyEnabledProperty and locallyEnabledProperty.

@jbphet
Copy link
Contributor

jbphet commented Jul 21, 2021

Sorry, I should have read #55 (comment) more carefully. I thought you were still looking for a solution. I think using fullyEnabledProperty is fine.

As far as being locally versus fully enabled, locally enabled is akin to having visible set to true in a Scenery node. It could be visible, but it depends on the state of the parent nodes. For a sound generator, being locally enabled means that it will produce sound as long as all the things external to it will allow it to. In other words, it must have also been added to the sound manager, and all of its enable control Properties must have a value of true.

@jbphet jbphet removed their assignment Jul 21, 2021
@pixelzoom
Copy link
Contributor Author

Great, thanks! Then I'll stick with the current solution of observing fullyEnabledProperty. Closing.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jul 27, 2021

Reopening. Sound ducking is broken again. 'user-interface' category sounds are now ALWAYS ducked on the Discrete screen. They should only be ducked when the checkbox is checked for the Fourier series sound.

This was working in 1.0.0-dev.31 on 7/7/21.
It broke in 1.0.0-dev.32 on 7/14/21, and it's still broken in master.

Looks like listening to fullyEnabledProperty is not sufficient. A SoundGenerator can be fully enabled but not producing sound. I need to figure out what to listen to tell me when a SoundGenerator is actually producing audible sound.

@pixelzoom pixelzoom reopened this Jul 27, 2021
@pixelzoom
Copy link
Contributor Author

SoundGenerator fullyEnabledProperty is not sufficient to know whether sound is acutally being produced and is audible, because that Property is true even when the SoundGenerator is not playing. And there's no isPlayingProperty to listen to. So in the above commit, I'm using a Multilink with fullyEnabledProperty my own fourierSeriesSoundEnabledProperty. It seems to be doing the job. Closing again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants