C2LC-253: Ensure that Tone is started on the first user action.

[the-t-in-rtf]

See C2LC-253 for details.

[simonbates]

I'm still seeing the warnings in Chrome with this branch. I think what we'd need to do is call the Tone.js start(), as you are doing now, but wait on the returned Promise before creating audio objects. At the moment, we are still making the AudioManager instance at App construction. And if we move to this lazy creation of the AudioManager, we will need to manage the possibility of multiple initialisations happening.

[simonbates]

The general approach seems sound to me: create a Promise that we can wait on and then tie that Promise resolution/rejection to the Promise from starting Tone.js.

However, I'm still seeing the warnings in Chrome in the console. The warnings are generated at page load, before any user interaction. It looks like we need to delay more of the work until Tone.js has been started.

I also have some notes (which may not be relevant after refactoring to delay more work):

• In playAnnouncement() and playPitchedSample(), you check if this.toneStartPromise is truthy -- this isn't necessary as this.toneStartPromise is created at construction
• We have 2 Promises with the same name, toneStartPromise: the one that we create at construction, and the one returned from tone.start() which we tie to our Promise in startTone(); we could use different names for these Promises that make their relationship clearer

[the-t-in-rtf]

Just to be sure I wasn't missing anything, I tried making a double-promise system. The first promise was for Tone itself, at which point I would try to create the Player and Sampler grades, and then trigger a separate promise indicating that the audio manager was ready to do its job. In practice this resulted in Player and Sampler grades not having enough time to load their sounds before that check was hit, so consistently the first sound would not play.

I went back to a single promise approach where I create the Player/Sampler classes ahead of time but don't try to start them until Tone has started. I haven't seen any further complaints about user interaction and the first sound (announcement or movement) plays consistently for me. My latest work addresses the naming concerns and removes the checks to see if the promise has been set.

I also relocated the promise check to playSoundForCharacterState. When you commented earlier on making playPitchedSample an internal method, what approach did you have in mind?

[the-t-in-rtf]

In practice this resulted in Player and Sampler grades not having enough time to load their sounds before that check was hit, so consistently the first sound would not play.

Just to comment further, sadly there is no promise infrastructure within Tone.js to wait for the load to complete. We could create something with polling, but if we can live with what I have that seems less fiddly.

[simonbates]

This PR is still not addressing the issue: the warnings in Chrome continue to be displayed at page load.

You have moved the code to set up the audio objects to a new method createSoundInfrastructure(), but that method is still being called from the constructor, rather than after Tone has been started.

Could you also edit this PR to change the target branch to develop-0.6 and merge the current develop-0.6 into your C2LC-253 branch and resolve the conflicts? Thanks.

[the-t-in-rtf]

@simonbates, I have updated with develop-0.6 and rebased the pull.

I previously tried tying the sound initialisation to the start promise, but had trouble with the first sound that should be played not playing. I reset things back to the earlier approach and committed for your review.

[simonbates]

I'm still seeing the "The AudioContext was not allowed to start. It must be resumed (or created) after a user gesture on the page." warnings on page load in Chrome Developer Tools console when I open https://deploy-preview-96--c2lc-build.netlify.app/

[the-t-in-rtf]

OK, so we're now in a state where the audio components are not created until after the first user action. For keyboard navigation this is fine, as there's generally enough time to load the actual sounds before the user gets to something that should trigger a sound. However, it's pretty easy using a mouse to click something and not have the first sound play. We need to discuss how to handle this, perhaps adding a promise wrapper with polling so that we can start the sound once it's loaded.

We also need to figure out a couple of issues. First, @chosww was reporting not being able to hear sounds at all when using the preview site.

Second, and more critically, even though I don't see any errors locally, I do indeed see "The AudioContext was not allowed to start" messages when loading the deployed preview site. I added a breakpoint at the point where I call Tone.start, and confirmed that I wasn't managing to trigger that on startup or anything. Looking at the stack traces, I can't see anywhere in our code that's actually performing the action that results in the error.

[the-t-in-rtf]

I suspect that Tone's tests are getting bundled and are run when they're sourced.

[the-t-in-rtf]

Those files are "tests" as in "checks", not tests as in unit, integration, etc. The files come from the package standardized-audio-context which is included by Tone.js.

[the-t-in-rtf]

Pretty sure we're hitting this known and unfixed issue with Tone.js. His assertion is that these are warnings and should not affect playback. I will work on at least getting the first sound working.

[the-t-in-rtf]

OK, I added the promise wrapper I had in mind around the Player and Sampler classes from Tone, which resolves a promise once the sound is loaded. It does what I hoped, ensures the first sound plays, and there's no noticeable delay in playing the first sound. I suspect we were checking the loaded variable milliseconds before the sound loaded.

I suspect we've eliminated as many of the warnings as we can at this point, as far as I know this is ready for review.

[the-t-in-rtf]

Once deployed, the load time for the sounds goes up high enough that there is a noticeable delay if you immediately trigger a sound. We should review together and discuss whether / how to address it.

The easiest way to reproduce is to shift+reload and click one of the action buttons with the mouse. The "move" announcement will be delayed.

[simonbates]

As we discussed this morning, we won't continue working any more on this issue for now. It looks like the warnings are triggered by Tone.js code that is out of our control. If Tone.js is changed in the future to remove these warnings, at that time we can see if we need to make any code changes.

[the-t-in-rtf]

@sbates-idrc, I think we may need to revisit this, as the work I'm doing with Julien is often resulting in unplayed first sounds.

[sbates-idrc]

@sbates-idrc, I think we may need to revisit this, as the work I'm doing with Julien is often resulting in unplayed first sounds.

Interesting. We also experienced issues with delayed sound playing due to audio file download time. Is it possible that what you are seeing with Julien is caused by that?

[the-t-in-rtf]

I don't think so, the new approach doesn't use any samples at all.

[sbates-idrc]

Got it.

[the-t-in-rtf]

As discussed, we will handle this (if needed) in the alternate tuning work.