godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.58k stars 21.1k forks source link

Importing .glb with parent as RigidBody, cannot import a child CollisionShape3D #79086

Open wilfredjonathanjames opened 1 year ago

wilfredjonathanjames commented 1 year ago

Godot version

4.0.3

System information

MacOS 12.3

Issue description

The Godot 3D import features are impressive and easy to use, but I think I've found an edgecase.

When a user wants to import a 3D model and have the resulting node be a Body3D, they can change this in the import settings as Root Type. However Body3Ds expect collider shapes to be direct children.

The only way I can see to create a collider shape on import is to use the -col, -convcol, colonly and -convcolonly object prefixes defined here.

The problem is that these all create a Body3D parent node with the collision shape as a child. This is the correct behaviour in any case other than the intended Body3D being the object root node.

E.g.: In blender:

image

In import settings:

image

Import preview tree:

image

The ChocolateBarCollider collision shape is put inside a StaticBody3D, when I need it to be placed directly under the parent node.

My work around has been to use the New Inherited Scene option and add a BoxCollider3D manually, which works fine in this instance but may not be so straightforward with more complex meshes that are updated frequently.

image

It would be great to know if I've just missed something

Steps to reproduce

  1. Make a simple object in blender and an overlapping shape to be the collider. Give the collider the -convcolonly prefix
  2. Export as .glb
  3. Import into Godot after changing the Root Type import setting to a RigidBody3D
  4. In a separate scene, place your object over a StaticBody3D with a large box collider
  5. Place a camera pointing at both objects
  6. Run the scene

The object will fall through the StaticBody3D collider

Minimal reproduction project

Repro repo here

fire commented 1 year ago

@aaronfranke We've talked at length about this issue. Do we have a summary of the discussion.

aaronfranke commented 1 year ago

This will be easy to support after https://github.com/godotengine/godot/pull/77937 is merged.

capnm commented 1 year ago

This will be easy to support after #77937 is merged.

I personally don't think that this is the right approach for Godot's tree based scenes. The collision shape node should always remain a child of the corresponding body. The importer should be able to correctly detect the object relations and only attach the -convcolonly shape as a collision object to the parent body:

image

wilfredjonathanjames commented 1 year ago

Hi guys, what does #77937 do that would solve this problem @aaronfranke, and why is it not the right approach @capnm?

aaronfranke commented 1 year ago

77937 would allow us to generate the collision shape node anywhere in the node hierarchy, and the shape will search upwards arbitrarily far for a body (static, rigid, etc). Then we would change the importer to import only the shape instead of a bunch of static bodies, which would cause the shapes to apply to the rigid body.

wilfredjonathanjames commented 1 year ago

It does seem to partly solve my problem, but I'm not clear on how it allows us to define specific Body parents on import. For context, I use import hints.

What I have

If I have this blender scene, with a lowpoly collider and a higher poly mesh

image

It imports as

image

Like I said in the original post, I can change the root node in import settings, but the ColliderShape3Ds are still parented to other import-generated colliders.

What I want

image

That seems like an important primitive, so maybe I'm just missing how to do this with import hints.

What currently kind of works

If I do this in blender

image

And turn off the visibility of the additional MeshInstance3D created on the -rigid node, I get this

image

It's not ideal that the Body isn't the scene parent, and it creates an additional MeshInstance3D that needs to be disabled.

Solutions

I would suggest either of the following:

I prefer Solution 1. Seems cleaner.

aaronfranke commented 1 year ago

@wjagodfrey For "Add new import hints", note that special node names are kind of a hacky solution, and they are not very flexible (for example, it only works with mesh-based shapes, no way to specify a box collider for your crate). Long-term, the solution is to make Blender export explicit physics information like OMI_physics_shape.

wilfredjonathanjames commented 1 year ago

@aaronfranke it is hacky, but it's the solution we've got. It's incomplete, and it would be great to plug the gap.

capnm commented 1 year ago

@wjagodfrey The situation seems to be improving :) this could plug the gap. https://github.com/eoineoineoin/glTF_Physics

aaronfranke commented 1 year ago

@capnm See https://github.com/omigroup/gltf-extensions/tree/main/extensions/2.0/OMI_physics_body

samsface commented 8 months ago

Workaround import script if anyone else is stuck with this. Set the import node type of the .glb to rigid body. Then in blender, name your object "yourobject-rigid" in the tree view.

