Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multiple new Methods for Vec2 and impl VecXZ #4

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

theaddonn
Copy link

@theaddonn theaddonn commented Nov 2, 2024

This is a PR (currently still a draft) that implements multiple new methods for Vec2 such as rotate, and is aimed to implement a VecXZ type. All this includes conversions between Vec3 <--> Vec2 <--> VecXZ.

@stirante
Copy link
Member

stirante commented Nov 3, 2024

I think if you're aiming to do a separate VecXZ class, there should be some shared implementation. Maybe some private static class with methods like addImpl(x1, y1, x2, y2). Or maybe some kind of shared base class, that uses customizable getters and setters for each component. For example you would have getDimension(<0/1>) and setDimension(<0, 1>, value).

Overall idea is to not duplicate too much code (more that I already did unfortunately).

@theaddonn
Copy link
Author

Well, it is very important to recognize that abstracting it further might cause performance problems due to the level of indirection.
And in the end, I dont think that this duplication is really that problematic.

But I do wonder, you mentioned that you think that I am focusing on having 2 seperate classes for both Vec2 and VecXZ. This makes it sounds like you have been thinking about combining them into 2 class, may you expand on that?

Though maybe, there is a way to have this abstraction in Ts but not in Js.. Im here thinking about possibly using macros, or something similar, to some regard?

@stirante
Copy link
Member

stirante commented Nov 4, 2024

My implementation of Vec2 accepts both Vector2 and VectorXZ. After that it will perform all operations on X and Y components l. If you convert VectorXZ it will assign Z to Y. There probably should also be option to get VectorXZ back from Vec2 and then we have all functionalities in one class for both interfaces. That was my idea.

As for compilation step to use something like macros, maybe there's an option for inlining pure functions in the esbuild?

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.

2 participants