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

global_scale property wrong in Node2D when scale is negative in x #17405

Closed
Piet-G opened this issue Mar 10, 2018 · 16 comments
Closed

global_scale property wrong in Node2D when scale is negative in x #17405

Piet-G opened this issue Mar 10, 2018 · 16 comments
Milestone

Comments

@Piet-G
Copy link
Contributor

Piet-G commented Mar 10, 2018

Version

3.0

Issue description:

When a Node2D has a negative scale in x, global scale will swap the - around to the .y part.

e.g: .scale = Vector2(-2,1) then .global_scale = Vector2(2,-1)

The inverse doesn't happen when y is negative

Steps to reproduce:

Make a scene with a Node2D, give it a negative scale in .x and check the .global_scale.

@robfram
Copy link
Contributor

robfram commented Mar 14, 2018

I will add some information to this. If you create a scene with Node > Node2D > Node2D2, and you set Node2D.scale to (-1.5, 1.25) and Node2D2.scale to (3, -5), and you script in Node this:

func _ready():
	# Called every time the node is added to the scene.
	# Initialization here
	prints("Node2D:", "LScale", $Node2D.scale,
			"LRotate", $Node2D.rotation_degrees,
			"GScale", $Node2D.global_scale,
			"GRotate", $Node2D.global_rotation_degrees)

	prints("Node2D2:", "LScale", $Node2D/Node2D2.scale,
			"LRotate", $Node2D/Node2D2.rotation_degrees,
			"GScale", $Node2D/Node2D2.global_scale,
			"GRotate", $Node2D/Node2D2.global_rotation_degrees)

The output is:

Node2D: LScale (-1.5, 1.25) LRotate 0 GScale (1.5, -1.25) GRotate 180
Node2D2: LScale (3, -5) LRotate 0 GScale (4.5, 6.25) GRotate 180

Why the 180º rotation? It maybe the cause of the x->y sign swap.

@tagcup
Copy link
Contributor

tagcup commented Mar 14, 2018

It's not a bug per se, there's just no way to uniquely decompose a basis matrix into rotation and scaling when reflection or negative scaling is involved.

You can read more about it here for 3D.

All the current API guarantees is that the when you query the scale and the rotation, they combine to give you the original basis matrix.

I proposed storing rotation and scaling separately which would solve this problem (as opposed to only using a monolithic basis matrix), to solve this problem but it was deemed to be a no-go.

One reason in favor of that argument is the current physics engine (bullet, or the old bullet fork labelled "godot") simply doesn't work correctly for bodies with negative scales or reflections anyway, and current bullet wrapper includes a bunch of hacks that makes this assumption.

@robfram
Copy link
Contributor

robfram commented Mar 14, 2018

I assumed something like this was the reason of this behaviour. That's why I printed rotation too, because I suspected the scale was being transfered to the rotation somehow.

I understand why separating both is a no-go, and I think in a similar way. Perhaps the only bug here maybe a lack of documentation, as the user will never be warned about scaling afecting rotating and vice versa (as numerical values shown, not as transformations applied to the object).

Thanks for the info!

@Piet-G
Copy link
Contributor Author

Piet-G commented Mar 14, 2018

After some research myself I understood that indeed is impossible to retrieve the correct scale from the basis matrix when it is negative.

Maybe it would just be better to have .global_scale and .global_rotation calculated directly from the stored values instead of using the transform2D?

Or maybe we could add separate functions to retrieve the true scale and rotation.

Also the documentation should surely be updated for those functions

@ghost
Copy link

ghost commented Aug 14, 2018

I would second DualMatrix's thoughts. It's going to definitely throw a really serious curve ball at any new comers who run into it.

I lost much time to my first encounter with it, because I thought the issue was surely in my code.

@MightyPrinny
Copy link
Contributor

MightyPrinny commented Mar 17, 2020

I think it would be better if transforms stored scale,rotation and position separately and kept the transform matrix cached and update it when needed.