This script will sneakily re-parent the mesh and phys model to the root node and you should end up with something like this: image

@tool
extends EditorScenePostImport

func _post_import(scene):
    if scene.get_child_count() == 0:
        return

    var rigid_body:RigidBody3D = scene.get_child(0)
    if not rigid_body:
        return

    for child in rigid_body.get_children():
        rigid_body.remove_child(child)
        scene.add_child(child)

    scene.remove_child(rigid_body)

    return scene
hmans commented 3 months ago

I came up with an alternative import script that works with any imported scene that only has a single root node (returning that node instead of the "container" node) and doesn't require you to reconfigure the type of the generated root node -- just enable physics for the node of the object you want to import and you're set:

@tool
extends EditorScenePostImport

func _post_import(scene):
    # If the scene has more (or fewer) than a single child node, we can't do anything,
    # so just return the unmodified scene instead.
    if scene.get_child_count() != 1:
        return scene

    var new_root: Node = scene.get_child(0)

    # Keep the original name so instances of this scene will have the
    # imported asset's filename by default
    new_root.name = scene.name

    # Recursively set the owner of the new root and all its children
    _set_new_owner(new_root, new_root)

    # That's it!
    return new_root

func _set_new_owner(node: Node, owner: Node):
    # If we set a node's owner to itself, we'll get an error
    if node != owner:
        node.owner = owner

    for child in node.get_children():
        _set_new_owner(child, owner)

Additional information/rationale/explainer here.

Aman-Anas commented 2 months ago

Here is yet another alternative. I have some unique use cases where I want to make multiple convex colliders and such in Blender, then import it into a single rigid body or static body. This method works well for me, all it does is check if the scene root is a physics body, and if it is, it converts all of the static bodies into bare CollisionShape3Ds.

I'm using it as a post-import plugin, so I don't have to manually add the script to each object. Whenever I want to import a rigid body as root, I can just specify that in the import settings and the script activates.

@tool
extends EditorScenePostImportPlugin

func _post_process(scene: Node):
    # In case it's needed
    if scene.name.contains("nopost"):
        return scene

    # Remove all Rigify controls    
    for node in scene.get_children():
        if node.name.begins_with("WGT"):
            scene.remove_child(node)

    # Only affect physics bodies
    if not scene is PhysicsBody3D:
        return scene

    for node in scene.get_children():
        if node is PhysicsBody3D:
            if node.name.contains("nopost"):
                continue

            var body_to_remove = node
            scene.remove_child(body_to_remove)
            body_to_remove.owner = null

            for body_child in body_to_remove.get_children():
                body_to_remove.remove_child(body_child)
                body_child.owner = null
                scene.add_child(body_child)
                body_child.owner = scene

                body_child.position += body_to_remove.position
                body_child.rotation += body_to_remove.rotation
                body_child.scale *= body_to_remove.scale

    return scene
linker-err0r commented 1 month ago

It does seem to partly solve my problem, but I'm not clear on how it allows us to define specific Body parents on import. For context, I use import hints.

What I have

If I have this blender scene, with a lowpoly collider and a higher poly mesh image

It imports as image

Like I said in the original post, I can change the root node in import settings, but the ColliderShape3Ds are still parented to other import-generated colliders.

What I want

image

That seems like an important primitive, so maybe I'm just missing how to do this with import hints.

What currently kind of works

If I do this in blender image

And turn off the visibility of the additional MeshInstance3D created on the -rigid node, I get this image

It's not ideal that the Body isn't the scene parent, and it creates an additional MeshInstance3D that needs to be disabled.

Solutions

I would suggest either of the following:

  • Solution 1

    • If Godot finds a single root node on importing a model, don't wrap it in a new root node

    • Add new import hints

    • -colonlyrigid - Like -colonly but uses a RigidBody3D instead of a StaticBody3D

    • -convcolonlyrigid - You get the idea

  • Solution 2

    • Implement Allow CollisionShape nodes to be indirect children of bodies #77937

    • Add new import hints

    • -colshape - a concave CollisionShape3D without the parent Body, follows the -col and -colonly naming convention

    • -convcolshape

    • Some new hints that allow the user to tell Godot to import a node as a RigidBody. Only makes sense on parent empties. Without which we won't be able to make use of this anywhere other than on the root node if it's changed to a Body on import.

