godot
godot copied to clipboard
Add sample playback support
⚠️ Here be dragons ⚠️
When this PR will be merged for 4.3, this feature will be marked as experimental and could change in 4.4.
tl;dr
This PR adds (back) the concept of samples to the Godot Engine. Currently, the PR enables only the web platform to play samples.
It principally fixes #87329, as that issue would plague any non-threaded web releases with crackling audio.
Example
| Single-threaded web example using streams (old way) |
Single-threaded web example using samples (this PR) |
|---|---|
| https://adamscott.github.io/2d-platformer-demo-main-thread/ | https://adamscott.github.io/2d-platformer-demo-main-thread-samples/ |
Introduction
Godot uses streaming to mix game audio. Each active stream is registered and then the engine mix on-the-fly the needed audio frames together to output audio based on the audio latency parameter. It works very well on modern platforms.
Samples are another way to handle sound instead of mixing streams. Instead of handling mixing sound and music by the game processes, it relies on off-loading it to the host system. While it doesn't permit full access to the mixing apparatus, it's super useful on systems that don't have a lot of processing power.
To use samples, you register a sample, and then tell the system to play it when needed. And to stop it. It's like a music player, you set the file, then you click on play. You don't control how the software do it, but you know it does.
Godot used to have samples back in Godot 1 and 2, especially to support platforms like the PSP, and the web (thanks to the Web Audio API).
As newer console platforms let developers handle their own mixing logic and that SharedArrayBuffers were introduced in browsers (permitting WebWorkers (web threads) to share memory with the game), samples support was dropped from Godot. Everything was fine.
Anyway, the implementation was somewhat lacking. You had to specifically want to play samples, you couldn't use common nodes to play both streams and samples.
The problem
But on the web platform, Spectre and Meltdown happened. And it completely changed where SharedArrayBuffers were able to be used. Enter "cross-origin isolated" websites, where it's impossible to contact other websites or display ads, and complicating hosting of simple games, greatly reducing the appeal for our web builds.
Hence the work on #85939 in order to compile Godot to run on the main thread. This enables exporting Godot games on the browser without having to cross-origin isolate your website. Unfortunately, this brought an unexpected issue: software mixing is pretty much incompatible with single-threaded games. Especially running on older/less powerful hardware.
Wanna hear for yourself? Try the single-threaded platformer demo (without this PR applied) on your phone or on a computer that doesn't have a great CPU.
The investigation
My colleague @Faless and I considered every solution imaginable: augmenting latency for the web, traced the processes on the web and on mobile and refactor the AudioWorklet processing the audio. But alas. Nothing substantial could have been done.
The only solution we found was to resort to Web Audio samples.
And it's not uncommon for web game engines. We were the uncommon ones not using web audio samples. So, a few weeks ago, I began work on this PR.
The sole requirement: seamlessness
My main focus was to reuse as possible as many features that already exist. It means that in order to play samples, I wanted the UX to keep as close as possible to existing tools.
Godot strives itself to offer the same experience for every target that it exports to. Imagine making the developer choose between having samples for the web export and streaming nodes for the rest. And having to manually add or remove nodes based on the platform with scripts.
This has such a big impact that it's a clear no go for us. We don't want that poor UX.
The solution
My solution is a big hack. (But it does works wonderfully.)
The idea is to reuse all the existing stream nodes and systems. And make the stream elements capable of producing samples.
This story begins with the new project setting audio/general/default_playback_type (hidden currently in the advanced options). Usually, it should stay with the value "Stream", as normally, that's how Godot works currently. But the magic happens with audio/general/default_playback_type.web set as "Sample".
That's because AudioStreamPlayer, AudioStreamPlayer2D and AudioStreamPlayer3D now have a new property called playback_type, which is set by default to... "Default". That's where the magic happens! On standard exports, the nodes will be defined as "Stream", but on web exports, "Sample" will be used instead!
The magic operates behind the scenes though.
The man behind the curtain
Essentially, when a stream is considered a "sample", it doesn't get mixed at all in the mixing phase. Instead, it relies on callbacks by the StreamPlayer nodes.
The StreamPlayer nodes, when their play() method is called, are calling internally AudioServer::start_sample_playback(). All the AudioServer does is to call AudioDriver::start_sample_playback(). If the driver doesn't implement that function, it just doesn't play any sound. But if it does, the driver can now tell the backend to play that sound.
The same thing happens for stop, pause, etc. You can even update the sample, like when the position of the node changes!
Isn't this fascinating?
Registering samples
Before playing the samples, it's important to register them first.
If played without previous registration, the player will make sure to register it first. Though, it's recommended to register manually streams. That's because, on single threaded games, memory transfer is synchronous, so it may make your game stutter. You register a stream as a sample by calling this method:
# optional step
const my_stream_resource = preload("res://assets/my_stream.wav")
AudioServer.register_stream_as_sample(my_stream_resource)
Under the hood, Godot will call the mix() method of the stream playback for the entire duration of the clip. This makes it so that it's possible to play any type of sound media that Godot supports (WAV, mp3, ogg vorbis).
Is it really seamless, though?
These demos were exported to the web (single-threaded) using samples without ever touching the project nodes, resources, nor files.
| Demo title | Playable link | Source |
|---|---|---|
| 2D Platformer | Play | Link |
| Dodge the Creeps | Play | Link |
| 3D Platformer | Play | Link |
| Hell of Mirrors | Play | Link |
| Catburglar | Play | Link |
Bugs yet to fix before merge
- [x] ~~Buses don't chain properly~~
- [ ] Only forward loop is supported (fix may not make it to the final release)
Known limitations
- Effects don't apply (will certainly not be part of the initial release and GDScript based effects cannot be used)
Technical diagrams
Registering and playing samples
sequenceDiagram
participant Script
participant AudioStreamPlayer2D
participant AudioStreamPlayerInternal
participant AudioServer
participant AudioDriverWeb
participant JavaScriptAudioLibrary
Script ->> AudioStreamPlayer2D: set_stream(Ref<AudioStream>)
AudioStreamPlayer2D ->> AudioStreamPlayerInternal: set_stream(Ref<AudioStream>)
Script ->> AudioServer: register_stream_as_sample(Ref<AudioStream>)
activate AudioServer
AudioServer ->> AudioServer: register_sample(Ref<AudioSample>)
AudioServer ->> AudioDriverWeb: register_sample(Ref<AudioSample>)
deactivate AudioServer
AudioDriverWeb ->> JavaScriptAudioLibrary: godot_audio_sample_register_stream()
Script ->> AudioStreamPlayer2D: play()
activate AudioStreamPlayer2D
AudioStreamPlayer2D ->> AudioStreamPlayerInternal: play_basic()
activate AudioStreamPlayerInternal
opt _is_sample() && stream->can_be_sampled() && stream_playback->sample_playback is null
AudioStreamPlayerInternal ->> AudioServer: is_stream_registered_as_sample(Ref<AudioStream>)
activate AudioServer
deactivate AudioServer
AudioServer ->> AudioStreamPlayerInternal:
opt stream is not registered
AudioStreamPlayerInternal ->> AudioServer: register_stream_as_sample(Ref<AudioStream>)
activate AudioServer
AudioServer ->> AudioServer: register_sample(Ref<AudioSample>)
AudioServer ->> AudioDriverWeb: register_sample(Ref<AudioSample>)
deactivate AudioServer
AudioDriverWeb ->> JavaScriptAudioLibrary: godot_audio_sample_register_stream()
end
AudioStreamPlayerInternal ->> AudioStreamPlayerInternal: set stream_playback->sample_playback
end
AudioStreamPlayerInternal ->> AudioStreamPlayer2D: Ref<AudioStreamPlayback>
deactivate AudioStreamPlayerInternal
opt stream playback sample exists
AudioStreamPlayer2D ->> AudioStreamPlayer2D: Update stream_playback->sample_playback for specific AudioStreamPlayer2D stuff
AudioStreamPlayer2D ->> AudioServer: start_sample_playback(Ref<AudioSamplePlayback>)
deactivate AudioStreamPlayer2D
AudioServer ->> AudioDriverWeb: start_sample_playback(Ref<AudioSamplePlayback>)
AudioDriverWeb ->> JavaScriptAudioLibrary: godot_audio_sample_start()
end
Samples and streams
classDiagram
namespace AudioStreams {
class AudioStream
class AudioStreamWAV
class AudioStreamPlayback
class AudioStreamPlaybackWAV
}
namespace AudioSamples {
class AudioSample
class AudioSamplePlayback
}
%% Inheritance
AudioStream <|-- AudioStreamWAV
AudioStreamPlayback <|-- AudioStreamPlaybackWAV
%% Links
AudioStream ..|> AudioSample
AudioStreamWAV ..|> AudioSample
AudioStreamPlayback ..|> AudioSamplePlayback
%% Cardinality
AudioStreamPlaybackWAV "1" --> "many" AudioSamplePlayback
AudioSample "1" --> "many" AudioStream
AudioSamplePlayback "1" --> "many" AudioStream
%% Classes
class AudioStream {
+can_be_sampled()*
+get_sample()
}
class AudioStreamWAV {
+can_be_sampled()
+get_sample()
}
class AudioStreamPlayback {
+set_is_sample()*
+get_is_sample()*
+set_sample_playback()*
+get_sample_playback()*
}
class AudioStreamPlaybackWAV {
-bool _is_sample
-Ref~AudioSamplePlayback~ sample_playback
+set_is_sample()
+get_is_sample()
+set_sample_playback()
+get_sample_playback()
}
class AudioSample {
+Ref~AudioStream~stream
+data
+num_channels
+sample_rate
+loop_mode
+loop_begin
+loop_end
}
class AudioSamplePlayback {
+Ref~AudioStream~stream
+float offset
+float volume_db
+PositionMode position_mode
+Vector3 position
+StringName bus
}
Fixes
Fixes #87329
You're missing some class and enum bindings to properly expose the new APIs:
diff --git a/doc/classes/AudioSample.xml b/doc/classes/AudioSample.xml
new file mode 100644
index 0000000000..2d0beece95
--- /dev/null
+++ b/doc/classes/AudioSample.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<class name="AudioSample" inherits="RefCounted" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../class.xsd">
+ <brief_description>
+ </brief_description>
+ <description>
+ </description>
+ <tutorials>
+ </tutorials>
+</class>
diff --git a/doc/classes/AudioServer.xml b/doc/classes/AudioServer.xml
index aa00b5b34f..05ed7919d5 100644
--- a/doc/classes/AudioServer.xml
+++ b/doc/classes/AudioServer.xml
@@ -356,5 +356,13 @@
<constant name="SPEAKER_SURROUND_71" value="3" enum="SpeakerMode">
A 7.1 channel surround setup was detected.
</constant>
+ <constant name="PLAYBACK_TYPE_DEFAULT" value="0" enum="PlaybackType">
+ </constant>
+ <constant name="PLAYBACK_TYPE_STREAM" value="1" enum="PlaybackType">
+ </constant>
+ <constant name="PLAYBACK_TYPE_SAMPLE" value="2" enum="PlaybackType">
+ </constant>
+ <constant name="PLAYBACK_TYPE_MAX" value="3" enum="PlaybackType">
+ </constant>
</constants>
</class>
diff --git a/servers/audio_server.cpp b/servers/audio_server.cpp
index cf19e3cca9..ef21de5361 100644
--- a/servers/audio_server.cpp
+++ b/servers/audio_server.cpp
@@ -1906,6 +1906,11 @@ void AudioServer::_bind_methods() {
BIND_ENUM_CONSTANT(SPEAKER_SURROUND_31);
BIND_ENUM_CONSTANT(SPEAKER_SURROUND_51);
BIND_ENUM_CONSTANT(SPEAKER_SURROUND_71);
+
+ BIND_ENUM_CONSTANT(PLAYBACK_TYPE_DEFAULT);
+ BIND_ENUM_CONSTANT(PLAYBACK_TYPE_STREAM);
+ BIND_ENUM_CONSTANT(PLAYBACK_TYPE_SAMPLE);
+ BIND_ENUM_CONSTANT(PLAYBACK_TYPE_MAX);
}
AudioServer::AudioServer() {
diff --git a/servers/register_server_types.cpp b/servers/register_server_types.cpp
index 932e339eb0..d3446a2dbc 100644
--- a/servers/register_server_types.cpp
+++ b/servers/register_server_types.cpp
@@ -173,6 +173,7 @@ void register_server_types() {
GDREGISTER_CLASS(AudioStream);
GDREGISTER_CLASS(AudioStreamPlayback);
+ GDREGISTER_CLASS(AudioSample);
GDREGISTER_CLASS(AudioSamplePlayback);
GDREGISTER_VIRTUAL_CLASS(AudioStreamPlaybackResampled);
GDREGISTER_CLASS(AudioStreamMicrophone);
Testing the current version on https://github.com/johncoffee/ngj-2024, it works really well!
On thing I noticed is that volume on the AudioStreamPlayer's doesn't seem to work. That's why if you test that demo, the footstep and attack sounds are much louder on web than on desktop. Is that a limitation of samples, or something that can be fixed? I know we can't do fancy bus effects but volume is quite important for most games IMO.
Is the configurable stereo separation strength supported as well? We have properties for this on a per-AudioStreamPlayer2D/3D node, as well as manual panning in AudioStreamPlayer and a project setting that acts as a global multiplier for stereo separation.
Testing the current version on https://github.com/johncoffee/ngj-2024, it works really well!
On thing I noticed is that volume on the AudioStreamPlayer's doesn't seem to work. That's why if you test that demo, the footstep and attack sounds are much louder on web than on desktop. Is that a limitation of samples, or something that can be fixed? I know we can't do fancy bus effects but volume is quite important for most games IMO.
I'm on the volume issues!
as well as manual panning in AudioStreamPlayer
@Calinou How? I was never able to do this.
@akien-mga I think I fixed the volume issue.
@Calinou How? I was never able to do this.
Nevermind, I thought we had this property in AudioStreamPlayer, but we don't.
Remember to add descriptions for newly exposed classes/properties/methods in the class reference.
Yes! I'll add them for sure. Thanks for the reminder.
Also, are there are any advantages to using sample playback on non-web platforms? If so, this should be documented.
Absolutely not. Sample playback for non-supported platforms does nothing, so no sound.
Also, are there are any advantages to using sample playback on non-web platforms? If so, this should be documented.
Absolutely not. Sample playback for non-supported platforms does nothing, so no sound.
I'm a bit concerned about this when it comes to the UX of having a new property on audio players. We now have a property on each player, and a global setting, to confirmed whether to use streams or samples. But on any platform other than web choosing anything else than stream would break the project.
Wouldn't it be better to make this an implementation detail that's handled behind the scenes in the web platform, and not expose this at all to the user? Or do we need users to still be able to choose running specific AudioStreamPlayers as streams even on web?
Or do we need users to still be able to choose running specific AudioStreamPlayers as streams even on web?
For dynamic streams, absolutely, as they are not supported currently as samples. Maybe we could add a ⚠️ emoji if we see that an incompatible setting has been selected?
Tested the current PR with https://github.com/gdquest-demos/godot-4-3d-third-person-controller, it seems to break the music playback.
Tested the current PR with https://github.com/gdquest-demos/godot-4-3d-third-person-controller, it seems to break the music playback.
Added the bug in my list of TODOs before merging: the autoplay property doesn't seem to trigger a play.
I've tested all the games mentioned in the OP. The sound worked pretty well for all of them on both FF 126 and Brave 1.66 on Linux. Only Truck Town had a bit of crackling on both, but I think it might just be something with the game itself. And yeah, as mentioned above, the autoplay does not trigger. I had to click once inside the game to start the sound. Everything was pretty smooth on mobile as well.
And yeah, as mentioned above, the autoplay does not trigger. I had to click once inside the game to start the sound.
That's a Web feature (called autoplay blocking)! Sound doesn't play before an interaction event from the user is triggered.
I was rather talking about AudioStreamPlayer's internal "autoplay" feature.
I can't compile this branch on my Godot build, it errors at the end with this wall of text:
em++: warning: -pthread + ALLOW_MEMORY_GROWTH may run non-wasm code slowly, see https://github.com/WebAssembly/design/issues/1271 [-Wpthreads-mem-growth]
file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:3567
var err = new SyntaxError(message);
^
SyntaxError: Unexpected token (11023:19)
static _samples = new Map();
^
at Parser.pp$4.raise (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:3567:13)
at Parser.pp$9.unexpected (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:766:8)
at Parser.pp$9.expect (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:760:26)
at Parser.pp$5.parseMethod (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:3321:8)
at Parser.pp$8.parseClassMethod (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:1527:35)
at Parser.pp$8.parseClassElement (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:1485:10)
at Parser.pp$8.parseClass (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:1403:24)
at Parser.pp$5.parseExprAtom (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:2909:17)
at Parser.pp$5.parseExprSubscripts (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:2709:19)
at Parser.pp$5.parseMaybeUnary (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:2675:17) {
pos: 396985,
loc: Position { line: 11023, column: 19 },
raisedAt: 396986
}
em++: error: '/home/mbcx/sources/emsdk/node/16.20.0_64bit/bin/node /home/mbcx/sources/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs /tmp/emscripten_temp_wvhct4ms/godot.web.template_debug.wasm32.js growableHeap --closureFriendly -o /tmp/emscripten_temp_wvhct4ms/godot.web.template_debug.wasm32.jso1.js' failed (returned 1)
scons: *** [bin/godot.web.template_debug.wasm32.js] Error 1
scons: building terminated because of errors.
@MBCX Which version of emscripten are you using? I used the latest one, maybe it has an impact.
Only Truck Town had a bit of crackling on both, but I think it might just be something with the game itself.
How much FPS do you get when running the project? Stuttering might happen at low FPS because the engine sound pitch only changes once per frame, so having low FPS (below 30 or so) will cause discrete pitch changes to be noticeable. A lot of racing games run into this issue as it's difficult to fix.
@MBCX Which version of emscripten are you using? I used the latest one, maybe it has an impact.
@adamscott I am using emscripten 3.1.56, I'm going to update to 3.1.59 and see if that fixes it.
@adamscott Nope, even after updating to latest emscripten (3.1.59), it still fails with same errors:
em++: warning: -pthread + ALLOW_MEMORY_GROWTH may run non-wasm code slowly, see https://github.com/WebAssembly/design/issues/1271 [-Wpthreads-mem-growth]
file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:3567
var err = new SyntaxError(message);
^
SyntaxError: Unexpected token (11023:19)
static _samples = new Map();
^
at Parser.pp$4.raise (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:3567:13)
at Parser.pp$9.unexpected (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:766:8)
at Parser.pp$9.expect (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:760:26)
at Parser.pp$5.parseMethod (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:3321:8)
at Parser.pp$8.parseClassMethod (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:1527:35)
at Parser.pp$8.parseClassElement (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:1485:10)
at Parser.pp$8.parseClass (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:1403:24)
at Parser.pp$5.parseExprAtom (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:2909:17)
at Parser.pp$5.parseExprSubscripts (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:2709:19)
at Parser.pp$5.parseMaybeUnary (file:///home/mbcx/sources/emsdk/upstream/emscripten/node_modules/acorn/dist/acorn.mjs:2675:17) {
pos: 396985,
loc: Position { line: 11023, column: 19 },
raisedAt: 396986
}
em++: error: '/home/mbcx/sources/emsdk/node/16.20.0_64bit/bin/node /home/mbcx/sources/emsdk/upstream/emscripten/tools/acorn-optimizer.mjs /tmp/emscripten_temp_wvhct4ms/godot.web.template_debug.wasm32.js growableHeap --closureFriendly -o /tmp/emscripten_temp_wvhct4ms/godot.web.template_debug.wasm32.jso1.js' failed (returned 1)
scons: *** [bin/godot.web.template_debug.wasm32.js] Error 1
scons: building terminated because of errors.
emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.59 (0e4c5994eb5b8defd38367a416d0703fd506ad81)
Copyright (C) 2014 the Emscripten authors (see AUTHORS.txt)
This is free and open source software under the MIT license.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
@MBCX Could you share the exact build command you're using?
@MBCX Could you share the exact build command you're using?
@akien-mga I build Godot in a automated manner through a script, this is the command that gets formed before executing:
SCRIPT_AES256_ENCRYPTION_KEY="<key>"
# Debug command (fails)
scons -j4 \ # ${THREADS_PER_JOB}
platform="web" precision="single" module_text_server_fb_enabled=yes \ # $common_options
threads="yes" \ # $thread_mode
target="template_debug"
# Release command (not tried yet)
scons -j4 \ # ${THREADS_PER_JOB}
platform="web" precision="single" module_text_server_fb_enabled=yes \ # $common_options
threads="yes" \ # $thread_mode
target="template_release" production="yes" use_closure_compiler="yes"
@MBCX It seems related to using the closure compiler. I'll take a look why it has issues with my new JavaScript code.
@MBCX It seems related to using the closure compiler. I'll take a look why it has issues with my new JavaScript code.
Well, I haven't tried compiling with closure compiler, compiling template_debug is enough to trigger the bug
I can reproduce the issue too with scons p=web threads=yes target=template_debug, I guess you just haven't compiled this specific branch with threads enabled @adamscott (since the whole point is to improve things without threads).
Fixing the style issues from CI would likely also have surfaced this in the CI build :)
Fixed the issue with closure-compiler. I made the "classes" prototypes instead.
When testing, I always get an exception in the console.
Bus.count; -> return GodotAudio.Bus._buses.length; -> GodotAudio.Bus is null?
tmp_js_export.js:10089 Uncaught (in promise) TypeError: Cannot read properties of null (reading '_buses')
at Function.get (tmp_js_export.js:10089:29)
at Object.GodotAudioClassesInit [as classesInit] (tmp_js_export.js:10116:8)
at tmp_js_export.js:14275:16
at tmp_js_export.js:16297:8
I don't do anything crazy with the audio buses:
When testing, I always get an exception in the console.
Bus.count;->return GodotAudio.Bus._buses.length;->GodotAudio.Busis null?tmp_js_export.js:10089 Uncaught (in promise) TypeError: Cannot read properties of null (reading '_buses') at Function.get (tmp_js_export.js:10089:29) at Object.GodotAudioClassesInit [as classesInit] (tmp_js_export.js:10116:8) at tmp_js_export.js:14275:16 at tmp_js_export.js:16297:8I don't do anything crazy with the audio buses:
@Maran23 Can you reproduce the bug with my latest changes? If so, can you join a minimal reproducible project?
Is it enforced by CI somehow? E.g. is the type definition used by the Closure Compiler (docs doesn't seem to mention typescript imports support).
And if not, how are going to make sure the type definition actually matches the JS code?
On Sun, Jun 2, 2024, 07:34 Adam Scott @.***> wrote:
@.**** commented on this pull request.
In platform/web/js/libs/library_godot_audio.d.ts https://github.com/godotengine/godot/pull/91382#discussion_r1623330140:
@@ -0,0 +1,158 @@ +export type PositionMode = "none" | "2D" | "3D";
Yes. It adds typing for easier coding. I'll review the code to make sure that it's still relevant.
— Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/pull/91382#discussion_r1623330140, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM4C3XZM5DKLNB3ECJ2IVTZFKVFLAVCNFSM6AAAAABHBFL5QWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOJSGIYTQNZYGY . You are receiving this because you were mentioned.Message ID: @.***>
As far as I can tell (see https://github.com/jsdoc/jsdoc/issues/1929 and https://github.com/jsdoc/jsdoc/issues/1645 ) it seems that the jsdoc syntax used to import from TS type definition is a VSCode dialect. I'm not sure the CC supports it, but we should be able to provide types using standard JSDoc (or a dialect supported by the CC). See https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler
On Sun, Jun 2, 2024, 09:06 Fabio Alessandrelli < @.***> wrote:
Is it enforced by CI somehow? E.g. is the type definition used by the Closure Compiler (docs doesn't seem to mention typescript imports support).
And if not, how are going to make sure the type definition actually matches the JS code?
On Sun, Jun 2, 2024, 07:34 Adam Scott @.***> wrote:
@.**** commented on this pull request.
In platform/web/js/libs/library_godot_audio.d.ts https://github.com/godotengine/godot/pull/91382#discussion_r1623330140:
@@ -0,0 +1,158 @@ +export type PositionMode = "none" | "2D" | "3D";
Yes. It adds typing for easier coding. I'll review the code to make sure that it's still relevant.
— Reply to this email directly, view it on GitHub https://github.com/godotengine/godot/pull/91382#discussion_r1623330140, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM4C3XZM5DKLNB3ECJ2IVTZFKVFLAVCNFSM6AAAAABHBFL5QWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOJSGIYTQNZYGY . You are receiving this because you were mentioned.Message ID: @.***>
Is it enforced by CI somehow? E.g. is the type definition used by the Closure Compiler (docs doesn't seem to mention typescript imports support). And if not, how are going to make sure the type definition actually matches the JS code?
I removed the .d.ts file and replaced everything with Closure Compiler compatible JSDoc.