@akien-mga
Copy link
Member

@aaronfranke
Copy link
Member

aaronfranke commented Jul 2, 2020

#21020 describes another symptom of this problem, and has another reproduction project that can be used for testing.

@MightyPrinny I think it would be better if transforms stored scale,rotation and position separately and kept the transform matrix cached and update it when needed.

This is not going to happen for many reasons, and I don't think it would actually be an improvement that solves problems. Negative X scales should not be used because they can't be decomposed from the transformation matrix.

@aaronfranke
Copy link
Member

#21849 describes another symptom of this problem, and has another reproduction project that can be used for testing.

@chucklepie
Copy link

chucklepie commented Sep 21, 2020

Hello, the following works equivalent to scale.x or scale.y for kinematic bodies with no difference as far as I can tell. So why not add a 'physics flip' property to physics bodies that does this and optionally disable scale, then lots of people won't get confused as much by setting scale and finding it goes all weird and freaky on them?

set_transform(Transform2D(Vector2(xscale,0), Vector2(0,yscale), Vector2(position.x, position.y))

@VladoCC
Copy link

VladoCC commented Oct 20, 2020

@aaronfranke

Negative X scales should not be used because they can't be decomposed from the transformation matrix.

How to rotate characters then? I mean, not just rotating sprite, but raycasts and other nodes too? Reposition one by one& Is there any good solution that I'm missing?

@knightofiam
Copy link

knightofiam commented Sep 24, 2021

@chucklepie Your code didn't work for me; I had to modify it to:

set_transform (Transform2D (Vector2 (xscale * (xflip ? -1 : 1), 0), Vector2 (0, yscale * (xflip ? -1 : 1)), Vector2 (position.x, position.y)))

where the boolean xflip decides on original or mirrored (horizontal only). If you only flip the x component, when you try to flip it back from mirrored to original, it goes upside down.

All of this indirectly sets scale.y to -1 anyway, so it's functionally equivalent. This not a workaround, just another way to reproduce the same bug.

(And no, that's not a typo. When flipped horizontally, scale.y is -1. When flipped vertically, scale.x is -1.)

@zzLinus
Copy link

zzLinus commented Jan 26, 2022

var transformer = Transform2D()
if flip to left:
    transformer.x.x = -1
    transformer.x.y = 0
if flip to right:
    transformer.x.x = 1
    transformer.x.y = 0

this block of code work for me

@aaronfranke
Copy link
Member

aaronfranke commented Jul 28, 2022

Closing this issue as resolved due to documentation improvements. The limitation of negative X scales not working is expected and will not change. The documentation now states the limitations of negative scale #61877.

I also proposed adding a warning to let users know about this, but this was decided against: #37376 (comment). When duplicates show up, we should just close them too. Like #21020, #21849, #31980, #42219, #62544, and #62663.

@hsandt
Copy link
Contributor

hsandt commented Feb 8, 2023

Thanks, the new note in the documentation was really helpful to understand this. To complete this closure rampage, I also found all the old questions scattered on the Q&A hub related to this and pointed them to this ticket, via one of my answers.

I'll also give the link to the most complete answer I gave: https://godotengine.org/qa/92282/why-my-character-scale-keep-changing?show=146969#a146969

because it's filled with alternative ways to flip on X, so useful for people who landed on this ticket in search of a workaround.

@aaronfranke aaronfranke closed this as not planned Won't fix, can't repro, duplicate, stale Feb 24, 2023
@akien-mga akien-mga added this to the 4.0 milestone Feb 24, 2023
@chunribu
Copy link

var transformer = Transform2D()
if flip to left:
    transformer.x.x = -1
    transformer.x.y = 0
if flip to right:
    transformer.x.x = 1
    transformer.x.y = 0

this block of code work for me

thx! it works for me. future guys, replace scale.x = -1 with transform.x.x = -1

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

Successfully merging a pull request may close this issue.