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] Consider changing how defines are used in raylib to support configuration from build systems #4411

Open
planetis-m opened this issue Oct 21, 2024 · 15 comments
Labels
enhancement This is an improvement of some feature

Comments

@planetis-m
Copy link
Contributor

planetis-m commented Oct 21, 2024

Description:
Currently, raylib uses #if defined(SUPPORT_X) to conditionally enable/disable certain features like file format support. However, setting a define to 0 from the build system (e.g., -DSUPPORT_FILEFORMAT_PNG=0) doesn't actually disable the feature because the macro is still considered "defined."

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.

Proposed Change:
Switch the current #if defined(SUPPORT_X) checks to something like #if SUPPORT_X == 1. This would allow features to be properly toggled off by defining them as 0 from the build system.

Example:

#ifndef SUPPORT_FILEFORMAT_PNG
#define SUPPORT_FILEFORMAT_PNG 1
#endif

#if SUPPORT_FILEFORMAT_PNG
    // PNG support code
#endif

This would enable features to be turned off using -DSUPPORT_FILEFORMAT_PNG=0 in the build system, without requiring edits to the default configuration file.

Discussion Summary:
This topic was discussed on the raylib Discord server. The current method was found to be limiting for build systems that rely on command-line defines. While the change is agreed to be useful, it was also acknowledged that it's better suited for a future release after the current planned version has been completed.

@znichola
Copy link

znichola commented Oct 27, 2024

I think this could be achieved by passing the define with the -U flag to undefine it, at least this is available with gcc. Not sure how easy this would be to make portable, but it does seem like a lower impact solution.

@planetis-m
Copy link
Contributor Author

planetis-m commented Oct 27, 2024

@znichola no that won't work, we have already tried it. Here's a test you can try:

#include <stdio.h>

#ifndef EXAMPLE_OPTION
#define EXAMPLE_OPTION  1
#endif

int main(void)
{
#if EXAMPLE_OPTION == 1
    puts("option enabled");
#endif
}

@znichola
Copy link

znichola commented Oct 27, 2024

Ah yes of course I see, that's dumb of me, I was thinking from the angle of disabling flags that were added with -D (if for instance using a raylib wrapper and not wanting to modify its build script), but that is not the case with the config.h.

@sagehane
Copy link
Contributor

In regards to the current Zig build system, there's currently no way to undefine a macro so it's currently impossible to disable something like SUPPORT_FILEFORMAT_PNG.

I was thinking if it would make sense to make it so if the value of a SUPPORT macro is set to 0, it should be equivalent to not setting the value.

@Peter0x44
Copy link
Contributor

@sagehane that's exactly what this issue is about, changing all the #if defined (...) to #if will do that
There is no point to do the == 1

@planetis-m
Copy link
Contributor Author

I missed that -DEXTERNAL_CONFIG_FLAGS exists, the change seems unnecessary. Close?

@Peter0x44
Copy link
Contributor

This means nothing in config.h ends up defined at all, so you need to put everything in from scratch and it's more inconvenient.
And potentially your default config options could end up out of sync from what upstream raylib has.

For the case of "stock raylib + jpg" or so you then have to pick every config option that is on by default, then also enable jpg.

But I've taken a look and adjusting raylib for this doesn't seem particularly trivial either.

@JeffM2501
Copy link
Contributor

Yes, we need to do this change, the current system is not really useable.
Has anyone made a PR that does this?

@JeffM2501
Copy link
Contributor

I have created PR#4554 That implements this.

@raysan5
Copy link
Owner

raysan5 commented Nov 29, 2024

@JeffM2501 I have to review this change carefully, all my tools pipelines depend on a custom config and I don't know if it could break my implementations.

  1. What is the specific use case for this change?
  2. Is it required to address any project/issue?
  3. Is it intended for future convenience?
  4. Should any build system be modified to accomodate this change?

