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

Use impl Into<T> #71

Closed
suprohub opened this issue Aug 12, 2024 · 4 comments · Fixed by #88
Closed

Use impl Into<T> #71

suprohub opened this issue Aug 12, 2024 · 4 comments · Fixed by #88
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Milestone

Comments

@suprohub
Copy link

some functions like set_color or set position use multiple arguments, it bit discomfort if you want set position of tuple or Array4.
Solution: position: impl Into<Array4>

@suprohub
Copy link
Author

or use from

@ElhamAryanpur
Copy link
Collaborator

The issue with that is that the function does more than setting just values, it also updates the uniform buffer and transform matrices.

A solution can be to add a whole new copy of the functions that accepts from array/list and internally maps them. But that will add extra confusion and cluttering.

e.g.

// the main
fn set_position(&mut self, x: f32, y: f32, z: f32);

// for array, an extra function can be added
fn set_position_with_array(&mut self, value: impl Into<[f32; 3]>) {
    self.set_position(value[0], value[1], value[2]);
}

@ElhamAryanpur ElhamAryanpur self-assigned this Aug 12, 2024
@ElhamAryanpur ElhamAryanpur added the enhancement New feature or request label Aug 12, 2024
@ElhamAryanpur ElhamAryanpur added this to the 0.6.0 milestone Aug 12, 2024
@ElhamAryanpur ElhamAryanpur added help wanted Extra attention is needed question Further information is requested labels Aug 12, 2024
@ElhamAryanpur
Copy link
Collaborator

@Gipson62 @NickJasonHagen What do you guys think?

@Gipson62
Copy link
Contributor

Honestly, having more functions like this would help, it'll help as much as functions overloading is useful in OOP languages and don't most people use the set_position() like this already?

let pos: Position3D = ...;
set_position(pos.x, pos.y, pos.z);

So making it takes an impl Into<[f32; 3]> instead of 3 f32 wouldn't be stupid imho

Gipson62 added a commit to Gipson62/BlueEngine that referenced this issue Jan 11, 2025
This should resolve AryanpurTech#71 as all of the function that set a position taks `impl Into<Position3D>` as input and you can trivially cast a Position3D to a nalgebra_glm::Vec3 with `.into()` when needed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants