Mix and Balance sound levels - Githubissues

phetsims / mean-share-and-balance

"Mean: Share and Balance" is an educational simulation in HTML5, by PhET Interactive Simulations.

GNU General Public License v3.0

2 stars 1 forks source link

Mix and Balance sound levels #212

Closed marlitas closed 3 months ago

marlitas commented 5 months ago

Once sounds have all been implemented we need to balance the sound levels and make sure the mix is sounding good.

https://github.com/phetsims/mean-share-and-balance/issues/144 is the parent issue.

jbphet commented 5 months ago

@Ashton-Morris and I met today for ~1 hr and adjusted the mix prior to creating a dev version for interviews (see https://github.com/phetsims/mean-share-and-balance/issues/221).

marlitas commented 3 months ago

@jbphet is going to reach out to @Ashton-Morris for the best way to proceed here.

jbphet commented 3 months ago

@Ashton-Morris - We are ready for the pre-release mix step. Please let me know when you're available for this and how you'd like to do it. My preference would be to work on it together over Zoom so we could get it done quickly, but we could also do the thing where I mark up the code and you search through it and make adjustments on your own.

jbphet commented 3 months ago

@Ashton-Morris and I just met on this over Zoom, and for the most part he thinks it sounds good. He had two suggestions, but neither of them are trivial to change, so I'll note them here and start investigating, and will discuss more with the team once I know how hard they will be to address.

On the 4th screen, when switching back and forth between the fixed and movable fulcrums, there are often two sounds played - one for the radio button interaction and one for the tilting of the beam. This seems like a bit much.
Also on the 4th screen, the sound made when dragging a soccer ball on the field is a bit loud.

Number 1 is tricky because it would be hard to mute the tilt sound since it is so tied in to the movement of the beam. It may be best to address it by having the tilt be instantaneous instead of animated in this case, but I'm not sure we want that, so I'd need to run it by the design team.

Number 2 is tricky because the sound is coming from the code in the soccer-common repo, and the code appears to have been implemented without a means of adjusting the sound level. I'll look into adding such an option and go from there.

jbphet commented 3 months ago

I added the ability to set the output level in the static class NumberTone, though I'm not very happy about it. This doesn't seem to me like it should be a static class - it should be an instance with default volume levels that can be adjusted as needed. However, I don't have the time or inclination to refactor this code, so am willing to live with this for now.

jbphet commented 3 months ago

I've created a patch that will cause the balance beam to tilt instantly when switch between movable and fixed fulcrum, which could potentially be used to address number 1 in the comment above. I'll share it with the team tomorrow and decide whether to use it.

jbphet commented 3 months ago

I've committed the changes that prevent the tilt sound from being played when switching between the fixed and movable fulcrum. This turned out to be a bit more involved than I'd originally thought, so I'm assigning to @marlitas for review. The reason for the additional complication is that when I really started scrutinizing the behavior of the tilt sound, it was playing in a number of situations when it shouldn't. One of the primary reasons for this was that the positions for the two ends of the balance beam where being set independently, which would cause a transient tilt change, which would trigger sound generation. To remedy this, I had to make the left and right Y positions of the beam into a single atomic unit whose value could be set in one fell swoop.

I implemented this and tested it a fair amount in the PhET brand version and also in the state and studio wrappers for phet-io. This changes the phet-io tree a little bit, but I don't think it warrants additional review (@marlitas - please let me know if you disagree).

Assigning to @marlitas for review.

jbphet commented 3 months ago

By the way, that comment above is for the last sound-mix-related item, so if it all looks acceptable, this can be closed.

marlitas commented 3 months ago

I'm wondering if BalanceBeamEndpointYValue should be it's own file? Especially since it's creating it's own IOType as well... Not sure how strongly I feel about that. Can be swayed either way.
I did some phet-io memory testing since I wasn't sure exactly how disposal needed to be handled for PhET-iO Types, but then I realized the class isn't even a PhET-iO object, so that seems fine and the memory testing didn't show anything.

I think this is good to go. Thanks for doing this work @jbphet!