X Tutup
Skip to content

Fix resource shared when duplicating an instanced scene#64487

Merged
Repiteo merged 2 commits intogodotengine:masterfrom
Rindbee:fix-instantiated-scene-duplicate
Dec 2, 2025
Merged

Fix resource shared when duplicating an instanced scene#64487
Repiteo merged 2 commits intogodotengine:masterfrom
Rindbee:fix-instantiated-scene-duplicate

Conversation

@Rindbee
Copy link
Contributor

@Rindbee Rindbee commented Aug 16, 2022 •
edited
Loading

Fix #45350, fix #47918.
Depends on #65011.

For resources with resource_local_to_scene enabled in the subscene, the resource is already set when the subscene is instantiated, so does not need to be set again.

@Rindbee Rindbee requested a review from a team as a code owner August 16, 2022 08:44
@Rindbee Rindbee changed the title Fix resource duplication when duplicating an instanced scene Fix resource shared when duplicating an instanced scene Aug 16, 2022
@Calinou Calinou added this to the 4.0 milestone Aug 16, 2022
@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from c6f8718 to 661579b Compare August 17, 2022 11:18
@Rindbee Rindbee mentioned this pull request Aug 17, 2022
Open
@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from 661579b to f2afbb6 Compare August 17, 2022 23:27
@akien-mga akien-mga requested a review from RandomShaper August 28, 2022 21:06
RandomShaper
RandomShaper reviewed Aug 29, 2022
View reviewed changes
RandomShaper
RandomShaper requested changes Aug 29, 2022
View reviewed changes
Copy link
Member

@RandomShaper RandomShaper left a comment

Choose a reason for hiding this comment

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

Looks good. There's just the comment about structure to improve readability.

@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from f2afbb6 to a3af0c7 Compare August 29, 2022 08:41
@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from a3af0c7 to c4f6fe5 Compare August 29, 2022 09:39
@Rindbee
Copy link
Contributor Author

Rindbee commented Aug 29, 2022 •
edited
Loading

There is also a similar situation here #64999 (comment)

I suggest to review #65011 first, this pr also needs to use a similar structure.

@RandomShaper RandomShaper added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Aug 29, 2022
@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from c4f6fe5 to e8e727f Compare August 29, 2022 12:54
Rindbee
Rindbee commented Aug 29, 2022
View reviewed changes
@akien-mga
Copy link
Member

akien-mga commented Aug 30, 2022

The build seems to fail.

@Rindbee
Copy link
Contributor Author

Rindbee commented Aug 30, 2022 •
edited
Loading

Yes, here uses the copy_from after the modified in #65011. Compared with previous, the parameter list is different.

@RandomShaper
Copy link
Member

RandomShaper commented Aug 30, 2022

@Rindbee, you may want to rebase this PR on top of the other one this depends on so CI build passes. When the other one is merged, this one will get automatically trimmed down to the only additional commit.

@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from e8e727f to f8398dd Compare August 30, 2022 10:27
@Rindbee
Copy link
Contributor Author

Rindbee commented Aug 30, 2022 •
edited
Loading

I think there are fewer records about the correlation between resources between tscn.

All the related (merged and not yet merged) PRs are based on a judgment: Resources with local-to-scene enabled on the same property of the same object that recorded in the main scene, is a shared copy of the source resource with local-to-scene enabled which recorded in the sub-scene.

In most cases, this may be correct. But in fact, this situation cannot be ruled out: Really hope to use new resources to cover the resources on the scene root.

@akien-mga akien-mga modified the milestones: 4.0, 4.x Feb 10, 2023
@KoBeWi
Copy link
Member

KoBeWi commented Jun 13, 2023

Needs rebase.

@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from f8398dd to 01becb4 Compare June 14, 2023 10:16
@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch 5 times, most recently from 8e68044 to ff6a538 Compare September 10, 2023 01:26
@Rindbee
Copy link
Contributor Author

Rindbee commented Sep 10, 2023

Okay, the cases of enabling/disabling Editable Children has been completed for scene initialization and node duplication. I think it's ready to be reviewed again.

@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch 3 times, most recently from e58ae4b to 498e3fc Compare September 10, 2023 09:09
@Rindbee
Copy link
Contributor Author

Rindbee commented Sep 10, 2023

Node::_duplicate() does not seem to be a suitable place to remap resources. It seems more appropriate to modify the mapping logic in SceneTreeDock::paste_nodes().

I'll try to modify the mapping logic there later.

@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from 498e3fc to 4b56fe2 Compare September 11, 2023 06:56
@Rindbee Rindbee requested a review from a team September 11, 2023 06:56
@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from 4b56fe2 to 4f81116 Compare September 11, 2023 08:01
@Rindbee Rindbee mentioned this pull request Oct 23, 2023
Merged
@Rindbee Rindbee force-pushed the fix-instantiated-scene-duplicate branch from 4f81116 to 2737783 Compare October 23, 2023 02:47
@akien-mga akien-mga modified the milestones: 4.2, 4.3 Oct 26, 2023
@YuriSizov YuriSizov added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Oct 26, 2023
@KoBeWi KoBeWi modified the milestones: 4.3, 4.4 Aug 3, 2024
@KoBeWi KoBeWi added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Aug 3, 2024
@Repiteo Repiteo added the cherrypick:4.4 Considered for cherry-picking into a future 4.4.x release label Feb 24, 2025
@Repiteo Repiteo modified the milestones: 4.4, 4.5 Feb 24, 2025
@Rindbee
Supplement the case of scene instantiation for "Editable Children"
1b4f116 
This is a follow-up to godotengine#65011.

For scenes with **Editable Children** enabled, the main scene will record
more information and resource mapping will be valid for multiple nodes.
KoBeWi
KoBeWi approved these changes Dec 2, 2025
View reviewed changes
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.

Gave it some testing and it seems to work correctly.

Might need core re-review after latest changes.

@Rindbee
Fix resource shared when duplicating an instanced scene
e0532a7 
For resources with `resource_local_to_scene` enabled in the sub-scene,
the resource is already set when the sub-scene is instantiated, so does
not need to be set again. Just needs to update the property of the
resource according to the value in the main scene.
Rindbee
Rindbee commented Dec 2, 2025
View reviewed changes
editor/docks/scene_tree_dock.cpp
Comment on lines +961 to +967
for (KeyValue<Node *, HashMap<Ref<Resource>, Ref<Resource>>> &KV : resources_local_to_scenes) {
for (KeyValue<Ref<Resource>, Ref<Resource>> &R : KV.value) {
if (R.value->is_local_to_scene()) {
R.value->setup_local_to_scene();
}
}
}
Copy link
Contributor Author

@Rindbee Rindbee Dec 2, 2025

Choose a reason for hiding this comment

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

This ensures that local-to-scene resources in newly copied child scene instances are effective when Editable Children is enabled.

The cache is now only cleared only when a new copy begins. As users edit the scene, it may lead to the use of released node instances as keys.

@Repiteo
Copy link
Contributor

Repiteo commented Dec 2, 2025

Thanks!

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

Reviewers

@KoBeWi KoBeWi KoBeWi approved these changes

@AThousandShips AThousandShips Awaiting requested review from AThousandShips

@RandomShaper RandomShaper Awaiting requested review from RandomShaper

@reduz reduz Awaiting requested review from reduz

Assignees

No one assigned

Labels

bug topic:core

Projects

None yet

Milestone

4.6

9 participants

@Rindbee @akien-mga @RandomShaper @KoBeWi @YuriSizov @reduz @Repiteo @AThousandShips @Calinou
X Tutup