Add function to transform position

[vessd]

Description

Function to transform position from screen space into world space

Additions

• fn screen_to_world

[AnneKitsune]

Looks pretty good! I just have a minor comment.

Also, can you add an entry in docs/CHANGELOG.md? There are two lines to add, the description and the link to this pull request. See how the other entries are made. Thanks!

[Moxinilian]

The tests unexpectedly failed, but it probably is a glitch. This will not prevent merging once 24 hours will have passed.

[Moxinilian]

Actually, come to thing of it, why don't you make this a function on the Camera component?
So we could do something like camera.coordinates_from_screen(screen_coords, screen_dim).
Maybe that would be more convenient.

[AnneKitsune]

Making it a function of the camera makes sense.

The camera might be a child of another entity, so it would also make sense to use the global transform instead of the translation. However, this will make the math harder, so I don't know if we want that in this PR or not. We don't want to make you work too hard :D

[vessd]

Should I remove the translation argument then?

[Moxinilian]

Oh right, the translation. Hmmm. Maybe the function could take its own transform or something.
Ah that's a tricky one actually. The reason I would like it to be on the Camera component is because it is an obvious place to look for people coming from non-ECS places. We should look at how Unity is doing it with their ECS.

[Moxinilian]

Okay so apparently they're using globals, which is not very good. We should not do that.
I guess we should have it be a method on the Camera, to which we pass the camera's transform, the screen position and the ScreenDimension. What do you think?

[Moxinilian]

Excellent work, thank you very much!
Please consider my suggested nitpick, then squash the commits so we can merge it.

[Moxinilian]

Actually, while I'm at it, why not return a Vector3 instead of just X and Y? It can be useful when your camera is not on the Z = 0 plane. Also, why are the input coordinates f64? It could be a Vector2, even.

[vessd]

Also, why are the input coordinates f64? It could be a Vector2, even.

Because the InputHandler::mouse_position() returns the Option<(f64, f64)>.

Also, why are the input coordinates f64? It could be a Vector2, even.

Hmm, I don’t understand how it works clearly, do I need to take Vec3 for this?
I mean, is it ok that screen_point is always equal to 0.0 for the z plane for this? Or do i need to accept it as a parameter?
I didn't test the z coordinate, since I have no idea how to interpret it in my 2D game

[Moxinilian]

I think introducing a Z coordinate here as input would just shift how "deep" the click is made. It should not really matter here, we only are converting click position, not raycasting.

However, the Z output can be useful if your camera is in 3D space. As we are doing an inverted camera projection, we are effectively taking the screen plane at Z=0 and applying it to the 3D plane orthogonal to the camera's direction. That plane can be a Z=0 plane, but not necessarily, if the camera is in a weird position. Therefore the output Z coordinate might be of use. Try it in a simple 3D world using one of our 3D examples (fly camera would be appropriate I think).

[vessd]

@Moxinilian ok, should I leave a f64 as an input argument?

[Moxinilian]

I think you should do as little casting as possible in the body of the function so the user does not have to do useless casting if they already work with f32. Set it to f32 in the parameter, and let the user do the casting.

[vessd]

I think it is ready to merge.
r? @Moxinilian @Jojolepro @azriel91

[azriel91]

woosh 💯!

reminder for future:

• https://stackoverflow.com/a/7702895/1576773
• https://learnopengl.com/Getting-started/Coordinate-Systems

[azriel91]

bors r=jojolepro,Moxinilian,azriel91

[bors]

Build succeeded

• continuous-integration/travis-ci/push