Zallist / unity.zallist.universal-simple-lit-shadergraph-target

This plugin simply adds a Simple Lit material (SubTarget) to the Universal target for Shader Graph for URP
Other
85 stars 14 forks source link

Error CS0117 in Unity 2022.2.15f #20

Closed RyanJVR closed 1 year ago

RyanJVR commented 1 year ago

Hi, firstly want to say thanks for implementing this in shadergraph.

Im getting multiple CS0117 errors after upgrading Unity to build 2022.2.15f. Was working fine in previous builds

okuligowski commented 1 year ago

Same issue. Is someone actively supporting this repo?

Zallist commented 1 year ago

Don't have time to look into it right now. If you can paste the full visual studio error screenshot / message and line numbers, I might be able to guess the fix.

okuligowski commented 1 year ago

Screenshot 2023-04-17 at 18 14 38

Zallist commented 1 year ago

Looks like unity must have renamed the internal classes in a minor version update. Which is insane but hey it's internal. Probably an easy fix to look at the target c# file and look at what they renamed the classes to, then do a find/replace in the simple lit target. Or alternatively just copy those missing classes into the package, rename them and then use the copied versions to avoid conflicts of they rename it in the future.

okuligowski commented 1 year ago

Thanks for a quick response Zallist. Is someone skilled enough to fix it? I don't understand how this works internally.

Zallist commented 1 year ago

If you're in a rush, none of those keywords (in the screenshot) are super important, so you can probably just fork the repo and comment out each of the lines that have that error on them. It might make the performance of the shadergraphs using the simple lit target perform slightly worse on newer devices, but should be fine for development. And then I can do it when I have some free time (or someone else wants to look at what unity changed them to)

If someone actually wants to dive into the code, the simple lit subtarget is based on https://github.com/Unity-Technologies/Graphics/blob/2022.2/staging/Packages/com.unity.render-pipelines.universal/Editor/ShaderGraph/Targets/UniversalLitSubTarget.cs

So in essence you'd just need to rematch the code in the simple lit target to that one. Or replicate whatever changes they did.

Zallist commented 1 year ago

Exact commit they broke it with

https://github.com/Unity-Technologies/Graphics/commit/584e10efb36cb33d6f67461da75eb3b035f3798f

Looks like they simply renamed the property names. Dropping a lot of the sm45

RyanJVR commented 1 year ago

Thanks. I dropped the sm45 and commented out the DOTSDepthNormal and WriteRenderingLayers error lines and everything seems to be fine now have to test still.

okuligowski commented 1 year ago

Thanks!

NMP-ffischer commented 1 year ago

Hi, there are two forks from 3 weeks ago which has a fix. Maybe you can pull from there and make a release? Thanks.

Zallist commented 1 year ago

Thanks for letting me know. Both https://github.com/okuligowski/unity.zallist.universal-simple-lit-shadergraph-target and https://github.com/VoxelAgentSimon/unity.zallist.universal-simple-lit-shadergraph-target are correct (and the same) but both will cause errors in earlier (pre minor version 15) versions of 2022.2, so can't be merged directly. Only obvious solution I can think of right now is to make a pseudo define at the top of the file to conditionally change it.

if UNITY_2022_2_1 || UNITY_2022_2_2 || (continue to 14)

define UNITY_2022_2_14_OR_OLDER

endif

And then using that in each of the changes made in the forks.

I can see if I can find time to do that this week but might not get to it until next week. If someone's else did that I'd merge it through immediately.

Zallist commented 1 year ago

Fixed with a custom implementation for UNITY_2022_2_15_OR_NEWER which is really really really annoying and stupid but it is what it is. File is looking a bit #if happy which is kinda making it hard to work out what's what, so will probably remove all that in the next major tag, and ask people who want to use an older version to use a specific tag.

NMP-ffischer commented 1 year ago

Thank you, works perfectly.