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

Fix loading packed scene with editable children at runtime #49664

Merged
merged 1 commit into from
Aug 28, 2021

Conversation

pouleyKetchoupp
Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp commented Jun 16, 2021

Fixes #49660

At runtime, packed scenes with nodes marked as editable instance where saved with node type tags, which prevented the scene to be then loaded as an instance, causing duplicated nodes in the tree.

This change ensures nodes marked as editable instances and their owned children are properly set as instances.

That doesn't make a difference in the editor, since such nodes where already set as instances based on their instance state, but it helps at runtime where instance states are disabled.

Credits to @latorril for finding the issue and investigating the cause.

@pouleyKetchoupp pouleyKetchoupp added bug topic:core cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Jun 16, 2021
@pouleyKetchoupp pouleyKetchoupp added this to the 4.0 milestone Jun 16, 2021
@pouleyKetchoupp pouleyKetchoupp requested a review from a team June 16, 2021 17:59
@akien-mga akien-mga requested a review from reduz June 29, 2021 10:46
Copy link
Member

@reduz reduz left a comment

Choose a reason for hiding this comment

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

It looks good, although it would make me happy if someone more recently involved in this code can give it a check.

@mhilbrunner
Copy link
Member

@pouleyKetchoupp This needs a rebase :)

At runtime, packed scenes with nodes marked as editable instance where
saved with node type tags, which prevented the scene to be then loaded
as an instance, causing duplicated nodes in the tree.

This change ensures nodes marked as editable instances and their owned
children are properly set as instances.

That doesn't make a difference in the editor, since such nodes where
already set as instances based on their instance state, but it helps
at runtime where instance states are disabled.

Co-authored-by: latorril <latorril@gmail.com>
Copy link
Member

@mhilbrunner mhilbrunner left a comment

Choose a reason for hiding this comment

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

Built & tested locally, tests run fine, works in my tests projects, and I could verify that it fixes the issue. I couldn't find anything that this breaks, and the code looks fine too.

Looking at the history of the file, it doesn't seem like anyone besides @pouleyKetchoupp has touched this code (outside of small style changes or as side effect or changes elsewhere) much for at least the last year.

@mhilbrunner mhilbrunner merged commit 00e66e2 into godotengine:master Aug 28, 2021
@mhilbrunner
Copy link
Member

Thanks for this!

@akien-mga
Copy link
Member

Cherry-picked for 3.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 14, 2021
@pouleyKetchoupp pouleyKetchoupp deleted the fix-editable-duplicated branch September 16, 2021 18:23
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.

Duplicated nodes in tree after loading packed scene with editable children at runtime
4 participants