Refactor audio (pre)loading #2006
Conversation
There was a problem hiding this comment.
This PR also makes HowlerSound use Composition over Inheritence, something that is preffered for this codebase if I understood correctly (Though currently it is done in a dirty way).
Yes composition is very usually better, in this case there is no downside to do it and is a preferred approach (because we don't need polymorphism or deal with objects of various types).
That way it is also possible to add to audio resources an attribute to let the user preload and cache the Howl instance on the loading screen.
That would be interesting! While reading the PR I was thinking of this.
Currently I think it's too risky to ask browsers to "really load" at the beginning all the music and sound files. If you do that for a game with 20 unique musics of 5mb each, you'll probably exhaust resources, memory or something bad.
We could add an attribute to the audio resources to "Preload as a sound" or "Preload as a music" - so that we have a Howl object ready to be started whenever we need it :)
We could also add 3 advanced actions: "Preload a sound in memory", "Preload a music in memory" (these two actions are doing the same thing as the audio resources marked as "Preload as a sound" or "Preload as a music". Basically, the loading screen is running these "actions" for each resource that has the attribute) and an action "Clear the preloaded sounds/musics" (that would clear this "cache" of Howl objects).
Let me know what you think!
|
Ok, now it is ready for proper reviewing. |
Well, I assumed so siince there is the whole IDs system, and that there is this as an example in the docs: |
Ah nice, I think I did not understand how it works! If that's the case that's even better then :) |
|
Sounds better than the old system then 👍👍 |
|
Very excited about these changes :) Let's triple check that all conditions/actions are working, in particular with multiple sounds and loading of preloaded sound/musics/not preloaded. |
|
I began to try adding typescript typecheking, sadly it doesn't support well overloaded functions :/ |
Let me know what you think, but as explained in the comment I'm not a huge fan of these magic functions, they are impossible to statically type and probably a nightmare for JIT. Not sure why people like them, writing getXxx/setXxx is not really long nor verbose and very clear. Anyway, let me know what you think we should do. Keeping compatibility with Howler is interesting, but on the other hand in the future if we replace it, we'll have this kind of "fluent" api that does not look like anything else in the codebase to maintain. |
|
I think overloading is indeed not a very good idea. As we have autocompletion in the IDE and API docs (though I believe their generation is currently broken due to bad typing in gd.js), it shouldn't be really a problem for the users to find the syntax for GDevelop. In the worst sceneario, they can use the public Do you think I should also remove the chaining for setters? I think that a fluent api is better when mass-setting properties of a sound. |
Yep. Or we could add an accessor to the howler sound if really needed one day (let's not do it now). Totally ok for the fluent setters 👍 |
|
Ok, I think all that's left is testing. Silver gave me a project for testing, let's try it out :) |
And cleanup the Howler type definitions
|
Thanks for the additional cleanup, I'll make a few more tests on larger games to ensure this works well, and then we can merge this :) |
|
I vaguely remember something related to |
|
I've tested on a game and can confirm preloading works well :) Background music is noticeably faster to start when marked as preloading. That's very cool. Nothing suspicious it seems, all sounds and musics working well, |
|
Nice! Is there something left to test or change or do you think this is ready to merge? |
|
Almost ready but still checking everything that I can. Notably this: when I call the action to unload all audio, the sound effect I was playing stops (as expected), but then when I try to play it again it does not play (but other sound effects work). In other words: when unloading, any sound that was already loaded and played will stop (ok) but won't be able to be played again. Other sounds not yet loaded will work. This sounds counter intuitive because if the engine can load sounds never played before, it should be able to also load sounds that have been unloaded? |
|
I've fixed this last bug. Would be good to do another pass to be sure there is nothing else like this :) |
|
Ah nice catch, thanks for fixing it! |
|
_checkForPause is unused right? I'm removing it now. |
|
To be honest it looked so hacky that I was too scared to touch it |
|
Looks good overall! Let's merge this, seems to work correctly now with the "__default" for the play method :) Could you maybe write about this workaround in your GitHub issue on the Howler repo? |
I already did ;) |
|
Merged! :) |
PR 4ian#2006 effectively reverted the behavior of preloaded sounds to state from before 4ian#431, bringing back old memory consumption issues. For games that want to download all their assets during the loading screen, but don't want to actually decode all sounds right away because of memory consumption concerns, being able to dynamically load/unload Howler objects via events is not enough, as the application may already go out-of-memory during loading, and not doing that will cause the game to download additional assets during gameplay. This change adds a new property, "preloadAsFile", which makes GDJS request the sound asset via XHR. This puts the file into browser cache, so it can be reasonably expected to be decoded later without issuing network requests.
PR 4ian#2006 effectively reverted the behavior of preloaded sounds to state from before 4ian#431, bringing back old memory consumption issues. For games that want to download all their assets during the loading screen, but don't want to actually decode all sounds right away because of memory consumption concerns, being able to dynamically load/unload Howler objects via events is not enough, as the application may already go out-of-memory during loading, and not doing that will cause the game to download additional assets during gameplay. This change adds a new property, "preloadAsFile", which makes GDJS request the sound asset via XHR. This puts the file into browser cache, so it can be reasonably expected to be decoded later without issuing network requests.
PR 4ian#2006 made the preloaded sounds be decoded right away, bringing back old memory consumption issues from before 4ian#431. For games that want to download all their assets during the loading screen, but don't want to actually decode all sounds right away because of memory consumption concerns, being able to dynamically load/unload Howler objects via events is not enough, as the application may already go out-of-memory during loading, and not doing that will cause the game to download additional assets during gameplay. This change adds a new property, "preloadInCache", which makes GDJS request the sound asset via XHR. This puts the file into browser cache, so it can be reasonably expected to be decoded later without issuing network requests. While at it, add user visible descriptions for all preload options.
PR 4ian#2006 made the preloaded sounds be decoded right away, bringing back old memory consumption issues from before 4ian#431. For games that want to download all their assets during the loading screen, but don't want to actually decode all sounds right away because of memory consumption concerns, being able to dynamically load/unload Howler objects via events is not enough, as the application may already go out-of-memory during loading, and not doing that will cause the game to download additional assets during gameplay. This change adds a new property, "preloadInCache", which makes GDJS request the sound asset via XHR. This puts the file into browser cache, so it can be reasonably expected to be decoded later without issuing network requests. While at it, add user visible descriptions for all preload options.
PR 4ian#2006 made the preloaded sounds be decoded right away, bringing back old memory consumption issues from before 4ian#431. For games that want to download all their assets during the loading screen, but don't want to actually decode all sounds right away because of memory consumption concerns, being able to dynamically load/unload Howler objects via events is not enough, as the application may already go out-of-memory during loading, and not doing that will cause the game to download additional assets during gameplay. This change adds a new property, "preloadInCache", which makes GDJS request the sound asset via XHR. This puts the file into browser cache, so it can be reasonably expected to be decoded later without issuing network requests. While at it, add user visible descriptions for all preload options.
The current way of loading audio has a problem. There is no real preloading, so when audio is played there is a slight delay (the time for the audio file to be actually loaded). This is because we use one new Howl instance every time we want to play a sound.
This PR has for goal to cache the Howl instance of every file and reuse them. That way it is also possible to add to audio resources an attribute to let the user preload and cache the Howl instance on the loading screen.
This PR also makes HowlerSound use Composition over Inheritance, something that is preferred for this codebase if I understood correctly (Though currently it is done in a dirty way).
There is one downside though, as the Howl instances are cached, it can make the game take some more memory as they never get properly unloaded. This could be fixed with an action to clear the cache and to clear one file/resource from the cache.