Skip to content

Conversation

@ngokevin
Copy link
Member

@ngokevin ngokevin commented Nov 3, 2017

Description:

I think there's potential for surprise if we're listening to an object3dset event, and accidentally catch an object3dset event from a child. One would have to know to check the event target, which is unlikely to remember.

Changes proposed:

  • Don't bubble object3dset/remove events.

@dmarcos dmarcos merged commit db4ea55 into aframevr:master Nov 3, 2017
@donmccurdy
Copy link
Member

donmccurdy commented Nov 3, 2017

I'm not sure about this change. There are side effects:

  1. No reliable way to listen for a new mesh entering the scene. Can try model-loaded, but you'll miss primitive geometries. Can try child-attached, but you'll get noise from entities that haven't finished loading, or that aren't intended to contain meshes.
  2. The new raycaster refresh logic relies on this event reaching the scene, and will break with this change.

The risk of catching object3dset from children is real too, of course, but at least it's a risk that we share with most DOM APIs. Better the devil you know, etc.

@ngokevin
Copy link
Member Author

ngokevin commented Nov 3, 2017

That's true. I think we can revert then, especially given the raycaster refresh logic. Too bad the tests missed that, good catch.

ngokevin added a commit that referenced this pull request Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants