Various brush entity improvements and fixes

[EIREXE]

This includes:

• Generate one mesh instance per entity, instead of one mesh instance per texture, this seems like a much more sensible way than using one meshinstance per texture
• Fix the rotation of brush entities, previously the transform was applied on top, however this is wrong as the brush entity's geometry is already in godot space (and brush entities have no rotation anyways).
• Always generate visuals for brush entities, this is because a brush entity doesn't have to make use of collision at all
• Generate brush entity collisions as siblings of the mesh instance if the brush entity is of a physics type, this is so the parent is the one that handles all the physics of the brush entity

[codecat]

Thanks for the PR! I'll take a closer look at the code hopefully tomorrow.

a brush entity doesn't have to make use of collision at all

Right, but the opposite is also correct; not all brushes need visual but do need collision, for example invisible walls. Would that still be possible with this change?

[EIREXE]

Thanks for the PR! I'll take a closer look at the code hopefully tomorrow.

a brush entity doesn't have to make use of collision at all

Right, but the opposite is also correct; not all brushes need visual but do need collision, for example invisible walls. Would that still be possible with this change?

Invisible walls IMO should be done with a special clip texture like quake/source does that is configured in the TBLoader node, I have such thing implemented already in my own branch but it's not in this PR, my plan was to add it together with nodraw textures for skipping faces as a separate PR.

[codecat]

Most of the code looks good! 🎉

One thing I don't understand though is why are you passing ColliderType::None to build_entity_mesh and then calling add_collider_from_mesh later? That should already happen correctly in build_entity_mesh, shouldn't it?

I also have some minor style nitpicks if you wouldn't mind changing them 😊

[codecat]

Invisible walls IMO should be done with a special clip texture like quake/source does that is configured in the TBLoader node

I agree, but I think it should work both ways too. Perhaps this is not a problem though, particularly because I also want to allow customizing whether visual/collision is enabled through entity properties. (But that's for a separate issue/commit)

[EIREXE]

One thing I don't understand though is why are you passing ColliderType::None to build_entity_mesh and then calling add_collider_from_mesh later? That should already happen correctly in build_entity_mesh, shouldn't it?

Because the idea is that in the case of brush entities collision shapes should always be children of the entity instead of children of the mesh as is the case in static mode.

I also have some minor style nitpicks if you wouldn't mind changing them blush

Should be OK now

[codecat]

Because the idea is that in the case of brush entities collision shapes should always be children of the entity instead of children of the mesh as is the case in static mode.

This is why, when passing ColliderType::Mesh to build_entity_mesh, it adds the collider using add_collider_from_mesh to the specified parent. I think it would do the same if you pass a mesh collider type using need_collider = ColliderType::Mesh;, similarly to how it is done for RigidDynamicBody3D.

[EIREXE]

Because the idea is that in the case of brush entities collision shapes should always be children of the entity instead of children of the mesh as is the case in static mode.

This is why, when passing ColliderType::Mesh to build_entity_mesh, it adds the collider using add_collider_from_mesh to the specified parent. I think it would do the same if you pass a mesh collider type using need_collider = ColliderType::Mesh;, similarly to how it is done for RigidDynamicBody3D.

Hmm yeah you are right, I've gone ahead and done that

[codecat]

Thank you! 🎉