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

Vector2.reflect() result is unexpected #11678

Closed
cbscribe opened this issue Sep 29, 2017 · 26 comments · Fixed by #89404
Closed

Vector2.reflect() result is unexpected #11678

cbscribe opened this issue Sep 29, 2017 · 26 comments · Fixed by #89404

Comments

@cbscribe
Copy link
Contributor

cbscribe commented Sep 29, 2017

Operating system or device, Godot version, GPU Model and driver (if graphics related):
3.0master build 0dbec3a

When reflecting a vector by a normal, for example to bounce off a surface, the resulting vector isn't reflected as expected (it's reversed):

var v1 = Vector2(100, 100)
var n = Vector2(0, -1)
var v2 = v1.reflect(n)

Expected result: (100, -100)
Actual result: (-100, 100)

As a workaround, I've been using -v1.reflect(n)

@cbscribe cbscribe changed the title Vector2.reflect() Vector2.reflect() result is unexpected Sep 29, 2017
@cbscribe
Copy link
Contributor Author

Actually, now that I've looked at the code, 3.0 has a Vector2.bounce() method that does produce the expected result. Is that intended to replace reflect()?

@ghost
Copy link

ghost commented Sep 29, 2017

Reflecting along y axis will flip the sign of x coordinate.

Reflect is just reflection in mathematical sense. Bounce is what you'd get for example if you had a ray that bounces off from a surface

@Boothand
Copy link

That looks unexpected to me too.
In this image: https://docs.unity3d.com/StaticFiles/ScriptRefImages/Vec3ReflectDiagram.png
The normal vector in Godot pixel units is Vector2(0, -1).
In-direction is equivalent to Vector2(1, 1), out-direction is Vector2(1, -1).

@ghost
Copy link

ghost commented Sep 29, 2017

That image describes a bounce, as I tried to explain above

@ghost
Copy link

ghost commented Sep 29, 2017

This is what reflection means

@Boothand
Copy link

Boothand commented Sep 29, 2017

Then it must be different from other Vector classes I've come across.

In Unity:
`Vector2 v1 = new Vector2(1, 1);

Vector2 normal = new Vector2(0, -1);

Vector2 reflectedVec = Vector2.Reflect(v1, normal);

print(reflectedVec);
`

Output: (1, -1)

@ghost
Copy link

ghost commented Sep 29, 2017

In Godot, reflect means mathematical reflection and bounce is physical reflection. Documentation describes this behavior.

Reflect typically refers to mathematical reflection, just like glsl reflect function, because that's what reflecting a vector, an abstract mathematical entity, means. A mathematical vector doesn't move. Reflection of a light from a surface, both represented by time-dependent vectors, ray is different. Unlike light, a pure mathematical vector doesn't move in time.

We don't need to do whatever Unity does.

@groud
Copy link
Member

groud commented Sep 29, 2017

This probably have to be better documented then. The lastest documentation says:

Vector2 reflect ( Vector2 n )
Reflects the vector along the given plane, specified by its normal vector.

I think in fact it use the direction vector instead of the normal.

@ghost
Copy link

ghost commented Sep 29, 2017

I think that's an accurate description of vector reflection although could be made more clear

@groud
Copy link
Member

groud commented Sep 29, 2017

Yup, but in OP's case: if reflect uses the normal of the "plane" (in fact a straight line here), with Vector2(0,-1), the plane is the X axis. So the reflection should change the Y coordinate instead of the X one.

@ghost
Copy link

ghost commented Sep 29, 2017

Ah, I missed that part, yse I agree, that parameter is the normalized reflection line

@ghost
Copy link

ghost commented Sep 29, 2017

You do have a plane for bouncing though,cant checwhat the docs say for that now

@groud
Copy link
Member

groud commented Sep 29, 2017

For the bounce doc:

Vector2 bounce ( Vector2 n )
Bounce returns the vector “bounced off” from the given plane, specified by its normal vector.

Actually I think bounce and reflect are doing more or less the same thing in fact. A "bounced" vector is reflected relatively to the plan it's bouncing on I'd say. Like illustrated here:
http://ogldev.atspace.co.uk/www/tutorial19/reflected_light.png

