feat: make Vec2 able to do math on noninstances#968
Conversation
commit: |
mflerackers
left a comment
There was a problem hiding this comment.
- Is there a reason to call it SerializedVec2, and not Vec2Like or something similar?
- Why is sometimes T used and sometimes SerializedVec2?
- How did chrome react? Don't these methods suddenly become de-optimized, as the argument types are no longer fixed?
No. I kind of wanted to change it to Vec2Like but didn't want to change everything since X.serialize() typically returns SerializedX and Vec2 does this too.
The type parameter is for the inout mutating methods, that mutate the object and then return the same one you passed in. That way, if you use the return value it will be the same type as the one you passed in (if it's a Vec2 instance the return value will be that type too, if it is just a Vec2Like then so will the return value).
I didn't notice any performance penalty, and it really shouldn't anyway, since all I changed were the types for the most part and by the time chrome sees the code the types are long gone. |
|
What I mean is, someone doing Vec2.add(v, w, v) with w a vertex { x, y, u, v, r, g, b, a } will cause the Vec2.add be deoptimized. |
Well, I mean yeah that would, but I doubt that will ever happen considering that that flattened format isn't even how kaplay stores verticies: /**
* @group Rendering
* @subgroup Shaders
*/
export interface Vertex {
pos: Vec2;
uv: Vec2;
color: Color;
opacity: number;
} |
|
Yes, It was just an example. I'm going to change that vertex format btw, once I rewrite the renderer. |
|
Typescript types will always let you do a lot of silly things that you really shouldn't. This was just to let you do some things that you ideally should be able to do (in my opinion at least...) but would get typescript yelling at you before. |
|
I don't have anything against the feature itself, I just want to know whether there would be any unexpected issues. |
couldn't figure out how to force it to use x and y and nothing else
now all Vec2 methods can be given a SerializedVec2 as arguments, so you don't have to deserialize everything
(for example:
JSON.parse(JSON.stringify(vec2(...).serialize())) instanceof Vec2 === false)Summary