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 docs to implementation-notes.md about not needing to dispose much #177

Closed
jbphet opened this issue Aug 23, 2023 · 3 comments
Closed
Assignees

Comments

@jbphet
Copy link
Contributor

jbphet commented Aug 23, 2023

Related to code review, see #165.

There's a line item in the code review checklist that says, "Are there leaks due to registering observers or listeners? The following guidelines should be followed unless documentation (in-line or in implementation-notes.md) describes why following them is not necessary."

Very few of the link calls in the sim have accompanying unlink calls, but I think this is just the nature of the sim - there just aren't a lot of things coming and going. The only dynamic elements are the particles, and there is mention of them and their disposal functions in implemenation-notes.md. I'd recommend adding something to that same doc that says something like, "Other than particles, everything else pretty much stays around for the lifetime of the sim and therefore doesn't need to be unlinked or disposed".

To be clear, there's no need to add comments or change the code for this, as it's generally pretty clear why there aren't any unlinks.

@jbphet jbphet mentioned this issue Aug 23, 2023
71 tasks
@zepumph zepumph removed their assignment Aug 25, 2023
@zepumph
Copy link
Member

zepumph commented Aug 25, 2023

To be looped into #94

@zepumph
Copy link
Member

zepumph commented Aug 28, 2023

  • Maybe in this disposal section, also talk about how this.particles is like a "main list" and once it is removed from it, the particle is disposed.

@Luisav1
Copy link
Contributor

Luisav1 commented Sep 14, 2023

Added in the commit above. Closing.

@Luisav1 Luisav1 closed this as completed Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants