Skip to content

Added editor dialog for easily creating shaders. - #51356

Merged
Chaosus merged 1 commit into
godotengine:masterfrom
Chaosus:shader_dialog
Aug 7, 2021
Merged

Added editor dialog for easily creating shaders.#51356
Chaosus merged 1 commit into
godotengine:masterfrom
Chaosus:shader_dialog

Conversation

@Chaosus

@Chaosus Chaosus commented Aug 7, 2021

Copy link
Copy Markdown
Member

Added a dialog similar to GDScript but for the shaders instead.

image

Language options is to select between Shader and VisualShader.

Mode options are Spatial,CanvasItem,Particles and Sky.

Template options may be Default (which adds some code for functions) and Empty (shader_mode is defined only).

Demo GIF

shader_dialog

This will helps new users to better engage with the shaders + it speeds up the creation of shaders by allowing adding some template code afterward.

Bugsquad edit: Closes #20152

@Chaosus
Chaosus requested a review from a team August 7, 2021 10:32
@Chaosus
Chaosus requested review from a team as code owners August 7, 2021 10:32
@Chaosus Chaosus added this to the 4.0 milestone Aug 7, 2021
@Chaosus
Chaosus force-pushed the shader_dialog branch 3 times, most recently from 8d306fc to b65b6f8 Compare August 7, 2021 11:46
@Chaosus
Chaosus requested a review from Calinou August 7, 2021 11:49
@Chaosus
Chaosus force-pushed the shader_dialog branch 4 times, most recently from ebbb54d to 29cfa0d Compare August 7, 2021 12:04
@KoBeWi

KoBeWi commented Aug 7, 2021

Copy link
Copy Markdown
Member

There should be an option for built-in shaders.

@Calinou

Calinou commented Aug 7, 2021

Copy link
Copy Markdown
Member

In the long run, we may want to rename the Shader resource to TextShader to distinguish it from VisualShader. Right now, it's not obvious from the name alone that Shader is for text-based shaders.

@Chaosus

Chaosus commented Aug 7, 2021

Copy link
Copy Markdown
Member Author

There should be an option for built-in shaders.

@KoBeWi Done

In the long run, we may want to rename the Shader resource to TextShader to distinguish it from VisualShader. Right now, it's not obvious from the name alone that Shader is for text-based shaders.

Yeah, but it's not a subject for this PR...

@KoBeWi

KoBeWi commented Aug 7, 2021

Copy link
Copy Markdown
Member

This warning seems to be permanently in the dialog:
image
It's irrelevant to shaders.

@Chaosus

Chaosus commented Aug 7, 2021

Copy link
Copy Markdown
Member Author

This warning seems to be permanently in the dialog:
It's irrelevant to shaders.

I've removed it in the last commit. Check again.

@Chaosus
Chaosus requested a review from KoBeWi August 7, 2021 14:42
@Chaosus
Chaosus force-pushed the shader_dialog branch 4 times, most recently from bea2877 to 1e28737 Compare August 7, 2021 15:12
Comment thread editor/editor_resource_picker.cpp Outdated
Comment thread editor/scene_tree_dock.cpp Outdated
@Chaosus
Chaosus force-pushed the shader_dialog branch 2 times, most recently from 44ed73e to 9b37dfa Compare August 7, 2021 15:28
Comment thread editor/shader_create_dialog.h Outdated
@Chaosus
Chaosus force-pushed the shader_dialog branch 2 times, most recently from f7c307b to bd74aa0 Compare August 7, 2021 15:51
Comment thread editor/shader_create_dialog.cpp Outdated
Comment thread editor/shader_create_dialog.cpp Outdated
@Chaosus
Chaosus force-pushed the shader_dialog branch 3 times, most recently from 6032d06 to ce130d0 Compare August 7, 2021 17:02
@Chaosus

Chaosus commented Aug 7, 2021

Copy link
Copy Markdown
Member Author

Removed almost all comments outside the constructor. ^^

@KoBeWi

KoBeWi commented Aug 7, 2021

Copy link
Copy Markdown
Member

Looks great now!

The last thing to fix is the file path in the dialog:

	if (path == "") {
		String root_path = editor_data->get_edited_scene_root()->get_filename();
		if (root_path == "") {
			path = String("res://").plus_file(selected_shader_material->get_name());
		} else {
			path = root_path.get_base_dir().plus_file(selected_shader_material->get_name());
		}
	}

Using selected_shader_material->get_name() exclusively is not a good idea, because the material is unnamed in most cases. If the name doesn't exist, it should fallback to scene name.

Also editor_data->get_edited_scene_root() might return null if the resource was created in the inspector and no scene is open. It's rare, but leads to editor crash, so you should check first if a scene exists before getting filename.

@Chaosus

Chaosus commented Aug 7, 2021

Copy link
Copy Markdown
Member Author

@KoBeWi Well, I think I'm done with it, check again.

Comment thread editor/scene_tree_dock.cpp Outdated
@Chaosus

Chaosus commented Aug 7, 2021

Copy link
Copy Markdown
Member Author

Ok, let this be, hope users would like it as I did 😃

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.

Add basic template by default when creating new shader file

3 participants