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

Unsaved scripts don't prompt when closing editor #55026

Closed
KoBeWi opened this issue Nov 16, 2021 · 14 comments · Fixed by #67503
Closed

Unsaved scripts don't prompt when closing editor #55026

KoBeWi opened this issue Nov 16, 2021 · 14 comments · Fixed by #67503

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Nov 16, 2021

Godot version

098e3cf / 3.4

System information

W10

Issue description

When you modify a script, it gets the "modified" status, but it's ignored by the editor. When you quit, any script changes are lost. Only unsaved scenes are handled.

Resolving this would effectively resolve #36281 (after #54653)

Steps to reproduce

  1. Open any script in script editor
  2. Modify it
  3. Close Godot

Minimal reproduction project

No response

@ator-dev
Copy link
Contributor

Sorry, I had this open in a different tab and copied the wrong issue tag... oops

@ator-dev
Copy link
Contributor

@KoBeWi Should a 'save script' dialogue appear for each unsaved script, or just a 'save all script changes' overall dialogue; or should this be an option toggled via the settings like for scenes?
Alternatively, should script changes just count as part of their scene/s, and not show any additional popups?

@Calinou
Copy link
Member

Calinou commented Nov 30, 2021

Should a 'save script' dialogue appear for each unsaved script, or just a 'save all script changes' overall dialogue; or should this be an option toggled via the settings like for scenes?

I think it should be an overall dialog, like we do for unsaved scenes.

@ator-dev
Copy link
Contributor

ator-dev commented Dec 2, 2021

@Calinou I have an effectively complete solution working at this point, but I've run into a problem. Built-in scripts don't seem to get a 'modified' status (though they are recognised as unsaved when quitting), and furthermore aren't saved by the save_all_scripts method meaning that my code doesn't work for them. I can't work out how to actually save all scripts, or to do it separately for built-in and external scripts (without saving scenes at the same time).
Do you have any advice for how I could resolve this, or will #54653 be adapted for 3.x (making both script types save in a similar way), meaning I should just pull request anyway?

@KoBeWi
Copy link
Member Author

KoBeWi commented Dec 2, 2021

The PR should be made against master branch first.

@ator-dev
Copy link
Contributor

ator-dev commented Dec 2, 2021 via email

@KoBeWi
Copy link
Member Author

KoBeWi commented Dec 2, 2021

Should I make separate pull requests for 3.x and 4.0?

Depends on how much different they are. Sometimes the changes are the same for both branches, so the commit can be cherry-picked. But if they are too different, 3.x needs a separate PR.

@ator-dev
Copy link
Contributor

ator-dev commented Dec 2, 2021 via email

@ator-dev
Copy link
Contributor

ator-dev commented Dec 3, 2021

There's one aspect of my changes (and my changes in 3.x) that I don't like; I had to add a variable bool close_without_saving to script_editor_plugin, as I couldn't find a better way of preventing scripts from being automatically saved on quit. I've added a TODO comment to this and might find a different workaround later. Apart from that, my fixes worked as expected in my tests and seem to fit in with the rest of the code workings.

@me2beats
Copy link

Is this still relevant?
I tested it on 3.5 beta — scripts are saved when closing the editor (there's no prompt tho)
I tried different editor options.

@me2beats
Copy link

me2beats commented Mar 31, 2022

I also tested it in 3.4 — scripts are saved
Windows

@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 31, 2022

there's no prompt tho

That's a bug: #18010
There should be a prompt. Also did you test with built-in scripts?

@me2beats
Copy link

me2beats commented Apr 1, 2022

Well yeah built-in scripts are not saved when project reloading.
So is this only built-in scripts problem (script changes are lost)

@ator-dev
Copy link
Contributor

ator-dev commented Sep 7, 2022

I had to add a variable bool close_without_saving

Just providing an update: I found my PR again recently, and have reworked it to work again and to remove poor practices such as the above unnecessary statefulness. I'm happy with how it works now, though shaders and built-in scripts would definitely benefit from special handling in future.

built-in scripts are not saved when project reloading

Built-in scripts are saved when their scene is saved, which means there is some additional weirdness and inconsistency in the saving system. I've addressed this temporarily by displaying a warning with the list of unsaved built-in scripts (since their scenes will be saved without prompting), but it should be addressed fully whenever possible.

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