search

CesiumGS / cesium-unreal

Bringing the 3D geospatial ecosystem to Unreal Engine
https://cesium.com/platform/cesium-for-unreal/
Apache License 2.0
902 stars 287 forks source link

Fix IntrusivePointer threadID assert #1365

Closed csciguy8 closed 5 months ago

csciguy8 commented 5 months ago

This assert is triggering in debug, when it shouldn't.

It may look like the cause is the latest cesium-native changes, https://github.com/CesiumGS/cesium-native/pull/806 ... but really the problem is that the cesium-for-unreal build defines NDEBUG, even in debug builds (!!).

For the previously mentioned PR, this resulted in a mismatch in the header of ReferenceCounted. The member declaration order was not the same in the cesium-native and cesium-for-unreal builds. A subtle bug.

Since there may be other subtle bugs we may not be aware of, change the build of cesium-native to always define NDEBUG, even in debug, to avoid these code mismatches.

csciguy8 commented 5 months ago

@kring If I could borrow your eyes for a minute or so... this is my first time touching this file. Also, you're already familiar with what I'm trying to solve. Thanks!

kring commented 5 months ago

That looks perfect to me @csciguy8!

csciguy8 commented 5 months ago

This issue has been raised in the Unreal forums. Here's one of the posts... https://forums.unrealengine.com/t/why-is-ue-build-debug-never-defined/1213123

In this case, the user defined their own debug define in the end, which may be what we need to do in cesium-native eventually.

kring commented 5 months ago

Thanks @csciguy8!