Re: [Pyode-user] Kinematic state patch

[<al...@ci...>]

Hello Ethan,

Thank you very much for the review and the comments.

Since this is my first patch submitted to an open source project, I expected to
received some critics.
Next time I will try to read the manual more thoroughly.


You are correct with suggesting setKinematic(True/False), it's more intuitive.
To keep the API
similar to ODE, and also take into account your suggestion, I'm thinking at
something like this:

body.setKinematic(kinematic = True)
body.setDynamic()
body.isKinematic() => True/False

Therefore, setKinematic(False) will be identical to setDynamic().

The default dynamic state is "Dynamic", and currently there are only two
possible values: kinematic and dynamic.

P.S. I am glad you switched to git. It would be a good idea to add the git link
to Pyode web site
( http://pyode.sourceforge.net )

Regards,
Alex

> al...@ci... wrote:
>> Hello,
>>
>> I have patched PyODE by adding the following functions, which set the
>> kinematic
>> state of a rigid body:
>>
>> body.setKinematic()
>> body.setDynamic()
>> body.isKinematic()
>>
>> The functions are described here:
>> http://opende.sourceforge.net/wiki/index.php/Manual_%28Rigid_Body_Functions
>> %29#Kinematic_State
>
> This is very cool. I've reviewed the patch. Some suggestions, if I may:
>
> 1. Don't send patches with trailing whitespace.
> 2. Send patches that apply from the root of the project (i.e. do cvs
> diff from outside of src).
>
> I'd also suggest changing the API to setKinematic(True/False), or
> setDynamism(DEFAULT/DYNAMIC/KINEMATIC), but I guess it is useful to have
> similar APIs to ODE.
>
> I've cleaned up the whitespace and rewritten one of the docstrings for
> clarity, and committed the resulting patch. Thanks.
>
>> I have also attached the same patch to the project tracker a few days ago
>> (at https://sourceforge.net/tracker/?group_id=73553&atid=538152 ),
>> but I'm not sure whether anyone noticed it.
>
> As you guessed, I don't check the project tracker and I doubt anyone
> else does either :)
>
> Ethan
>