Bug XRBaseInteractable attachtransform incorrect attaching

I was having problems with attachment of my grab interactable that has two colliders where I enable/disable some during runtime. Looking at the code I saw weird references to rigidbody.worldCenterOfMass.

I removed usage of worldCenterOfMass. I suspect this is very old code before introducing attachpoints?
I removed unnecessary checks for m_AttachTransform != null. AFAIK this is guaranteed to exist.

This is the original code in XRGrabInteractable:

Code (CSharp):

1. void UpdateInteractorLocalPose(XRBaseInteractor interactor)
2. {
3. #pragma warning disable IDE0029 // Use coalesce expression -- Do not use for UnityEngine.Object types
4.     var attachTransform = m_AttachTransform != null ? m_AttachTransform : transform;
5. #pragma warning restore IDE0029
6.     var attachPosition = m_AttachTransform != null ? m_AttachTransform.position : transform.position;
7.     var attachOffset = m_Rigidbody.worldCenterOfMass - attachPosition;
8.     var localAttachOffset = attachTransform.InverseTransformDirection(attachOffset);
9.  
10.     var inverseLocalScale = interactor.attachTransform.lossyScale;
11.     inverseLocalScale = new Vector3(1f / inverseLocalScale.x, 1f / inverseLocalScale.y, 1f / inverseLocalScale.z);
12.     localAttachOffset.Scale(inverseLocalScale);
13.  
14.     m_InteractorLocalPosition = localAttachOffset;
15.     m_InteractorLocalRotation = Quaternion.Inverse(Quaternion.Inverse(m_Rigidbody.rotation) * attachTransform.rotation);
16. }

This is the fixed code, that at least works for me:
It still feels a little suspect, because I had to negate the m_InteractorLocalPosition meaning it's relativeness is used in the wrong way. I'm not sure how anything worked before because (m_Rigidbody.worldCenterOfMass - attachPosition) results in a relative "local" position, and this was used as input to InverseTransformDirection() that requires a world space point. :shrugs:

Code (CSharp):

1. void UpdateInteractorLocalPose(XRBaseInteractor interactor)
2. {
3.     // localPosition relative to interactable transform root.
4.     Vector3 localAttachOffset = transform.InverseTransformPoint(attachTransform.position);
5.  
6.     var inverseLocalScale = interactor.attachTransform.lossyScale;
7.     inverseLocalScale = new Vector3(1f / inverseLocalScale.x, 1f / inverseLocalScale.y, 1f / inverseLocalScale.z);
8.     localAttachOffset.Scale(inverseLocalScale);
9.  
10.     // Convert attachTransform.localPosition to interactor.attachTransform.localPosition.
11.     Vector3 worldAttachOffset = interactor.attachTransform.TransformPoint(localAttachOffset);
12.     m_InteractorLocalPosition = -interactor.attachTransform.InverseTransformPoint(worldAttachOffset);
13.  
14.     m_InteractorLocalRotation = Quaternion.Inverse(Quaternion.Inverse(m_Rigidbody.rotation) * attachTransform.rotation);
15. }

Why don't you just put the whole project on GitHub so the community can comment on suspect code and suggest improvements?

 

Since you don't have the project public on github yet, and to keep myself sane, I had to add it myself. It's the only way to keep the overwritten function between Unity upgrades etc. (also good way to see what has changed between your releases)

I'll keep it public so others can see. As far as I understand the license this should be OK. Please notify me otherwise and I'll make it private.
https://github.com/fresolina/com.unity.xr.interaction.toolkit

PS. Merry Christmas!