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

[ShaderGraph] Replace TextAsset usage with ShaderInclude #4095

Merged
merged 7 commits into from Apr 15, 2021

Conversation

esmelusina
Copy link
Contributor

@esmelusina esmelusina commented Apr 6, 2021


Purpose of this PR

Trunk is moving Text files with shader related extensions to a separate asset type, ShaderInclude. This change originally caused a regression for us (which has been resolved), but we need to move to the new asset type. This change only effects Custom Function Node's asset selector for File sources, improving the filtering to exclude unsupported extensions by default.

Related issues/bugs:
https://jira.unity3d.com/browse/GSG-387
https://fogbugz.unity3d.com/f/cases/1324528/
https://fogbugz.unity3d.com/f/cases/1322466/

Note that our previous soft-checking for file extensions remains (as it impacts some ui/error msgs), as the goal of this PR is just to comply with Trunk.


Testing status

Manually checked asset type filtering on the ObjectField for all supported types. QA will do some additional testing. Extensions are further tested (to an extent) in the Custom Function Node related graphics tests.


Notes for Reviewers

Min version needed to be bumped for ShaderGraph, for which requires a min version bump for all of graphics.

@esmelusina esmelusina requested review from cdxntchou and a user April 6, 2021 21:44
@esmelusina esmelusina marked this pull request as ready for review April 6, 2021 21:45
@esmelusina esmelusina requested a review from a team as a code owner April 6, 2021 21:45
@esmelusina esmelusina requested review from a team as code owners April 8, 2021 00:55
@esmelusina esmelusina marked this pull request as draft April 8, 2021 17:14
@esmelusina esmelusina marked this pull request as ready for review April 9, 2021 15:47
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Should be good now!

@esmelusina esmelusina merged commit 4e4e400 into master Apr 15, 2021
@esmelusina esmelusina deleted the sg/textasset-to-shaderinclude branch April 15, 2021 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants