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

Sound code total remake #74

Merged
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
750e006
Deleted sound.py, removed CMUSound from shape_logic.py, removed calls…
Mohamed-Waiel-Shikfa Jun 9, 2024
b159bf0
Update cmu_graphics.py
Mohamed-Waiel-Shikfa Jun 9, 2024
c995b5a
Update cmu_graphics.py
Mohamed-Waiel-Shikfa Jun 9, 2024
b780cc5
Update shape_logic.py
Mohamed-Waiel-Shikfa Jun 9, 2024
ad60499
Update cmu_graphics.py
Mohamed-Waiel-Shikfa Jun 9, 2024
4455dc5
Update cmu_graphics.py
Mohamed-Waiel-Shikfa Jun 9, 2024
c951fb4
Update cmu_graphics.py
Mohamed-Waiel-Shikfa Jun 9, 2024
8ac2387
Update cmu_graphics.py
Mohamed-Waiel-Shikfa Jun 10, 2024
95d0477
Merge branch 'main' into Sound-code-total-remake
schmave Jul 9, 2024
8dc6d37
removed code that belongs to the old way of cmu_graphics sound handling
Mohamed-Waiel-Shikfa Jul 10, 2024
f03d213
Added Type checking to new implementation of sound
Mohamed-Waiel-Shikfa Jul 11, 2024
f0194ec
Game continues without sound if there was an error during initializat…
Mohamed-Waiel-Shikfa Jul 13, 2024
7228f4f
chore: parallel sound handling
Mohamed-Waiel-Shikfa Aug 24, 2024
f30d1a5
Add setVolume and getVolume methods to Sound class
Mohamed-Waiel-Shikfa Aug 24, 2024
674144f
Merge branch 'main' of https://github.com/Mohamed-Waiel-Shikfa/deskto…
Mohamed-Waiel-Shikfa Aug 25, 2024
6f2abfb
Fixed Bug
Mohamed-Waiel-Shikfa Sep 16, 2024
7671296
Use os.path.join instead of str cat when handling url value
Mohamed-Waiel-Shikfa Sep 16, 2024
a9c6483
Merge branch 'Sound-code-total-remake' of https://github.com/Mohamed-…
Mohamed-Waiel-Shikfa Sep 16, 2024
33bbaed
Fix bug in sounds with restart
Mohamed-Waiel-Shikfa Sep 16, 2024
94df9de
Sound type checks even if pygame mixer not initialized
Mohamed-Waiel-Shikfa Oct 29, 2024
a4dcee3
add docstring to getVolume
Mohamed-Waiel-Shikfa Oct 29, 2024
b6c09e1
Changed tests to force audio driver to be dummy and removed check of …
Mohamed-Waiel-Shikfa Oct 30, 2024
5e17139
Merge branch 'main' into Sound-code-total-remake
Mohamed-Waiel-Shikfa Nov 1, 2024
85bdf4c
minor code style change
Mohamed-Waiel-Shikfa Nov 8, 2024
aefaa26
Bump version number, add one more gitignore
schmave Nov 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 64 additions & 5 deletions cmu_graphics/cmu_graphics.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,44 @@ def __iter__(self): return iter(self._shape)
def __len__(self): return len(self._shape._shapes)

class Sound(object):
number_of_sounds = 0
def __init__(self, url):
self.sound = slNewSound(url)
if not pygame.mixer.get_init():
pygame.mixer.init()
pygame.mixer.set_num_channels(1)

if not isinstance(url, str):
callSpec = '{className}.{attr}'.format(className=t('Sound'), attr=t('url'))
err = t(
'{{error}}: {{callSpec}} should be {{typeName}} (but {{value}} is of type {{valueType}})',
{'error': t('TypeError'), 'callSpec': callSpec, 'typeName': 'string', 'value': repr(url), 'valueType': type(url).__name__}
)
raise Exception(err)

Sound.number_of_sounds += 1
if pygame.mixer.get_num_channels()==Sound.number_of_sounds:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request change: code style (whitespace around operators)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

pygame.mixer.set_num_channels(Sound.number_of_sounds * 2)

if url.startswith('file://'):
url = url.split('//')[-1]
if url.startswith('http'):
for i in range(10):
try:
response = webrequest.get(url)
self.sound = io.BytesIO(response.read())
except:
if i<9:
continue
else:
raise Exception('Failed to load sound data')
break
Comment on lines +232 to +241
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request change: I don't think this is a good way to handle failed requests; something like the image fetching logic might be more appropriate here (also, pyThrow might be preferable over generic Python error types).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rationale behind this code is to make the cmu_graphics more robust; if a network error happens, we don't just want to directly crash the program and instead try again a couple of times. In this spirit, I actually think that the sound fetching logic should be updated to reflect the behaviour seen here

pyThrow isn't defined in cmu_graphics.py nor was it ever used there; instead, normal exception was used. pyThrow is only defined inside shape_logic.py and is merely a simple wrapper of the exception class. We could do something about this, but if we are going to, we need to be consistent with it. So, any new thoughts on this?

elif hasattr(__main__, '__file__'):
self.sound = os.path.abspath(os.path.join(__main__.__file__, "..", url))
else:
self.sound = os.path.abspath(os.path.join(os.getcwd(), url))

self.sound = pygame.mixer.Sound(self.sound)
self.channel = None

def play(self, **kwargs):
default_kwargs = {'loop': False, 'restart': False}
Expand All @@ -227,10 +263,34 @@ def play(self, **kwargs):
raise Exception('The loop argument to Sound.play must be True or False, got ' + repr(loop))
if not isinstance(restart, bool):
raise Exception('The restart argument to Sound.play must be True or False, got ' + repr(restart))
self.sound.play(loop, restart)

