Skip to content

Conversation

@dragoncoder047
Copy link
Contributor

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

  • Changeloged

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 2, 2025

Open in StackBlitz

npm i https://pkg.pr.new/kaplayjs/kaplay@968

commit: a64aed2

Copy link
Member

@lajbel lajbel left a comment

Choose a reason for hiding this comment

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

LGTM

@lajbel lajbel requested a review from mflerackers December 3, 2025 17:11
Copy link
Member

@mflerackers mflerackers left a comment

Choose a reason for hiding this comment

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

  • 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?

@dragoncoder047
Copy link
Contributor Author

  • Is there a reason to call it SerializedVec2, and not Vec2Like or something similar?

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.

  • Why is sometimes T used and sometimes SerializedVec2?

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).

  • How did chrome react? Don't these methods suddenly become de-optimized, as the argument types are no longer fixed?

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.

@mflerackers
Copy link
Member

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.

@dragoncoder047
Copy link
Contributor Author

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;
}

@mflerackers
Copy link
Member

Yes, It was just an example. I'm going to change that vertex format btw, once I rewrite the renderer.

@dragoncoder047
Copy link
Contributor Author

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.

@mflerackers
Copy link
Member

I don't have anything against the feature itself, I just want to know whether there would be any unexpected issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants