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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a way for plugins to signal that they've made unsaved changes to a scene #2153

Closed
Calinou opened this issue Jan 19, 2021 · 18 comments 路 May be fixed by godotengine/godot#90130
Closed

Add a way for plugins to signal that they've made unsaved changes to a scene #2153

Calinou opened this issue Jan 19, 2021 · 18 comments 路 May be fixed by godotengine/godot#90130

Comments

@Calinou
Copy link
Member

Calinou commented Jan 19, 2021

Describe the project you are working on

The Godot editor 馃檪

Describe the problem or limitation you are having in your project

Now that the quit confirmation has been removed when there are no unsaved changes, we need to be extra sure that no data can be lost when closing the editor.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add a property/method in an editor class to signal that unsaved changes have been made to a scene.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Some design questions remain:

  • Should we imply that the currently edited scene is always the one being marked as dirty (has unsaved changes), or should we allow marking an arbitrary open scene as dirty even if it's not the currently edited one?
    • If we go for the former, then exposing this feature to plugins should be as simple as adding a EditorInterface.set_dirty(bool p_dirty) method.
    • There should be a way to mark a scene as non-dirty for the UndoRedo class to use. This is why the above suggested method uses a boolean parameter rather than just having EditorInterface.mark_as_dirty().

If this enhancement will not be used often, can it be worked around with a few lines of script?

Not that I know of.

Is there a reason why this should be core and not an add-on in the asset library?

This is about preventing data loss, so it's essential to have in core.

@Calinou Calinou changed the title Add a way for plugins to signal that they've made unsaved changes Add a way for plugins to signal that they've made unsaved changes to a scene Jan 19, 2021
@Calinou Calinou added this to the 4.0 milestone Jan 19, 2021
@YuriSizov
Copy link
Contributor

Should we imply that the currently edited scene is always the one being marked as dirty (has unsaved changes), or should we allow marking an arbitrary open scene as dirty even if it's not the currently edited one?

Most plugins that I make where I would hate for the data loss to happen deal with editing resources, not scenes. And plugins can also deal with non-Godot files when their core functionality is integration of some sort. So I would propose we do not make any assumptions as to what exactly is unsaved.

Besides, this call (EditorInterface.set_dirty(bool p_dirty)) would cause conflicts between two plugins setting it to true and only one of them setting it to false, overriding state for both. More granularity is needed. There is no way internally to figure out which plugin accesses the EditorInterface singleton (and EditorScript has access to it as well, so this is not limited to plugins).

As a more general solution which would also give greater benefits to plugins I'd propose two new methods instead:

  • EditorInterface.set_dirty(String p_identifier, bool p_dirty) to mark an arbitrary thing dirty which the plugin can later unmark when it's done with it. A resource path can be used as this identifier, but it would not be limited to it and plugins doing something unrelated to the file system or dealing with virtual files not yet saved can still use it.
  • EditorInterface.is_dirty(String p_identifier) to get current dirty status for the specified identifier, returning false if this identifier was never marked as dirty in the first place.

An API like that can be used for the quit confirmation as well as for internal tracking of changed resources by plugins. Hits two birds with one stone, so I think it would be simply beautiful to do it this way.

@KoBeWi
Copy link
Member

KoBeWi commented May 20, 2022

My alternate solution would be this: add a virtual method in EditorPlugin called _has_unsaved_changes(). When trying to close the editor, it would first ask all plugins if they are unsaved. If any of them returns true, stop closing and display a prompt. The issue mostly applies to script and shader editors (godotengine/godot#55026) as most of the other editors will make the scene modified.

@whogben
Copy link

whogben commented Jun 18, 2022

Has anyone figured out a workaround / way to trick the editor into considering the scene changed? I'm open to workarounds ranging from inelegant to abominable.

@Calinou
Copy link
Member Author

Calinou commented Jun 18, 2022

Has anyone figured out a workaround / way to trick the editor into considering the scene changed? I'm open to workarounds ranging from inelegant to abominable.

You can probably change a property of any node in the scene tree, wait for one frame, then change it back to its previous value.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 18, 2022

Commit no-op UndoRedo action.

@monsieurmax
Copy link

Same as @whogben , I'm trying to figure out a way to manually set a scene as dirty from editor plugin.
Modifying a property on a node from script (C#) is not making the scene as dirty, at least the ones I tried.

If someone has a workaround, whatever it is, I'd be very grateful to read it.

@CookieBadger
Copy link

Any update on this? I would also greatly appreciate info on any workaround

@KoBeWi
Copy link
Member

KoBeWi commented Jan 2, 2023

godotengine/godot#67503 will addres this. I'm going to modify it to allow per-scene unsaved status, but right now it's pending on another PR.

I mentioned a workaround in my previous comment.

@KoBeWi
Copy link
Member

KoBeWi commented Aug 15, 2023

The aforementioned PR was merged. Is _get_unsaved_status() enough to resolve this proposal?

@CookieBadger
Copy link

@KoBeWi from how I understood this solution, it shows the desired popup when closing a scene with changes, but it might have also been useful to be able to signal changes at any point in time, such that the asterisk (*) is displayed next to the scene name in the tab bar.

@KoBeWi
Copy link
Member

KoBeWi commented Aug 18, 2023

You can mark the scene as dirty using EditorUndoRedoManager. But what if you don't use UndoRedo or use a custom history (e.g. ScriptEditor)? A generic make_dirty(scene_id: int) method doesn't allow to remove the dirty status in case you e.g. undo. A set_dirty(dirty: bool) method could be abused to remove the dirty flag from other changes.

The only reasonable solution I can think of is adding some set_dirty(id: String, enabled: bool) method. This way a scene tab can keep a list of "dirty flags" and show * as long as there is at least one dirty flag in the list.

@TokisanGames
Copy link

TokisanGames commented Aug 18, 2023

We use the EditorUndoRedoManager in Terrain3D, and all of our edits show up in the history tab, but it doesn't mark the scene dirty and the scene can be closed without warning. And I don't see anything in the documentation about manually marking the scene dirty.

Now that I've looked at this new function, I suppose it can help, but it seems like an unexpected implementation. I was expecting that the function would return an int or bool, not a string, and it's purpose would be to solely tell Godot if there are changes or not. Yes or no. Then Godot could warn the user with the standard, do you want to save/cancel/don't save? We already capture the pre_save notification so we can save our resource, we're just missing the scene dirty status. We don't need to ask the user any special question beyond the above.

I suppose providing a list of changes to the user can be useful, and as long as Godot interprets a full string as yes and an empty string as no, that serves the same purpose.

@KoBeWi
Copy link
Member

KoBeWi commented Aug 18, 2023

We use the EditorUndoRedoManager in Terrain3D, and all of our edits show up in the history tab, but it doesn't mark the scene dirty and the scene can be closed without warning. And I don't see anything in the documentation about manually marking the scene dirty.

They probably show up as global changes instead of scene-specific.

@CookieBadger
Copy link

They probably show up as global changes instead of scene-specific.

Please elaborate on that. I have the same issue, commiting an action never marked my scenes dirty.

As to the function, in my opinion, the dirty flag not disappearing on undo is only a minor issue, and something that happens in many software products. It's preferable to ask once too often for saving, than to lose changes.

@KoBeWi
Copy link
Member

KoBeWi commented Aug 20, 2023

Please elaborate on that. I have the same issue, commiting an action never marked my scenes dirty.

Check the description of EditorUndoRedoManager, it explains how action context works. In short, your first operation in undo action should involve an object that belongs to a scene, i.e. a node or built-in resource (but not nested resource). If you can't provide object for operation, create_action() has custom_context argument that you can use. If the object can be identified as part of a scene, the action will be performed in the context of the scene and mark it as dirty.

Scene actions show as white in History dock, global actions show with "accent color":
image
They also have unique undo messages:
image

Unlike scene actions, the global actions only make the project dirty (you can see * in the Godot's title bar).

Here's example plugin that makes scene dirty. Put it in addons/ , enable, select Node2D and press D.
dirtyplugin.zip

As to the function, in my opinion, the dirty flag not disappearing on undo is only a minor issue, and something that happens in many software products. It's preferable to ask once too often for saving, than to lose changes.

Dirty flag is irrelevant here. get_unsaved_status() will prevent exiting regardless if anything is dirty.

I suppose providing a list of changes to the user can be useful, and as long as Godot interprets a full string as yes and an empty string as no, that serves the same purpose.

That's how it works.

@CookieBadger
Copy link

Thank you for the clarification and the example, this has been confusing me, since I think I followed an unofficial tutorial. UndoRedo now also works as it should in my project. Otherwise, I cannot think of a situation that would require another implementation, so for me the proposal would be resolved with get_unsaved_status().

@ArtyIF
Copy link

ArtyIF commented Oct 21, 2023

Currently, I tag my scene dirty by creating and commiting a no-op EditorUndoRedoManager action as KoBeWi suggested above. The issue is that it creates an entry in history that can be undone, even though I want the operation to be undoable (mostly because undoing what I'm doing is way too complex as it involves changing the scene root and reorganizing the nodes). It would be nice to add an option to commit_action to make an action undoable by, say, clearing the scene/global history depending on the context. Probably would be up to the plugin creator to add a sufficient warning that it can't be undone.

@KoBeWi
Copy link
Member

KoBeWi commented Apr 2, 2024

Seems like godotengine/godot#77750 added mark_scene_as_unsaved() that allows this.

@KoBeWi KoBeWi closed this as completed Apr 2, 2024
@KoBeWi KoBeWi modified the milestones: 4.x, 4.1 Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants