Initialize shared sound clips at sim construction time instead of load time

[jbphet]

I'm working to improve the tambo API by addressing https://github.com/phetsims/tambo/issues/189, and I keep running into loading dependency issues with the shared sound players (e.g. resetAllSoundPlayer). The more I look at this - and try to work around it - the more I think my original approach here was a bit flawed. The code as of this writing is trying to decode and load the audio buffers at module load time, This can be problematic because not all modules are guaranteed to be loaded when trying to initialize these sounds. This is exactly the problem I'm running into. The error in the console looks like this:

Before going any further on https://github.com/phetsims/tambo/issues/189, I think I should revamp the shared sound players such that they are functions that return a SoundClipPlayer instance instead the players themselves. The first invocation of the function will cause the instance to be created, subsequent invocations will return that already-created instance. By doing it this way I'll move the creation and initialization of the shared sound players to the construction phase of the sim, which should be less problematic.

[jbphet]

I have created a centralized sharedSoundPlayers object from which the shared sound players can be retrieved. This has less code duplication than the previous approach, and loads up the sounds the first time a given sound player is requested.

I tested this in the tambo demo and in a number of sims. I'll keep an eye on CT and, if all looks good, I'll assign this to someone for review.

[jbphet]

Assigning to @pixelzoom for review (as was discussed in today's developer meeting). The focus should be on the change made in the tambo repo, and perhaps some spot checking of sim changes, though these will be pretty similar.

[pixelzoom]

I reviewed the primary change in https://github.com/phetsims/tambo/commit/27ff7a5e23f5f7c6ef6f6a371fd35a2dc03cc4e5, and spot checked a few of the sim-specific changes in the other commits. Deferring creating of SoundClipPlayer instances seems like a good idea, and the implementation provides a nice API. It will be interesting to see how large sharedSoundPlayers.ts gets in the future, but it's reasonable for now.

One questions I have is about naming. This is about creation of SoundClipPlayer instances. So why have you used some names that indicate "sound player" instead of sticking with consistent "sound clip player"? e.g: sharedSoundPlayer, SharedSoundPlayerName, sharedSoundPlayerInstanceMap,

[pixelzoom]

Also... Why does SoundClipPlayer not implement TSoundPlayer? I discovered this when I looked at nullSoundPlayer. Then I could get on board with the "shared sound player" name. But then I'd also expect sharedSoundPlayers.get to be:

-  get( sharedSoundPlayerName: SharedSoundPlayerName ): SoundClipPlayer {
+  get( sharedSoundPlayerName: SharedSoundPlayerName ): TSoundPlayer {

[jbphet]

Excellent points @pixelzoom. I think I actually meant to change SoundClipPlayer to extend TSoundPlayer and use that type in sharedSoundPlayers, but just forgot to do so as part of this. So I'm glad you brought it up.

I've made the types more consistent with the names and fixed up one common code element that needed to be adjusted for this. Please have another look and, if you're good with it and have no more comments, feel free to close.

[pixelzoom]

Looks great. One minor doc fix in https://github.com/phetsims/sun/commit/43ac9f1394995c5b5a1b9f6969c2b3ede9bab509. Closing.

[marlitas]

Looks like CloseButton accidentally got generalOpen in https://github.com/phetsims/scenery-phet/commit/9e8eea2da52306fcd31871b2fbf2f36bc82e2b7b. I went ahead and fixed that. I don't think there's a need to reopen.