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

Bring back cpRemoveCollisionHandler #104

Closed
ewmailing opened this issue Mar 16, 2015 · 2 comments
Closed

Bring back cpRemoveCollisionHandler #104

ewmailing opened this issue Mar 16, 2015 · 2 comments
Assignees

Comments

@ewmailing
Copy link

It looks like cpRemoveCollisionHandler was removed in Chipmunk 7.
I request you bring it back.

As far as I can tell, you can't use NULL to unset a function. Doing so will cause a crash within the engine. The only way to reset it is to have the original default function pointer addresses which are private internals of the engine and not accessible publicly. Also, it looks like recalling cpAddCollisionHandler doesn't reset previous set callbacks.

I use Chipmunk with language bindings so there is a potential issue that the callback functions can go away during runtime. Being able to reset the default functions is useful to prevent it from calling dead functions.

Incidentally, I also miss having a function to set the functions as in Chipmunk 6. The new struct technique is harder for language bindings and the persistence of setting things in a returned struct is not obvious, nor memory management/life-cycle issues that come with it.

@slembcke
Copy link
Owner

There used to be a lot of (really gross) code to implement removing collision handlers in the past. Even after all the work that went into fixing all the known bugs it caused, there were still a lot of really nasty gotchas. For example, removing a handler was actually deferred and could cause nasty dangling pointer issues if you freed your user data object after removing the handler.

I decided a couple years ago that it was a misfeature, and that there really was no way to make it work correctly, sensibly, or safely. The new cpCollisionHandler structs were supposed to fix a lot of that. Since collision pairs reference the handler instead of copying the callback pointers, changes to the handler are immediate with no surprises or dangling pointer issues.

You are unfortunately right that the new struct way has a couple new issues. Setting a callback back to the default is difficult (not really on purpose). I'm not entirely certain how to improve that since the default changes based on if the handler is a regular or wildcard handler.

the persistence of setting things in a returned struct is not obvious, nor memory management/life-cycle issues that come with it

Since you cannot remove a handler, it exists as long as the space does. This is the same as how it used to work if you weren't explicitly removing handlers. This was meant to simplify the (basically undocumented and complicated) memory lifecycle problems with removing handlers in v6. Since Chipmunk only reads from the handlers, whatever values you write into them can be considered permanent.

@slembcke slembcke self-assigned this Jun 29, 2015
@ewmailing
Copy link
Author

I made manual fixes to the .def file in my branch. It also contains other things that may be of interest to you.
https://github.com/ewmailing/Chipmunk2D/commits/Blurrr7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants