search

cmu-cs-academy / desktop-cmu-graphics

BSD 3-Clause "New" or "Revised" License
15 stars 9 forks source link

Sound code total remake #74

Open Mohamed-Waiel-Shikfa opened 3 months ago

Mohamed-Waiel-Shikfa commented 3 months ago

Remade Sound to natively use pygame instead of using subprocesses for a better more robust way to play multiple sound at the same time with no noticeable effect for user. Of course, the URL argument of sound was fixed at the same time and now expects a normal file path.

Mohamed-Waiel-Shikfa commented 3 months ago

This would thus also solve:

Mohamed-Waiel-Shikfa commented 3 months ago

Hello! So, tests are failing but I have done some reading and the issue might be because there is no audio output device connected to the machine running the test. The code works fine on my laptop so I am not sure what we can do about the tests. Please let me know so that we can merge these changes afterwards :-)

(Here is what I read: https://stackoverflow.com/questions/62125225/pygame-error-mixer-not-initialized-how-to-fix)

Yuxiang-Huang commented 1 month ago

The currently released version of desktop CMU graphics supports multiple sounds, but this functionality is no longer available after this remake. For example with the code below, if I press the mouse, the background music will stop in this branch, but the background music continues in the main branch.

from cmu_graphics import *

background_music = Sound("file:///Users/yuxianghuang/desktop-cmu-graphics/background.mp3")
sound_effect = Sound("file:///Users/yuxianghuang/desktop-cmu-graphics/se1.wav")

background_music.play()

def onMousePress(mouseX, mouseY):
    sound_effect.play()

cmu_graphics.run()
Mohamed-Waiel-Shikfa commented 1 month ago

Unfortunately, I can't test or work on this right now.

I will be back helping with development in about two weeks.

Sorry for the inconvenience and thanks for the wait.

On Tue, 30 Jul 2024, 22:13 Yuxiang Huang, @.***> wrote:

The currently released version of desktop CMU graphics supports multiple sounds, but this functionality is no longer available after this remake. For example with the code below, if I press the mouse, the background music will stop in this branch, but the background music continues in the main branch.

from cmu_graphics import *

background_music = Sound("file:///Users/yuxianghuang/desktop-cmu-graphics/background.mp3") sound_effect = Sound("file:///Users/yuxianghuang/desktop-cmu-graphics/se1.wav")

background_music.play()

def onMousePress(mouseX, mouseY): sound_effect.play()

cmu_graphics.run()

— Reply to this email directly, view it on GitHub https://github.com/cmu-cs-academy/desktop-cmu-graphics/pull/74#issuecomment-2259034016, or unsubscribe https://github.com/notifications/unsubscribe-auth/BBRSZJNOWTVFOC3EBOOAAX3ZO7QWVAVCNFSM6AAAAABJA5TD4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJZGAZTIMBRGY . You are receiving this because you authored the thread.Message ID: @.***>

Mohamed-Waiel-Shikfa commented 3 weeks ago

@Yuxiang-Huang YES! you are right. My implementation did not allow for multiple sound being played at the same time. Nevertheless, I looked into it, and it should now be fixed. As a bonus I also added a quick way to change the volume of individual sounds.

Hope this now is good enough to be merged into main :)

schmave commented 2 weeks ago

Thanks, @Mohamed-Waiel-Shikfa! I hope to review this again this week.

Mohamed-Waiel-Shikfa commented 1 week ago

So... any news? Perhaps a comment on the code?

Also, just wanted to update you—I'm also a CA for 15112 this semester. I've talked with Professor Riley about using 1 to 2 hours of my paid time each week to work on cmu_graphics. If you have anything new you want me to focus on, let me know; otherwise, I'll keep going as usual.