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

Compatibility improvements: remove zsh and graceful errors if sound fails #262

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

krs013
Copy link
Collaborator

@krs013 krs013 commented Oct 25, 2023

What it says in the title. WSL had cryptic float errors if libpulse0 wasn't installed, now the sound module will report an error and revert to emulating the sound.

Also replaced zsh in makefile with find. This should be available on all Unix systems as far as I know.

@Baekalfen
Copy link
Owner

Baekalfen commented Oct 25, 2023

I rebased your code so we don't have the merge commit, and the extra commit, that fixes logging in the first. Also, I removed the self.device = 0, as that isn't need (if it was, we should have added it in the error case above it).

@krs013
Copy link
Collaborator Author

krs013 commented Oct 25, 2023

Sounds good, and sorry for the messy history. I appreciate the changes

self.device = 0 was just there so the module doesn't leave __init__() with that field undeclared, but that's more of a tidiness thing than functional. It might lead to errors that won't show up in Python but will in CPython/PyPy. If there's a better place to put it, that's fine--we could even have it initialized to 0 before calling SDL_Init and leave it at that. Not a big deal either way once we remove SDL code from sound.py, which is still something I intend to do to match how we did lcd.py.

@Baekalfen
Copy link
Owner

It might lead to errors that won't show up in Python but will in CPython/PyPy

I checked. All uses are protected by if self.enabled, so it won't be a problem.

It would be awesome if you have time to move the SDL2 stuff out, like you say.

@Baekalfen Baekalfen merged commit 8e27df9 into Baekalfen:master Oct 25, 2023
28 checks passed
@krs013 krs013 deleted the small-fixes-1023 branch October 25, 2023 15:31
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.

2 participants