Inconsistency

[Nivek92]

Hey, I am currently going through the "tutorial" and there are a few things I find inconsistent which in my opinion could be improved.

First there is some inconsistency between the shorthand methods on the builders.

On the RigidBodyBuilder for example we call RigidBodyBuilder::new_static() while on the ColliderBuilder it's ColliderBuilder::ball(0.5)

It would make more sense to stay consistent and name the calls either

RigidBodyBuilder::static(); or ColliderBuilder::new_ball(0.5) respectively

Something similar with linvel(mut self, x: f32, y: f32, z: f32) and angvel(mut self, angvel: AngVector<f32>). Why use separate values for one and a vector for the other. It would make more sense to either add linvel(mut self, linvel: LinVector<f32>) and angvel(mut self, x: f32, y: f32, z: f32) as well or just allow for construction via seperate values or vector only.

Another thing that's inconsistent is the creation of joints which unlike RigidBodies and Colliders does not use the builder pattern. Why?

Lastly JointSet::insert<J>( &mut self, bodies: &mut RigidBodySet, body1: RigidBodyHandle, body2: RigidBodyHandle, joint_params: J, ) should be JointSet::insert<J>( &mut self, joint_params: J, body1: RigidBodyHandle, body2: RigidBodyHandle, bodies: &mut RigidBodySet, ) to be consistent with ColliderSet::insert( &mut self, mut coll: Collider, parent_handle: RigidBodyHandle, bodies: &mut RigidBodySet, )

PS: Awesome project. Thank you for your effort.

[sebcrozet]

Hi!

Thank you for pointing out these inconsistencies. There are reasons for most of these inconsistencies, but I agree we should make it better.

RigidBodyBuilder::static(); or ColliderBuilder::new_ball(0.5) respectively

Ideally, I would prefer keeping ColliderBuilder the way it is and use RigidBodyBuilder::static(), RigidBodyBuilder::dynamic(), etc. The reason it has not been done is because static is a reserved keyword so RigidBodyBuilder::static() cannot be defined. We could use RigidBodyBuilder::r#static() (raw identifier) but that looks ugly.

Perhaps this means we should leave RigidBodyBuilder as-is and use ColliderBuilder::new_ball instead.

Something similar with linvel(mut self, x: f32, y: f32, z: f32) and angvel(mut self, angvel: AngVector)

The reason here is because with x: f32, y: f32, y: f32 users may think euler angles are expected. With a vector, it is more clear that the representation of the rotation is axis-angle. Then if we want to keep things consistent I guess we need to change linvel so it use vectors.

Lastly JointSet::insert<J>( &mut self, bodies: &mut RigidBodySet, body1: RigidBodyHandle, body2: RigidBodyHandle, joint_params: J, ) should be JointSet::insert<J>( &mut self, joint_params: J, body1: RigidBodyHandle, body2: RigidBodyHandle, bodies: &mut RigidBodySet, ) to be consistent with ColliderSet::insert( &mut self, mut coll: Collider, parent_handle: RigidBodyHandle, bodies: &mut RigidBodySet, )

Agreed. Perhaps we should change them to:

JointSet::insert<J>(&mut self, bodies: &mut RigidBodySet, joint_params: J, body1: RigidBodyHandle, body2: RigidBodyHandle)
ColliderSet::insert(&mut self, bodies: &mut RigidBodySet, coll: Collider, parent_handle: RigidBodyHandle)

I'm not sure if there is a conversion for this kind of thing but I find it more aesthetic to move the sets in the first places of the method arguments.

[Nivek92]

I should find some time to implement those changes on the weekend.

Another thing that's inconsistent is the creation of joints which unlike RigidBodies and Colliders does not use the builder pattern. Why?

I know there is no really "configuration" which would make the builder pattern more appropriate but for consistency I think one could still justify to use it here.

[revillo]

There are also inconsistencies in the setTranslation() functions taking either vectors or components between RigidBody and RigidBodyDesc