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

Fix initialization order in AudioStreamInteractive to allow initial_clip to be properly played #100552

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

mdelorme
Copy link
Contributor

Fixes #100514

Problem comes from the initialization order of initial_clip coming before the actual clips. When the AudioStreamInteractive is loaded in the scene, no clips are loaded yet. initial_clip is reset to 0 triggering the error. Then the clips are loaded, and the initial_clip will inevitably become the first of the list.

I propose a fix by simply changing the order of the two properties in the class. Having the initial_clip being declared after the actual clips, fixes everything.

@mdelorme mdelorme requested a review from a team as a code owner December 18, 2024 09:30
@mdelorme mdelorme changed the title Fixed order intiialization to allow initial_clip to be properly played Fixed order initialization in AudioStreamInteractive to allow initial_clip to be properly played Dec 18, 2024
@akien-mga akien-mga changed the title Fixed order initialization in AudioStreamInteractive to allow initial_clip to be properly played Fix initialization order in AudioStreamInteractive to allow initial_clip to be properly played Dec 18, 2024
@akien-mga akien-mga added this to the 4.4 milestone Dec 18, 2024
@akien-mga akien-mga added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Dec 18, 2024
@akien-mga
Copy link
Member

Looks good! Could you squash the commits? See PR workflow for instructions.

@mdelorme mdelorme force-pushed the mdelorme/issue_100514 branch from 00e6fad to 193fccf Compare December 18, 2024 13:43
@mdelorme mdelorme requested a review from a team as a code owner December 18, 2024 13:43
@mdelorme
Copy link
Contributor Author

I think I messed up and included another commit in the PR. Lemme fix that. Sorry

@mdelorme mdelorme force-pushed the mdelorme/issue_100514 branch from 193fccf to 95ff87d Compare December 18, 2024 13:48
@mdelorme
Copy link
Contributor Author

All good now

@mdelorme
Copy link
Contributor Author

Sorry for that, am I the one supposed to fix this or can the formatting changes be approved by a maintainer ?

@akien-mga
Copy link
Member

You need to do the formatting fixes yourself ideally. You'd do it by removing the trailing tab on the line flagged in the diff: https://github.com/godotengine/godot/actions/runs/12394304279/job/34597974426?pr=100552

And then amend the commit and force push to udpate the PR, keeping a single commit.

@mdelorme mdelorme force-pushed the mdelorme/issue_100514 branch from 95ff87d to ea97d44 Compare December 18, 2024 14:24
@mdelorme
Copy link
Contributor Author

Ok ! Thank you very much for the guidance :)
Fix applied !

@akien-mga akien-mga merged commit 56d11c1 into godotengine:master Dec 18, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:audio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clip index problem when changing “Initial Clip” value other than 0
2 participants