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 bug in AudioStreamPlaybackPolyphonic where stream IDs returned from play_stream may not work #86054

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xerothermic06
Copy link

Intended to fix this bug: #86053

The docs state that invoking play_stream on an instance of AudioStreamPlaybackPolyphonic returns an integer ID that can be used to control the volume or stop playback of a given stream, but in GDScript none of these IDs work.

While debugging I noticed that the _find_stream method seems to have platform-dependent behavior because an implicitly typed enum value is used as a bitmask. This line here appears to be the problem:

int64_t id = p_id & ID_MASK;

The usage here seems to be casting ID_MASK undesirably on my system; the result of p_id & ID_MASK is always exactly equal to p_id. My guess is that MSVC is reading 0xFFFFFFFF as -1 and then casting that up to an int64_t also with the value of -1. Added an explicit type to the enum that should remove this ambiguity.

My compiler version is: Microsoft (R) C/C++ Optimizing Compiler Version 19.34.31937 for x64

@akien-mga akien-mga changed the title Fix bug in AudioStreamPlaybackPolyphonic where stream IDs returned from play_stream may not work Fix bug in AudioStreamPlaybackPolyphonic where stream IDs returned from play_stream may not work Dec 12, 2023
@akien-mga akien-mga added platform:windows cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Dec 12, 2023
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Dec 12, 2023
Co-authored-by: Rémi Verschelde <rverschelde@gmail.com>
@@ -54,7 +54,7 @@ class AudioStreamPolyphonic : public AudioStream {
class AudioStreamPlaybackPolyphonic : public AudioStreamPlayback {
GDCLASS(AudioStreamPlaybackPolyphonic, AudioStreamPlayback)

enum {
enum : uint64_t {
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
enum : uint64_t {
enum : uint32_t {

should be sufficient, as ID_MASK equals max unsigned 32

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

Successfully merging this pull request may close these issues.

None yet

5 participants