Closed jbphet closed 5 years ago
This is implemented, I'm going to assign to @jessegreenberg, who is going to publish a dev release. @jessegreenberg - once the dev release is out, please assign this to @Ashton-Morris and @emily-phet to see if they have any last feedback on the implementation before we start the release process.
OK, will do, thanks!
OK, here is the latest dev version: https://phet-dev.colorado.edu/html/ohms-law/1.4.0-dev.26/phet/ohms-law_en_phet.html
From https://github.com/phetsims/ohms-law/issues/125#issuecomment-463419740, assign to @emily-phet and @Ashton-Morris for review.
@jbphet Could we have the looping sound fade a little more slowly. It feels pretty abrupt right now. Also, could the loudness of the slider click sound be lowered slightly? If it feels more efficient, we could discuss this briefly tomorrow in our usual meeting.
Otherwise, as always, I'm pleasantly surprised by this one. I must have some bad memory stuck in my mind regarding some prior looping sound. Every time I'm surprised by how much I like this one. :)
+1 for working on it during the weekly sound design meeting.
Overall I think the levels sound good. My only observation is that the slider clicks when going up on voltage slider don't click right when the battery appears causing a slight visual to audio offset feeling. If you move the slider very slowly upward with a mouse you can see it.
In #127 the slider values were constrained with a mouse so that the voltage can only increase in increments of 0.1. Just thought I would mention this in case it addresses what @Ashton-Morris mentioned in https://github.com/phetsims/ohms-law/issues/125#issuecomment-465166333.
@emily-phet, @Ashton-Morris, and I reviewed the input they made in the comments above in our 2/19/2019 weekly sound design meeting, and the decision was to change the fade out for the current to be an exponential decay instead of the currently implemented linear method. This was decided after trying a couple of different linear variations.
As for the other sound design items mentioned above, at least for now we are going to leave the click volume and timing where as it is.
@jessegreenberg - I've implemented the requested change. As far as sound design is concerned, this is ready for a new dev version or RC. I'm happy to do it, but thought I'd check in with you and see if there are other pending changes that you'd like to finish before the next release.
Awesome, thanks @jbphet. Yes, I would like to look into #132 before publishing and wait until the current dev test completes before starting the next test (https://github.com/phetsims/QA/issues/289)
Sorry this took so long to finalize, but description is ready for publication and other blocking issues are closed. @jbphet can you please verify that sound is still ready for publication?
@jessegreenberg - I just tested sound on a built version from master on Win+Chrome, Win+Firefox, Win+Edge, Mac+Safari, and iOS+Mobile Safari. The sound implementation appeared to work well on all. I also verified that the sim loads and runs but has no sound support on IE. You are go for launch! I'd suggest moving pretty quickly, since I may be making some changes to tambo later in the week.
Thanks @jbphet, an RC has been created at https://github.com/phetsims/QA/issues/313. Is there anything else that should be done for this issue or can it be closed?
Closing - will log and follow up on any sound-related issues separately.
The sound design has been approved, it's time to add it to the sim. The sound design document is available here and the most recent sound design prototype is here. Also, there is some additional information in issue https://github.com/phetsims/ohms-law/issues/120 that should be taken into account when implementing.