Review comments from bot, good :bot:

Author: oddbookworm

buildconfig/stubs/pygame/mixer.pyi

@@ -5,6 +5,7 @@ from pygame.event import Event
 from pygame.typing import FileLike
 from typing_extensions import (
     Buffer,  # collections.abc 3.12
+    Self,
     deprecated,  # added in 3.13
 )
 
@@ -72,8 +73,8 @@ class Sound:
     def get_num_channels(self) -> int: ...
     def get_length(self) -> float: ...
     def get_raw(self) -> bytes: ...
-    def copy(self) -> Sound: ...
-    def __copy__(self) -> Sound: ...
+    def copy(self) -> Self: ...
+    def __copy__(self) -> Self: ...
 
 class Channel:
     def __init__(self, id: int) -> None: ...

src_c/mixer.c

@@ -816,33 +816,49 @@ snd_copy(PyObject *self, PyObject *_null)
     pgSoundObject *newSound =
         (pgSoundObject *)Py_TYPE(self)->tp_new(Py_TYPE(self), NULL, NULL);
 
-    PyObject *kwargs = PyDict_New();
-    PyObject *key = PyUnicode_FromString("buffer");
+    if (!newSound) {
+        if (!PyErr_Occurred()) {
+            PyErr_SetString(PyExc_MemoryError,
+                            "Failed to create new Sound object for copy");
+        }
+        return NULL;
+    }
+
+    PyObject *dict = PyDict_New();
+    if (!dict) {
+        if (!PyErr_Occurred()) {
+            PyErr_SetString(PyExc_MemoryError,
+                            "Failed to create internal dictionary to create "
+                            "copy of Sound");
+        }
+        Py_DECREF(newSound);
+        return NULL;
+    }
+
     PyObject *bytes = snd_get_raw(self, NULL);
     if (bytes == NULL) {
         // exception set already by PyBytes_FromStringAndSize
         PG_SAVE_EXCEPTION
-        Py_DECREF(key);
-        Py_DECREF(kwargs);
+        Py_DECREF(dict);
+        Py_DECREF(newSound);
         PG_UNSAVE_EXCEPTION
         return NULL;
     }
 
-    if (PyDict_SetItem(kwargs, key, bytes) < 0) {
+    if (PyDict_SetItemString(dict, "buffer", bytes) < 0) {
         // exception set already
         PG_SAVE_EXCEPTION
-        Py_DECREF(key);
         Py_DECREF(bytes);
-        Py_DECREF(kwargs);
+        Py_DECREF(dict);
+        Py_DECREF(newSound);
         PG_UNSAVE_EXCEPTION
         return NULL;
     }
-    Py_DECREF(key);
     Py_DECREF(bytes);
 
-    if (sound_init((PyObject *)newSound, NULL, kwargs) != 0) {
+    if (sound_init((PyObject *)newSound, NULL, dict) != 0) {
         PG_SAVE_EXCEPTION
-        Py_DECREF(kwargs);
+        Py_DECREF(dict);
         Py_DECREF(newSound);
         PG_UNSAVE_EXCEPTION
         // Exception set by sound_init
@@ -858,7 +874,7 @@ snd_copy(PyObject *self, PyObject *_null)
         Mix_VolumeChunk(dst, vol);
     }
 
-    Py_DECREF(kwargs);
+    Py_DECREF(dict);
     return (PyObject *)newSound;
 }
 

test/mixer_test.py

@@ -1,3 +1,4 @@
+import copy
 import os
 import pathlib
 import platform
@@ -1348,6 +1349,8 @@ def __init__(self, *args, **kwargs):
         new_volumes = [0.2, 0.3, 0.7, 1.0, 0.1]
         if pygame.mixer.get_sdl_mixer_version() >= (2, 6, 0):
             filenames.append("house_lo.mp3")
+            old_volumes.append(0.9)
+            new_volumes.append(0.5)
 
         for f, old_vol, new_vol in zip(filenames, old_volumes, new_volumes):
             filename = example_path(os.path.join("data", f))
@@ -1377,6 +1380,35 @@ def __init__(self, *args, **kwargs):
             sound_copy.play()
             self.assertEqual(sound_copy.get_num_channels(), 1)
 
+        # Test __copy__
+        for f, old_vol, new_vol in zip(filenames, old_volumes, new_volumes):
+            filename = example_path(os.path.join("data", f))
+            try:
+                sound = mixer.Sound(file=filename)
+                sound.set_volume(old_vol)
+            except pygame.error:
+                continue
+            sound_copy = copy.copy(sound)
+            self.assertEqual(sound.get_length(), sound_copy.get_length())
+            self.assertEqual(sound.get_num_channels(), sound_copy.get_num_channels())
+            self.assertEqual(sound.get_volume(), sound_copy.get_volume())
+            self.assertEqual(sound.get_raw(), sound_copy.get_raw())
+
+            sound.set_volume(new_vol)
+            self.assertNotEqual(sound.get_volume(), sound_copy.get_volume())
+
+            del sound
+
+            # Test on the copy for playable sounds
+            channel = sound_copy.play()
+            if channel is None:
+                continue
+            self.assertTrue(channel.get_busy())
+            sound_copy.stop()
+            self.assertFalse(channel.get_busy())
+            sound_copy.play()
+            self.assertEqual(sound_copy.get_num_channels(), 1)
+
         # Test copying a subclass of Sound
         for f, old_vol, new_vol in zip(filenames, old_volumes, new_volumes):
             filename = example_path(os.path.join("data", f))
@@ -1386,7 +1418,7 @@ def __init__(self, *args, **kwargs):
             except pygame.error:
                 continue
             sound_copy = sound.copy()
-            self.assertTrue(sound_copy, SubSound)
+            self.assertIsInstance(sound_copy, SubSound)
             self.assertEqual(sound.get_length(), sound_copy.get_length())
             self.assertEqual(sound.get_num_channels(), sound_copy.get_num_channels())
             self.assertEqual(sound.get_volume(), sound_copy.get_volume())