I prefer Solution 1. Seems cleaner.

I'm not sure that solution would work for my use case... Or it is incomplete. The reason being is that there are other scenarios to consider other than just _rigid. For instance, if I want to create a VechicleBody3D using the importers and then assign a collision mesh to it inside my modeling software, I would be unable to create a vehicle that both is a _vehicle and has a collision shape child. To do that we would have to complicate the API further if we use solution 1. We'd have to add import hints for vehicle bodies as well. For that reason, I think I prefer solution 2 -- just tell it to explicitly create the thing you want.

As such, I created my own EditorScenePostImport which produces a result similar to your Solution 2:

@tool # Needed so it runs in editor.
extends EditorScenePostImport
## This importer enables more seamless creation of collision nodes using 
## 3D modeling and animation software. 
##
## To use it, when naming a `_col` or `_convcol`, simply add the prefix `_shape`
## or `_shapeonly`. For example, you have a node named `body` you'd like to turn
## into a convex collision shape. To achieve this, you would rename it to 
## `body_shape_convcol`. To keep the shape and discard the mesh, you would 
## rename it to `body_shapeonly_convcol`.

const _shape := "_shape"
const _shapeonly := "_shapeonly"
const _collision_shape := "_collision_shape"

# Called right after the scene is imported and gets the root node. 
func _post_import(scene: Node) -> Node:
  iterate(scene)
  return scene # Remember to return the imported scene

## Recursive function that is called on every node and checks for 
## a collision shape that can be pulled out of the heirarchy and assigned
## to it's nearest parent PhysicsBody3D. This prevents warnings from Godot
## that cause the collision shape to be imported in such a way as to require
## this manual manipulation to eliminate. Also renames nodes to avoid
## anomalous node names (e.g. "body2" or "chest2" for instance) and appends
## suffixes `_mesh` and `_collision_shape` respectively
func iterate(node: Node) -> void:
  if not node:
    return

  if node.name.contains(_shape) and is_collison_nodeset(node):
    var mesh := node as MeshInstance3D
    var physics_body := mesh.get_parent() as PhysicsBody3D
    var static_body = Array(node.get_children()).front() as StaticBody3D
    var shape = Array(static_body.get_children()).front() as CollisionShape3D

    # reparent the shape to the parent PhysicsBody3D, kill the StaticBody
    kidnap_node(physics_body, shape)

    if mesh.name.contains(_shapeonly):
      # leave only the shape, get rid of the mesh
      var new_name := mesh.name.replace(_shapeonly, "")
      mesh.queue_free()
      prune_by_name(physics_body, new_name)
      shape.name = new_name + _collision_shape

    elif mesh.name.contains(_shape):
      # leave the mesh intact
      var new_name := mesh.name.replace(_shape, "")
      prune_by_name(physics_body, new_name)
      shape.name = new_name + _collision_shape
      mesh.name = new_name + "_mesh"

  for child in node.get_children():
    iterate(child)

## Looks for a specific combination of nodes that indicate that the importer
## found a nested collision shape that can be reparented to an ancestor 
## physics body to create a valid PhysicsBody
func is_collison_nodeset(node: Node) -> bool:
  var is_mesh := node is MeshInstance3D
  var is_static_body := Array(node.get_children()).front() is StaticBody3D
  var is_col_shape := Array(Array(node.get_children()).front().get_children()).front() is CollisionShape3D
  var parent_is_physics_body := node.get_parent() is PhysicsBody3D
  return is_mesh and is_static_body and is_col_shape and parent_is_physics_body

## Get rid of extra empty nodes in the tree that might conflict with our new name
func prune_by_name(physics_body: PhysicsBody3D, new_name: String) -> void:
  for child in physics_body.get_children():
    if child.name == new_name and child.get_child_count() == 0:
      child.queue_free()

## Parent the child to the new_parent and then destroy the old parent. 
## Sets the correct owner and prevents warnings and error conditions
## that would cause the scene to become inconsistent
func kidnap_node(new_parent: Node, child: Node) -> void:
  var old_parent := child.get_parent()
  old_parent.remove_child(child)
  child.owner = null
  new_parent.add_child(child)
  child.owner = new_parent.owner

  # always kill the node being kidnapped from
  old_parent.queue_free()