Skip to content

Fix custom joke sound playback#4

Merged
AkshajSinghal merged 5 commits into
TruFoundation:mainfrom
ded-furby:fix-joke-sound-file
Jun 2, 2026
Merged

Fix custom joke sound playback#4
AkshajSinghal merged 5 commits into
TruFoundation:mainfrom
ded-furby:fix-joke-sound-file

Conversation

@ded-furby
Copy link
Copy Markdown
Contributor

Closes #2

Summary

  • play the selected joke sound asset instead of always triggering the generic alarm
  • fall back to the existing alarm when file playback is unavailable
  • add a regression test for filename-based playback and pin the existing alarm test to the Linux branch it validates

Testing

  • python -m pytest -q

@AkshajSinghal
Copy link
Copy Markdown
Collaborator

Hey! Really appreciate the effort! The logic in play_audio_file looks solid, and the fallback to play_alarm() ensures we don’t break existing functionality.

In pyfunny.py, if play_audio_file succeeds but somehow raises an exception after starting playback, we might hear both the custom sound and the default alarm. To be safe, maybe we can add a simple flag or move the play_alarm() call strictly into the except block?

@AkshajSinghal
Copy link
Copy Markdown
Collaborator

Thanks for the fix. Please address these three points before merge:

  1. Prevent double playback by ensuring play_alarm() is strictly inside the except block.
  2. Explicitly convert paths to strings (str(sound_path)) in play_audio_file to ensure compatibility with all system subprocess calls.
  3. Add a note to the README that .mp3 and .wav are recommended for cross-platform support across the different audio modules.

@ded-furby
Copy link
Copy Markdown
Contributor Author

I traced the current CI failure to the workflow setup rather than the custom-sound changes themselves. The Python CI job installs the package with python -m pip install -e ., then immediately runs python -m pytest, so the job fails before exercising the branch because pytest is only in the dev extra.

I verified locally that this branch passes with:

python -m pip install -e ".[dev]"
python -m pytest -q

That ran cleanly here: 21 passed in 0.36s. Updating the workflow install step to include the dev extra should unblock the PR checks.

@AkshajSinghal AkshajSinghal merged commit d93fce0 into TruFoundation:main Jun 2, 2026
0 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_play_sound in pyfunny.py ignores the filename and always plays a generic beep

2 participants