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

Avoid saving scene while already saving the scene #85154

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Nov 20, 2023

Fixes #85093
Does not fix my crash unfortunately.

@KoBeWi KoBeWi added this to the 4.2 milestone Nov 20, 2023
@YuriSizov
Copy link
Contributor

I wasn't sure about a solution like this. I think that this will prevent the updated script from saving. We first save the scene then the script editor modifies the script to add an empty new line, and then we attempt to save the same scene again, but this won't happen as far as I can tell.

In a pinch this is not the worst thing, but we need to handle this somehow, ideally.

@KoBeWi
Copy link
Member Author

KoBeWi commented Nov 20, 2023

There are multiple callbacks for applying some changes before saving, like NOTIFICATION_EDITOR_PRE_SAVE or apply_changes(). ScriptEditor should use that instead of modifying the script while it's being saved.

@akien-mga
Copy link
Member

There are multiple callbacks for applying some changes before saving, like NOTIFICATION_EDITOR_PRE_SAVE or apply_changes(). ScriptEditor should use that instead of modifying the script while it's being saved.

Do you want to try that approach? 👀

@KoBeWi
Copy link
Member Author

KoBeWi commented Nov 22, 2023

I was thinking that it can be done independently (tbh the case prevented in this PR might even deserve an error message), but I could do it in this PR.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Discussed with Yuri, should be a good approach for now to prevent the crash.

@akien-mga akien-mga merged commit 51bca1b into godotengine:master Nov 22, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the yo_dawg_I_heard_you_like_saving_scen-JUST_STOP_IT branch November 22, 2023 13:15
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.

Editor crashes when saving built-in script with a non empty last line
3 participants