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

output level is not being set to 0 for some joist buttons #680

Closed
pixelzoom opened this issue Dec 1, 2020 · 6 comments
Closed

output level is not being set to 0 for some joist buttons #680

pixelzoom opened this issue Dec 1, 2020 · 6 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Dec 1, 2020

Fourier has sim-specific sound, and (for now) we want to disable all of the sounds that come with UI components. So I've added this to fourier-making-waves-main.js:

soundManager.setOutputLevelForCategory( 'user-interface', 0 );

This silences most UI components. But I'm still hearing sounds associated with the home screen buttons and navigation bar screen buttons.

For the home screen buttons, I'm only hearing the sound when a new screen is actually selected, not the sound when a new icon is selected.

For the navigation bar screen buttons, I'm not hearing the sound for the home button, just the buttons for the non-home screens.

Everything else in the sim is silent. (PhET menu, Reset All button, other UI components,...)

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 4, 2021

Raising the priority of this because it's a requirement for Fourier 1.0, and it's annoying when I'm fuzz testing. Labeling for dev meeting so we discuss who should do this and when.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 10, 2021

This is continuing to annoy me, so investigating... And I think I've discovered the cause of the problem.

Screen selection uses ScreenSelectionSoundGenerator. Unlike the sounds for other UI components, this is a SoundClip, not a SharedSoundClip. It therefore does not register itself as categoryName: 'user-interface'.

Is there some reason why this should not be converted to a SharedSoundClip, using the same pattern as (e.g.) checkboxCheckedSoundPlayer.js?

@pixelzoom
Copy link
Contributor Author

Worked on this with @jbphet, fixed in above commit. Closing since we collaborated.

@samreid
Copy link
Member

samreid commented Feb 16, 2021

In phetsims/circuit-construction-kit-common#667 (comment) @arouinfar said:

@samreid can you add 991d6a4 to the list? There are joist sounds in the latest rc, which is odd because all other UI sounds are disabled, see #680

We know this problem affects at least 2 release branches (cck-dc and cck-dc-virtual-lab). While working on phetsims/circuit-construction-kit-common#667 @jonathanolson recommended using the maintenance release process to update all affected release branches (including but not limited to the CCK ones mentioned).

@jbphet or @pixelzoom can you please take the lead? Please also coordinate with @jessegreenberg since there is one maintenance release in process and another one recommended in #658 (comment)

Marking as high priority since this part is blocking CCK RC.2.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Feb 16, 2021

Sorry, I don't have time to take the lead on this.

This presumably affects only sims that were turning off the 'user-interface' category of sounds. Searching for setOutputLevelForCategory( 'user-interface', I find only 5 occurrences: Fourier (1) and the CCK suite (4). Fourier is unpublished, so I would guess that only CCK is affected. @samreid is the responsible dev, so can he sort out what release branch(es) need to be patched?

The fix is 991d6a4, trivial even if it can't be cherry-picked.

@pixelzoom pixelzoom assigned samreid and unassigned pixelzoom and jbphet Feb 16, 2021
samreid added a commit to phetsims/circuit-construction-kit-dc that referenced this issue Feb 16, 2021
samreid added a commit to phetsims/circuit-construction-kit-dc-virtual-lab that referenced this issue Feb 16, 2021
@samreid
Copy link
Member

samreid commented Feb 16, 2021

Thanks, I ran this maintenance script to update the CCK 1.2 sims:

Maintenance.reset()
Maintenance.createPatch( 'joist', 'https://github.com/phetsims/joist/issues/680', 'patch-1' )
Maintenance.addNeededPatch( 'circuit-construction-kit-dc', '1.2', 'patch-1' )
Maintenance.addNeededPatch( 'circuit-construction-kit-dc-virtual-lab', '1.2', 'patch-1' )
Maintenance.addPatchSHA( 'patch-1', '991d6a448a869bff9ed7836676eae9fdd2c9aeed' )
Maintenance.applyPatches()
Maintenance.updateDependencies()

Closing.

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

3 participants