Skip to content

Commit

Permalink
Fix Mesh not being optional (#405)
Browse files Browse the repository at this point in the history 
# Objective

Avian 0.1.0 introduced a regression where `bevy_render` is now needed because `Mesh` is not optional. It should be feature-gated behind `collider-from-mesh`.

## Solution

Put all mentions of `Mesh` behind the `collider-from-mesh` feature.
  • Loading branch information
@Jondolf
Jondolf authored Jul 7, 2024
1 parent d73349f commit d25341a
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 121 deletions.
142 changes: 77 additions & 65 deletions src/collision/collider/backend.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,16 +284,16 @@ pub(crate) fn init_colliders<C: AnyCollider>(
/// Panics if the [`ColliderConstructor`] requires a mesh but no mesh handle is found.
fn init_collider_constructors(
mut commands: Commands,
meshes: Res<Assets<Mesh>>,
#[cfg(feature = "collider-from-mesh")] meshes: Res<Assets<Mesh>>,
#[cfg(feature = "collider-from-mesh")] mesh_handles: Query<&Handle<Mesh>>,
constructors: Query<(
Entity,
Option<&Handle<Mesh>>,
Option<&Collider>,
Option<&Name>,
&ColliderConstructor,
)>,
) {
for (entity, mesh_handle, existing_collider, name, constructor) in constructors.iter() {
for (entity, existing_collider, name, constructor) in constructors.iter() {
let name = pretty_name(name, entity);
if existing_collider.is_some() {
warn!(
Expand All @@ -303,8 +303,9 @@ fn init_collider_constructors(
commands.entity(entity).remove::<ColliderConstructor>();
continue;
}
#[cfg(feature = "collider-from-mesh")]
let mesh = if constructor.requires_mesh() {
let mesh_handle = mesh_handle.unwrap_or_else(|| panic!(
let mesh_handle = mesh_handles.get(entity).unwrap_or_else(|_| panic!(
"Tried to add a collider to entity {name} via {constructor:#?} that requires a mesh, \
but no mesh handle was found"));
let mesh = meshes.get(mesh_handle);
Expand All @@ -317,13 +318,17 @@ fn init_collider_constructors(
None
};

#[cfg(feature = "collider-from-mesh")]
let collider = Collider::try_from_constructor(constructor.clone(), mesh);
#[cfg(not(feature = "collider-from-mesh"))]
let collider = Collider::try_from_constructor(constructor.clone());

if let Some(collider) = collider {
commands.entity(entity).insert(collider);
} else {
error!(
"Tried to add a collider to entity {name} via {constructor:#?}, \
but the collider could not be generated from mesh {mesh:#?}. Skipping.",
but the collider could not be generated. Skipping.",
);
}
commands.entity(entity).remove::<ColliderConstructor>();
Expand All @@ -335,13 +340,14 @@ fn init_collider_constructors(
/// If an entity has a `SceneInstance`, its collider hierarchy is only generated once the scene is ready.
fn init_collider_constructor_hierarchies(
mut commands: Commands,
meshes: Res<Assets<Mesh>>,
#[cfg(feature = "collider-from-mesh")] meshes: Res<Assets<Mesh>>,
#[cfg(feature = "collider-from-mesh")] mesh_handles: Query<&Handle<Mesh>>,
#[cfg(feature = "bevy_scene")] scene_spawner: Res<SceneSpawner>,
#[cfg(feature = "bevy_scene")] scenes: Query<&Handle<Scene>>,
#[cfg(feature = "bevy_scene")] scene_instances: Query<&SceneInstance>,
collider_constructors: Query<(Entity, &ColliderConstructorHierarchy)>,
children: Query<&Children>,
mesh_handles: Query<(Option<&Name>, Option<&Collider>, Option<&Handle<Mesh>>)>,
child_query: Query<(Option<&Name>, Option<&Collider>)>,
) {
for (scene_entity, collider_constructor_hierarchy) in collider_constructors.iter() {
#[cfg(feature = "bevy_scene")]
Expand All @@ -360,73 +366,79 @@ fn init_collider_constructor_hierarchies(
}

for child_entity in children.iter_descendants(scene_entity) {
if let Ok((name, existing_collider, handle)) = mesh_handles.get(child_entity) {
let pretty_name = pretty_name(name, child_entity);

let default_collider = || {
Some(ColliderConstructorHierarchyConfig {
constructor: collider_constructor_hierarchy.default_constructor.clone(),
..default()
})
};

let collider_data = if let Some(name) = name {
collider_constructor_hierarchy
.config
.get(name.as_str())
.cloned()
.unwrap_or_else(default_collider)
} else if existing_collider.is_some() {
warn!("Tried to add a collider to entity {pretty_name} via {collider_constructor_hierarchy:#?}, \
let Ok((name, existing_collider)) = child_query.get(child_entity) else {
continue;
};

let pretty_name = pretty_name(name, child_entity);

let default_collider = || {
Some(ColliderConstructorHierarchyConfig {
constructor: collider_constructor_hierarchy.default_constructor.clone(),
..default()
})
};

let collider_data = if let Some(name) = name {
collider_constructor_hierarchy
.config
.get(name.as_str())
.cloned()
.unwrap_or_else(default_collider)
} else if existing_collider.is_some() {
warn!("Tried to add a collider to entity {pretty_name} via {collider_constructor_hierarchy:#?}, \
but that entity already holds a collider. Skipping. \
If this was intentional, add the name of the collider to overwrite to `ColliderConstructorHierarchy.config`.");
continue;
} else {
default_collider()
};
continue;
} else {
default_collider()
};

// If the configuration is explicitly set to `None`, skip this entity.
let Some(collider_data) = collider_data else {
continue;
};
// If the configuration is explicitly set to `None`, skip this entity.
let Some(collider_data) = collider_data else {
continue;
};

// Use the configured constructor if specified, otherwise use the default constructor.
// If both are `None`, skip this entity.
let Some(constructor) = collider_data
.constructor
.or_else(|| collider_constructor_hierarchy.default_constructor.clone())
else {
continue;
};

// Use the configured constructor if specified, otherwise use the default constructor.
// If both are `None`, skip this entity.
let Some(constructor) = collider_data
.constructor
.or_else(|| collider_constructor_hierarchy.default_constructor.clone())
else {
#[cfg(feature = "collider-from-mesh")]
let mesh = if constructor.requires_mesh() {
if let Ok(handle) = mesh_handles.get(child_entity) {
meshes.get(handle)
} else {
continue;
};
}
} else {
None
};

let mesh = if constructor.requires_mesh() {
if let Some(handle) = handle {
meshes.get(handle)
} else {
continue;
}
} else {
None
};
#[cfg(feature = "collider-from-mesh")]
let collider = Collider::try_from_constructor(constructor, mesh);
#[cfg(not(feature = "collider-from-mesh"))]
let collider = Collider::try_from_constructor(constructor);

let collider = Collider::try_from_constructor(constructor, mesh);

if let Some(collider) = collider {
commands.entity(child_entity).insert((
collider,
collider_data
.layers
.unwrap_or(collider_constructor_hierarchy.default_layers),
collider_data
.density
.unwrap_or(collider_constructor_hierarchy.default_density),
));
} else {
error!(
if let Some(collider) = collider {
commands.entity(child_entity).insert((
collider,
collider_data
.layers
.unwrap_or(collider_constructor_hierarchy.default_layers),
collider_data
.density
.unwrap_or(collider_constructor_hierarchy.default_density),
));
} else {
error!(
"Tried to add a collider to entity {pretty_name} via {collider_constructor_hierarchy:#?}, \
but the collider could not be generated from mesh {mesh:#?}. Skipping.",
but the collider could not be generated. Skipping.",
);
}
}
}

Expand Down
54 changes: 24 additions & 30 deletions src/collision/collider/constructor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ pub struct ColliderConstructorHierarchyConfig {
#[derive(Clone, Debug, PartialEq, Reflect, Component)]
#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serialize", reflect(Serialize, Deserialize))]
#[cfg_attr(all(feature = "3d", feature = "collider-from-mesh"), derive(Default))]
#[cfg_attr(all(feature = "3d", feature = "collider-from-mesh"), reflect(Default))]
#[cfg_attr(feature = "collider-from-mesh", derive(Default))]
#[cfg_attr(feature = "collider-from-mesh", reflect(Default))]
#[reflect(Debug, Component, PartialEq)]
#[non_exhaustive]
#[allow(missing_docs)]
Expand Down Expand Up @@ -405,7 +405,7 @@ pub enum ColliderConstructor {
scale: Vector,
},
/// Constructs a collider with [`Collider::trimesh_from_mesh`].
#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
#[default]
TrimeshFromMesh,
/// Constructs a collider with [`Collider::trimesh_from_mesh_with_config`].
Expand All @@ -416,7 +416,7 @@ pub enum ColliderConstructor {
))]
TrimeshFromMeshWithConfig(TrimeshFlags),
/// Constructs a collider with [`Collider::convex_decomposition_from_mesh`].
#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
ConvexDecompositionFromMesh,
/// Constructs a collider with [`Collider::convex_decomposition_from_mesh_with_config`].
#[cfg(all(
Expand All @@ -426,28 +426,22 @@ pub enum ColliderConstructor {
))]
ConvexDecompositionFromMeshWithConfig(VhacdParameters),
/// Constructs a collider with [`Collider::convex_hull_from_mesh`].
#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
ConvexHullFromMesh,
}

impl ColliderConstructor {
/// Returns `true` if the collider type requires a mesh to be generated.
#[cfg(feature = "collider-from-mesh")]
pub fn requires_mesh(&self) -> bool {
#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
{
matches!(
self,
Self::TrimeshFromMesh
| Self::TrimeshFromMeshWithConfig(_)
| Self::ConvexDecompositionFromMesh
| Self::ConvexDecompositionFromMeshWithConfig(_)
| Self::ConvexHullFromMesh
)
}
#[cfg(not(all(feature = "3d", feature = "collider-from-mesh")))]
{
false
}
matches!(
self,
Self::TrimeshFromMesh
| Self::TrimeshFromMeshWithConfig(_)
| Self::ConvexDecompositionFromMesh
| Self::ConvexDecompositionFromMeshWithConfig(_)
| Self::ConvexHullFromMesh
)
}
}

Expand All @@ -470,7 +464,7 @@ mod tests {
assert!(app.query_err::<&ColliderConstructor>(entity));
}

#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
#[test]
#[should_panic]
fn collider_constructor_requires_mesh_on_computed() {
Expand All @@ -481,7 +475,7 @@ mod tests {
app.update();
}

#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
#[test]
fn collider_constructor_converts_mesh_on_computed() {
let mut app = create_test_app();
Expand Down Expand Up @@ -516,7 +510,7 @@ mod tests {
assert!(app.query_err::<&Collider>(entity));
}

#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
#[test]
fn collider_constructor_hierarchy_does_nothing_on_self_with_computed() {
let mut app = create_test_app();
Expand All @@ -537,7 +531,7 @@ mod tests {
assert!(app.query_err::<&Collider>(entity));
}

#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
#[test]
fn collider_constructor_hierarchy_does_not_require_mesh_on_self_with_computed() {
let mut app = create_test_app();
Expand Down Expand Up @@ -592,7 +586,7 @@ mod tests {
assert!(app.query_ok::<&Collider>(child3));
}

#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
#[test]
fn collider_constructor_hierarchy_inserts_computed_colliders_only_on_descendants_with_mesh() {
let mut app = create_test_app();
Expand Down Expand Up @@ -653,7 +647,7 @@ mod tests {
assert!(app.query_ok::<&Collider>(child8));
}

#[cfg(all(feature = "3d", feature = "collider-from-mesh", feature = "bevy_scene"))]
#[cfg(all(feature = "collider-from-mesh", feature = "bevy_scene"))]
#[test]
fn collider_constructor_hierarchy_inserts_correct_configs_on_scene() {
use parry::shape::ShapeType;
Expand Down Expand Up @@ -730,7 +724,7 @@ mod tests {
radius: 0.5,
};

#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
const COMPUTED_COLLIDER: ColliderConstructor = ColliderConstructor::TrimeshFromMesh;

fn create_test_app() -> App {
Expand All @@ -748,7 +742,7 @@ mod tests {
app
}

#[cfg(all(feature = "3d", feature = "collider-from-mesh", feature = "bevy_scene"))]
#[cfg(all(feature = "collider-from-mesh", feature = "bevy_scene"))]
fn create_gltf_test_app() -> App {
use bevy::{diagnostic::DiagnosticsPlugin, winit::WinitPlugin};

Expand All @@ -771,7 +765,7 @@ mod tests {
!self.query_ok::<D>(entity)
}

#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
fn add_mesh(&mut self) -> Handle<Mesh>;
}

Expand All @@ -782,7 +776,7 @@ mod tests {
component.is_ok()
}

#[cfg(all(feature = "3d", feature = "collider-from-mesh"))]
#[cfg(feature = "collider-from-mesh")]
fn add_mesh(&mut self) -> Handle<Mesh> {
self.world_mut()
.get_resource_mut::<Assets<Mesh>>()
Expand Down
Loading

0 comments on commit d25341a

Please sign in to comment.