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

SCons: Cleanup DEBUG, _DEBUG and NDEBUG defines #66303

Merged
merged 1 commit into from
Sep 23, 2022

Conversation

akien-mga
Copy link
Member

  • _DEBUG is MSVC specific so it didn't make much sense to define for Android and iOS builds.
  • iOS was the only platform to define DEBUG. We don't use it anywhere outside thirdparty code, which we usually don't intend to debug, so it seems better to be consistent with other platforms.
  • Consistently define NDEBUG to disable assert behavior in both release and release_debug targets. This used to be set for release for all platforms, and release_debug for Android and iOS only.
  • Due to the above, I removed the only use we made of assert() in Godot code, which was only implemented for Unix anyway, should have been DEV_ENABLED, and is in PoolAllocator which we don't actually use.
  • The denoise and recast modules keep defining NDEBUG even for the debug target as we don't want OIDN and Embree asserting all over the place.

- `_DEBUG` is MSVC specific so it didn't make much sense to define for
  Android and iOS builds.
- iOS was the only platform to define `DEBUG`. We don't use it anywhere
  outside thirdparty code, which we usually don't intend to debug, so it
  seems better to be consistent with other platforms.
- Consistently define `NDEBUG` to disable assert behavior in both `release`
  and `release_debug` targets. This used to be set for `release` for all
  platforms, and `release_debug` for Android and iOS only.
- Due to the above, I removed the only use we made of `assert()` in Godot
  code, which was only implemented for Unix anyway, should have been
  `DEV_ENABLED`, and is in PoolAllocator which we don't actually use.
- The denoise and recast modules keep defining `NDEBUG` even for the `debug`
  target as we don't want OIDN and Embree asserting all over the place.
@akien-mga akien-mga added this to the 4.0 milestone Sep 23, 2022
@akien-mga akien-mga requested review from a team as code owners September 23, 2022 13:22
@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 23, 2022
Copy link
Contributor

@JFonS JFonS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in Embree and OIDN look good.

Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for Android!

@akien-mga akien-mga merged commit f74491f into godotengine:master Sep 23, 2022
@akien-mga akien-mga deleted the scons-cleanup-debug-defines branch September 23, 2022 14:45
@akien-mga
Copy link
Member Author

Cherry-picked for 3.6.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants