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 finished() signal acts as stop() #33579

Closed
rezgi opened this issue Nov 12, 2019 · 13 comments
Closed

AudioStreamPlayer finished() signal acts as stop() #33579

rezgi opened this issue Nov 12, 2019 · 13 comments

Comments

@rezgi
Copy link

rezgi commented Nov 12, 2019

Godot version:
3.2 beta 1

OS/device including version:
OSX Sierra

Issue description:
I have a button that stops the music and an AudioStreamPlayer finished() signal with $music.play() in it to play again. When I press the button to stop the music, it plays again, and I guess that Godot executes the stop() method the same way as the finished() signal ?

Btw I have no loop option in the inspector I guess it's because it's a wav file ?

Steps to reproduce:
In any project, add a Button and an AudioStreamPlayer with a song (1mn in my case but a shorter one would be better for testing). _on_button_pressed() -> $AudioStreamPlayer.stop() and _on_AudioStreamPlayer_finished() -> $AudioStreamPlayer.play()and you'll notice that when you press the stop button, the song plays again.

Minimal reproduction project:
Empty Godot project with one wav file.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 12, 2019

Sounds like stopping AudioStreamPlayer emits finished signal. This is somewhat expected.

@rezgi
Copy link
Author

rezgi commented Nov 12, 2019

Sounds like stopping AudioStreamPlayer emits finished signal. This is somewhat expected.

Makes sense. Which means my method is dead wrong? Or maybe the two methods should be different? There's a redundancy here.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 12, 2019

Seems like you are trying to achieve looping. You can do this easier by importing the audio sample with loop enabled.
image

@rezgi
Copy link
Author

rezgi commented Nov 13, 2019

Seems like you are trying to achieve looping. You can do this easier by importing the audio sample with loop enabled.
image

Thanks for the info ! But isn't it a problem that stop() emits a finished() signal ? Or is it by design ?

If so I'll close the issue. Thanks again !

@KoBeWi
Copy link
Member

KoBeWi commented Nov 13, 2019

It looks like by design to me, although I'm not an audio expert to say 100% sure.

@rezgi
Copy link
Author

rezgi commented Nov 13, 2019

Mmm. I'd like a core contributor opinion before closing the issue. To me it seems a bit redundant.

@Xrayez
Copy link
Contributor

Xrayez commented Nov 13, 2019

I believe stopping forcibly should not ever mean that a track is finished. See a fix at #33602.

Also would be good if this reproduction project is linked:
audio-stream-player-stop-finished.zip (Intro.ogg taken from demo projects).

@Xrayez
Copy link
Contributor

Xrayez commented Nov 13, 2019

The alternative to this could be introducing end_reached signal and treat finished signal as before:

  • finished = end of audio or stopped
  • end_reached = end of audio.

@rezgi
Copy link
Author

rezgi commented Nov 14, 2019

Ok makes more sense ! Should I close this issue ? Sorry, still beginner in how issues work.

@Xrayez
Copy link
Contributor

Xrayez commented Nov 14, 2019

@rezgi I think you've raised a valid point, so I'd keep this issue open for now until the issue is fixed, which will be closed automatically for you, most of the time. 🙂

@rezgi
Copy link
Author

rezgi commented Nov 15, 2019

Oh ok I see. I've seen commits and stuff about this, and since I'm not experienced, I'm not confident if I raise a valid point or not. So I'm leaving this issue opened. Thanks !

@KoBeWi
Copy link
Member

KoBeWi commented Jul 22, 2021

I just encountered it myself and I can say it's a bug. stop() is an action done by the user, so they don't need to be notified by the signal. Thus the current behavior is not consistent with the usual Godot design, which is to avoid redundant signal emissions.

@KoBeWi
Copy link
Member

KoBeWi commented Feb 25, 2022

Looks like it's fixed both in 4.0 and the newest 3.5 beta.

@KoBeWi KoBeWi closed this as completed Feb 25, 2022
@KoBeWi KoBeWi added this to the 4.0 milestone Feb 25, 2022
@akien-mga akien-mga modified the milestones: 4.0, 3.5 Feb 25, 2022
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.

5 participants