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

Fix shader preprocessor include resource check #73975

Merged

Conversation

bitsawer
Copy link
Member

Fixes #73925

This extra check is needed because both Shader and ShaderInclude implement their resource loader (which gets eventually called by ResourceLoader::load()) in a somewhat strange way comapred to many other loaders. Neither loader can never really "fail" and they don't check for any errors, they will always return a valid resource, but with empty text content. In short, previously almost all include paths "worked" even if the file did not even exist and these places simply included an empty string without raising any erros.

A more fundamental fix would be to add error checking to both resource loaders, but that would be a high-risk fix as much of the exisiting codebase might not be prepared for these loaders to suddenly start returning null resources.

I can make a separarete PR to add error checking if that sounds like a good idea. It could be tested and merged later when 4.0 is out and there is more room for risky changes.

@akien-mga akien-mga merged commit ad32b22 into godotengine:master Feb 26, 2023
@akien-mga
Copy link
Member

Thanks!

@bitsawer bitsawer deleted the fix_preprocessor_include_check branch February 26, 2023 16:01
@naturally-intelligent
Copy link

Is this related to shaders precompiling?

In rc5 I was getting lag when a shader appeared. In rc6 that seems to be fixed for me.

So that's good

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.

Shader #include with absolute path without res:// leads to confusing error
3 participants