First a question.
Shader class allows us to find shader property in a specific shader:
int propertyId= shader.GetPropertyNameId( shader.FindPropertyIndex( propertyName ) );
Let’s say I have a property in my shader float windForce;
My package will work fine, but if my customer has such a property in one of his shaders and uses Shader.PropertyToID() to find it, will his project be at risk of finding my shader property instead?
That would make his project stop working properly the moment he installs my package.
If that’s true, then here’s my first request - make Shader.PropertyToID() obsolete and display a warning during compilation that this method is dangerous with a hint of using FindPropertyIndex and GetPropertyNameId in instead.
My other request is that ComputeShader class should have an equvalent of Shader.FindPropertyIndex, Shader.GetPropertyNameId and other property related methods.
Right now, if your’e developing a tool that uses compute shader, I see no safe way other than using name convention like this:
<organization_name>_<variable_name>
You can imagine how long variable names will become to remain unique and not cause conflicts with shaders in projects.
Or we could use names like:
<variable_name>_<random_string>
This is never 100% safe (Murphy’s laws) and still makes peoprrty names unnecessarily long and messy.
In my opinion this is a serious issue for tool developer like me, who wants to develop high-performance tools using compute shaders.
Right now, my property names in compute shaders are like 50-60 characters long. Go figure…
This doesn’t “find” anything, it just gives you that this “name” has this ID. If two different variables share a name, they share an ID as well.
This is an issue if either you or your customer does Shader.SetGlobalWhatever. Otherwise if you just set it to a specific material, there is no issue.
Why?
I’m not 100% sure because I haven’t really used GetPropertyNameId, but my guess is if the variables are named the same, this would have the exact same issue.
If you are an asset developer the issue is fixed by having your shader variables have a prefix related to your asset.
Ok, then the issue is not as bad as I thought.
Thanks for clarifying - it’s not something you could easily deduct from documentation.
My first instinct was to assume that each shader instance has its own ID for each of its properties.
This was also my conclusion because of existence of mentioned SetGlobal methods.
Or a GUID-like suffix, like unity shader graph does.
Either way, you’re making your code less friendly to read.
Yes but the problem is not with the IDs really. Shader.SetGlobal***** also accepts string (which internally calls PropertyToID, but anyway), so if you have overlapping shader names, you have an issue.
I’m not sure how you would be able to give whatever name you want without caring about collisions and still be able to Set stuff Globally.
That is understndable.
The issue is when you develop a tool for sale in Asset Store - you will not be using SetGlobal yourself, but someone using your tool may do that.
So I never know my properties are set correctly, unless I set them every time I’m about to use one of my shaders or I use property names with prefixes or suffixes that are unique enough and hoping Murphy’s Law won’t bite me:)
Thanks for all your comments