Samsung / GearVRf

The GearVR framework(GearVRf) is an Open Source VR rendering library for application development on VR-supported Android devices.
http://www.gearvrf.org
Apache License 2.0
407 stars 217 forks source link

Improvements to rag doll demo #1918

Closed ragner closed 6 years ago

ragner commented 6 years ago

GearVRf-DCO-1.0-Signed-off-by: Ragner Magalhaes <ragner.n@samsung.com>

ragner commented 6 years ago

Hi @NolaDonato, could you comment in the code line where that inconsistency happens?

Because I am not setting the owner without adding the component to a scene object.

NolaDonato commented 6 years ago

From GVRComponentGroup:

public void addChildComponent(T child) { synchronized (mComponents) { mComponents.add(child); if (owner != null) { child.setOwnerObject(owner); } } } You are setting the owner of the child component to be the same as the component group. The component group has been added to the scene object. The child object has only been added to the component group, not the scene object. If you want the owner to be propagated FOR CONSTRAINTS, then put that code in GVRConstraint instead of putting it in a general place where it can cause unforeseen side effects.

ragner commented 6 years ago

I see your point, onAttach should happens when the component is attached to a scene object as you know but the Component group is forwarding the onAttach() to its children.

ragner commented 6 years ago

I don't would like to implement some "special" code in the components to handle group support.

I mean "special" code to Constraints, to Colliders, to spatial audio... we need some logic that works for all components

crbozz commented 6 years ago

@liaxim I've made some changes as required by @NolaDonato

liaxim commented 6 years ago

Looks good to me. @NolaDonato ?

liaxim commented 6 years ago

Planning to merge on Wednesday.