It's a big change and affects config.h readability. I want to be sure about it is really needed.

@JeffM2501
Copy link
Contributor

It is a big change yes.

1 & 2) The main use case for this is that I need to disable built in screen capture and GIF recording. I do not want to maintain a fork of raylib with a custom config.h. I do not want to provide an entire config.h. Both of these thigns have a maintnenance and update cost that I do not want to pay. I simply want to disable the one feature I don't need via the build system since I am building raylib with my project.

  1. This makes it very easy in the future to enable/disable features one at a time. So yes, there is future use here as well.

  2. Build systems do not need to change. If they provide a custom config.h that is still used. If they do not then the same defaults are in place and nothing changes. They simply now have the option to disable/enable individual features.

The platform defines do NOT change, only the support and built in RLGL limits. Code that was checking
#If defined(SUPPORT_FLAG)
simply needs to change to
#if SUPPORT_FLAG

I agree this is a HUGE change, but I feel it is absolutely necessary. The current way that #defines are used is basically useless unless you want to maintain a fork of raylib, or duplicate an entire file. This detracts from the simple nature of raylib.

Because all the checks are just checking #if defined(), then it doesn't matter what the define in the config is.
The config has values set to 1, so that makes it look like the value is used, when it is not. If the defines are going to have values, then those values should be used. If the config is going to define something it should check if it's not already defined, that's just common sense to avoid a duplicate definition error. Basically the config.h file looks like you can just override one thing, but in reality you can't, and that's just confusing. This method is also in line with other projects.

The only other real option is to remove many things from config.h entirely and make things be more dynamically settable at runtime, not compile time.
Compile time flags make sense for code based things, like what modules are supported, but I feel that it's gotten way too bloated with things that are not compile time, such as screen capture, and gif recording. If you don't want to do this change, then we should discuss removing as many things from config.h as possible and making them dynamic.

@znichola
Copy link

znichola commented Nov 30, 2024

config.h

#define OPTION

custom.h

#undef OPTION

main.c

#include <stdio.h>
#include "config.h"
#include "custom.h"

int main(void)
{
#ifdef OPTION
    puts("option enabled");
#else
    puts("option disabled");
#endif
}

This does not allow you to disable from command line, but unless I'm missing something you don't need a full copy of the config, only a custom config to undef what you need.

EDIT:
I was looking through how the build files are setup, and I think you would need to add EXTERNAL_CONFIG_FLAGS to your build system then in your custom.h config, put

#inlcude "config.h"

#undef OPTION

It's annoying, but I don't think quite as bad as you describe.

@orcmid
Copy link
Contributor

orcmid commented Nov 30, 2024

@znichola
I think the challenge around config.h is that it is part of raylib/src and modifying it locally creates a maintenance problem when using a clone or fork of the main repository. Any time there is synchronization with raylib in some manner, including installing a new raylib release source, the user-modified config.h will be clobbered. This is not made prominent in raylib.

I have distributions like that also, where some files are user-modifiable with respect to compiler options. I keep those all near each other along with instructions about how to preserve modified forms before installing or re-installing the distributed version in place.

@znichola
Copy link

znichola commented Dec 1, 2024

@orcmid
I might be missing something as I've not had to do this setup, I came to this post through zig.

But, could I not have a custom config that #include the raylib config, and then undef what you want after, my post might not be very clear, so I'll try again using a more concrete example.

my_config.h

#ifndef MY_CONFIG
# define MY_CONFIG

# include "raylib/config.h" // include raylib config, it's transparent to any changes upstream

#undef SUPPORT_FILEFORMAT_PNG // disabled for my custom build 

#endif // MY_CONFIG

@Peter0x44
Copy link
Contributor

@znichola surely this requires editing raylib source code anyway, right?
That doesn't make it any more convenient than just editing config.h yourself to enable/disable what you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an improvement of some feature
Projects
None yet
Development

No branches or pull requests

7 participants