Confusing Capsules

[anthonysharpy]

Describe the bug

I've been trying to transition from AABB to capsules and ran into a lot of bumps, so I thought I'd mention what I found

Originally I was defining my capsules with their actual position, like this:

private Capsule GetPlayerCollisionCapsule()
{
   var radius = PLAYER_WIDTH / 2;

   return new Capsule( GameObject.Transform.Position + Vector3.Up * radius,
      GameObject.Transform.Position + new Vector3( 0, 0, PlayerHeight ) + Vector3.Down * radius,
      radius );
}

When doing traces this lead to super weird behaviour because it was giving it a positional offset. The correct code is more like this:

private Capsule GetPlayerCollisionCapsule()
{
   var radius = PLAYER_WIDTH / 2;

    return Capsule.FromHeightAndRadius(PlayerHeight, radius);
}

The issue is that the API is misleading because it takes two Vector3s, giving the impression it's asking for an actual position. I think it should ditch the Vector3s and just use a float to describe the height / cylinder height / whatever instead. It's made more confusing by the code which says things like "Position of point A" etc.

If the above is added, then what I'm about to say becomes moot. But I'll say it anyways just in case...

The next problem is this: as just mentioned, the capsule class takes an A and B Vector3, but it's not explained in the code what A and B are

With some experimentation you can find out that A is the bottom of the cylinder (NB: not the bottom of the capsule) and B is the top of the cylinder (not the top of the capsule). I understand mathematically why it's like this but IMO it's confusing since you would intuitively expect A and B to refer to the highest and lowest point of the capsule. The FromHeightAndRadius function solves this issue, but IMO this should probably be a constructor, and the constructor taking A and B should probably be yeeted because it's too confusing.

Was literally about to give 5000 reasons why the capsule class is broken until I found out my above mistake, but IMO I think the API could be a bit more forgiving here 😄

To Reproduce

see above

Expected behavior

see above

Media/Files

No response

Additional context

No response

[anthonysharpy]

sorry this is probably more of a feature request than a bug, I pressed submit too soon 🤓

[garrynewman]

I don't think this is confusing, is it?

[anthonysharpy]

Yes it is confusing:

1. The fact that the capsule class has a constructor taking points A and B, but that the definition of A and B is not mentioned anywhere, is confusing (you literally have to guess what these mean)
2. The fact that the Capsule class says it takes a position in its comments and through being a Vector3 when it does not really is confusing
3. The fact that points A and B do not refer to the top and bottom of the capsule is unintuitive and confusing

[yuberee]

There's a reason why A and B are not the top/bottom. How would you define a capsule with a radius greater than the distance between A and B if that were the case?

[anthonysharpy]

There's a reason why A and B are not the top/bottom. How would you define a capsule with a radius greater than the distance between A and B if that were the case?

I think the maths still works although correct me if I'm wrong

If B is the topmost point A is the bottomost R is the radius

The height of the half-spheres is R The height of the cylinder is the difference between A and B minus 2*R The radius of the cylinder is R

I think that works regardless of the distance between A and B

[yuberee]

With your suggestion of having A and B representing the top and bottom of the capsule, the moment the distance between A and B is less than the radius you won't have a capsule anymore. The shortest possible capsule with the current method is a sphere, which is still a capsule by definition. The shortest possible capsule with your suggested method is a prolate spheroid, which is not a capsule by definition.

[anthonysharpy]

With your suggestion of having A and B representing the top and bottom of the capsule, the moment the distance between A and B is less than the radius you won't have a capsule anymore.

Okay thanks I see your point about how it can generate invalid capsules now. From a code perspective though to be fair you could just throw an error in this probably quite rare situation.

My main point was just that this:

var radius = 15;

var myCapsule = new Capsule( new Vector3(),
  new Vector3( 0, 0, PlayerHeight ),
  radius );

is simpler to write and explain than this:

var radius = 15;

var myCapsule = new Capsule( new Vector3( 0, 0, radius ),
  new Vector3( 0, 0, PlayerHeight - radius ),
  radius );

Interestingly enough, having thought about what you said, it seems Capsule.FromHeightAndRadius also gives weird results where H < R (maybe even < 2R, IDK I didn't check), albeit never an invalid capsule. E.g. in this example, the height is passed in as 3, but definitely does not come out as 3:

To be fair if you take the function comment literally then it does technically do what it says it does, although it is surprising:

This is one of the reasons I was confused as if Capsule.FromHeightAndRadius works then surely having A and B as endpoints must work too. But it seems it does not actually always work.

Unity seems to define a capsule with just a height and a radius: https://docs.unity3d.com/Manual/class-CapsuleCollider.html. IMO this is the way we should be doing it too, although I'm assuming that this means they also run into weird problems when H < R?