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

Clear the previously set state when configuring for a new scene root node #79201

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

Rindbee
Copy link
Contributor

@Rindbee Rindbee commented Jul 8, 2023

Saving a subscene (or other reasons) causes the main scene to be re-instantiated. And the resource instance in the main scene will be reused when the main scene is re-instantiated. So for resources with resource_local_to_scene enabled, resetting state may be necessary (at least for ViewportTexture).

Fix #69469, fix #79176, fix #77981, fix #74080, fix #80417.

@Rindbee Rindbee requested a review from a team as a code owner July 8, 2023 12:21
@wlsnmrk
Copy link
Contributor

wlsnmrk commented Jul 8, 2023

The fix is much appreciated, but these changes only sporadically resolve #79176 for me - sometimes the scene with the Subviewport and ViewportTexture will open correctly, but other times they will not.

@Rindbee
Copy link
Contributor Author

Rindbee commented Jul 8, 2023

Comparing addresses seems to be unreliable. Because the node pointed to by local_scene is freed without resetting the variable local_scene to nullptr, local_scene may point to the same address.

@wlsnmrk
Copy link
Contributor

wlsnmrk commented Jul 8, 2023

This version does appear to fix #79176, thanks!

@KoBeWi
Copy link
Member

KoBeWi commented Aug 8, 2023

I think the method name should be more descriptive, like reset_local_to_scene().

…node

Saving a subscene causes the main scene to be re-instantiated. And the resource
instance in the main scene will be reused when the main scene is re-instantiated.
So for resources with `resource_local_to_scene` enabled, resetting state may be
necessary (at least for `ViewportTexture`).
@Rindbee Rindbee force-pushed the fix-setup-state-not-cleared branch from ec39551 to 4795c3c Compare August 8, 2023 15:52
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

This adds a new method to Resource, so needs core review.
Other than that works correctly.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

@reduz said on chat that this one seems fine to him.

@akien-mga akien-mga merged commit e73a4a3 into godotengine:master Aug 17, 2023
@akien-mga
Copy link
Member

Thanks!

@Rindbee Rindbee deleted the fix-setup-state-not-cleared branch August 17, 2023 13:59
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment