Change bounds creation to use colliders instead. #2942

[CodeLeopard]

Changes the way a part's Bounds are computed from using the part's meshes to using colliders.
Colliders that are triggers are excluded.

Fixes #2942

See #2905 for additional information.

[Dunbaratu]

I was just playing with this installed today and I noticed the bounds are still including the realfuels' engine plumes. I'm not sure why those are "colliders" but it may be that it's not enough to check for triggers. Maybe it's also necessary to filter some colliders out by layer number?

[mgalyean]

This is an interesting problem. A plume, for realism, would need colliders. But this gets into the realm of "flavors" of collision. One certainly wouldn't want the same collision rules for smoke as for a 3kt engine. Is there some kind of mask that tells a collision mesh what it is allowed to collide with? I'm gathering that the plume is perhaps considered a part of the craft and so brings its collision mesh along with when trying to determine bounds. If not the layer number one would have to find something different about the plume meshes to filter them out by for sure and I'm curious what that ends up being, but if the plume is considered a craft part, maybe filtering it out on the part level prior to looking at meshes might work. And I don't even know if that makes sense

[Dunbaratu]

Okay, so the plumes are NOT the problem.
Verified the problem also happens on Stock.
Something about this PR changing things from Meshes to Colliders also made the bounds all bugged out.
If you use the TestBounds script from here (http://69.25.178.18/kostemp/tutorials/display_bounds.html?highlight=testbounds) , on stock, you can see this PR messed things up - and I can't figure out why unless colliders exist in some other alternate reference frame that's different from the reference frame Meshes use. Because literally all that changed is the collider bounds instead of the mesh bounds are used.

[Dunbaratu]

I think this is the issue:

The documentation for Mesh.bounds in Unity says this:

The bounding volume of the Mesh.

This is the axis-aligned bounding box of the mesh in its local space (that is, not affected by the transform)

The Unity documentation for Collider.bounds says this:

The world space bounding volume of the collider (Read Only).

The Mesh bounds is in local space, which we then find the 8 corners of it and transform that into World space afterward.

The Collider bounds starts out already in world space, which means:

(A) It's wrong to do that transform on them, and

(B) They'll have extra space around the corners because world axes don't align with the part's orientation.

It looks like there is a solution but it's messier than just a drop-in replacement. It goes like so:

Get the Type of the Collider. Colliders come in 4 types: Sphere, Capsule, Box, and Mesh.

If the type is MeshCollider, then use the code already written for Meshes, but you have to get the MeshCollider.sharedMesh.bounds (which is local space like the code expects) and not the MeshCollider.bounds (which is world space).

If the type is BoxCollider, CapsuleCollider, or SphereCollider then they have no direct API to return the local bounds, but kOS can manually work out the what the local bounds would be based on the relatively simple parameters of these shapes.

[CodeLeopard]

Thanks for looking in to this and identifying what exactly was wrong, I'll update this PR to include the suggested fix.

[CodeLeopard]

I have implemented the suggested solution. Some notes though:

• I came across WheelCollider and had to add a reference to be able to deal with it.
• Currently if an unexpected collider type appears an exception will be thrown. Should that be handled differently, perhaps with a default value or best guess?

[Dunbaratu]

Currently if an unexpected collider type appears an exception will be thrown. Should that be handled differently, perhaps with a default value or best guess?

Good question. I think throwing an exception here is probably inappropriate because it's not something the user can fix in their script. Maybe just a SafeHouse.Logger.Log() message rather than an exception. Then we can see it is happening but it won't break the script execution. Maybe when it's an unknown collider we just say it's "not there" and make it zero size. (or very very small size since a zero-size bounds may cause some of our values to be zero-length vectors making them have no defined direction, causing possible math errors elsewhere.)

Edit: Better idea - When the type of collider is unknown, just skip over it like we would for a "trigger" collider. Still give that SafeHouse.Logger.Log message, but treat it as if that collider wasn't even present. That's probably the "safest" in terms of protecting the code from crashing.

[CodeLeopard]

Okay I changed it to log and skip.
Log message includes the Part's toString and collider index to help diagnosing.

[Dunbaratu]

I started playing with this installed today and now it seems to be working properly. Good Job!