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

Object3D: Honor recursive parameter of copy() in subclasses. #24119

Merged
merged 2 commits into from
May 25, 2022

Conversation

frading
Copy link
Contributor

@frading frading commented May 24, 2022

Description

I've had a situation where I would need to clone a Spotlight or PointLight which had children, but without wanting those children. It did not seem possible as the copy method did not pass the recursive argument to the inherited Object3D.copy method.

So I've added the recursive argument to the copy method of a few classes.

Unless there is a reason of not allowing to pass the recursive argument is intentional, which I may have missed?

Mugen87
Mugen87 previously approved these changes May 25, 2022
Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good to make this change in order for consistency reasons. Please honor all remaining classes like InstancedMesh, Points or Sprite.

@frading
Copy link
Contributor Author

frading commented May 25, 2022

Ah yes, I missed those. Done now.
And I realise now there are also MorphAnimMesh and LightningStorm which could then do with this in that case, so I've also updated them.

@mrdoob mrdoob added this to the r141 milestone May 25, 2022
@mrdoob mrdoob merged commit 0ccddfd into mrdoob:dev May 25, 2022
@mrdoob
Copy link
Owner

mrdoob commented May 25, 2022

Thanks!

@Mugen87 Mugen87 changed the title honor recursive argument in copy method of classes derived from Object3D Object3D: Honor recursive parameter in copy() of subclasses. May 25, 2022
@Mugen87 Mugen87 changed the title Object3D: Honor recursive parameter in copy() of subclasses. Object3D: Honor recursive parameter of copy() in subclasses. May 25, 2022
@VeinSyct
Copy link

VeinSyct commented Jun 8, 2022

The changes in the Object3D is causing error when loading animated gltf "THREE.Object3D.add: object not an instance of THREE.Object3D. undefined"

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 8, 2022

Can you please explain in more detail how this change produce the mentioned error?

The examples load animated glTF assets without issues e.g. https://threejs.org/examples/webgl_animation_skinning_blending

@VeinSyct
Copy link

VeinSyct commented Jun 8, 2022

I trace the error at the car engine sound, I think the car engine is obsolete https://github.com/Antonio-R1/engine-sound-generator

@VeinSyct
Copy link

VeinSyct commented Jun 8, 2022

release 141 feels fast and lighter,

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 8, 2022

The above project relies on r127dev so there is probably a migration task required if it wants to support latest three.js versions.

@VeinSyct
Copy link

VeinSyct commented Jun 8, 2022

There was nothing wrong with the car engine sound https://github.com/Antonio-R1/engine-sound-generator works perfectly with threejs release 141, the error was occurring from my code bug when I load the gltf gltf.scene i fixed it with gltf.scene.children[0]. so sorry guys my bad.

abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
…t3D (mrdoob#24119)

* honor recursive argument in copy method of classes derived from Object3D

* add recursive argument to copy method of classes inheriting from Object3D
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.

None yet

4 participants