GLTF: Update OMI_physics_body, add OMI_physics_shape, keep OMI_collider

[aaronfranke]

This PR adds support for importing OMI_physics_shape, which supersedes OMI_collider. This PR also updates OMI_physics_body with recent OMI changes.

The change from OMI_collider to OMI_physics_shape includes not only a rename, but also a change in responsibility for where the behavior of a shape is defined, and of how the data is formatted. This is based on a combination of consensus between OMI, Microsoft, and Khronos. In the future the goal is to end up with a Khronos standard, which will likely end up looking very similar to OMI_physics_shape but with a name change.

The ability to load OMI_collider is kept for backwards compatibility, and uses mostly the same code. Note that for exporting new files, it will only use OMI_physics_shape. As such, do not backport to Godot 4.2.x. If desired I could backport only the import changes so that files exported in 4.3 will be loadable in 4.2.

The change in OMI_physics_body is similarly designed to bring the OMI spec closer to the consensus of what Khronos wants. The body definition is now inside of the motion sub-JSON, and collider/trigger shapes are now located inside properties with those names in OMI_physics_body.

Production edit: closes godotengine/godot-roadmap#43

[fire]

Comment from @aaronfranke about how to test.

Can you send me links?

[aaronfranke]

Test files for the newest format:

• Add example files for OMI_physics_body omigroup/omi-godot#2
• https://github.com/omigroup/gltf-extensions/tree/main/extensions/2.0/OMI_physics_shape/examples
• https://github.com/omigroup/gltf-extensions/tree/main/extensions/2.0/OMI_physics_body/examples

Test files for the older format that still load correctly:

• https://github.com/omigroup/gltf-extensions/tree/39afb6453ea36b4825c1d9e5140529af7ca180d2/extensions/2.0/OMI_collider/examples
• https://github.com/omigroup/gltf-extensions/tree/39afb6453ea36b4825c1d9e5140529af7ca180d2/extensions/2.0/OMI_physics_body/examples

[lyuma]

Overall looks good. I think the main blocker for now is indirect child collision shapes

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

[lyuma]

I do not understand where the string animatable comes from.

I think we should stick for now with what is in the schema and add this as extras or a separate GODOT_ extension if needed
https://github.com/omigroup/gltf-extensions/blob/main/extensions/2.0/OMI_physics_body/schema/node.OMI_physics_body.motion.schema.json#L12..L16

[aaronfranke]

As I have explaines before, the in-memory type list is closer to Godot's types for flexibility, and is expanded on import / flattened on export. We only ever deal with the types in the OMI schema at the level of glTF files.

This allows reading "dynamic" from OMI data, then setting it in memory as "rigid", then other extensions can change this to "vehicle". Or in this case, reading "kinematic", in memory "animatable", and other extensions can change this to "character".

This mechanism exists precisely to allow a separate extension to exist in the future, with code to support new types not needing to fight with the rest of the physics code. It gives full access to Godot's physics node types.

[lyuma]

In that case, since these types are not GLTF types, I think it could be helpful to use an enum rather than string constants to help follow how these types are used in various points in the code (easier to search for uses of enum values).

The only reason I would be ok with strings here is so that future extensions can add additional physics types. But since there is an ERR_FAIL "Error converting GLTFPhysicsBody to a node: Body type X is unknown", I think this is not the reason... so in that light, and given that body_type was already exposed, I would prefer that this be addressed in one or two ways:

1. in gltf_physics_body.cpp, ADD_PROPERTY(PropertyInfo(Variant::STRING, "body_type"), "set_body_type", "get_body_type"); should be modified to use an enum hint with the list of possible body types
2. in gltf_physics_body.cpp, set_body_type and get_body_type convert from string to enum value and vice versa, such that internally it becomes an enum.

[lyuma]

Nitpick but I think to make compile-time StringName, you're supposed to use SNAME

[aaronfranke]

Please see the comment above the SNAME macro "If in doubt, do not use":

/*
 * The SNAME macro is used to speed up StringName creation, as it allows caching it after the first usage in a very efficient way.
 * It should NOT be used everywhere, but instead in places where high performance is required and the creation of a StringName
 * can be costly. Places where it should be used are:
 * - Control::get_theme_*(<name> and Window::get_theme_*(<name> functions.
 * - emit_signal(<name>,..) function
 * - call_deferred(<name>,..) function
 * - Comparisons to a StringName in overridden _set and _get methods.
 *
 * Use in places that can be called hundreds of times per frame (or more) is recommended, but this situation is very rare. If in doubt, do not use.
 */

[fire]

@aaronfranke I don't know this is the best place to write this, but do you know if you or anyone else has implemented OMI_physics_body and OMI_physics_shape in Blender?

[aaronfranke]

@fire No, I have not implemented in Blender yet and nobody else has either.

[aaronfranke]

@lyuma "I think the main blocker for now is indirect child collision shapes"

It's not a blocker, because that is not necessary for this PR. In master, indirect shapes do not work. In this PR, indirect shapes do not work. The behavior is unchanged. This PR does not require that PR.

[lyuma]

I agree that this can be merged without resolving the discussion on indirect collision shapes.

I think my concern still remains about having a bunch of hardcoded strings in GLTFPhysicsBody which do not match one-to-one with gltf strings. I would rather these use an enum.

[lyuma]

In that case, since these types are not GLTF types, I think it could be helpful to use an enum rather than string constants to help follow how these types are used in various points in the code (easier to search for uses of enum values).

The only reason I would be ok with strings here is so that future extensions can add additional physics types. But since there is an ERR_FAIL "Error converting GLTFPhysicsBody to a node: Body type X is unknown", I think this is not the reason... so in that light, and given that body_type was already exposed, I would prefer that this be addressed in one or two ways:

1. in gltf_physics_body.cpp, ADD_PROPERTY(PropertyInfo(Variant::STRING, "body_type"), "set_body_type", "get_body_type"); should be modified to use an enum hint with the list of possible body types
2. in gltf_physics_body.cpp, set_body_type and get_body_type convert from string to enum value and vice versa, such that internally it becomes an enum.

[fire]

3d Asset Pipeline Meeting:

1. Lyuma approved the pr and we think it should be merged.

[YuriSizov]

Thanks!