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

Add missing dispose() methods #21577

Merged
merged 6 commits into from
Apr 5, 2021
Merged

Add missing dispose() methods #21577

merged 6 commits into from
Apr 5, 2021

Conversation

acu192
Copy link
Contributor

@acu192 acu192 commented Apr 5, 2021

Fixed #21572

@mrdoob mrdoob added this to the r128 milestone Apr 5, 2021
@mrdoob
Copy link
Owner

mrdoob commented Apr 5, 2021

Seems like the docs changes use spaces for indentation instead of tabs.

@acu192
Copy link
Contributor Author

acu192 commented Apr 5, 2021

@mrdoob Fixed now.

@mrdoob mrdoob merged commit 1bbe806 into mrdoob:dev Apr 5, 2021
@mrdoob
Copy link
Owner

mrdoob commented Apr 5, 2021

Thanks!

@JaidenDeChon
Copy link

After updating recently, I found dispose() is also missing from Scene. Was this intentional? If it was intentional, is there a replacement or does the move to webgl2 mean we don't need this anymore?

Please let me know if I'm asking in the wrong place and I'll move this question. Thank you

@acu192
Copy link
Contributor Author

acu192 commented Apr 27, 2021

After updating ...

Was there ever a dispose() in the Scene? If so, I never noticed it.

I don't think Scene needs one; the Scene itself does not create disposable objects. Rather, you create disposable objects and add them to the Scene. The purpose of dispose() is to dispose of resources that are created internally by the object.

For a larger discussion, you should probably file a new issue for it since this PR is already merged.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 28, 2021

Was there ever a dispose() in the Scene?

Yes. And it was removed in #19972 after internal data-structures of the engine where converted to weak maps.

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.

Inconsistent existence of dispose() across API
4 participants