-
Notifications
You must be signed in to change notification settings - Fork 96
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
class Sound does not implement copy ctor properly #137
Comments
After digging around in raylib for a while I realised that copying a Sound struct is not that easy, so the copy ctor/assigmnent should be marked as deleted. |
Perhaps something like this? Sound(const ::Sound& sound) {
set(sound);
}
Sound(const raylib::Sound& sound) {
set(sound);
sound.stream.buffer = NULL;
}
inline void Unload() {
if (stream.buffer != NULL) {
::UnloadSound(*this);
}
} |
That would not work, because in the constructor you're trying to modify the other sound object which is marked as const. Imo the best solution is to delete the copy operators, because raylib does not provide a mechanism to copy a sound struct (as far as I know). Sound(const Sound&) = delete;
Sound& operator=(const Sound&) = delete;
Sound(Sound&& other) {
set(other);
other.sampleCount = 0;
other.stream = { 0 };
};
Sound& operator=(Sound&& other) {
if (this == &other)
return *this;
Unload();
set(other);
other.sampleCount = 0;
other.stream = { 0 };
return *this;
};
~Sound() {
Unload();
} I also added proper move ctor/assigmnent, because the default one also caused memory errors. |
Oh, that's pretty cool. A test of it would be awesome too. Something simple would be enough... Could adopt a similar pattern for the other classess too. |
I think that #102 is a symptom of the same problem that I encountered here surfacing in the other classes. |
Copying a Sound object does not copy the rAudioBuffer of AudioStream from the parent class. Multiple copies of one sound instance getting destroyed causes then a double-free error on the buffer (since they all share the same buffer).
The text was updated successfully, but these errors were encountered: