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

Explain circular references and how to break them #82942

Merged
merged 1 commit into from
Nov 11, 2023

Conversation

timothyqiu
Copy link
Member

Closes #82882

  • Adds circular reference related content in RefCounted
  • Improves description of weakref behavior

@produno
Copy link

produno commented Oct 7, 2023

Fyi, there is a spelling mistake in the paragraph below the one you added.

garbage collection is free to destroy the referent

I believe this should be

garbage collection is free to destroy the reference ?

Sorry, ignore me :)

@AThousandShips
Copy link
Member

"referent" means what's referred to, the target of the reference

@AThousandShips AThousandShips added this to the 4.2 milestone Oct 7, 2023
@Mickeon
Copy link
Contributor

Mickeon commented Oct 7, 2023

While I'm glad the example in RefCounted exists, it feels quite verbose. It may sound easy to shrug off the next time someone falls into the same pit with a "Well, it's actually written right here", but perhaps it could be written differently to be easier to digest. Perhaps a more visual code example? Could that be necessary?

@timothyqiu
Copy link
Member Author

I expect most people won't read this description because RefCounted is not a class that people will interact with directly.

The target audience for this description is someone who has encountered a circular reference problem and wants to find the cause and solution in the documentation.

Previously, the documentation of RefCounted didn't metion weak reference at all. So people experiencing problems can't get any relevant information from the documentation, unless they already know about the concept of weak references (but they don't need to rely on the documentation to find the solution in that case).

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

This seems good to me, other than the suggestion that I've left.

Also improves the documentation of `weakref`.
@akien-mga akien-mga merged commit 8179ad5 into godotengine:master Nov 11, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@timothyqiu timothyqiu deleted the circulation branch November 12, 2023 05:37
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.

Memory leak for the objects storing self reference
6 participants