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

Consider impl Into<PointX<_>> in kiss3d API #237

Open
jerry73204 opened this issue Jul 19, 2020 · 3 comments
Open

Consider impl Into<PointX<_>> in kiss3d API #237

jerry73204 opened this issue Jul 19, 2020 · 3 comments

Comments

@jerry73204
Copy link
Contributor

A series of From traits are defined on Point type (doc). We could combine it with impl Into<PointX<_>> to help user to write more compact syntax.

For example, the Window::draw_line receives a pair of point and color. We can turn it to the following.

pub fn draw_line(
    &mut self,
    a: impl Into<Point3<f32>>,
    b: impl Into<Point3<f32>>,
    color: impl Into<Point3<f32>>
)

In this way, we can call by window.draw_line([0.0, 0.0, 0.0], [0.8, 0.8, 0.8], [1.0, 1.0, 1.0]) instead of Point3::new() everywhere.

@jerry73204
Copy link
Contributor Author

I tried once the run the tests. It does not work with borrowed types draw_line(&Point3::new(1., 2., 3.)), so imposing this feature will lead to breaking change.

Though using more complex trait bounds can solve this issue, we could add impl From<&Point<...>> for Point<...> to nalgebra as a simple solution.

@Nevsden
Copy link

Nevsden commented Jul 24, 2020

Would it suffice, if you'd write a wrapper function for that? I wrote a wrapper around the kiss3d framework to adapt the window add_... functions to handle my own structs.

Would look something like this:

fn Wrapper::add_jerrys_line(input: &JerryInput, window: &mut Window) {
// first convert JerryInput into something-nalgebra like
// call Window::add_line_...
}

@jerry73204
Copy link
Contributor Author

It's definitely a good idea. Writing a wrapper with Deref to the Window would be enough.

I opened this issue for a mild suggestion to make the API more handy. I dived into the impl and realized that we need both &Point -> Cow<Point> and Point -> Cow<Point> conversions to make the idea come true. It's not yet done on nalgebra.

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

No branches or pull requests

2 participants