Add clip and skip (nodraw) texture support#29
Conversation
codecat
left a comment
There was a problem hiding this comment.
Thanks for the PR! I have some comments.
| // Create material | ||
| Ref<Material> material; | ||
|
|
||
| if (String(tex.name) == m_loader->get_skip_texture_name()) { |
There was a problem hiding this comment.
It would probably be good to have a comment here explaining what this is doing (even if it's clear by looking at the code)
There was a problem hiding this comment.
Additionally, casting tex.name to a String might invoke a memory allocation & string copy for each texture, we can probably avoid that? (Doesn't String have a compare method against const char*?)
| add_surface_to_mesh(mesh, surf); | ||
| add_surface_to_mesh(collision_mesh, surf); | ||
|
|
||
| // Give mesh material | ||
| if (material != nullptr) { | ||
| mesh->surface_set_material(mesh->get_surface_count() - 1, material); | ||
| // Add surface to mesh |
There was a problem hiding this comment.
Misplaced comment? These should ultimately have 2 different comments, one for adding the surface to the visible mesh, and one for adding the surface to the collision mesh.
| if (material != nullptr) { | ||
| mesh->surface_set_material(mesh->get_surface_count() - 1, material); | ||
| // Add surface to mesh | ||
| if (String(tex.name) != m_loader->get_clip_texture_name()) { |
There was a problem hiding this comment.
It would be cleaner to do an if / continue here I think
| StaticBody3D* static_body = memnew(StaticBody3D()); | ||
| static_body->set_name(String(mesh_instance->get_name()) + "_col"); | ||
| parent->add_child(static_body, true); | ||
| static_body->set_owner(m_loader->get_owner()); | ||
| add_collider_from_mesh(static_body, collision_mesh, colshape); |
There was a problem hiding this comment.
Does this change the behavior from mesh_instance->create_multiple_convex_collisions() or mesh_instance->create_trimesh_collision()?
| void TBLoader::set_clip_texture_name(String clip_texture_name) { | ||
| m_clip_texture_name = clip_texture_name; | ||
| } | ||
|
|
||
| String TBLoader::get_clip_texture_name() { | ||
| return m_clip_texture_name; | ||
| } | ||
|
|
||
| void TBLoader::set_skip_texture_name(String skip_texture_name) { | ||
| m_skip_texture_name = skip_texture_name; | ||
| } | ||
|
|
||
| String TBLoader::get_skip_texture_name() { | ||
| return m_skip_texture_name; | ||
| } | ||
|
|
There was a problem hiding this comment.
{ should be on a new line here. Also I think we can pass the strings as const String&, no?
5c8905f to
c0621c0
Compare
|
all fixed |
|
Thank you! 🎉 |
Adds clip and nodraw texture support, as a property to TBLoader