Skip to content

Commit

Permalink
Make HierarchyPlugin optional (#423)
Browse files Browse the repository at this point in the history
# Objective

Avian 0.1.0 introduced a regression where `HierarchyPlugin` and `ColliderHierarchyPlugin` are needed for colliders to function, as `ColliderParent` was only inserted by `ColliderHierarchyPlugin`. Hierarchies shouldn't be necessary.

## Solution

Update `ColliderParent` for colliders that are on the `RigidBody` entity in the `ColliderBackendPlugin`, and change panics to warnings when using `ColliderHierarchyPlugin` without `HierarchyPlugin`.
  • Loading branch information
Jondolf authored Jul 13, 2024
1 parent ac1fd02 commit 3425b7a
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 25 deletions.
33 changes: 33 additions & 0 deletions src/collision/collider/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,14 @@ impl<C: ScalableCollider> Plugin for ColliderBackendPlugin<C> {
),
);

// Update collider parents for colliders that are on the same entity as the rigid body.
app.add_systems(
self.schedule,
update_root_collider_parents::<C>
.after(PrepareSet::InitColliders)
.before(PrepareSet::Finalize),
);

let physics_schedule = app
.get_schedule_mut(PhysicsSchedule)
.expect("add PhysicsSchedule first");
Expand Down Expand Up @@ -274,6 +282,31 @@ pub(crate) fn init_colliders<C: AnyCollider>(
}
}

/// Updates [`ColliderParent`] for colliders that are on the same entity as the [`RigidBody`].
///
/// The [`ColliderHierarchyPlugin`] should be used to handle hierarchies.
#[allow(clippy::type_complexity)]
fn update_root_collider_parents<C: AnyCollider>(
mut commands: Commands,
mut bodies: Query<
(Entity, Option<&mut ColliderParent>),
(With<RigidBody>, With<C>, Or<(Added<RigidBody>, Added<C>)>),
>,
) {
for (entity, collider_parent) in &mut bodies {
if let Some(mut collider_parent) = collider_parent {
collider_parent.0 = entity;
} else {
commands.entity(entity).try_insert((
ColliderParent(entity),
// TODO: This probably causes a one frame delay. Compute real value?
ColliderTransform::default(),
PreviousColliderTransform::default(),
));
}
}
}

/// Generates [`Collider`]s based on [`ColliderConstructor`]s.
///
/// If a [`ColliderConstructor`] requires a mesh, the system keeps running
Expand Down
26 changes: 12 additions & 14 deletions src/collision/collider/hierarchy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,35 +117,33 @@ impl Plugin for ColliderHierarchyPlugin {
.in_set(NarrowPhaseSet::First),
);
}

fn finish(&self, app: &mut App) {
if !app.is_plugin_added::<HierarchyPlugin>() {
warn!("`ColliderHierarchyPlugin` requires Bevy's `HierarchyPlugin` to function. If you don't need collider hierarchies, consider disabling this plugin.",);
}
}
}

#[derive(Reflect, Clone, Copy, Component, Debug, Default, Deref, DerefMut, PartialEq)]
#[reflect(Component)]
pub(crate) struct PreviousColliderTransform(pub ColliderTransform);

/// Updates [`ColliderParent`] for descendant colliders of [`RigidBody`] entities.
///
/// The [`ColliderBackendPlugin`] handles collider parents for colliders that are
/// on the same entity as the rigid body.
#[allow(clippy::type_complexity)]
fn update_collider_parents(
mut commands: Commands,
mut bodies: Query<(Entity, Option<&mut ColliderParent>, Has<ColliderMarker>), With<RigidBody>>,
mut bodies: Query<Entity, (With<RigidBody>, With<AncestorMarker<ColliderMarker>>)>,
children: Query<&Children>,
mut child_colliders: Query<
Option<&mut ColliderParent>,
(With<ColliderMarker>, Without<RigidBody>),
>,
) {
for (entity, collider_parent, has_collider) in &mut bodies {
if has_collider {
if let Some(mut collider_parent) = collider_parent {
collider_parent.0 = entity;
} else {
commands.entity(entity).try_insert((
ColliderParent(entity),
// TODO: This probably causes a one frame delay. Compute real value?
ColliderTransform::default(),
PreviousColliderTransform::default(),
));
}
}
for entity in &mut bodies {
for child in children.iter_descendants(entity) {
if let Ok(collider_parent) = child_colliders.get_mut(child) {
if let Some(mut collider_parent) = collider_parent {
Expand Down
10 changes: 3 additions & 7 deletions src/sync/ancestor_marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ impl<C: Component> Plugin for AncestorMarkerPlugin<C> {
},
);

// Initialize `HierarchyEvent` in case `HierarchyPlugin` is not added.
app.add_event::<HierarchyEvent>();

// Update markers when changes are made to the hierarchy.
// TODO: This should be an observer. It'd remove the need for this scheduling nonsense
// and make the implementation more robust.
Expand All @@ -90,13 +93,6 @@ impl<C: Component> Plugin for AncestorMarkerPlugin<C> {
app.add_systems(self.schedule, update_markers_on_hierarchy_changes::<C>);
}
}

fn finish(&self, app: &mut App) {
assert!(
app.is_plugin_added::<HierarchyPlugin>(),
"`AncestorMarkerPlugin` requires Bevy's `HierarchyPlugin` to function.",
);
}
}

/// A marker component that marks an entity as an ancestor of an entity with the given component `C`.
Expand Down
10 changes: 6 additions & 4 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ fn create_app() -> App {
app.add_plugins((
MinimalPlugins,
TransformPlugin,
HierarchyPlugin,
PhysicsPlugins::default(),
PhysicsPlugins::default()
.build()
.disable::<ColliderHierarchyPlugin>(),
bevy::asset::AssetPlugin::default(),
#[cfg(feature = "bevy_scene")]
bevy::scene::ScenePlugin,
Expand Down Expand Up @@ -256,8 +257,9 @@ fn no_ambiguity_errors() {
App::new()
.add_plugins((
MinimalPlugins,
HierarchyPlugin,
PhysicsPlugins::new(DeterministicSchedule),
PhysicsPlugins::new(DeterministicSchedule)
.build()
.disable::<ColliderHierarchyPlugin>(),
bevy::asset::AssetPlugin::default(),
#[cfg(feature = "bevy_scene")]
bevy::scene::ScenePlugin,
Expand Down

0 comments on commit 3425b7a

Please sign in to comment.