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

Add TorusMesh #60843

Merged
merged 1 commit into from
Aug 6, 2022
Merged

Add TorusMesh #60843

merged 1 commit into from
Aug 6, 2022

Conversation

hoontee
Copy link
Contributor

@hoontee hoontee commented May 7, 2022

PrimitiveMesh counterpart to CSGTorus3D. Required for #59077.

image

@hoontee
Copy link
Contributor Author

hoontee commented May 8, 2022

Added tangents. All it's missing now is the resource icon.

@akien-mga
Copy link
Member

What's the rationale for adding this as a primitive mesh? This is the kind of change that requires a proposal to discuss the use case and whether it makes sense to add things to the API.

@hoontee
Copy link
Contributor Author

hoontee commented May 10, 2022

CSGTorus3D exists and this was intended to replace it internally when removing redundant CSG primitive generation code.

@hoontee
Copy link
Contributor Author

hoontee commented Aug 3, 2022

Now that #59077 is closed this is solely to have a torus primitive mesh. CSG has one already.

@Calinou
Copy link
Member

Calinou commented Aug 4, 2022

I tested this PR locally, it works as expected.

However, if inner_radius and outer_radius are set to the same value, mesh generation fails with the message:

ERROR: _create_mesh_array must return at least a vertex array.
   at: _update (scene/resources/primitive_meshes.cpp:53)

I suggest adding an human-readable error message by checking for the inner and outer radius' values with ERR_FAIL_COND_MSG().

The resource icon is missing because I don't know how to make one.

I made an icon for TorusMesh based on the CSGTorus3D icon 🙂

Save this file as editor/icons/TorusMesh.svg then recompile the editor to see changes: TorusMesh.svg.zip

@hoontee
Copy link
Contributor Author

hoontee commented Aug 4, 2022

@Calinou Added your suggestions!

@akien-mga akien-mga merged commit 21f6916 into godotengine:master Aug 6, 2022
@akien-mga
Copy link
Member

Thanks!

@hoontee hoontee deleted the CSG3 branch August 6, 2022 19:11
@Calinou Calinou mentioned this pull request Aug 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants