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 4 commits 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

@raysan5
Copy link
Owner

raysan5 commented Dec 11, 2024

@JeffM2501 I see the usefulness of this big change but trying to understand what problem it is currently addressing. As per the issue description:

This limitation makes it difficult to selectively disable certain features without manually editing config.h, especially when working with different build systems or when trying to configure raylib via command-line flags alone.

Yeah, that's right but, afaik, there have been no real use-case issues related since config.h was introduced. Also, this change, makes config.h bigger and less readable for new users. For my specific use cases, the tools I develop, I just overwhite config.h (defining EXTERNAL_CONFIG_FLAGS) and define the flags I want, depending on the build system I use (VS2022 example, Linux/Web). I have to see how this change affects my pipelines because it could require updating +40 projects...

I don't know... I got the feeling this redesign addresses a potential issue that nobody really had... at least yet.

@JeffM2501
Copy link
Contributor Author

I am trying to make a game where I allow the user to map items to F12, I can not because the built in screen capture and recording is using that key. That is my real world case.

@JeffM2501
Copy link
Contributor Author

You can still overwrite config.h and it will be the same as before. This just means you do not have to if you want another option. You are the author of raylib so it's not that big of a deal for you to just make a new config for each project, You always know what is in config.h. But if someone does not want to fork raylib, or does not know everything that is in raylib, that makes it much harder for them to keep raylib up to date.

@raysan5
Copy link
Owner

raysan5 commented Dec 18, 2024

@JeffM2501 Thanks for the further explanation, just one last concern, if I define make -DSUPPORT_FILEFORMAT_KTX, does it default to 1 and gets enabled? Or is it required to make -DSUPPORT_FILEFORMAT_KTX=1?

@JeffM2501
Copy link
Contributor Author

@JeffM2501 Thanks for the further explanation, just one last concern, if I define make -DSUPPORT_FILEFORMAT_KTX, does it default to 1 and gets enabled? Or is it required to make -DSUPPORT_FILEFORMAT_KTX=1?

It is required that you set the value to 1.
#define NAME does not add any default value, since #define is just a string replacement, The previous version simply checked if the define existed, not what it's value is. This was the core problem, there was no way to change it's value once it's defined. #define NAME defines NAME as nothing, so when anything that uses NAME, it gets replaced with nothing.
so #if NAME comes out to just #if and is a syntax error.

@Peter0x44
Copy link
Contributor

@JeffM2501 @raysan5 That's not true!!! I just tested myself

#include <stdio.h>
int main(void)
{
#if EXAMPLE_OPTION
                puts("option enabled");
#else
                puts("option disabled");
#endif
}
$ gcc test.c && ./a.exe
option disabled
$ gcc test.c -DEXAMPLE_OPTION && ./a.exe
option enabled
$ gcc test.c -DEXAMPLE_OPTION=0 && ./a.exe
option disabled
$ gcc test.c -DEXAMPLE_OPTION=1 && ./a.exe
option enabled

It doesn't need to be =1 to enable, no compromise necessary

@raysan5
Copy link
Owner

raysan5 commented Dec 18, 2024

@Peter0x44 thanks for the test! that's really great to know! Is that behaviour consistent between compilers or it is actually "Undefined Behaviour" and could be managed in other ways by other compilers? I'm trying to find this info but I couldn't...

UPDATE: ChatGPT tells me:

Yes, if you define a macro using the -D flag without specifying a value, it implicitly evaluates to 1. 
This behavior follows the convention of the C preprocessor.

@Peter0x44
Copy link
Contributor

https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html

-D name
Predefine name as a macro, with definition 1.

@raysan5
Copy link
Owner

raysan5 commented Dec 18, 2024

**Testing ChatGPT, quite impressive! 😄 **


The choice between checking if a flag is defined (#ifdef/#if defined()) or checking its value (#if CUSTOM_FLAG) depends on your specific use case and coding style. Here's a breakdown of the two approaches and their implications:


1. Checking if a Flag is Defined (#ifdef or #if defined())

Syntax:

#ifdef CUSTOM_FLAG
// Code for enabled feature
#endif

Or:

#if defined(CUSTOM_FLAG)
// Code for enabled feature
#endif

Behavior:

  • The feature is considered "enabled" if CUSTOM_FLAG is defined, regardless of its value.
  • The flag does not need to have a numeric value.
  • If CUSTOM_FLAG is not defined, the code is excluded.

Use Case:

  • When you only care whether a flag is defined or not.
  • Ideal for simple on/off toggles without requiring numeric or value-based logic.

Pros:

  • Simple and clean for basic feature toggles.
  • Does not require explicitly setting the value of the flag.

Cons:

  • Cannot easily support more complex configurations (e.g., different modes).
  • The code behavior depends solely on whether the flag exists, not its value.

2. Checking a Flag's Value (#if CUSTOM_FLAG)

Syntax:

#if CUSTOM_FLAG
// Code for enabled feature
#endif

Behavior:

  • The feature is considered "enabled" if CUSTOM_FLAG evaluates to a truthy value (non-zero).
  • The flag must either be defined with a numeric value (-DCUSTOM_FLAG=1) or default to 1 when defined with -DCUSTOM_FLAG.

Use Case:

  • When you might want to assign the flag a specific value, or use it in numeric or multi-level configurations.
  • Ideal for more flexible setups, e.g., enabling multiple modes or levels of a feature.

Pros:

  • Supports more nuanced configurations (e.g., -DFEATURE_MODE=2 for mode-specific behavior).
  • Explicitly checking values makes the intent clearer when toggling features programmatically.

Cons:

  • If the macro is not defined, you need to ensure it defaults to 0 to prevent compilation errors. Use:
    #ifndef CUSTOM_FLAG
    #define CUSTOM_FLAG 0
    #endif

Comparison

Aspect #if defined(CUSTOM_FLAG) #if CUSTOM_FLAG
Simplicity Good for simple on/off toggles Slightly more complex
Flexibility Limited to "defined" vs "not" Can use numeric values
Error Handling No issues if undefined Requires #ifndef fallback
Multi-level Config Not supported Supported
Code Clarity Clearly indicates "defined check" Explicit about value logic

Best Practice

  1. For simple toggles (e.g., enable/disable a feature):
    Use #if defined(CUSTOM_FLAG) or #ifdef CUSTOM_FLAG.

  2. For configurable features (e.g., modes, levels):
    Use #if CUSTOM_FLAG and assign meaningful values during compilation (e.g., -DFEATURE_LEVEL=2).

  3. To ensure robustness:
    Combine both patterns by defining defaults for macros in your code to avoid accidental undefined behavior:

    #ifndef CUSTOM_FLAG
    #define CUSTOM_FLAG 0
    #endif
    
    #if CUSTOM_FLAG
    // Feature is enabled
    #endif

This way, your code is safer and can handle cases where the macro is not defined.

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.

3 participants