erincatto / box2d

Box2D is a 2D physics engine for games
https://box2d.org
MIT License
8.09k stars 1.52k forks source link

Added changeable UserData typedef (#161) #635

Closed 1aam2am1 closed 3 years ago

1aam2am1 commented 4 years ago

Based on issue from some time ago. I have wrote typedef definition to change user data type based on their needs. This code isn't invasive and don't change current behavior in any way.

And now user can change data type in simple way by configuring settings. Example:

class b2Virtual;
typedef std::shared_ptr<b2Virtual> b2BodyUserData;

Future development way: typedef change based on cmake variable on configure step.

thegrb93 commented 4 years ago

I thought the point of void* was so you can give a pointer to any type you want. Not seeing the benefit to this change.

thegrb93 commented 4 years ago

Ah shared_ptr makes sense, nevermind.

1aam2am1 commented 4 years ago

As I write program using box2d I always forget to destroy my data. And sometimes my data is shared between multiple objects. Therefore making this change don't break existing code. I would even use.

class b2Virtual; typedef std::shared_ptr<b2Virtual> b2BodyUserData;

And compile it as that. In this way user can create its own b2Virtual class. As box2d can only see declaration and its only need this declaration and not definition i can even replace dll without recompiling.

thegrb93 commented 4 years ago

The only question I have is if bodies' and fixtures' deconstructors are reliable and if those types get copied a lot (lots of copying shared_ptrs will degrade performance). I think the answers are yes for reliable and no for copied a lot, but would be good to be sure.

1aam2am1 commented 4 years ago

If even body would not reliable than what could be? But destructor should be reliable as it is called on b2_world:216, body would not be copied to much as this could degrade performance even without shared pointer. But this is question for @erincatto

erincatto commented 4 years ago

Thanks for the PR. I have a different solution for this problem. When I get a PR up I'll add you guys to review.

erincatto commented 4 years ago

Please look at https://github.com/erincatto/box2d/pull/643

thegrb93 commented 4 years ago

lgtm

1aam2am1 commented 4 years ago

@erincatto I see that you want to give user macro that enables including of our own macros and user data type. As it is good idea. As we don't need to change Box2d code at all. I would write it somewhat different.

First I don't want to change all defines only part but in existing way I need to define them all. I would write checks before each define if it was defined before if not then define default.

Second check if macro "B2USERFILEPATH" exists and include it instead of default file name as I then don't need to add my file to Box2d search paths.

Third deleting user data from b2userdef and so on. Is controversial idea as it could be helpful.

thegrb93 commented 3 years ago

This can be closed now