search

phetsims / vibe

Library for handling audio for PhET simulations. Provides cross-platform support and enables usage of base64 audio embedded in an HTML document.
MIT License
3 stars 3 forks source link

Canary warning message #31

Closed jonathanolson closed 6 years ago

jonathanolson commented 7 years ago

Sound.js?bust=1509001415460:37 [Deprecation] GainNode.gain.value setter smoothing is deprecated and will be removed in M64, around January 2018. Please use setTargetAtTime() instead if smoothing is needed. See https://www.chromestatus.com/features/5287995770929152 for more details.

It would be good to know (a) what we should use instead, (b) whether we'll need to push out maintenance releases (if old versions of sims will error out in chrome)

ariel-phet commented 7 years ago

Worthy of investigation, but @jbphet quick looks suggest it will not "break" our sims, and this feature is currently rarely used.

jbphet commented 7 years ago

I have committed a fix for this and tested on Chrome Canary, Chrome, Firefox, IE11, and Edge - all on my Win 10 machine - and saw no problems. This should prevent the warning for all future release.

I'm about 95% sure that this won't cause any problems for published sims when it moves into the main release of Chrome. We will likely see the error message on the console, but I don't think it will cause any issues (it didn't cause any audible glitches or prevent any audio from playing on the sims I tested before fixing it). I'm not sure what to do with this issue - maybe put a reminder on the calendar to follow up in a couple of months? @ariel-phet - any suggestions?

ariel-phet commented 7 years ago

@jbphet yes I think a calendar reminder would be good. Perhaps close this issue, make a calendar reminder, and open a new issue at that time if necessary.

jbphet commented 7 years ago

Okay, I'll put a reminder on my calendar and will check to see whether this feature has made it into the main Chrome release and verify that no problems occur with our sims if and when it does. If it does introduce problems, I'll either reopen this issue or log a new once, dependent upon the details. Closing (at lease for now).

pixelzoom commented 7 years ago

Searching for "gain.value", I still see the deprecated feature in john-travoltage (21 occurrences) and phet-io-wrapper (42 occurrences). Shouldn't those occurrences also be replaced by setTargetAtTime?

jbphet commented 7 years ago

Over slack, @pixelzoom asked which sims are affected by this, and the answer is any that use the Sound.js type, either directly or through the inclusion of another library that includes it. For instance, vegas (the game support library) has a type called GameAudioPlayer.js that is used in most if not all sims with game screens, and vegas/GameAudioPlayer uses vibe\Sound.js, so sims that use GameAudioPlayer would manifest this issue.

@pixelzoom also pointed out that this pattern appears elsewhere in the code. Good observation. I've logged two new issues to make sure that it is corrected there as well. Please see:

phet-steele commented 6 years ago

My Canary sounds fine as of this writing. Assigning to myself to check again once this deprecation is in the stable release.

phet-steele commented 6 years ago

My computer is flipping out! Hooray for a double comment!

phet-steele commented 6 years ago

@jbphet I should mention there is another warning too. How bad is this one?

[Deprecation] AudioParam value setter will become equivalent to AudioParam.setValueAtTime() in
M65, around March 2018  See https://webaudio.github.io/web-audio-api/#dom-audioparam-value for 
more details.
jbphet commented 6 years ago

@phet-steele - It is essentially the same warning, just in more general form, and they are saying that it won't stop working, so there should be no detrimental effects on our sims. Still, I'll make it a priority to address this, since I think it should be pretty straightforward, and I know it's disconcerting to see these warnings.

jbphet commented 6 years ago

@phet-steele - what were you running when you saw this?

phet-steele commented 6 years ago

@phet-steele - what were you running when you saw this?

The production version of john-travoltage.

jbphet commented 6 years ago

The production version of john-travoltage, and any other currently published sim with the code prior to the fix, will print out this error. The only thing we could do about it is to publish maintenance releases, but I feel reasonably confident (as described above) that these warning do not indicate that the sim will behave incorrectly or sound bad.

I'm going to leave this open so that I remember to fix all usages of direct value setting for Web Audio, but I'm going to lower the priority since I thing much of this will be taken care of as sonification is integrated into PhET sims.

jbphet commented 6 years ago

I tested this on the currently published version of John Travoltage, which is v1.3.1, and the current version of Chrome, which on my machine is 64.0.3282.167 (Official Build) (64-bit) (cohort: 64_167_win), and the warning messages appear on the console, but the audio appears to work fine. This is what I expected.

I am interpreting this to mean that there is no need for maintenance releases to correct this problem.

This issue will remain open until I've addressed all places in the code base where Web Audio values are being directly set.

jbphet commented 6 years ago

I had a reminder on my calendar to check this, and I just did, and the warning message appears to have gone away. I checked this on the 1.3.1 version of JT using the URL https://phet-dev.colorado.edu/html/john-travoltage/1.3.1/john-travoltage_en.html?sonification=jostle, and used the current version of Chrome - 67.0.3396.99 - as well as the current version of Canary - 69.0.3486.0. The message did not appear in the console for either of these.

jbphet commented 6 years ago

I just searched through the code base for all instances of .gain.value = and found 56 matches in 26 files. All of these files are in either the prototype sonification code in the john-travoltage repo or the phet-io-wrapper-sonification repo. The former is due to be replaced in about a month as the 'real' sonification is added. The latter is prototyping code used for demonstrating sound designs, and will not be shared with users. It's my opinion that it isn't work the time and effort to replace all of these calls, since they don't break anything and the warning has gone away. The worst case is that we hear some abrupt gain transitions in the prototype sound designs and, if this is bothering us, I'll fix them. Bottom line: I'm going to close this issue and address anything that comes up on a case-by-case basis (which I think is unlikely) and not take the time to address them all.