Spatial.look_at affects more than just rotation

[erodozer]

Operating system or device - Godot version: Windows 10 Godot 3.0 alpha-1

Issue description: look_at reverts the scale on the affected scale, expected behavior should only be changing the rotation of the model instead of the whole transformation.

Steps to reproduce: Create a spatial with a scale vector other than (1,1,1) and perform a look_at on the spatial towards any point.

[ghost]

Ah, OK. I think I know what you mean. Yes, look_at and related functions set the Basis from scratch based on the orientation provided, discarding the old Basis, which includes any scaling information you had earlier.

I'll submit a PR later today for it.

[ghost]

I'm not sure how we should do this actually. The problem is, we're currently storing a single matrix (Basis) which is the combination of scaling and rotation, and if you want to operate only on rotation or scaling, you'll need to decompose Basis into a rotation and a scaling matrix. This decomposition is called polar decomposition, and unfortunately, it's not unique in general.

You can see the comment here for further details. The correct solution to this problem is to store the rotation and scaling separately, and use (if, at all) a combined Basis matrix that cannot be directly changed by the user (user should only be able to change the rotation or scaling, separately, after which we can internally reconstruct the Basis).

I can't remember the exact issue titles, but this has already come up several times. Short answer to your question is, it's currently not possible to do this correctly in the engine that would work correctly all the time, and I'd prefer to leave look_at it as-is and document this behavior, rather than generalizing it with an implementation that doesn't work for all the inputs.

As a workaround, you can use an auxiliary parent/child node, such that one node handles rotation only, and the other one handles scaling only.

[erodozer]

I feel that storing rotation and scaling separately and combining might be an optimal solution for the future. I'm aware of the workaround, but it feels hacky at best having to implement two nodes for a single entity just because of this issue.

According to the comment, one of the issues is that the user can directly access points in the matrix. It might be necessary to turn those into getters/setters that are exposed as properties in GDScript to insure if the values are changed they can invoke the reconstruction of the Basis.

[ghost]

I'm not exactly sure but this may already be how Unity implements it. It doesn't look like they allow element-wise write access to the Basis matrix.

I wonder what @reduz thinks about this, actually. 3.0 hasn't been released yet, and I'd be willing to make changes overall in the engine.

[erodozer]

In my experience not even the rotation aspect of look_at is working properly. If I have an enemy trying to rotate to face my character, as I run behind him look at seems to fail, restricting rotation to 180 degrees of initial facing. It's pretty weird.

[ghost]

The look at code looks correct to me, I think it's a problem elsewhere (in your script?). Attach a minimal example project maybe?

[erodozer]

Not exactly a minimal project, but I was noticing the issue in this project of mine. I had to use the workaround in this implementation. The facing logic is in the enemy prefab, it might just be a programming error of mine but I wouldn't think so since it's pretty basic https://github.com/nhydock/get-in-the-robot

The character is only supposed to rotate on the y-axis.

var x = self.rotation.x
var z = self.rotation.z
self.look_at(player.get_global_transform().origin, Vector3(0, 1, 0))
self.rotation.x = x
self.rotation.z = z

Regardless, that issue I'd prefer discussing on discord or just somewhere else because it's not relevant to the initial topic. If it does turn out to be a valid problem, though, we might need to expand this issue to be a full evaluation of the matrix class.

[ghost]

I've gone through and changed/fixed the transform recently and I really don't think there're any such bugs there. Unless you can show me a complete snippet that shows a bug, I'd bet it's a mistake in your script.

[ghost]

And after a quick glance, the problem is indeed in your code. Rotations don't work like that. A rotation isn't a vector, they are linear operators which act on vectors. Rotations around different axes don't commute and they don't have X,Y,Z components that can be used like that.

Also you should be using SLERP to interpolate rotations.

There is unfortunately no documentation regarding rotations in 3D but there will be soon.

Anyway, I think Q&A is a better place to ask this question.

[shiMusa]

I just ran into the issue with the reset of the scaling. Any plans on fixing it? Or at least document it! It's really frustrating to spend an hour tracking down the bug just to find out that it's a build in one... :S Please, use separate matrices/vectors for scale, translation and rotation so that you can separate them. Thanks.

[stewlab]

I recently ran into this problem as well. It seems most people who modify Spatial scaling at run time would encounter this. Hopefully it will be resolved soon, as it looks to have been an issue for quite a while now. Thanks to those that are looking into it.

[rodolforg]

I just ran into this problem too. A parent node (Sun) has 3 nodes (Planet) in its orbit. The planets should always facing outward the sun.

In the test scene, an instance of Sun is placed with scale of 0.25 . At runtime, the planets are scaled up to 4 if they have a Spatial.look_at() function in its _physics_process()…

Will it be fixed?

My current workaround:

    var original_scale = planet.transform.basis.get_scale()
    planet.look_at(self.transform.origin, UP)
    var current_scale = planet.transform.basis.get_scale()
    var fix_scale = original_scale / current_scale
    planet.transform.basis = planet.transform.basis.scaled(fix_scale)