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

Document AudioStreamPlayer.get_playback_position() intentionally always returning 0.0 when using AudioStreamInteractive #99200

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

KingTheFifth
Copy link
Contributor

Fix #97791
AudioStreamPlayer.get_playback_position() always returns 0.0 if the AudioStream for the AudioStreamPlayer is AudioStreamInteractive. This is intended behaviour that lacks documentation, as confirmed by #97806 (comment) and #97791 (comment). This pull requests adds documentation for the behaviour to the class reference.

@KingTheFifth KingTheFifth requested a review from a team as a code owner November 13, 2024 21:09
@KingTheFifth KingTheFifth changed the title Documented AudioStreamPlayer.get_playback_position() intentionally aways returning 0.0 when using AudioStreamInteractive` Documented AudioStreamPlayer.get_playback_position() intentionally aways returning 0.0 when using AudioStreamInteractive Nov 13, 2024
@@ -22,6 +22,7 @@
<description>
Returns the position in the [AudioStream] of the latest sound, in seconds. Returns [code]0.0[/code] if no sounds are playing.
[b]Note:[/b] The position is not always accurate, as the [AudioServer] does not mix audio every processed frame. To get more accurate results, add [method AudioServer.get_time_since_last_mix] to the returned position.
[b]Note:[/b] The returned position is always [code]0.0[/code] if the [AudioStream] is [AudioStreamInteractive]. [AudioStreamInteractive] may have multiple clips with transitions between them. This means that the playback position loses meaning, especially during transitions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[b]Note:[/b] The returned position is always [code]0.0[/code] if the [AudioStream] is [AudioStreamInteractive]. [AudioStreamInteractive] may have multiple clips with transitions between them. This means that the playback position loses meaning, especially during transitions.
[b]Note:[/b] The returned position is always [code]0.0[/code] if the [member stream] is of type [AudioStreamInteractive], since it can have multiple clips playing at once.

I think it can be a bit shorter while still getting the point across. + refers to the "stream" property now instead of the class. Feel free to tweak my suggestion further if you like :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also add particularly during transitions. at the end if wanted, but I didn't think it's necessary to understand this behavior. And the sentence would get a bit long :p

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continuing with the above suggestion.

Suggested change
[b]Note:[/b] The returned position is always [code]0.0[/code] if the [AudioStream] is [AudioStreamInteractive]. [AudioStreamInteractive] may have multiple clips with transitions between them. This means that the playback position loses meaning, especially during transitions.
[b]Note:[/b] This method always returns [code]0.0[/code] if the [member stream] is an [AudioStreamInteractive], since it can have multiple clips playing at once.

I do agree to keep verbosity at a minimum, especially because this method doesn't have a "strict" dependance on this class. That is to say that you don't need to know about AudioStreamInteractive to make use of this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes very much sense! Thank you very much for the feedback. I have fixed it now :)

@Chaosus Chaosus added this to the 4.4 milestone Nov 14, 2024
@AThousandShips AThousandShips changed the title Documented AudioStreamPlayer.get_playback_position() intentionally aways returning 0.0 when using AudioStreamInteractive Document AudioStreamPlayer.get_playback_position() intentionally aways returning 0.0 when using AudioStreamInteractive Nov 14, 2024
…ways returning `0.0` when using `AudioStreamInteractive`
@Repiteo Repiteo merged commit 1117d91 into godotengine:master Nov 22, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 22, 2024

Thanks! Congratulations on your first contribution! 🎉

@akien-mga akien-mga changed the title Document AudioStreamPlayer.get_playback_position() intentionally aways returning 0.0 when using AudioStreamInteractive Document AudioStreamPlayer.get_playback_position() intentionally always returning 0.0 when using AudioStreamInteractive Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When using AudioStreamInteractive, AudioStreamPlayer.get_playback_position() always returns 0
5 participants