Implicit conversion of Quat to Basis not raising error. Assignment works in debug, in release does nothing.

[MagellanicGames]

Godot version

3.4.2

System information

Kubuntu 18

Issue description

If you assign a Quat to Basis the editor will not flag this is an error and allow compilation. At runtime this will be implicitly converted from Quat to Basis, but it should require a cast.

Editor is fine with this:
global_transform.basis = current_rotation.slerp(m_target_rotation, m_weight)

It should raise an error unless it is stated as follows:
global_transform.basis = Basis(current_rotation.slerp(m_target_rotation, m_weight))

Run from the editor and debug builds there will be no issues. In release builds the rotation will simply not happen. No error or crash, just silent failure to do the slerp.

Steps to reproduce

The only script code is that stated below attached to a MeshInstance with a cube mesh, of course add a camera to the scene to see what the cube is doing.

If run as is in debug there will be no issue and the cube will rotate. If you export as release build and run, the cube will not rotate.

Then uncomment the commented line and comment out the last line and export (so now explicitly create the Basis from a Quat). The cube will now correctly rotate in a release build.

var m_target_rotation : Quat = Quat(Vector3(0.0,360.0,0.0))
var m_weight := 0.0
var m_speed := 0.05

func _process(delta):
	m_weight += delta * m_speed
	var current_rotation : Quat = global_transform.basis
	#global_transform.basis = Basis(current_rotation.slerp(m_target_rotation, m_weight))
	global_transform.basis = current_rotation.slerp(m_target_rotation, m_weight)

Minimal reproduction project

BasisQuatBug.zip

[akien-mga]

As pointed out on Twitter, this might be related to #48090 (and #48999).

[akien-mga]

As pointed out on Twitter, this might be related to #48090 (and #48999).

Tested with a target=release build of the 3.x branch with #57851 merged, this bug is still reproducible.

[vnen]

This one was quite tricky to understand what was happening. It boils down to some type information not available on release builds, which is used to detect if a conversion is needed.

GDScript rely on the information from the return type of the getter for the property (global_transform in this case), but this is only constructed in debug builds. The property itself can be checked instead of the getter, but that's unreliable in many cases, especially when the hint shows multiple valid classes, which is useful for the inspector (since it may allow multiple objects) and difficult to parse for GDScript (which relies on it being a specific type).

There are a few potential solutions for this.

1. Enable that type information on release builds. While it's likely more beneficial, I'm not sure how much it would impact on binary size.
2. Remove the discrepancy by not checking that in debug either. It still won't work, but at least you'll see it when running in the editor and will be able to solve it. Could potentially become some warning in the editor as well, though that requires some extra effort.
3. Use the property type on release as a fallback since the getter return type isn't available. Maybe only for built-in types in which the class name is not used. This keeps a difference still even though it solves this particular issue.
4. Using the property type on both release and debug is also an option.

[vnen]

1. Enable that type information on release builds. While it's likely more beneficial, I'm not sure how much it would impact on binary size.

I just noticed this was done in master with #53545. Maybe it should be cherry-picked for 3.x as well.

[Calinou]

I just noticed this was done in master with #53545. Maybe it should be cherry-picked for 3.x as well.

The commit has a lot of conflicts when cherry-picked to 3.x, so it needs to be redone from scratch.

[vnen]

This was solved by #59793.