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

Don't duplicate custom type scripts #100177

Closed
wants to merge 1 commit into from

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Dec 8, 2024

Fixes #100154

@KoBeWi KoBeWi added this to the 4.4 milestone Dec 8, 2024
@KoBeWi KoBeWi requested a review from a team as a code owner December 8, 2024 15:28
Copy link
Contributor

@Synzorasize Synzorasize left a comment

Choose a reason for hiding this comment

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

Hey, I just ran into this issue, so I decided to test out this commit and it works perfectly. I don't know much about C++ or the Godot source code or code style, though, but it looks good enough to me.

@sfreed141
Copy link
Contributor

I tried this fix out since I was hitting #100154 in a personal project but it resulted in .duplicate(true) failing to make a deep copy. Not sure what's different about my code compared with Synzorasize, but replacing the .duplicate(true) with some code to manually copy my resource works fine.

Fwiw, I have nested custom resources so perhaps that's the issue: a custom resource AttributeSet which contains an array of another custom resource, AttributeResources. Calling attribute_set.duplicate(true) performs a shallow copy.

@KoBeWi
Copy link
Member Author

KoBeWi commented Dec 23, 2024

Do you have the same issue in dev5? It might be another bug.

@sfreed141
Copy link
Contributor

If I launch my project through the editor on dev4/5/6 I hit #100154. However if I run the project directly without the editor then I do still hit the duplicate bug (on dev4/5/6). So perhaps it is a different (but more severe) bug?

@adamscott
Copy link
Member

adamscott commented Jan 3, 2025

Oops. Didn't see your PR when I did mine: #101001.

Mine seems to be forward-compatible with additional scripts as metadata though.

@adamscott
Copy link
Member

@sfreed141 Can you test my fix #101001? It seems to solve #100154 with the latest builds.

@sfreed141
Copy link
Contributor

@adamscott yes, your PR does fix #100154, however deep copying a resource is still broken unfortunately :(

@KoBeWi KoBeWi closed this Jan 7, 2025
@KoBeWi KoBeWi added the archived label Jan 7, 2025
@KoBeWi KoBeWi removed this from the 4.4 milestone Jan 7, 2025
@KoBeWi KoBeWi deleted the deep_scripts branch January 7, 2025 12:13
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.

Deep copy custom resource created in 4.4dev4 or newer throws Parser Error: Class hides a global script class
4 participants