@ghost
Copy link

ghost commented Sep 29, 2017

Great we can them simply say a normal vector along reflection line direction doesn't matter it's same either way

@cbscribe
Copy link
Contributor Author

I'm still not sure I see a use for the reflect method as written.

If this is the intended behavior then we definitely need to clarify the documentation. Especially since in 2.1 reflect did what bounce now does.

@ghost
Copy link

ghost commented Sep 29, 2017

I'm still not sure I see a use for the reflect method as written.

Reflection is a basic affine transformation just like rotation. It allows you to do reflection without constructing a reflection matrix, similar to what Vector2.rotate does. Together they give you the functionality of rotation-reflection matrices (that is, 2D orthogonal matrices, O(2)).

You can read more about it here

@jack-vo
Copy link

jack-vo commented Feb 11, 2018

In the stable release of Godot3, reflect and bounce always return wrong value

var test = Vector2(100, 100)
var reflection = Vector2(0, -1).reflect(test) # returns (0, 0)
var bouncing = Vector2(0, 1).bounce(test); # return (-0, -0)

@akien-mga
Copy link
Member

@jack-vo No, it's just that the syntax changed: http://docs.godotengine.org/en/stable/classes/class_vector2.html#class-vector2-reflect

It's no longer normal.reflect(vector) but vector.reflect(normal), so you should do test.reflect(Vector2(0, -1)).

@aaronfranke
Copy link
Member

Yup, but in OP's case: if reflect uses the normal of the "plane" (in fact a straight line here), with Vector2(0,-1), the plane is the X axis. So the reflection should change the Y coordinate instead of the X one.

So then the problem seems to be that we're defining one dimensional lines with normal vectors instead of a vector in the actual direction of the line? Normal vectors for planes makes sense for 3D but for one dimensional lines in 2D it makes more sense to just define the line.

So, are we aiming for simpler 2D math, or consistency between 2D and 3D math?

And obviously this would be a breaking change so it'd be best to wait until 4.0 to make changes (though we can still discuss them now)

@vnen
Copy link
Member

vnen commented Jun 1, 2018

So then the problem seems to be that we're defining one dimensional lines with normal vectors instead of a vector in the actual direction of the line? Normal vectors for planes makes sense for 3D but for one dimensional lines in 2D it makes more sense to just define the line.

Not really, the problem (which is not really a problem but a more of a misunderstanding) is that reflect() does a mathematical reflection (which is more like a "flip") and not a physical reflection, like a ray reflecting of a mirror (which can be accomplished by the bounce() function).

The referred comment is more about how it's written in the docs. Using a normal vector in this case is perfectly fine, even in 2D.

@Chaosus
Copy link
Member

Chaosus commented Sep 14, 2018

cc @mhilbrunner Could you fix a documentation of reflect/bounce ?

@romlok
Copy link
Contributor

romlok commented Oct 13, 2018

Can I request that any documentation clarification come with at least one diagram which visualises the difference between bounce() and reflect(), and/or some practical examples of what each is useful for? I still don't understand how they would differ when operating on a vector.

It probably doesn't help that existing documentation seems to use the terms "reflect" and "bounce" interchangeably. Eg. Using KinematicBody2D or Vector Math.

@DleanJeans
Copy link

Just ran into this problem. I suggest renaming reflect() to mirror().

@vnen
Copy link
Member

vnen commented Dec 19, 2018

I don't think mirror() will clarify. I would call it flip().

@Calinou
Copy link
Member

Calinou commented Dec 24, 2020

So, what should we do in the end:

  • Rename reflect() to flip()?
  • Invert the value returned by reflect()?
  • Both?

@Exerionius
Copy link

It's been two years, and I've discovered this problem on my own practice. If not for googling this thread I would never know that reflect in Godot is actually called bounce, while whatever is called reflect does something completely different.

This is very counter-intuitive and goes against practices of other engines and math libraries, including Unity, Unreal, Monogame, XNA, Pygame and who knows what else.

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