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 shared exported variables of inherited scenes #88741

Conversation

adamscott
Copy link
Member

This PR fixes the "shared" exported variables of inherited scenes.

Supersedes

Supersedes #88739.

MRP

Many thanks to @exodrifter for her MRP.
dictionary-aliasing.zip

Notes to merging team

I added the label cherrypick:4.2, but this may break projects if they are "exploiting" the bug, ie. fake static variables.

Fixes

Fixes #81526.

@adamscott adamscott added bug topic:core cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Feb 23, 2024
@adamscott adamscott added this to the 4.3 milestone Feb 23, 2024
Copy link

@exodrifter exodrifter left a comment

Choose a reason for hiding this comment

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

I tested my MRP with this change and I wasn't able to figure out how to alias the Dictionary again using a similar strategy, so it seems to work.

scene/resources/packed_scene.cpp Outdated Show resolved Hide resolved
@adamscott adamscott force-pushed the fix-shared-variables-of-inherited-scenes-redux branch from 9997543 to d53a1b0 Compare February 24, 2024 21:35
@akien-mga akien-mga requested review from KoBeWi and dalexeev March 6, 2024 12:45
@KoBeWi
Copy link
Member

KoBeWi commented Mar 17, 2024

While this works, I wonder why is it even necessary. Why the issue happens in the first place, and only for inherited scenes?

@adamscott
Copy link
Member Author

Why the issue happens in the first place, and only for inherited scenes?

From what I remember, it's because inherited scenes will use the same Variant of the parent if we're not careful.

@akien-mga
Copy link
Member

CC @godotengine/gdscript

@KoBeWi
Copy link
Member

KoBeWi commented May 7, 2024

it's because inherited scenes will use the same Variant of the parent if we're not careful.

But when creating more instances should ensure duplication of variables. Like, when you have 2 instances with exported Dictionary then it's unique, but when these instances use inherited scene, the same variable is suddenly shared.

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.

The fix works and looks safe.

@akien-mga akien-mga merged commit 2960792 into godotengine:master May 7, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release topic:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exported arrays in extended scripts of inherited scenes all have the type of the first array declared
4 participants