loop = -1 if loop else 0
if not self.channel or not self.channel.get_busy():
self.channel = self.sound.play(loops=loop)
elif restart and self.channel.get_sound() != self.sound:
self.channel = self.sound.play(loops=loop)
elif restart:
self.channel.stop()
self.channel = self.sound.play(loops=loop)
else:
self.channel.unpause()

def pause(self):
self.sound.pause()
self.channel.pause()

def setVolume(self, volume: float):
"""
Volume in the range of 0.0 to 1.0 (inclusive)\n
If value < 0.0, the volume will not be changed\n
If value > 1.0, the volume will be set to 1.0
"""
self.sound.set_volume(volume)

def getVolume(self):
"""
Returns the volume (range: 0.0 - 1.0 (inclusive))
"""
return self.sound.get_volume()

SHAPES = [ Arc, Circle, Image, Label, Line, Oval,
Polygon, Rect, RegularPolygon, Star, ]
Expand Down Expand Up @@ -295,7 +355,6 @@ def translateKeyName(keyName, originalLanguage):
return KeyName(TRANSLATED_KEY_NAMES[originalLanguage].get(keyName, keyName))

def cleanAndClose():
shape_logic.cleanSoundProcesses()
try:
app._app.callUserFn('onAppStop', (), redraw=False)
except:
Expand Down Expand Up @@ -1012,6 +1071,7 @@ def interact(self):

import os
import sys
import io
from datetime import datetime
from datetime import timedelta
import json
Expand Down Expand Up @@ -1122,7 +1182,6 @@ def print_debug_info():
slGet = sli.slGet
rgb = sli.rgb
gradient = sli.gradient
slNewSound = sli.newSound
toEnglish = sli.toEnglish
accentCombinations = sli.accentCombinations
t = sli.t
Expand Down
51 changes: 0 additions & 51 deletions cmu_graphics/shape_logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -2289,52 +2289,6 @@ def scalexy(self, varName, k, scaleAnchor = None):
trans[i][i] = k
self.multMat(trans)

class CMUSound(object):
processes = []
def __init__(self, url):
current_directory = os.path.dirname(__file__)
sound_path = os.path.join(current_directory, 'sound.py')
self.soundProcess = subprocess.Popen(
[sys.executable, sound_path], stdout=subprocess.PIPE,
stdin=subprocess.PIPE, stderr=subprocess.PIPE,
cwd=current_directory)
CMUSound.processes.append(self.soundProcess)
self.sendProcessMessage({'url': url})

def sendProcessMessage(self, message):
packet = bytes(json.dumps(message) + '\n', encoding='utf-8')
self.soundProcess.stdin.write(packet)
self.soundProcess.stdin.flush()
self.soundProcess.stdout.readline()
self.soundProcess.poll()
if self.soundProcess.returncode is not None and self.soundProcess.returncode != 0:
print(self.soundProcess.stderr.read().decode('utf-8'))
raise Exception('Exception in Sound.')

def play(self, doLoop, doRestart):
self.sendProcessMessage({
'command': 'play',
'kwargs': {
'doLoop': doLoop,
'doRestart': doRestart
}
})

def pause(self):
self.sendProcessMessage({
'command': 'pause',
'kwargs': {}
})

def __del__(self):
self.soundProcess.kill()

def cleanSoundProcesses():
for p in CMUSound.processes: p.kill()

# clean up processes when the interpreter closes
atexit.register(cleanSoundProcesses)

class CMUImage(PolygonWithTransform):
def __init__(self, attrs):
if attrs is not None:
Expand Down Expand Up @@ -3046,11 +3000,6 @@ def gradient(self, *colors, start=None, **kwargs):

return Gradient(list(colors), 'center' if start is None else start)

def newSound(self, url):
checkArgCount(None, t('Sound'), [t('url')], (url,))
checkString(t('Sound'), t('url'), url, False)
return CMUSound(url)

def slNew(self, className, args):
return (objConstructors[className])(args)

Expand Down
72 changes: 0 additions & 72 deletions cmu_graphics/sound.py

This file was deleted.

8 changes: 3 additions & 5 deletions tests/test_image_gen.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def is_mac_pip_ci():
is_mac = sys.platform == 'darwin'
is_pip = 'pip' in os.getenv('TOX_ENV_NAME', '')
is_ci = os.environ.get('CI', False)
return is_ci and is_mac and is_pip
return is_ci and is_mac and is_pip

def compare_images(path_1, path_2, test_name, test_piece_i, threshold=25):
image_1 = Image.open(path_1)
Expand Down Expand Up @@ -89,10 +89,8 @@ def generate_test_source(test, run_fn, extras='', language='en'):
source_code = ''
source_code += 'import sys'
source_code += '\nimport os'
if sys.platform == 'darwin':
source_code += '\nos.environ["SDL_VIDEODRIVER"] = "dummy"'
if sys.platform == 'win32':
source_code += '\nos.environ["SDL_AUDIODRIVER"] = "dummy"'
source_code += '\nos.environ["SDL_VIDEODRIVER"] = "dummy"'
source_code += '\nos.environ["SDL_AUDIODRIVER"] = "dummy"'
source_code += '\nsys.path.insert(0, os.path.dirname(os.path.dirname(os.path.realpath(__file__))))'
source_code += '\nfrom cmu_graphics import *\n'
source_code += "setLanguage('%s')\n" % (language)
Expand Down
2 changes: 2 additions & 0 deletions tests/test_sound.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@
CMU_GRAPHICS_DEBUG = True
from cmu_graphics import *

os.environ["SDL_AUDIODRIVER"] = "dummy"

music = Sound('https://s3.amazonaws.com/cmu-cs-academy.lib.prod/sounds/Liberty_bell_march.mp3')
os._exit(0)