OGRECave / ogre

scene-oriented, flexible 3D engine (C++, Python, C#, Java)
https://ogrecave.github.io/ogre/
MIT License
3.91k stars 963 forks source link

Add set/get default color implementation for Bullet DebugDrawer class #3052

Open wil92 opened 6 months ago

wil92 commented 6 months ago

Currently, is only possible to debug OgreBullet wrapper with the default black wireframe color. The only workaround was possible if you create your own debug wrapper by inhering from the existing one and then overwriting set/get defaultColors methods involving a lot of code for such a simple thing.

paroj commented 6 months ago

with the default black wireframe color

the colors should not be all black by default: https://github.com/bulletphysics/bullet3/blob/6bb8d1123d8a55d407b19fd3357c724d0f5c9d3c/src/LinearMath/btIDebugDraw.h#L40-L47

to fix the CI, you either have to ignore the new methods or rename the nested class as: https://github.com/OGRECave/ogre/blob/1662ca06f318a704d47ae61e15914edd1830c806/OgreMain/include/Ogre.i#L734

wil92 commented 6 months ago

@paroj, sorry for my ignorance, but not sure how could I ignore the methods or rename them, can you please share some documentation that I can follow to fix the pipelines?

About the color: m_deactivatedObject should be equal to btVector3(0, 1, 0) (green), but instead I got it rendered in black, this is why I said black. In any case, the idea is to be able to manipulate this color from the Ogre wrapper.

paroj commented 6 months ago

but not sure how could I ignore the methods or rename them

the renaming snipped is linked in my post

should be equal to btVector3(0, 1, 0) (green), but instead I got it rendered in black,

I looked into this, as it should have been working and its a typo:

https://github.com/OGRECave/ogre/pull/3053/commits/f25eb7f8ee82aa70efac552ec7ddd89d41239aee

if the default colours work, is this PR still relevant?

wil92 commented 6 months ago

@paroj, nice that you found the error. I don't need the change I made, but still, I think it could be good if someone would like to change the default color values. I was trying to fix the pipeline, but I honestly never worked with SWIG before, and I have no idea what to change to make it work. And I can't reproduce the error locally.

paroj commented 6 months ago

ok, lets keep it as a draft for now then. It would be difficult to merge now as the change breaks ABI in 14.2.x. I can come back for 14.3 to this.