Re: [Pyode-user] Lots of missing functions added to PyODE

[Ethan Glasser-C. <gl...@cs...>]

al...@ci... wrote:
> A summary of changes:
> 
> * World: damping functions, QuickStep over-relaxation tuning
> * Body: damping, auto-disable, setMovedCallback
> * Joints:
>   - enable/disable joints
>   - JointType... constants
>   - 3 new joint classes: Piston, PR and PU.

Hmm.. why did you move the enable/disable functions for bodies to the
bottom of the file, instead of adding the autodisable stuff where the
enable stuff was?

I think _bodyMovedCallbacks should be a weakdict, shouldn't it? Also,
would it make more sense to add a _body_c2py_lut lookup table like for
geoms in ode.py? In that case you'd only need to store the callbacks in
_bodyMovedCallbacks. Of course, if it's a weakdict, you'd need to handle
the case where the body goes out of scope.. presumably the moved
callback won't be called if the body ceases to exist, but I'm not sure ;)

Maybe Body.getData() and Body.setData() should raise NotImplementedError?

You add a lot of blank lines between sections of code! I find this
distracting. Also, you have lots and lots of trailing whitespace, which
I really hate.

Also, it seems a little silly for code this simple, but you could send
smaller patches, especially if you're rearranging method declarations.

> Many declarations were generated automatically with a small script: auto.py (it
> is attached).

I added this, it's pretty neat. Thanks.

> The following functions are still not implemented (I need some help here):
> 
> dJointID dBodyGetJoint (dBodyID, int index);  (should return a Joint instance)

I would add a _joint_c2py_lut, like with geoms. Ignore the comment, that
was before I fully understood why tutorial3 used to break :)

> dGeomID dBodyGetFirstGeom (dBodyID);
> dGeomID dBodyGetNextGeom (dGeomID);           (these two should be wrapped into
> an iterable or a geom list)

Hmm, if nobody gets to this before the weekend I'll see if I can add
something.

> Regards,
> Alex

How would you like to be credited it the changelog? Just your email address?

Ethan