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

Core: Convert Object3D and EventDispatcher to ES6 classes. #21646

Merged
merged 2 commits into from
Apr 13, 2021

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Apr 13, 2021

Related issue: -

Description

Converts the remaining two class in the src/core folder.

@Mugen87 Mugen87 changed the title Core: Convert Object3D and EventDispatcher. Core: Convert Object3D and EventDispatcher to ES6 classes. Apr 13, 2021
@Mugen87 Mugen87 added this to the r128 milestone Apr 13, 2021
This was referenced Apr 13, 2021
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 13, 2021

WebXRManager also has been updated. Otherwise EventDispatcher can't be an ES6 class.

@mrdoob mrdoob merged commit 669e43a into mrdoob:dev Apr 13, 2021
@mrdoob
Copy link
Owner

mrdoob commented Apr 13, 2021

Thanks!

@drcmda
Copy link
Contributor

drcmda commented Nov 1, 2021

@Mugen87 do you still remember what prompted this change https://github.com/mrdoob/three.js/blob/master/src/core/Object3D.js#L66-L94

const v = new THREE.Vector3()
const m = new THREE.Mesh()
m.position = v

Screenshot 2021-11-01 at 14 32 45

the request we got was that people want to wire up their own objects which they can then control outside of the class. but it seems these props can't get overwritten any longer. looks like some props are non-replaceable while some others are (matrix, layers). could this block be removed so that all props can be driven externally?

@mrdoob
Copy link
Owner

mrdoob commented Nov 1, 2021

It was a bad practice.

People did this a lot:

mesh.position = new THREE.Vector3(1,1,1)

Instead of

mesh.position.set(1,1,1)

And they would do that in loops and would create a lot of garbage.

That pattern also broke mesh.rotation which we need to track to create mesh.quaternion.

@drcmda
Copy link
Contributor

drcmda commented Nov 1, 2021

for reference pmndrs/react-three-fiber#1419 they want to drive matrix externally, which is similar, mesh.matrix = externalMatrix. if this is the intended direction, i would communicate that overwriting props in threejs is no longer accepted.

@mrdoob
Copy link
Owner

mrdoob commented Nov 1, 2021

It definitely is discouraged.

@drcmda
Copy link
Contributor

drcmda commented Nov 1, 2021

understood. im glad i opted for .copy for <mesh position={external} some years ago ☺️

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

Successfully merging this pull request may close these issues.

3 participants