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

Remove static script reference from GDScript class #36358

Merged
merged 2 commits into from
Feb 19, 2020

Conversation

vnen
Copy link
Member

@vnen vnen commented Feb 19, 2020

With the changes in Variant it now causes a reference cycle. The script reference is now created on the function call, the same way as it's done for the instance reference.

Reverts #36295
Fix #36246
Fix #36298

vnen added 2 commits February 19, 2020 09:14
This is needed because of the new changes to Variant. The reference
counter is increased by adding it to a Variant, which means no GDScript
will be freed (or will be double freed if manually freed somewhere).
@vnen vnen added this to the 4.0 milestone Feb 19, 2020
@vnen vnen requested a review from bojidar-bg as a code owner February 19, 2020 13:37
@akien-mga akien-mga requested a review from reduz February 19, 2020 13:56
@akien-mga akien-mga merged commit b5729a8 into godotengine:master Feb 19, 2020
@akien-mga
Copy link
Member

Thanks!

@ghost
Copy link

ghost commented Feb 20, 2020

Not sure where to ask given the history of this one. Wondering though if one of the original issues #32383 will remain for 3.2?

There were PRs consider for 3.2 that were closed in favor of 867d073.

@vnen vnen deleted the gdscript-variant-ref-fix branch February 20, 2020 12:09
@vnen
Copy link
Member Author

vnen commented May 29, 2020

@avencherus I totally missed this. You should have asked in the issue itself, this PR has nothing to do with that one. But I think a solution was added for 3.2.

@vnen
Copy link
Member Author

vnen commented May 29, 2020

I came here to mention that I added a fixup: 0f1da72, since I forgot to actually set the new variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants