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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sound! 馃攰 #326

Merged
merged 6 commits into from Jul 28, 2019
Merged

Sound! 馃攰 #326

merged 6 commits into from Jul 28, 2019

Conversation

AstraLuma
Copy link
Member

@AstraLuma AstraLuma commented Jul 16, 2019 •
edited

Add a sound system!

Part of #327

@AstraLuma
Copy link
Member Author

So, a quirk: pygame has a limited number of parallel sounds (channels) that can be played at once, defaulting to 8.

My initial thought was to just allocate more channels if we run out, but then I realized that 1. Most naive implementations would spam this easily, 2. It's not obvious when to scale down.

@pathunstrom
Copy link
Collaborator

I figure we pick a number we think is reasonable and manageable and go with that. We can probably surface a parameter in the engine to modify it as well?

@AstraLuma
Copy link
Member Author

Yeah, a parameter is easy enough. The pygame mixer surfaces so many parameters, though, and tbh I don't think game developers should have to think about that?

@pathunstrom
Copy link
Collaborator

I agree. We can pick the ones that are important and let advanced users subclass and modify as they like.

@AstraLuma AstraLuma mentioned this pull request Jul 18, 2019
Closed
4 tasks
@AstraLuma
Copy link
Member Author

Reorganized things so sound is broken up across several PRs.

@AstraLuma AstraLuma marked this pull request as ready for review July 18, 2019 16:34
@AstraLuma AstraLuma requested a review from a team as a code owner July 18, 2019 16:34
@AstraLuma AstraLuma added this to Review in progress in Release v0.7.0 Jul 20, 2019
Release v0.7.0 automation moved this from Review in progress to Reviewer approved Jul 27, 2019
pathunstrom
pathunstrom approved these changes Jul 27, 2019
View reviewed changes
Copy link
Collaborator

@pathunstrom pathunstrom left a comment

Choose a reason for hiding this comment

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

Works wonderfully for basic use case.

@pathunstrom
Copy link
Collaborator

bors r+

@pathunstrom pathunstrom added the bors Someone has bors r+ this PR label Jul 27, 2019
bors bot added a commit that referenced this pull request Jul 27, 2019
@bors @AstraLuma
Merge #326
3ca7c5b 
326: Sound! 馃攰 r=pathunstrom a=astronouth7303

Add a sound system!

Part of #327

Co-authored-by: Jamie Bliss <jamie@ivyleav.es>
@bors
Copy link
Contributor

bors bot commented Jul 28, 2019

Build succeeded

  • docs
  • Linux python:3.6-slim
  • Linux python:3.7-slim
  • macOS PYTHON:3.6.8
  • macOS PYTHON:3.7.2
  • pep517
  • Windows python:3.6-windowsservercore-1809
  • Windows python:3.7-windowsservercore-1809

@bors bors bot merged commit b335b89 into ppb:master Jul 28, 2019
Release v0.7.0 automation moved this from Reviewer approved to Done Jul 28, 2019
@AstraLuma AstraLuma deleted the sound branch May 11, 2020 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pathunstrom pathunstrom pathunstrom approved these changes

Assignees
No one assigned
Labels
bors Someone has bors r+ this PR
Projects
No open projects
Release v0.7.0
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@AstraLuma @pathunstrom