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.

Sign up for GitHub

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

Jump to bottom

Avoid duplicating signals from scene instances into packed scenes #97303

Merged
merged 1 commit into from
Nov 29, 2024
Merged

Avoid duplicating signals from scene instances into packed scenes #97303

merged 1 commit into from
Nov 29, 2024
+76 −4

Conversation

cixil
Copy link
Contributor

@cixil cixil commented Sep 21, 2024 •
edited by AThousandShips
Loading

Fixes #48064 and fixes #86532

When packing a tree of nodes to a PackedScene, signal connections originating from the root node of a scene instance within that tree would get duplicated into the PackedScene and cause "duplicate signal" errors when instantiating it.

@cixil cixil requested review from a team as code owners September 21, 2024 23:42
@cixil
Copy link
Contributor Author

cixil commented Sep 22, 2024

I think this also fixes #85372

@cixil cixil mentioned this pull request Sep 22, 2024
Open
@fire fire changed the title fix bug where signals from scene instances are included in packed scenes Avoid duplicating signals from scene instances into packed scenes Sep 22, 2024
@AThousandShips AThousandShips added this to the 4.4 milestone Sep 22, 2024
@AThousandShips AThousandShips added cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Sep 22, 2024
@cixil cixil requested review from a team as code owners September 22, 2024 14:51
@AThousandShips AThousandShips removed request for a team September 22, 2024 14:53
@AThousandShips

This comment was marked as resolved.

Sign in to view
@cixil cixil force-pushed the fix-duplicate-signal-bug branch 2 times, most recently from a99174b to b19b1ed Compare September 22, 2024 17:13
KoBeWi
KoBeWi reviewed Oct 28, 2024
View reviewed changes
core/object/object.h Outdated Show resolved Hide resolved
@fire fire requested a review from SaracenOne October 29, 2024 03:09
AThousandShips
AThousandShips previously requested changes Oct 29, 2024
View reviewed changes
tests/scene/test_packed_scene.h Outdated Show resolved Hide resolved
tests/scene/test_packed_scene.h Outdated Show resolved Hide resolved
@jitspoe
Copy link
Contributor

jitspoe commented Nov 8, 2024

Thanks for working on this! The error flood from this issue has made it difficult to spot real errors in my project.

akien-mga
akien-mga reviewed Nov 12, 2024
View reviewed changes
tests/scene/test_packed_scene.h Outdated Show resolved Hide resolved
scene/resources/packed_scene.cpp Outdated Show resolved Hide resolved
@akien-mga akien-mga requested a review from a team as a code owner November 29, 2024 17:29
@akien-mga akien-mga merged commit 94082a4 into godotengine:master Nov 29, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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

@akien-mga akien-mga akien-mga left review comments

@KoBeWi KoBeWi KoBeWi approved these changes

@SaracenOne SaracenOne Awaiting requested review from SaracenOne

@AThousandShips AThousandShips Awaiting requested review from AThousandShips

Assignees
No one assigned
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release topic:core
Projects
None yet
Milestone
4.4
5 participants
@cixil @AThousandShips @jitspoe @akien-mga @KoBeWi