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

Rework deferred collider initialization #378

Merged
merged 81 commits into from
Jun 24, 2024

Conversation

janhohenheim
Copy link
Sponsor Contributor

@janhohenheim janhohenheim commented Jun 19, 2024

Objective

As we all know, Collider is not Reflect. This is a bit of a problem for people wanting to load collider shapes from code-external sources. Notably, people using @kaosat-dev 's excellent Blender-GLTF workflow as an editor need to setup their own Reflect version of Collider all the time, usually something like this:

#[derive(Debug, Component, Reflect)
#[reflect(Component)]
enum Collider {
  Cuboid {...},
  Sphere {...},
  Trimesh,
  ...
}

and then attach XPBD colliders to entities holding the above fake collider.
I've been thinking about opening a PR on the Blender workflow to do this automatically based on a cargo feature, but then I thought that maybe this would be better in XPBD / Avian proper so that other reflect-heavy workflows may profit from it as well. I'm thinking about scene structures, embedding colliders in BSN and future Bevy editor prototypes.

Solution

My approach is to extend ComputedCollider to cover all variants of creating a collider. Some of these require a Mesh, some don't. When going through AsyncColliders, I can see if the specified creation method needs a Mesh. If so, the entity that the AsyncCollider is on must also hold a Handle


Changelog

  • Reworked deferred / "async" collider initialization. Renamed most involved types.
    • ComputedCollider and AsyncCollider have been combined into ColliderConstructor and support all possible collider shapes except compounds.
      • ColliderConstructor is Reflect, Serialize and Deserialize. You can use it to statically configure your colliders when loading them from code external sources such as save files or serialized level definitions.
      • ColliderConstructor supports creating primitive shapes, so it does not need to be placed on an entity holding a mesh.
      • ColliderConstructor can be used in 2D when not using computed shapes
    • AsyncSceneCollider is now called ColliderConstructorHierarchy and no longer requires being placed on a Scene. It will generally generate a collider for all available descendants.
      • Changes to ColliderConstructor also apply here
      • If the new default feature bevy_scene is enabled, placing ColliderConstructorHierarchy on a Scene will still wait until the scene is loaded
      • When the desired collider shape should be created from a mesh, only descendants holding a mesh will receive a collider
      • This type is now especially useful for generating colliders on objects defined with the Blender to Bevy workflow
      • ColliderConstructorHierarchy::new now takes an impl Into<Option> instead of an Option, so you can drop the Some from your calls if you want: AsyncSceneCollider::new(Some(Foo)) becomes ColliderConstructorHierarchy::new(Foo)
      • ColliderConstructorHierarchy can be used in 2D when not using computed shapes
    • bevy_scene can now be feature gated off while still using deferred colliders
  • bevy_gltf is no longer pulled in

Migration Guide

  • Remove feature async-collider. If you need to use computed shapes, use the feature collider-from-mesh. If you depend on ColliderConstructorHierarchy waiting for a scene to load, use the feature bevy_scene
  • Remove AsyncCollider and use ColliderConstructor directly
  • Rename AsyncSceneCollider to ColliderConstructorHierarchy
    • Rename AsyncSceneCollider::default_shape to ColliderConstructorHierarchy::default_constructor
    • Rename AsyncSceneCollider::meshes_by_name to ColliderConstructorHierarchy::config
    • Rename AsyncSceneCollider::with_shape_for_name to ColliderConstructorHierarchy::with_constructor_for_name
    • Rename AsyncSceneCollider::without_shape_for_name to ColliderConstructorHierarchy::without_constructor_for_name
  • Rename AsyncSceneColliderData to ColliderConstructorHierarchyConfig
    • Rename AsyncSceneColliderData::shape to ColliderConstructorHierarchyConfig::constructor
  • Rename ComputedCollider to ColliderConstructor.
    • Rename ComputedCollider::TriMesh to ColliderConstructor::TrimeshFromMesh
    • Rename ComputedCollider::TriMeshWithFlags to ColliderConstructor::TrimeshFromMeshWithConfig
    • Rename ComputedCollider::ConvexHull to ColliderConstructor::ConvexHullFromMesh
    • Rename ComputedCollider::ConvexDecomposition to ColliderConstructor::ConvexDecompositionFromMeshWithConfig
  • Rename VHACDParameters to VhacdParameters
  • Rename Collider::halfspace to Collider::half_space

Linked Issues

@janhohenheim janhohenheim changed the title Make AsyncCollider Reflect Rework lazy collider initialization Jun 19, 2024
@janhohenheim janhohenheim marked this pull request as ready for review June 20, 2024 12:05
@janhohenheim janhohenheim changed the title Rework lazy collider initialization Rework deferred collider initialization Jun 20, 2024
@Jondolf Jondolf added the C-Breaking-Change This change removes or changes behavior or APIs, requiring users to adapt label Jun 23, 2024
Copy link
Owner

@Jondolf Jondolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, thank you for all the hard work and patience on this!

I see this also adds crate-wide allows for clippy::type_complexity and clippy::too_many_arguments. I agree we should probably do this. The existing per-method allows can be removed in a follow-up.

@Jondolf Jondolf merged commit 5eba1c8 into Jondolf:main Jun 24, 2024
4 checks passed
Jondolf added a commit that referenced this pull request Jun 29, 2024
…#386)

# Objective

When loading a (glTF) scene and generating colliders for it, it can be useful to be able to add all entities in that scene to some collision layer. However, `ColliderConstructorHierarchy` (previously `AsyncSceneCollider`, see #378) doesn't currently allow specifying default collision layer or density configurations despite allowing configurations for individual entities.

## Solution

Add `default_layers` and `default_density` properties for `ColliderConstructorHierarchy`. The `layers` and `density` properties of `ColliderConstructorHierarchyData` are now `Option`s, and if they are `None`, the default values are used.

I also moved the collider constructor types and tests to their own module, and updated some docs to remove mesh-specific wording.

For reviewing purposes, the first commit (8cbe48c) should be easier to read, the other commit just moves code to the `constructor` module.
@janhohenheim janhohenheim deleted the reflect-collider branch July 8, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Breaking-Change This change removes or changes behavior or APIs, requiring users to adapt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sensor not working on AsyncSceneCollider Remove bevy/bevy_gltf dependency from async-collider
2 participants