Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

oob-collider not setting el.object3D as default #5501

Open
shabarito opened this issue Mar 24, 2024 · 16 comments
Open

oob-collider not setting el.object3D as default #5501

shabarito opened this issue Mar 24, 2024 · 16 comments

Comments

@shabarito
Copy link

Description:

  • A-Frame Version: 1.5.0
  • Platform / Device: any
  • Reproducible Code Snippet or URL:

By checking the obb-collider component I have two doubts:

  1. Is there really a need of calling checkTrackedObject inside tick? As the trackedObject is changed via property update, it should be called inside update and not in every tick. I think it will work the same, but less work.

  2. The oob-collider system emit the event to both collider entities and uses component.trackedObject3D. However inside the component this.trackedObject3D is never defined as default to this.el.object3D and the emit goes empty.

In some of the component methods we see || this.el.object3D, but never assign it to this.trackedObject3D

Regards,

@dmarcos
Copy link
Member

dmarcos commented Mar 24, 2024

Can you share an example to reproduce issues with https://glitch.com/~aframe Thanks!

@shabarito
Copy link
Author

Sure.
https://glitch.com/edit/#!/thorn-dusty-conifer?path=index.html%3A23%3A6

I ve tested. Both the sphere and the model have the same behaviour. I think that just with the VR hands colliding it will work properly.

@dmarcos
Copy link
Member

dmarcos commented Mar 25, 2024

For some reason I cannot open the glitch

There was an error starting the editor. Maybe try to reload?

@dmarcos
Copy link
Member

dmarcos commented Mar 25, 2024

Maybe you want to recreate the glitch. I'm getting the same error.

@shabarito
Copy link
Author

@dmarcos
Copy link
Member

dmarcos commented Mar 26, 2024

Looks like a misunderstanding. Recommend reading the docs. Can improve if necessary:

https://aframe.io/docs/1.5.0/components/obb-collider.html

You don't have to call any function manually. Just setup the component in relevant entities.

I see collision events fired as expected:

 document.body.addEventListener("obbcollisionstarted", function (e) {
        console.log(e.detail.trackedObject3D);
 });

The value you have to check is el.detail.withEl as stated in docs

trackedObject3D is an internal variable that you wouldn't need to check but not sure I'm understanding what you are trying to do. If you can clarify...

@shabarito
Copy link
Author

On the fiddle, I have called the function manually just to show the behaviour.

You are right that withEl is sent on the emit.
But the main collider should also be sent.

Looking at the obb-collider system the emit has two parameters.

Also, in the docs.

trackedObject3D is explained as:
Variable path to the object3D used to calculate the collider. el.object3D by default

But this default behaviour does not work.

@dmarcos
Copy link
Member

dmarcos commented Mar 26, 2024

What are you trying to accomplish? Why are you trying to use / access the trackedObject3D variable?

@dmarcos
Copy link
Member

dmarcos commented Mar 26, 2024

trackedObject3D is only used in the rare occasions where you don't want to use the default this.el.object3D to calculate the collider. I don't think you need it in the presented example.

@dmarcos
Copy link
Member

dmarcos commented Mar 26, 2024

A simplified version of your glitch: https://glitch.com/edit/#!/phantom-bead-vacation?path=index.html%3A29%3A9

See that obbcollisionstarted fires as expected in both entities when they collide. You don't need to define trackedObject3D

@dmarcos
Copy link
Member

dmarcos commented Mar 26, 2024

Also notice you were using <a-sphere> for the model entity where you should be using <a-entity>

@shabarito
Copy link
Author

Indeed the component is working as intended.

To correct myself, it is the system that is emitting null.

componentA.el.emit('obbcollisionstarted', {trackedObject3D: componentA.trackedObject3D, withEl: componentB.el});
componentB.el.emit('obbcollisionstarted', {trackedObject3D: componentB.trackedObject3D, withEl: componentA.el});

The system was coded to have both trackedObject3D and withEl in the same emit.

I think, if you have many collisions happening, in one single event you have both entities involved in the collision. This is a good behaviour.

My idea was to increase consistency in the system.

@dmarcos
Copy link
Member

dmarcos commented Mar 26, 2024

We should probably remove trackedObject3D from the event details. Probably a vestige and not part of the documented public API

@dmarcos
Copy link
Member

dmarcos commented Mar 26, 2024

Also notice that trackedObject3D refers to an object path from the entity object el.path.to.your.object. In your initial example that path is undefined and hence this.trackedObject3D is undefined as well

@shabarito
Copy link
Author

But. Inside the hand-tracking-grab-controls component, the onCollisionStarted method uses this.grabbingObject3D = evt.detail.trackedObject3D;

The usage of trackedObject3D is probably when you have hands.

@shabarito
Copy link
Author

Hah.

Reviewing the hand-tracking-grab-controls component it seems this.grabbingObject3D is not used at all.

So probably we should do as you say, remove in the next revision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants