godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.59k stars 21.1k forks source link

Cannot Save Scene while Shader Editor is Open #84601

Closed WickedInsignia closed 11 months ago

WickedInsignia commented 11 months ago

Godot version

4.5 Beta 5

System information

Windows 11, Nvidia RTX4070ti, AMD Ryzen7700x

Issue description

Cannot save any scene after changes while the Shader Editor is open.

Steps to reproduce

Minimal reproduction project

ShaderEditorSaveBug.zip

jsjtxietian commented 11 months ago

I guess related to #84064.

Confirmed, also it seems only the scene tab is not saved, but with visual shader editor and script editor open at the same time, whichever get the focus will be saved.

reduz commented 11 months ago

Folks, we are paper cutting with this all the time. I don´t know where this should be documented so we don´t keep running into this issue again every some years, probably the source code.

To clarify, the intended behavior is:

No matter where you are, if script editor, shader editor, scene editor, etc. Ctrl-S (or the shortcut assigned to save-scene) should always take precedence over everything else. When you save scene, all edited subresources or external resources are also saved, so its always a "save all" shortcut. If the script or shader editors have the same shortcuts as the scene save, then it should check this and trigger a scene save instead (and opened scripts/shaders will be saved anyway).

WickedInsignia commented 11 months ago

Maybe a dedicated shortcut to save file separate from Ctrl+S is needed, such as Ctrl+Alt+S? There may be some cases someone wants to save a shader without saving their changes to a scene (maybe if they were experimenting with objects in the scene to test the shader and did not want to save those scene changes). Using File > Save File seems fine to me though and I agree with you. I don't believe a good use-case existed to justify that commit.

YuriSizov commented 11 months ago

Maybe a dedicated shortcut to save file separate from Ctrl+S is needed, such as Ctrl+Alt+S?

That's what we had in the script editor, and it was a usability nightmare for many years, until we resolved it in Godot 4, allowing contexts for shortcuts. Then this was extended to the shader editor as well, because it's no different from the script editor at this point.

You can edit any number of shaders, unrelated to the current scene included. You can detach it and work in a different window, within a separate context from the main editor. You don't even have to have the 2D/3D view open, so saving the scene instead of the currently edited shader would be completely unexpected.

We want to have a WYSIWYG experience, so you save with intent. Alternatively, we need a true save everything shortcut, but that never existed and is not currently possible. (What Juan refers to as "Save all" is "Save scene with related resources", so not the same thing.)

WickedInsignia commented 11 months ago

Not being able to save scene while the shader editor is open is a usability nightmare all its own unfortunately. Saving the scene with related resources is the expected behavior currently across the editor, and is only broken in a couple corner cases that aren't expected by the user. "Save" isn't usually contextual in apps like other shortcuts and almost always refers to the overall scene or project file. This is consistent with other engines and 3D DCC tools. The key to "saving with intent" is the intent part. There should be consistency the user can expect. Hitting Ctrl+S while editing a StandardMaterial shouldn't suddenly change behavior entirely when editing a VisualShader. StandardMaterials have a "Save Resource" button that is easily accessible, and there's ample space to include the same thing in the shader/visualshader editor. Alternatively Blender uses Alt+S to save files external from the project file, such as images, so there'll be some overlap with users already familiar with that.

Not exactly sure what the best solution is, but Ctrl+S being hijacked while the shader editor is open ain't it. That's inconsistent UX design of arguably the single most important operation in the editor.

YuriSizov commented 11 months ago

Not being able to save scene while the shader editor is open is a usability nightmare all its own unfortunately.

That's just a bug, and it's fixable without drastic changes 🙃

akien-mga commented 11 months ago

To clarify, the fact that it's not possible to save a scene with Ctrl+S while the shader editor is visible is a bug, which #84614 will fix. After that fix, you can save the scene by giving focus to any other editor (so e.g. the steps to reproduce here won't reproduce the bug, since the 3D viewport will have focus at the time of pressing Ctrl+S).

The other part of this discussion is that it's actually intentional that Ctrl+S saves the shader instead of the scene when the shader editor has focus. Giving focus to any other editor that doesn't use Ctrl+S will let the event bubble up to the main File menu where Ctrl+S is "Save Scene". The script editor has the same behavior.

This behavior is a change as it wasn't possible in the past, and might warrant more discussion / options, as not everyone seems to be happy with it. Especially since we don't make it very obvious to the user which context is currently active.

jsjtxietian commented 11 months ago

Just pop here to mention that before the shortcut commit, when no scene is opened and I'm editing my shader, if I press Ctrl +S to save my shader, godot's will have an Alert window as mentioned in #84039.

If we change ctrl + s 's behaviour back to global save, this alert window might need some consideration.

WickedInsignia commented 11 months ago

@akien-mga That makes sense but is a pretty serious UX inconsistency, even with adequate contextual highlighting. Although the Shader Editor can be broken out to float, it will still see plenty of use when docked and where Ctrl+S is expected to save the scene + resources. In every other docked window with the 3D editor open that's exactly what it does, including the ShaderMaterial editor. In most other engines and DDC tools Ctrl+S is expected to save the project and not change contextually, such as how Blender uses Alt+S to save external files and avoid throwing a curveball. Users will expect Ctrl+S to save the scene + shader file, just as it does with the StandardMaterial editor. If the user wants to save the shader file but not the scene, they know to look for it since they know Ctrl+S always saves the scene and resources. For Ctrl+S not to do that is not expected behavior.

I may be pushing the issue here but Save is the single operation where users should always know exactly what to expect. If there's one thing to respect the industry standard on, it's this.

akien-mga commented 11 months ago

I think we can consider this as fixed by #84614, and the discussion on the UX downgrade can be continued in #84623 which aims at addressing it.