Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AudioStreamPlayer does not call "finished" signal #87802

Closed
EnlightenedOne opened this issue Feb 1, 2024 · 2 comments · Fixed by #87830
Closed

AudioStreamPlayer does not call "finished" signal #87802

EnlightenedOne opened this issue Feb 1, 2024 · 2 comments · Fixed by #87830

Comments

@EnlightenedOne
Copy link

EnlightenedOne commented Feb 1, 2024

Tested versions

Reproducible on 313f623
Introduced by #87061

System information

Godot v4.3.dev.mono (313f623) - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3070 Laptop GPU (NVIDIA; 31.0.15.3742) - 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz (16 Threads)

Issue description

AudioStreamPlayer never calls "finished" and emits no notifications, it also does not call AudioStreamPlayerInternal::process the impact beyond no notifications is not fully understood.

The cause is that AudioStreamPlayerInternal::play_basic now calls:
_set_process(true);
Instead of:
set_process_internal(true);

In the Node flow this triggers a different notification type but AudioStreamPlayerInternal::notification still uses
case Node::NOTIFICATION_INTERNAL_PROCESS: {
process();
} break;

The 2D + 3D versions are fine as they use the internal physics loop notification to drive calls to process.

Steps to reproduce

If you open the attached MRP in 4.2 stable it will repeat a small sound immediately as the main scene loads and log a message saying everything is looking good.

If you run a local build of 4.3+ and open the MRP it will play the sound once everytime you reopen the scene and you wont see any log message occuring.

Minimal reproduction project (MRP)

MRP_AudioFinished.zip

@EnlightenedOne
Copy link
Author

I dont yet understand what determines if you should use the internal notiifcation type so im not sure which way would be considered the right way to fix it alignment wise.

@Calinou
Copy link
Member

Calinou commented Feb 1, 2024

cc @KoBeWi

@Calinou Calinou added this to the 4.3 milestone Feb 1, 2024
@AThousandShips AThousandShips changed the title AudioStreamPlayer does not call "finished" notification AudioStreamPlayer does not call "finished" signal Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants