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

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

Open
Stefotono opened this issue Sep 2, 2024 · 2 comments

Comments

@Stefotono
Copy link

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.

  • OS: Windows
  • Godot version 4.2.2.stable.mono
  • Module version 1.2.0
  • Forward+
@AmyGilhespy
Copy link

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
Copy link
Owner

Zylann commented Sep 10, 2024

See #699 (comment)

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?

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

No branches or pull requests

3 participants