RhenaudTheLukark / CreateYourFrisk

Rhenaud The Lukark's Unitale fork
GNU General Public License v3.0
132 stars 56 forks source link

shader.GetVector() returning values in incorrect order #96

Closed KeepDrive closed 3 years ago

KeepDrive commented 3 years ago

shader.GetVector() seems to return the vector as {w,x,y,z} instead of {x,y,z,w}, I'm not sure if this is intended behaviour, my guess is that this is a mistake that stemmed from unity docs displaying Vector4 properties in alphabetical order(https://docs.unity3d.com/2018.2/Documentation/ScriptReference/Vector4.html), while the constructor and other operations expect the order to be {x,y,z,w} (https://docs.unity3d.com/2018.2/Documentation/ScriptReference/Vector4-ctor.html). Judging from the code this also seems to affect shader.GetVectorArray(), but I haven't tested it so I cannot confirm that

RhenaudTheLukark commented 3 years ago

I see, in that case I may change the docs to refect that behavior. It's not a bug per se, just a difference in how these arguments are truly handled in Unity. Thanks for the issue!

RhenaudTheLukark commented 3 years ago

I just checked and you're completely right...

It seems SetVector() uses (x, y, z, w) but GetVector() explicitly uses (w, x, y, z) as shown below.

2021-08-22_18-46-11

It's an issue I'll have to fix later, but I'm afraid it'd break a lot of shaders right now.

I plan to revamp CYF for 0.7, so I will most likely make everything use (x, y, z, w) in that version.

However, in 0.6.6 (the next version), I'll just document these functions aren't used the same. Is that good enough for now?

KeepDrive commented 3 years ago

Sounds good, thank you for addressing this, this caused quite a bit of confusion when I made my own shader, I'll just make a simple version check so it uses the correct order for versions 0.7 and after, good luck with the revamp!