godot-cpp icon indicating copy to clipboard operation
godot-cpp copied to clipboard

Basis inconsistencies between GDScript and C++

Open Jummit opened this issue 5 years ago • 4 comments

godot::Godot::print(Basis(Vector3(1, 2, 3), Vector3(4, 5, 6), Vector3(7, 8, 9)))
// prints 1, 2, 3, 4, 5, 6, 7, 8, 9
godot::Godot::print(Basis(Vector3(1, 2, 3), Vector3(4, 5, 6), Vector3(7, 8, 9)).y);
// prints 2, 5, 8
print(Basis(Vector3(1, 2, 3), Vector3(4, 5, 6), Vector3(7, 8, 9)))
# prints ((1, 4, 7), (2, 5, 8), (3, 6, 9))
print(Basis(Vector3(1, 2, 3), Vector3(4, 5, 6), Vector3(7, 8, 9)).y)
# prints (4, 5, 6)

This could be related to how basis printing works, or with how they are stored, but it affects calculations (GDScript code ported directly to C++ behaves differently, so it should definitely made more consistent.

If I'm missing something here, I'd be happy if someone could clear it up.

Related: godotengine/godot#14553

Jummit avatar Jul 13 '20 21:07 Jummit

Printing might be different, but behaviour should at least be the same. You could check that with some test cases to see if the functionality works. If it does, then we can fix the printing to be transposed. If it doesn't work, in doubt, we could re-port the whole class by copying Godot's over to make it up to date. Although, it's also possible that GDScript exposes Godot's Basis differently than the way it's implemented internally, which could explain the difference.

Zylann avatar Jul 22 '20 01:07 Zylann

I think printing uses the same convention as GDScript already, even though operator String in Basis formats it without parentheses. The numbers are the same if you let Godot format it:

godot::Godot::print("{0}", Basis(Vector3(1, 2, 3), Vector3(4, 5, 6), Vector3(7, 8, 9)));

The linked issue and PR should have fixed x/y/z already too.

I assume the Basis(Vector3, Vector3, Vector3) constructor is what's causing the incorrect values in both C++ lines. The Basis is just being constructed with the wrong row/column convention, the one used in the core engine.

sheepandshepherd avatar Jul 22 '20 20:07 sheepandshepherd

I assume the Basis(Vector3, Vector3, Vector3) constructor is what's causing the incorrect values in both C++ lines. The Basis is just being constructed with the wrong row/column convention, the one used in the core engine.

If that is true (and from the above print statements I would conclude the same), that is a pretty serious bug

BastiaanOlij avatar Sep 08 '20 00:09 BastiaanOlij

Basis(row0, row1, row2) is still broken. #533 seems to be working correctly.

DmitriySalnikov avatar Aug 07 '22 19:08 DmitriySalnikov

PR #859 updated Basis to match the engine core, so if there is an inconsistency then it also exists in the engine.

aaronfranke avatar Jul 08 '23 17:07 aaronfranke

This issue is pretty old by now and things have changed in 4.0+, so closing.

akien-mga avatar Jul 08 '23 19:07 akien-mga