New distance functions and improvements to bbox / magnet sphere code. #1060
Conversation
…nted bounding boxes.
…cency of objects.
… surface an object is currently on.
…e arm. Also returning more information.
…as done for the IK controller).
…ere it reported the closest distance between a point and a non-convex mesh collider as being 0.
|
This pull request introduces 2 alerts and fixes 2 when merging 28692f1 into af96708 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 2 alerts and fixes 2 when merging 8e70929 into 7f79e43 - view on LGTM.com new alerts:
fixed alerts:
|
…r appears necessary (several bug reports that this causes problems).
|
This pull request introduces 2 alerts and fixes 2 when merging 663dd36 into 2a3a8b4 - view on LGTM.com new alerts:
fixed alerts:
|
… more complicated than we first thought.
|
This pull request introduces 2 alerts and fixes 2 when merging d16b3df into 2a3a8b4 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 2 alerts and fixes 2 when merging 7f0c32f into 2a3a8b4 - view on LGTM.com new alerts:
fixed alerts:
|
| public virtual ObjectMetadata[] generateObjectMetadata(SimObjPhysics[] simObjects = null) { | ||
| if (simObjects == null) { | ||
| if (this.simObjFilter != null) { | ||
| simObjects = this.simObjFilter; |
There was a problem hiding this comment.
This is a little surprising, passing null and getting metadata. Could be good for this function to just work with the simObjects it's passed, but I see the filtering needs to happen somewhere.
There was a problem hiding this comment.
generateObjectMetadata now has no default and will throw an error if passed a null value.
| VisibilityScheme visSchemeEnum; | ||
| if (visibilityScheme != null) { | ||
| visibilityScheme = visibilityScheme.ToLower(); | ||
| if (visibilityScheme.ToLower() == "collider") { |
There was a problem hiding this comment.
Change string literal comparison to -> Enum.GetName(typeof(VisibilityScheme), VisibilityScheme.Collider, ignoreCase: true) == visibilityScheme
There was a problem hiding this comment.
Results in an error:
Assets/Scripts/BaseFPSAgentController.cs(2956,107): error CS1739: The best overload for 'GetName' does not have a parameter named 'ignoreCase'
Will do
Enum.GetName(typeof(VisibilityScheme), VisibilityScheme.Collider).ToLower() == visibilityScheme
instead.
| # if UNITY_EDITOR | ||
| Vector3? lastPoint = null; | ||
| # endif | ||
| foreach (float x in xVals) { |
There was a problem hiding this comment.
For future reference you can do new List<float> {xVals, yVals, zVals}.CartesianProduct().Select(p => bc.center + new Vector3(p[0], p[1], p[2])) defined in our Extensions.cs. But I think we should keep it as it is because it's probably the fastest.
| Vector3 pLocal = c1.transform.InverseTransformPoint(p) - c1.center; | ||
| Vector3 size = c1.size; | ||
| if ( | ||
| (-0.5f * size.x < pLocal.x && pLocal.x < 0.5f * size.x) && |
There was a problem hiding this comment.
What does constant 0.5 mean perhaps create named constant?
There was a problem hiding this comment.
This is just because BoxCollider.size corresponds to the lengths of the box's sides whereas the comparisons we're doing here are w.r.t. the box's center so we want half of these lengths. Will add a comment.
| } | ||
| c1.enabled = false; | ||
|
|
||
| c0.enabled = true; |
There was a problem hiding this comment.
What does enabling/disabling each do?
There was a problem hiding this comment.
If the colliders aren't enabled when running functions that do physics checks (namely ClosestPoint in this code) then we will get incorrect results (this is a hard thing to debug as, iirc, it doesn't throw an error). I'll add comment.
| c is BoxCollider || | ||
| c is SphereCollider || | ||
| c is CapsuleCollider || | ||
| (c is MeshCollider && ((MeshCollider) c).convex) |
There was a problem hiding this comment.
Why are this sub type checks needed?
There was a problem hiding this comment.
The ClosestPoint function only works on these types of colliders (it fails on nonconvex mesh colliders). Note that we fallback to using an object's bounding box if it has no such colliders. I'll add a comment.
| @@ -168,7 +176,17 @@ public class PhysicsMaterialValues { | |||
| ContainedObjectReferences.Remove(t); | |||
| } | |||
|
|
|||
| public void syncBoundingBoxes(bool forceCacheReset = false) { | |||
| public void syncBoundingBoxes( | |||
There was a problem hiding this comment.
This name seems a misnomer if I understand correctly? It's not syncing the boxes just setting flags to do so at some point?
There was a problem hiding this comment.
I don't think so, it will actually regenerate the bounding boxes so long as:
- The object has moved/rotated or
- We explicitly pass the
forceCacheResetargument.
In this PR I added another argument to force the creation of an object-oriented bounding box as sometimes these are not created for certain objects..
|
Could you add a description to the PR for future reference? like what problem you're trying to solve and how it solved it/what is covered. Some minor comments, LGTM after that |
|
@AlvaroHG Made the requested changes and added a description. |
|
This pull request introduces 2 alerts and fixes 2 when merging 55ccf96 into 250b12c - view on LGTM.com new alerts:
fixed alerts:
|
This PR makes the following changes:
touchedNotHeldObjects) to the arm metadata which corresponds to a list of object ids that are currently being touched, but are not currently being held, by the arm's magnet sphere.GetObjectMetadataaction that returns the metadata for the objects corresponding to a given list of object ids.GetVisibleObjectsaction that returns a list of object ids of visible objects (given a max visible distance and visibility scheme).GetAllVisibleSimObjPhysicsDistancefunction was ignoring themaxDistanceargument.BBoxDistanceaction that computes the (approximate) distance between two objects using their bounding boxes. This should be far more accurate than using the distance between the centerpoints of objects (e.g. the distance between an apply siting on the side of a couch would be large when considering centerpoints but will be near 0 when computing the distance between bounding boxes).CheckUnobstructedPathBetweenObjectCentersaction that does what it says (sees if you can send a raycast between two objects' center points without hitting any other object). This is a nice way to check if two objects are actually side-by-side when they are nearby (e.g. imagine two objects on two sides of a wall, they can be close in L2 distance but are not actually "nearby" in a meaningful sense).CheckWhatObjectsOnaction that returns the object id of the object that a given object is sitting on. Useful when the receptacle bounding box is not usable (e.g. imagine a box that's been turned on its side with an apple sitting on it, the apple is "on" the box but the receptacle trigger box will, correctly, be empty but we'd still like to know that the apple is on the box in some settings).WhatIsInsideMagnetSphere.cs.