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

Raycaster raycaster-intersection event detail is not in line with the documentation #4974

Closed
diarmidmackenzie opened this issue Dec 2, 2021 · 2 comments

Comments

@diarmidmackenzie
Copy link
Contributor

Description:

  • A-Frame Version: 1.2.0
  • Platform / Device: All
  • Reproducible Code Snippet or URL: None. Can create if needed.

Raycaster documentation says that the raycaster-intersection event is generated as follows:

raycaster-intersection | Emitted on the raycasting entity. Raycaster is intersecting with one or more entities. Event detail will contain els, an array with the intersected entities, and intersections, and .getIntersection (el) function which can be used to obtain current intersection data.

https://aframe.io/docs/1.2.0/components/raycaster.html#events

However this is not what the code does for new intersections:
https://github.com/aframevr/aframe/blob/v1.2.0/src/components/raycaster.js#L272

    // Emit all intersections at once on raycasting entity.
    if (newIntersections.length) {
      this.intersectionDetail.els = newIntersectedEls;
      this.intersectionDetail.intersections = newIntersections;
      el.emit(EVENTS.INTERSECTION, this.intersectionDetail);
    }

It only includes new intersections in the event, rather than all current intersections, missing out any previous intersections.

We should fix either the code or the docs, so they are consistent.

I suspect the current implementation (code) actually gives more flexibility.

If you want to know all the currentl intersecting elements, raycaster.intersectedEls is a documented part of the public interface of raycaster (https://aframe.io/docs/1.2.0/components/raycaster.html#members), so you can get it direct from there, and don't need it on the event.

The set of newly intersected elements could be useful info for an application, and it's info the application wouldn't otherwise have access to.

On top of that, changing code would risk breaking existing code.

So I think the best fix is to fix up the docs. Trivial change, so I'll create a PR.

@dmarcos
Copy link
Member

dmarcos commented Dec 11, 2021

This one could be closed right? Fixed by #4975

@diarmidmackenzie
Copy link
Contributor Author

Agreed - closing.

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