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 default NodePaths saved in scene #92095

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented May 18, 2024

Fixes #91591

I spent whole day writing tests for both #91591 and #82670
In the end it turned out that disabling #83343, or at least part of it, fixes both issues. I don't know what to think about it, maybe I'm missing something.

Putting as draft. If it's a correct solution I assume more code needs to be removed.

@Cronos87
Copy link

Cronos87 commented May 28, 2024

Maybe it would be good to ask @warriormaster12 to try this MR; it could help to make progress on it?

That would be beneficial to prevent regression on #83343.

@warriormaster12
Copy link
Contributor

I can certainly give it a look however I'm not sure if I'm going to be able to give any useful input on the matter since my understanding of the code is as much as I touched.

@warriormaster12
Copy link
Contributor

Hmm the assumption that I made in property_utils.cpp in regards of affecting duplication was wrong. Duplicating a node still works even without if check. The only thing that changes is an ability to revert exported Node values in my_level.tscn provided by @Cronos87. Reverting them does nothing. Other variable types don't allow you to revert values in an instantiated scene unless a user has changed the values.

However, if this only causes a cosmetic inconsistency then I don't see an issue in reverting the change.

image

@KoBeWi KoBeWi force-pushed the TDD,_never_again branch from 224724f to 7fae1db Compare June 2, 2024 09:10
@KoBeWi KoBeWi marked this pull request as ready for review June 2, 2024 09:10
@KoBeWi KoBeWi requested review from a team as code owners June 2, 2024 09:10
@KoBeWi KoBeWi force-pushed the TDD,_never_again branch from 7fae1db to 2a9648c Compare June 2, 2024 09:10
@KoBeWi
Copy link
Member Author

KoBeWi commented Jun 2, 2024

Pushed a solution that doesn't break reverting.

@akien-mga

This comment was marked as resolved.

@KoBeWi KoBeWi force-pushed the TDD,_never_again branch from 2a9648c to be11100 Compare June 3, 2024 11:10
@akien-mga akien-mga merged commit 56cf773 into godotengine:master Jun 3, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Cronos87
Copy link

Cronos87 commented Jun 3, 2024

Many thanks for the fix and review! 🙏

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.

[4.3dev] Exported node path in child scene are reflected in the parent scene file
4 participants