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

Improvements to the shader editor #61459

Merged
merged 1 commit into from
Jun 20, 2022

Conversation

reduz
Copy link
Member

@reduz reduz commented May 27, 2022

  • Shader editor is permanent (no longer transient).
  • Can edit multiple files at the same time.

Likely fixes many usability issues (please lend me a hand Bugsquad team to identify them).

Obligatory gif
shader-editor

@reduz reduz requested review from a team as code owners May 27, 2022 09:11
@reduz reduz changed the title Redo the shader editor Improvements to the shader editor May 27, 2022
@Calinou Calinou added this to the 4.0 milestone May 27, 2022
@Calinou
Copy link
Member

Calinou commented May 27, 2022

I'd look into adding a button to collapse the list of shaders, similar to the one we already have at the bottom of the script editor between the list and the script:

image

This can be beneficial when editing shaders with long lines on narrow displays.

This button has a Toggle Scripts Panel shortcut which defaults to Ctrl + \. It probably makes sense to reuse the default binding (but with a different shortcut name, I assume).

@reduz
Copy link
Member Author

reduz commented May 27, 2022

@Calinou This not easy because that section is part of the shader editor itself, and the visual shader editor does not allow placing something there.

This is an initial implementation, other contributors can add small things to it and improve it afterwards.

@sztrovacsek92
Copy link

sztrovacsek92 commented May 27, 2022

  • Shader editor is permanent (no longer transient).
  • Can edit multiple files at the same time.

Likely fixes many usability issues (please lend me a hand Bugsquad team to identify them).

Obligatory gif shader-editor

Would it be possible, to add the Shader Editor's button next to the 2D/3D/Script buttons instead? And it would work like the regular script/visualscript window, except it could be split to 2 for previewing, like we can do it in 3D.

godshad

@reduz
Copy link
Member Author

reduz commented May 27, 2022

@sztrovacsek92 not really, the idea of the shader editor is that you always edit shaders while looking at the viewport to see the changes. This is what editors on the bottom dock are for. Having it on another main tab would be very annoying, because you would need to manually split it all the time. The way it is now happens on demand.

@KoBeWi
Copy link
Member

KoBeWi commented May 27, 2022

Built-in shader names should display similar to built-in scripts, as this isn't really useful:
image
Also closing a scene doesn't close its built-in shaders and you can edit them on another scene, but not save.
Also also saving a built-in shader opens a save file dialog. It should save its owning scene.

@reduz
Copy link
Member Author

reduz commented May 27, 2022

@KoBeWi That makes sense and it will have to be added eventually, if you can work on improving it later on that would be great.

I am not as familiarized to how all that logic in the editor works nowadays and it would take me longer. In general my idea is that as I have so many things pending I need to work on, I try to get something working that does what it has to do and then rely on others to bring it up to the standards they expect.

@KoBeWi
Copy link
Member

KoBeWi commented May 28, 2022

Guess I will just list some more problems that I found, but with an intention that they can be resolved later (unless it's something major):

  • middle-click doesn't close shader
  • File menu doesn't have any shortcut
  • the option to open in the inspector is interesting, but not sure what can it be used for 🤔
  • changing shader externally will force-reload it, even if you have unsaved changes. You can undo this though
  • shaders have no modified state (no (*) appears)
  • previously opened shaders aren't restored on project start

* Shader editor is permanent (no longer transient).
* Can edit multiple files at the same time.

Likely fixes many usability issues (please lend me a hand Bugsquad team to identify them).
@reduz reduz force-pushed the new-shader-editor branch from dd7ca4c to 73c102f Compare May 28, 2022 09:03
KoBeWi
KoBeWi previously approved these changes May 28, 2022
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Looks ok (aside from #61459 (comment), #61459 (comment) and some minor style stuff)

@KoBeWi
Copy link
Member

KoBeWi commented May 30, 2022

Actually it's not ok. It doesn't fix #43635, which doesn't make sense for a permanent editor.

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.

Looks good to me.

As pointed out above #43635 is still not solved, this will require further work to figure it out. But we can merge this refactor as is anyway.

@akien-mga akien-mga merged commit b4804a2 into godotengine:master Jun 20, 2022
@akien-mga
Copy link
Member

Thanks!

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.

5 participants