godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.13k stars 87 forks source link

Add the shader resource name (or path) inside of the error output when compilation fails #4887

Open paddy-exe opened 2 years ago

paddy-exe commented 2 years ago

Describe the project you are working on

VFX in Godot

Describe the problem or limitation you are having in your project

Trying out the Godot 4 Beginners Course I encountered a bug in a shader which happened due to hint_color being renamed to source_color.

The error message in the error log looks like this: ``` 1 | // NOTE: Shader automatically converted from Godot Engine 4.0.alpha1's StandardMaterial3D. 2 | 3 | shader_type spatial; 4 | render_mode blend_mix,depth_draw_never,cull_back,diffuse_burley,specular_schlick_ggx,depth_test_disabled; E 5-> uniform vec4 albedo : hint_color; 6 | uniform sampler2D texture_albedo : hint_albedo,filter_linear_mipmap,repeat_enable; 7 | uniform float point_size : hint_range(0,128); 8 | uniform float roughness : hint_range(0,1); 9 | uniform sampler2D texture_metallic : hint_white,filter_linear_mipmap,repeat_enable; 10 | uniform vec4 metallic_texture_channel; 11 | uniform sampler2D texture_roughness : hint_roughness_r,filter_linear_mipmap,repeat_enable; 12 | uniform float specular; 13 | uniform float metallic; 14 | uniform vec3 uv1_scale; 15 | uniform vec3 uv1_offset; 16 | uniform vec3 uv2_scale; 17 | uniform vec3 uv2_offset; 18 | 19 | uniform vec4 x_ray_color : hint_color; 20 | 21 | void vertex() { 22 | UV=UV*uv1_scale.xy+uv1_offset.xy; 23 | } 24 | 25 | 26 | 27 | 28 | void fragment() { 29 | vec2 base_uv = UV; 30 | vec4 albedo_tex = texture(texture_albedo,base_uv); 31 | ALBEDO = albedo.rgb * albedo_tex.rgb; 32 | float metallic_tex = dot(texture(texture_metallic,base_uv),metallic_texture_channel); 33 | METALLIC = metallic_tex * metallic; 34 | vec4 roughness_texture_channel = vec4(1.0,0.0,0.0,0.0); 35 | float roughness_tex = dot(texture(texture_roughness,base_uv),roughness_texture_channel); 36 | ROUGHNESS = roughness_tex * roughness; 37 | SPECULAR = specular; 38 | 39 | float depth = textureLod(DEPTH_TEXTURE, SCREEN_UV, 0.0).r; 40 | vec4 upos = INV_PROJECTION_MATRIX * vec4(vec3(SCREEN_UV, depth) * 2.0 - 1.0, 1.0); 41 | float scene_depth = upos.z / upos.w; 42 | 43 | vec4 vpos = INV_PROJECTION_MATRIX * vec4(vec3(SCREEN_UV, FRAGCOORD.z) * 2.0 - 1.0, 1.0); 44 | float v_depth = vpos.z / vpos.w; 45 | 46 | float depth_diff = scene_depth - v_depth; 47 | 48 | 49 | float rim = pow(1.0 - dot(VIEW, NORMAL), 1.0); 50 | ALBEDO.rgb = mix(ALBEDO.rgb, 3.0 * x_ray_color.rgb * rim, clamp(depth_diff * 5.0, 0.0, 1.0)); 51 | ALPHA = mix(ALPHA, rim, clamp(depth_diff * 5.0, 0.0, 1.0)); 52 | 53 | } 54 | :5 - Expected valid type hint after ':'. Shader compilation failed. ```

Finding this type of bug in shader files if it's a course and there are a lot of files is only possible to find if you go through every file or if you are the dev yourself and you can guess where you wrote the shader code.

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

This limitation can be worked around if the shader error log entails the shader resource name or path

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

Not sure how I would do this here

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

I highly doubt it

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

It's part of the shader editor

Calinou commented 2 years ago

Related to https://github.com/godotengine/godot-proposals/issues/3000 and https://github.com/godotengine/godot-proposals/issues/697.

Many shaders are part of the engine itself, so they don't have a filename to speak of.

bramreth commented 2 years ago

Note: this was also a case where the shader was part of a ShaderMaterial so didn't have a dedicated file that was easy to find

paddy-exe commented 2 years ago

@Calinou Would it be possible then to show the owner Node that entails the shader?

Calinou commented 2 years ago

@Calinou Would it be possible then to show the owner Node that entails the shader?

See my comment in https://github.com/godotengine/godot-proposals/issues/3000#issuecomment-894859165.

paddy-exe commented 2 years ago

@Calinou In that case the resource path/name would be the best way to implement this. I will update the proposal accordingly.