Collision-separate callback, destroyed shapes

[atis]

Is it a known bug that if you destroy shapes and remove them from space, you still get a collision-separate event for them (they were in contact at the time of their destruction), but the shape pointers in arbiter struct point to some freed/garbage memory?
Maybe I should provide more details.. but I'm lazy and hope the question makes it clear what the problem is.

[slembcke]

Hmm. No, that makes sense.

I actually thought about if removing shapes would work correctly, which it does. It didn't occur to me that it holds a dangling pointer to the shape until the callback is called however. I guess there isn't really that much that the API can do to help you out with that as it really is tending towards object management in your own game engine. I do need to document that people need to be careful of that though.

The big problem is when you have simultaneous collisions, one that removes and one that uses a separate() callback. If you are using manual memory management beyond just Chipmunk, I'm not sure there is anything the API can do to help you with that.

[atis]

Well, forget the destroying part.
Should it be so that a collision callback is invoked for shape that has been removed from space? I mean, the callbacks are not called for shapes that haven't been added to it, and (it seems to me) every other type of callback, except separate, will not be called if any of the shapes involved have been removed from space.
So it just seems a bit unexpected and counter-intuitive.

I would propose that during cpSpaceRemoveShape(), any references to the removed shape within the space are found, and the arbiters containing them removed; or alternatively, the shape pointer within arbiters could be set to NULL. Ok, I'm not too familiar with Chipmunk internals, so maybe that's hard to do. It's just that, in my mind, it seems like the logical thing to do.

[slembcke]

Ah. I see what you are getting at now.

Personally I think it's important that the separate() callback gets called even when an object is removed. At least for me, I can't think of a reason I wouldn't want the separate() callback to always be called. For instance, for tracking if an object is touching anything you increment a number in the begin() and decrement it in separate(). If the number is greater than zero, the object is touching something.

I've also used them (in Unity at least) for tracking certain special conditions. There are a few ways in Unity to get a collision begin but not separate event, and we still haven't figured out why exactly. As a result, we sometimes have "doppelganger" copies of our character object that hang in the air. It was very frustrating.

To fix the issue of calling separate() after the object is destroyed, it occurred to me that I could simply call all it's separate() callbacks before actually removing the shape. I think that is probably what I will do. I can also add boolean parameter to the callback that lets you know it was called because of a removal. This would make compiler warnings, but would be backwards compatible otherwise.

[atis]

Calling separate() before removal sounds like a great idea! I believe that would take care of all the issues, as well as intuitive expectations. Is it really necessary though to know that separate() was called due to one of the shapes being removed? Well, maybe..

[slembcke]

I dunno. I would say not, I guess I thought you thought that calling separate() on removal was undesirable. I figured there was probably a reason for not wanting it.

I'll finish up merging in the block allocation stuff and then get to work on this!

[slembcke]

Done and committed to trunk.

[atis]

I dunno. I would say not, I guess I thought you thought that calling separate() on removal was undesirable. I figured there was probably a reason for not wanting it.

Oh no, I just didn't think of the idea that separate() could be called before removing the shapes, so I came up with less elegant solutions..
Anyways, just tried trunk, and it works great! This means I won't have to code any silly workarounds for the dangling pointer issue.

Thank you so very much.