Inheriting from cpPivotJoint

[slembcke]

Well I don't actually care that much about the private-ness of functions really. It more just about organization. You aren't going to be using something like cpConstraintInit() unless writting a new constraint, and most people are not going to be writing new constraints. I also like to avoid putting too much into the public headers because once it's there, it's part of the API and people will get cranky if you change it.

Now, getting back to inheritance. The problem is that Chipmunk's class structures are really just little interfaces. It's not the same powerful object oriented programming that you are used to, preStep(), applyImpulse(), and getImpulse() is all you get. You don't get to add new overridable functions in subclasses or anything like that. The outside world only cares about those 3 functions, but a real fancy joint supertype would have overrideable functions for impulse calculation, impulse clamping, caching strategies, etc. Doing it this way would also be very slow. Considering how I've been able to refactor the function sizes down so much, I don't feel to bad about that.

Having thought about it a bit, I think that delegation and callbacks will likely be more flexible in C. Think about how the breakable joint works. It doesn't really do anything terribly useful itself, but takes on the behavior of the joint that it delegates to. Additionally it will need to have an action to perform when the joint is broken if no reason other than memory management. I rather like callbacks myself (real closures are better, but c'est la vive) being a fan of function programming. This setup works fairly well as it can map nicely onto other languages features. You can make it into a real closure call in a non-OO language or a virtual function in an-OO language.

[supertommy]

I must admit I'm an OO and C++ guy myself, although my day-job consists of developing code on a system which implements OO in C much like the chipmunk code. That's why I have so many opinions on the matter . I hope you don't take my rambling as criticism, though -- of the OO-in-C implementations I've seen, my favourite is probably the one in chipmunk.

I don't think I have anything more to add to this thread, except for a cute little trick I learned just the other day. It's related to callbacks, so I don't think it's entirely off-topic;

Turns out you can cast function pointers to other, compatible function pointers. So it's possible to define the pivotJointPreStep function with this signature:

Code: Select all

static void pivotJointPreStep(cpPivotJoint *joint, cpFloat dt, cpFloat dt_inv)

... and just cast it to the right type as you assign it to the cpConstraintClass struct. Like in the attached patch. Cool, eh?

functionPointers.diff

[slembcke]

Hmm interesting. I did know that you could do that, but I had avoided doing so because I thought it a bad idea to be subverting the type system in such a way. Though now that I'm looking at it, that was a naive thought because I'm either doing unsafe casts inside the function or an unsafe cast of the function pointer itself. Casting the function pointer like that actually cleans things up a bit.

Good tip!

[supertommy]

If you try to cast a function which isn't compatible at all (wrong number of arguments for instance) you get a compile error, so it seems like this is reasonably safe...

[slembcke]

Well, by unsafe I mean that casting one pointer type to another without runtime checking. C obviously doesn't do this, nor does it even know that the structs are "inheriting" from one another. Might not be a bad idea to add some assert statements someday to check the class pointers, but I'll leave that for a rainy day. I don't actually remember an instance where I've been burned by bad casting of pointers in this fashion so I'm not terribly worried about it.

[supertommy]

Yeah, I see what you mean. On the other system I'm working on, we have a Cast method for each class. In chipmunk they would have been implemented like:

Code: Select all

cpPivotJoint *cpPivotJointCast(cpJoint *joint)
{
    <assert that the pointer is of the right type>
    return (cpPivotJoint *)joint;
}

The good thing about the cast method is that it can be switched to just a define if you need to squeeze that extra bit of speed out of your program;

Code: Select all

#define cpPivotJointCast(joint) ((cpPivotJoint *)joint)

But, as you say, I don't think I've ever seen any of those asserts in action.

Btw, the patch above only does the function pointer trick with cpPivotJoint. If you want me to type out a patch which covers all the joints, just let me know.

[slembcke]

Nah, that's Ok. I think I'll work that in myself after work today. I'm trying to clean up some of the local variable naming conventions anyway.

Oh, and no I don't feel your critiquing is without merit. You seem to know what you are talking about.