search

Zylann / godot_voxel

Voxel module for Godot Engine
MIT License
2.71k stars 251 forks source link

Voxel Module dosen`t use the same convention as GridMap for its orthogonal rotation. #694

Open Stefotono opened 2 months ago

Stefotono commented 2 months ago

image The dark blue row is block meshes rotated using the GridMap methods. The dark green row is voxels which within the library have their rotation set. They aren`t the same.

The final top row represents how the values need to be converted to be proper (Left digit is Voxel Module rotation index with the right being GridMap rotation index)

To Reproduce just get a rotation from GridMaps and apply to mesh and compare that to how the library would treat it. they wouldn`t match!

link to minimal project which contains bug! (https://drive.google.com/file/d/1EcjGb_e5ZtDVXVgJkYMyN-8LgaMDTgcx/view?usp=sharing)

I expected the values to be the same between the two systems since I was told so by the description of the variable.

AmyGilhespy commented 2 months ago

The values are the same from the Godot source code as far as I can tell. Something else is wrong here.

https://github.com/godotengine/godot/blob/5675c76461e197d3929a1142cfb84ab1a76ac9dd/modules/gridmap/grid_map.cpp#L434C1-L459C1

Zylann commented 2 months ago

See https://github.com/Zylann/godot_voxel/issues/699#issuecomment-2341630413

Although #699 is about VoxelBlockyModelCube, not VoxelBlockyModelMesh... As was linked, considering the code for VoxelBlockyModelMesh rotation is litterally a copy-paste of Godot's rotation table, I don't know what to do about this issue. They don't have to match GridMap (do you need them to?), I just copied them so that I would not have to come up with a correct table manually, and so I thought of documenting that. But the fact you found somehow that they don't match is annoying.

My suspicion is the fact that Godot's Basis is internally transposed, yet the constructor taking 9 floats does not encapsulate this implementation detail. Therefore, axes are also transposed when passed to those constructors, and then results differ when used. (it's very annoying to distinguish which way the values are going with that kind of API tbh, and parameter names don't help). In the module's code, OrthoBasis is not transposed and xyz members directly represent the axes the basis would have if drawn in world space.

To make things match, I would then have to transpose everything, which will break compatibility with existing projects that used ortho_rotation_index (if Godot had a concept of serialized versions, that would be workable, but it doesn't).

I tried to use your project but it is broken.

@export var conversion_table : Dictionary = {} :
    set(new):
        conversion_table = new
        update_debug_display_corrected()

This is going to be set by the scene loader before the node enters the scene tree, so get_children fails. Also get_voxel_index_from_name no longer exists in latest version.

After fixing that, and transposing OrthoBasis, I get this: image Is that the expected result?