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

Decreasing Number of Bodies sound heard on Reset All #307

Closed
Nancy-Salpepi opened this issue Dec 4, 2023 · 14 comments
Closed

Decreasing Number of Bodies sound heard on Reset All #307

Nancy-Salpepi opened this issue Dec 4, 2023 · 14 comments
Assignees
Labels

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air M1 chip

Operating System
14.1.1

Browser
Safari 17.1

Problem description
For phetsims/qa#1008, on the Lab Screen, pressing Reset All with more than 2 bodies present will trigger the 'Decreasing Number of Bodies' sound.

Steps to reproduce

  1. On the Lab Screen, use the increment button to change the number of bodies OR change the orbital system in the combobox
  2. Press Reset All

Visuals

soundsonResetAll.mov
Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪My Solar System‬ URL: https://phet-dev.colorado.edu/html/my-solar-system/1.3.0-dev.7/phet/my-solar-system_all_phet.html Version: 1.3.0-dev.7 2023-12-01 16:11:20 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.1 Safari/605.1.15 Language: en-US Window: 1359x712 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
@AgustinVallejo
Copy link
Contributor

This sound is also coming up when selecting a new preset, and it shouldn't be. Looking into it.

@AgustinVallejo
Copy link
Contributor

@Nancy-Salpepi can you review this and close when ready? Thanks!

@Nancy-Salpepi
Copy link
Author

Sounds good to me now--I don't hear it on Reset All or when changing the orbital system.
Closing.

@Nancy-Salpepi
Copy link
Author

This sound is also coming up when selecting a new preset, and it shouldn't be. Looking into it.

This part is fixed, but on main I just realized that the sound made when selecting an orbital preset is missing. I just hear the combobox closing sound.

@Nancy-Salpepi Nancy-Salpepi reopened this Dec 7, 2023
@AgustinVallejo
Copy link
Contributor

This is weird and potentially a common code issue. I'm seeing that although the published version has a custom comboBox sound, neither MSS or Kepler's baseline do, nor sims like GeometricOptics, but I'm seeing Balancing-Chemical-Equations still uses the selection sound instead of the closing one. Even though there's nothing in their construction that seems to indicate a difference.

Any thoughts @pixelzoom ??

@pixelzoom pixelzoom self-assigned this Dec 7, 2023
@pixelzoom
Copy link
Contributor

pixelzoom commented Dec 7, 2023

@Nancy-Salpepi said:

... the sound made when selecting an orbital preset is missing

I'm not experiencing this problem -- when I select an item from orbitalSystemComboBox, I hear a sound.

And ComboBox has a default sound, so it's not necessary to specify a sound unless you want to change the default.

@AgustinVallejo
Copy link
Contributor

But that's not it. The ComboBox has three sounds: openedSoundPlayer, closedNoChangeSoundPlayer and the selection sound is defined by soundPlayer, all of them found in ComboBox.ts. If you compare the selection sounds in MSS-published with the baseline or with main, they are different. But I suspect this change might have come from common code.

Again, Balancing-Chemical-Equations still has a selection sound, but other sims do not.

Should we do anything about this or leave it be?

@pixelzoom
Copy link
Contributor

pixelzoom commented Dec 7, 2023

Oh, I see. Published version 1.2.3 (and baseline) are using a custom sound for the orbitalSystemComboBox. That was probably lost when we factored out OrbitalSystemComboBox -- I don't see it setting any custom sounds.(OrbitalSystemComboBox was formerly named LabModeComboBox.)

EDIT: No, that's not correct. The published version was using the default, a tone with decreasing pitch for each item. And baseline 1.3.0-dev.1 has the problem reported here -- same sound for every item.

@pixelzoom
Copy link
Contributor

pixelzoom commented Dec 7, 2023

Sounds can be provided by the items, see ComboBoxListBox:

    // Make a list of sound generators for the items, using defaults if nothing was provided.
    const itemSelectedSoundPlayers = items.map( item => {
      return item.soundPlayer ?
             item.soundPlayer :
             multiSelectionSoundPlayerFactory.getSelectionSoundPlayer( items.indexOf( item ) );
    } );

OrbitalSystemComboBox createItem is not providing any sounds. So we should be getting the default sounds. But it doesn't sound like what multiSelectionSoundPlayerFactory should return.

@pixelzoom
Copy link
Contributor

I compared geometric-optics published to main. The published version has the default sound -- a different tone for each item. main has the same tone for all items.

So something was changed in main, probaby unintentionally.

@pixelzoom
Copy link
Contributor

Posted to Slack#developer:

Has anyone been working on the sounds for combo box — ComboBoxListBox and/or multiSelectionSoundPlayerFactory? In published versions of sims, the default is to play a different pitch for each listBox item, decreasing in pitch from top-to-bottom of the listbox. In main, the same sound is being played for all items.

@pixelzoom
Copy link
Contributor

pixelzoom commented Dec 7, 2023

Since we've established that this is a common-code problem, I opened phetsims/sun#861. @jbphet volunteered to investigate. I'll put this issue on hold until that issue is resolved.

@pixelzoom
Copy link
Contributor

Assigning to me and @AgustinVallejo to keep an eye on phetsims/sun#861.

@pixelzoom
Copy link
Contributor

phetsims/sun#861 has been addressed (thanks @jbphet!) and is behaving nicely in this sim. So this issue can be closed.

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

No branches or pull requests

3 participants