Chipmunk 6.0 Beta

[dieterweb]

Thanks!

[dieterweb]

I have a problem with chipmunk 6.

Every now and then I get a BAD_ACCESS in this code block when I remove a shape from the space. Any idea what can cause this?

Code: Select all

static void
removeArbiterFromBodyList(cpBody *body, cpArbiter *arb)
{
	cpArbiter **prev_ptr = &body->arbiterList;
	cpArbiter *node = body->arbiterList;
	
	while(node && node != arb){
BAD ACCESS ->		prev_ptr = (node->body_a == body ? &node->next_a : &node->next_b);
		node = *prev_ptr;
	}
	
	// Need to double check, but there is probably quite a few situations where an arbiter isn't in the body list.
	//cpAssert(node, "Internal Error: Attempted to remove an arbiter from a body it was never attached to.");
	
	if(node){
		(*prev_ptr) = (node->body_a == body ? node->next_a : node->next_b);
	}
}

[dieterweb]

We did some debugging.

The BAD ACCESS happens when we free bodies we no longer need.

In our game we often change the body for a shapes. So the shape remains in space, but we set it to a new body. The old one will be removed. We also have often many shapes attached to the same body. This has not caused any trouble in chipmunk 5.

[slembcke]

Hmm. So Chipmunk stores some information on what shapes are connected to what body and which bodies are connected to which arbiters, etc. If that was triggering a crash now it was probably causing a mild bug with the sleeping code before too. I'll have to make the cpShapeSetBody() function update the relevant linked lists, and mark it as mandatory in the documentation.

I probably won't have time to get to this until next week. For now the simplest workaround I can think of is to remove the shape, set the body, then re-add the shape.

[slembcke]

@dieterweb
I fixed the issue with swapping a body, though I haven't had a chance to test it well yet.

Also, I looked at the patch from that other thread again on rouge static bodies. Since writing that cpBodyStaticRogue() function I added cpBodyNewStatic() as an alternative in the official API in 5.3.3 (I think?). Basically these two chunks of code are the same:

Code: Select all

cpBody *body = cpBodyNew(INFINITY, INFINITY);
cpBodyStaticRogue(body);

// is the same as:

cpBody *body = cpBodyNewStatic();

[dieterweb]

Cool, I will try that.

[dieterweb]

I tried the new version.

the cpBodyAdd/RemoveShape functions are doing what we already did manually. I still get a bad access when trying to free unused bodies. I will try to get more informations about when this happens exactly.

Sleeping does work with the cpShapeNewStatic, however it is crashing after some time

Code: Select all

static inline Node *
SubtreeRemove(Node *subtree, Node *leaf, cpBBTree *tree)
{
	if(leaf == subtree){
		return NULL;
	} else {
BAD ACCESS ->	Node *parent = leaf->parent;
		if(parent == subtree){
			Node *other = NodeOther(subtree, leaf);
			other->parent = subtree->parent;
			NodeRecycle(tree, subtree);
			return other;
		} else {
			NodeReplaceChild(parent->parent, parent, NodeOther(parent, leaf), tree);
			return subtree;
		}
	}
}

I will try what happens with the old spartial hash

[dieterweb]

sleep problems also happen when using the spartial hash.

It does not seem to happen with very small values for sleepTimeThreshold. When setting it to 0.01 it seems to work. values over 0.5 result in strange problems and crashes.

[dieterweb]

I fixed the sleeping problem by calling

cpBodyActivate(body);

at the start of cpBodyRemoveShape. As you call this when removing a shape from the space I thought I should give it a try. Seems to work.

[slembcke]

Yeah, after I looked at it a bit more and found some new edge cases for sleeping or static bodies that make swapping the body of a shape more difficult than I had thought. I'm going to have to revert the changes to cpShapeSetBody() to throw an assertion when changing the body of a shape that is in a space.

On a related note, I should probably change the names of cpBodyAddShape() and cpBodyRemoveShape() as they don't really do what it sounds like they should do. People peeking in the private API might think they found a gem when it doesn't do at all what they think.