search

xexuxjy / bullet-xna

Automatically exported from code.google.com/p/bullet-xna
Other
21 stars 5 forks source link

CollisionObject missing function "Vector3 GetInterpolationLinearVelocity(void)" #3

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
There's a setter method for CollisionObject.m_interpolationLinearVelocity but 
no getter method.   m_interpolationLinearVelocity is protected.

Original issue reported on code.google.com by dcoliva...@gmail.com on 7 Apr 2011 at 6:36

GoogleCodeExporter commented 9 years ago 
Adding a patch for this one.   Adds the proper getter method.

Original comment by dcoliva...@gmail.com on 7 Apr 2011 at 6:49

Attachments:

GoogleCodeExporter commented 9 years ago 
The .NET philosophy is that get and set methods should be properties unless 
they are computationally intensive.

For example, this allows to do:
myObject.Property++;
instead of:
myObject.SetProperty(myObject.GetProperty() + 1);

So properties can be used like fields and also make the code easier to read.

I understand the need to keep things as similar as possible to Bullet, but 
properties will be much more familiar to C# programmers, so I definitely 
recommend to use those.

Also, properties are optimized to be as fast as get/set methods, so there's no 
performance problem.

Original comment by andres.traks on 7 Apr 2011 at 2:43

GoogleCodeExporter commented 9 years ago 
@andres.traks, I've already had to do shenanigans like;
"using btVector3 = Microsoft.Xna.Framework.Vector3;"
"using btQuaternion = Microsoft.Xna.Framework.Quaternion;"
and 
"using btRigidBody = BulletXNA.BullettDynamics.Dynamics.RigidBody;" to make the 
API compatible with bullet.

I understand the use of properties and, unless you're planning on diverging 
from the normal bullet API making it /harder to maintain/ when bullet updates 
and requiring your own set of documentation and tutorial elements for this 
project, you'll want to tread carefully here.    "Just sayin"

Original comment by dcoliva...@gmail.com on 7 Apr 2011 at 3:23

GoogleCodeExporter commented 9 years ago 
Is there any reason to use btVector3 or btRigidBody in your C# code if you 
could use Vector3 and RigidBody just the same?

There are guidelines for .NET development concerning design and style: 
http://msdn.microsoft.com/en-us/library/czefa0ke(vs.71).aspx
It's not absolutely required to follow them, but they make a lot of sense.

For example, there's no need to have the "bt" prefix, because namespaces 
already distinguish between different software packages.
C++ has namespaces as well, but in C++ you might also link to C code, which is 
always in the global namespace, so Bullet does need the "bt" prefix.
Also, the use of abbreviations is discouraged in .NET for clarity.

Another thing that caught my attention is the enumerations. At the moment they 
are like this:
public enum CollisionObjectTypes
{
    CO_COLLISION_OBJECT = 1,
    CO_RIGID_BODY,
...

but they should be like this:
public enum CollisionObjectType
{
    CollisionObject = 1,
    RigidBody,
...
No need for the "CO_". In C++ you can use CO_RIGID_BODY without the 
CollisionObjectTypes identifier, but in C# you have to write 
CollisionObjectType.RigidBody ([enum name].[constant name]),
so CollisionObjectType already fills the role of "CO_". Also, the name of this 
enumeration is singular, because you can only set it to one value as opposed to 
flags.

You don't need a second set of documentation, because all that's needed to 
explain the differences between Bullet and Bullet-XNA is a straightforward 
checklist:
1. No "bt" in front of class names.
2. Enumerations are written in Pascal case.
3. Get/set methods are properties.
4. Maybe something else...
Nobody has ever complained about such differences in BulletSharp: 
http://code.google.com/p/bulletsharp

Remember, I'm not trying to enforce my own style or opinion. All of the above 
is second nature to .NET developers and these changes are all about consistency 
and predictability, which is what makes .NET so good. The goal should not be 
that C++ code must work without changes as C# code.

It's up to Mark to decide, but if he does what the guidelines say, then I'll 
help him make those changes and maintain them.

Original comment by andres.traks on 7 Apr 2011 at 7:50

GoogleCodeExporter commented 9 years ago 
Hey, I'm OK with the changes as long as they don't make it difficult to update 
when Bullet does.    I've seen bullet and ODE ports come and go and the ones 
that stick around the longest follow the original API because the further a 
port diverges from a line by line comparison, the harder it is to update the 
port when the original is updated.   

-========-IMPORTANT -======-
My point.  It doesn't really matter what the guidelines say if nobody updates 
it because it's diverged too much.

Original comment by dcoliva...@gmail.com on 7 Apr 2011 at 8:08

GoogleCodeExporter commented 9 years ago 
Okay, point taken. I don't see the guidelines affecting the project enough to 
derail it.

Original comment by andres.traks on 7 Apr 2011 at 8:42

GoogleCodeExporter commented 9 years ago 
Here is a patch with the fix and also the minimum of the changes that I'd like 
to make to CollisionObject.

Original comment by andres.traks on 9 Apr 2011 at 9:33

Attachments:

GoogleCodeExporter commented 9 years ago 
Also, if these changes seem too big or just not comfortable enough to work 
with, then a separate branch would be great for me.

Original comment by andres.traks on 9 Apr 2011 at 9:52

GoogleCodeExporter commented 9 years ago 
Thanks all, I'm going to review the submitted patches and update accordingly. 
Might have helped if I'd enabled the issue notifications email on google code. 
<sigh>

General point is that I am trying to keep things as close to the original c++ 
code as much as possible , just about every time I thought I'd done something 
clever I ended up going back to the c++ version anyway. Even things like order 
of matrix multiply ops I've put in a function call to allow it to match the 
bullet stuff more closely.
WRT to Properties vs's functions, I'm in two minds on this, a lot of the types 
in bullet are value types in xna so using properties often hid various errors. 
If I get enough comments of people really struggling with the function style 
methods I'll look at a re-write of those areas.

Thanks again for the feedback.

Original comment by xexu...@gmail.com on 11 Apr 2011 at 7:44

GoogleCodeExporter commented 9 years ago 
No worries on the !EmailNotifications and thanks for opening the source for 
this.  You've done an amazingly large amount of work here....   and... the 
solver works.   Congratulations...  that's harder to do then people might think!

Original comment by dcoliva...@gmail.com on 11 Apr 2011 at 11:56

GoogleCodeExporter commented 9 years ago 
added missing method, which the other issues were this easy :)

Original comment by xexu...@gmail.com on 12 Apr 2011 at 12:05