Bug in Example code, caused me problems!

[Beernutts]

In the collision callbacks example (http://files.slembcke.net/chipmunk/rele ... nCallbacks), there is this:

Code: Select all

postStepRemove(cpSpace *space, cpShape *shape, void *unused)
{
  cpSpaceRemoveBody(space, shape->body);
  cpBodyFree(shape->body);
  
  cpSpaceRemoveShape(space, shape);
  cpShapeFree(shape);
}

In cpSpaceRemoveShape() (in v5.3.4), it does this:

Code: Select all

cpBody *body = shape->body;
	if(cpBodyIsStatic(body)){
		cpSpaceRemoveStaticShape(space, shape);
		return;
	}

	cpBodyActivate(body);
	
	cpAssertSpaceUnlocked(space);
	cpAssertWarn(cpHashSetFind(space->activeShapes->handleSet, shape->hashid, shape),
		"Cannot remove a shape that was not added to the space. (Removed twice maybe?)");
	
	cpBodyRemoveShape(body, shape);
	
	removalContext context = {space, shape};
	cpHashSetFilter(space->contactSet, (cpHashSetFilterFunc)contactSetFilterRemovedShape, &context);
	cpSpaceHashRemove(space->activeShapes, shape, shape->hashid);

I don't think it should be messing with body when it's been freed. So, should you remove and free the shape, then the body? Or remove body, remove shape, and then free? (EDIT: Wait, you can't free shape before removing body, scratch that)

I was getting odd crashes, and other issues. Am I wrong on this?

Thanks.

[slembcke]

Ah yes, you are right. Sorry about that. Specifically, freeing a body before removing shapes that point to it is bad as the shapes need to remove themselves from the list of shapes on the body. This only became an issue as of 5.2 (I think) when I added the sleeping algorithm. So it is fairly new.

The best practice is to remove all chipmunk objects before freeing any of them. Doing that will prevent new issues like this from happening in the future. I need to add that to the documentation.

[Beernutts]

Hi,

I was looking back through the documentation, and I wanted to see if the example had been fixed, but it had not.

Please take one minute to change the example (so you remove the objects then free them) as I know there are people who will use that code, and will cause them problems (as it did for me 2 1/2 years ago).

Thanks.