Skip to content

SCons: Cleanup DEBUG, _DEBUG and NDEBUG defines#66303

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
akien-mga:scons-cleanup-debug-defines
Sep 23, 2022
Merged

SCons: Cleanup DEBUG, _DEBUG and NDEBUG defines#66303
akien-mga merged 1 commit into
godotengine:masterfrom
akien-mga:scons-cleanup-debug-defines

Conversation

@akien-mga

Copy link
Copy Markdown
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

@JFonS JFonS left a comment

Copy link
Copy Markdown
Contributor

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.

@m4gr3d m4gr3d left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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