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

Added miniaudio backend to cmake contrib options #334

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

danman113
Copy link

@danman113 danman113 commented Nov 17, 2021

Hello, still pretty new to cmake but this is what I needed to add to the cmake files in order to get miniaudio working correctly on both OSX (clang), windows (msvc) and linux (arm/g++).

I also turned the new Miniaudio setting on by default and turned sdl2 backend off by default. My reasoning being that since miniaudio is actually included in the project, it should be the default. LMK if you think differently and I'll change it back.

@danman113 danman113 changed the title Added miniaudio backend to cmake contrib options. Added miniaudio backend to cmake contrib options Nov 17, 2021
@danman113
Copy link
Author

Curious what the initial contributor @valid-ptr thinks of this

@valid-ptr
Copy link
Contributor

Curious what the initial contributor @valid-ptr thinks of this

Nice PR.

It can be a good idea to add ${BACKENDS_PATH}/miniaudio/miniaudio.h to the BACKENDS_SOURCES also.
Then it will appear in the project (generated by cmake)

@danman113 danman113 closed this Nov 23, 2021
@danman113 danman113 reopened this Nov 23, 2021
@danman113
Copy link
Author

It can be a good idea to add ${BACKENDS_PATH}/miniaudio/miniaudio.h to the BACKENDS_SOURCES also. Then it will appear in the project (generated by cmake)

Forgive my unfamiliarity with cmake but is this necessary? soloud_miniaudio.cpp already includes miniaudio.h? It seems to work just fine without adding to the backend sources. Is there a non-functional reason to add that?

@danman113
Copy link
Author

@jarikomppa any thoughts on this?

@valid-ptr
Copy link
Contributor

Header should be included because cmake can generate the project for Visual Studio or Xcode as instance.
The reason to include - is that the developer can access this header from IDE and at least can see that it's presented in the project.

PS
If you compile some program like
g++ main.cpp
you should not pass headers (which main.cpp uses) of course

PPS
Imaging the header-only library and how it will be presented in the IDE after cmake-generator phase ;^)

@danman113
Copy link
Author

Header should be included because cmake can generate the project for Visual Studio or Xcode as instance. The reason to include - is that the developer can access this header from IDE and at least can see that it's presented in the project.

PS If you compile some program like g++ main.cpp you should not pass headers (which main.cpp uses) of course

PPS Imaging the header-only library and how it will be presented in the IDE after cmake-generator phase ;^)

Got it, thanks for the explanation! I went ahead and added it to BACKENDS_SOURCES

contrib/src.cmake Show resolved Hide resolved
@danman113
Copy link
Author

Bump?

@danman113
Copy link
Author

Monthly Bump

@danman113
Copy link
Author

Bi-annual Bump

@TheSunCat
Copy link

I'm running into this issue using CMake, thanks for making this PR! I guess I'll switch to your fork until it gets merged.

@danman113
Copy link
Author

I'm running into this issue using CMake, thanks for making this PR! I guess I'll switch to your fork until it gets merged.

Glad I could help!

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.

4 participants