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

[Config.h] Make all the defines in config.h be override-able by the build system #4554

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JeffM2501
Copy link
Contributor

Per issue #4411 this PR makes all the config defines first check if they have been set by the build system, and doesn't just check for the define, but instead check the value.
This allows the config.h values to be individually set by the build system.

@Peter0x44
Copy link
Contributor

Are you sure this is enough? Anything you pass on the command line will still get redefined in config.h itself.
This lets you turn on extra things, but not turn off things, and not change the value of anything.

@JeffM2501
Copy link
Contributor Author

Are you sure this is enough? Anything you pass on the command line will still get redefined in config.h itself.

Yes because config.h now checks if the label is not already defined first before defining it, so things on the command line will be defined before the preprocessor runs and config.h will not replace them. This ensures that all things are still defined. I have tested this in msvc and get expected results, I can build raylib with a define in the build system only to disable screenshots. Please test it in other systems.

What this won't do is disable things at runtime, since it's all still baked into the compile with preprocessor directives.

@Peter0x44
Copy link
Contributor

I missed the change to config.h because GitHub collapsed it, sorry! I will test this with gcc/make on Linux later.

@Peter0x44
Copy link
Contributor

Peter0x44 commented Nov 30, 2024

I tested this with:
make CUSTOM_CFLAGS="-DSUPPORT_FILEFORMAT_PNG=0" and I was then unable to load PNGs as expected
LGTM

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