Skip to content

Commit

Permalink
address pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Vrixyz committed Jul 5, 2024
1 parent 0b9ca86 commit f4643ef
Show file tree
Hide file tree
Showing 9 changed files with 185 additions and 113 deletions.
2 changes: 1 addition & 1 deletion bevy_rapier3d/examples/ray_casting3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub fn setup_physics(mut commands: Commands) {
pub fn cast_ray(
mut commands: Commands,
windows: Query<&Window, With<PrimaryWindow>>,
rapier_context: DefaultRapierContextAccessMut,
rapier_context: DefaultRapierContextAccess,
cameras: Query<(&Camera, &GlobalTransform)>,
) {
let window = windows.single();
Expand Down
11 changes: 4 additions & 7 deletions src/plugin/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,16 +375,13 @@ impl RapierContext {
collision_event_writer: &mut EventWriter<CollisionEvent>,
contact_force_event_writer: &mut EventWriter<ContactForceEvent>,
) {
for collision_event in self.collision_events_to_send.iter() {
collision_event_writer.send(*collision_event);
for collision_event in self.collision_events_to_send.drain(..) {
collision_event_writer.send(collision_event);
}
self.collision_events_to_send.clear();

for contact_force_event in self.contact_force_events_to_send.iter() {
contact_force_event_writer.send(*contact_force_event);
for contact_force_event in self.contact_force_events_to_send.drain(..) {
contact_force_event_writer.send(contact_force_event);
}

self.contact_force_events_to_send.clear();
}

/// This method makes sure that the rigid-body positions have been propagated to
Expand Down
100 changes: 71 additions & 29 deletions src/plugin/context/systemparams/rapier_context_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,20 @@ use bevy::prelude::*;
use std::ops::{Deref, DerefMut};

use super::super::{DefaultRapierContext, RapierContext, RapierContextEntityLink};
/// Utility [`SystemParam`] to easily access the default world [`RapierContext`] immutably
/// Utility [`SystemParam`] to easily access the single default [`RapierContext`] immutably.
///
/// SAFETY: Dereferencing this struct will panic if its underlying query fails.
/// See [`RapierContextAccess`] for a safer alternative.
#[derive(SystemParam)]
pub struct DefaultRapierContextAccess<'w, 's, T: Component = DefaultRapierContext> {
rapier_context: Query<'w, 's, &'static RapierContext, With<T>>,
}

impl<'w, 's, T: Component> DefaultRapierContextAccess<'w, 's, T> {
/// Use this method if you only have one world.
/// Use this method if you only have one [`RapierContext`].
///
/// SAFETY: This method will panic if its underlying query fails.
/// See [`RapierContextAccess`] for a safe alternative.
pub fn single(&'_ self) -> &RapierContext {
self.rapier_context.single()
}
Expand All @@ -19,12 +25,19 @@ impl<'w, 's, T: Component> DefaultRapierContextAccess<'w, 's, T> {
impl<'w, 's> Deref for DefaultRapierContextAccess<'w, 's> {
type Target = RapierContext;

/// Use this method if you only have one [`RapierContext`].
///
/// SAFETY: This method will panic if its underlying query fails.
/// See [`RapierContextAccess`] for a safe alternative.
fn deref(&self) -> &Self::Target {
self.rapier_context.single()
}
}

/// Utility [`SystemParam`] to easily access the default world [`RapierContext`] mutably
/// Utility [`SystemParam`] to easily access the single default [`RapierContext`] mutably.
///
/// SAFETY: Dereferencing this struct will panic if its underlying query fails.
/// See [`RapierContextAccess`] for a safer alternative.
#[derive(SystemParam)]
pub struct DefaultRapierContextAccessMut<'w, 's, T: Component = DefaultRapierContext> {
rapier_context: Query<'w, 's, &'static mut RapierContext, With<T>>,
Expand All @@ -33,12 +46,20 @@ pub struct DefaultRapierContextAccessMut<'w, 's, T: Component = DefaultRapierCon
impl<'w, 's, T: Component> Deref for DefaultRapierContextAccessMut<'w, 's, T> {
type Target = RapierContext;

/// Use this method if you only have one [`RapierContext`].
///
/// SAFETY: This method will panic if its underlying query fails.
/// See [`RapierContextAccess`] for a safe alternative.
fn deref(&self) -> &Self::Target {
self.rapier_context.single()
}
}

impl<'w, 's> DerefMut for DefaultRapierContextAccessMut<'w, 's> {
/// Use this method if you only have one [`RapierContext`].
///
/// SAFETY: This method will panic if its underlying query fails.
/// See [`RapierContextAccess`] for a safe alternative.
fn deref_mut(&mut self) -> &mut Self::Target {
// TODO: should we cache the result ?
self.rapier_context.single_mut().into_inner()
Expand All @@ -58,11 +79,18 @@ pub struct RapierContextAccess<'w, 's> {

impl<'w, 's> RapierContextAccess<'w, 's> {
/// Retrieves the rapier context responsible for the entity owning the given [`RapierContextEntityLink`].
pub fn context(&self, link: RapierContextEntityLink) -> &'_ RapierContext {
self.rapier_context
.get(link.0)
///
/// SAFETY: This method will panic if its underlying query fails.
/// See [`Self::try_context`] for a safe alternative.
pub fn context(&self, link: &RapierContextEntityLink) -> &'_ RapierContext {
self.try_context(link)
.expect("RapierContextEntityLink.0 refers to an entity without RapierContext.")
}

/// Retrieves the rapier context responsible for the entity owning the given [`RapierContextEntityLink`].
pub fn try_context(&self, link: &RapierContextEntityLink) -> Option<&'_ RapierContext> {
self.rapier_context.get(link.0).ok()
}
}

impl<'w, 's> Deref for RapierContextAccess<'w, 's> {
Expand All @@ -86,40 +114,54 @@ pub struct RapierContextAccessMut<'w, 's> {

impl<'w, 's> RapierContextAccessMut<'w, 's> {
/// Retrieves the rapier context responsible for the entity owning the given [`RapierContextEntityLink`].
pub fn context(&mut self, link: RapierContextEntityLink) -> &'_ mut RapierContext {
self.rapier_context
.get_mut(link.0)
///
/// SAFETY: This method will panic if its underlying query fails.
/// See [`Self::try_context`] for a safe alternative.
pub fn context(&mut self, link: &RapierContextEntityLink) -> Mut<RapierContext> {
self.try_context(link)
.expect("RapierContextEntityLink.0 refers to an entity without RapierContext.")
.into_inner()
}

/// Retrieves the rapier context responsible for the entity owning the given [`RapierContextEntityLink`].
pub fn try_context(&mut self, link: &RapierContextEntityLink) -> Option<Mut<RapierContext>> {
self.rapier_context.get_mut(link.0).ok()
}

/// Retrieves the rapier context component on this [`Entity`].
///
/// Do not call this on a rapier managed entity. given entity should have a [`RapierContext`].
/// Calling this method on a rapier managed entity (rigid body, collider, joints...) will fail.
/// Given entity should have a [`RapierContext`].
///
/// SAFETY: This method will panic if its underlying query fails.
pub(crate) fn context_from_entity(
&mut self,
rapier_context_entity: Entity,
) -> &'_ mut RapierContext {
self.rapier_context
.get_mut(rapier_context_entity)
.unwrap_or_else(|_| panic!("entity {rapier_context_entity} has no RapierContext."))
.into_inner()
) -> Mut<RapierContext> {
self.try_context_from_entity(rapier_context_entity)
.unwrap_or_else(|| panic!("entity {rapier_context_entity} has no RapierContext."))
}

/// Retrieves the rapier context component on this [`Entity`].
///
/// Calling this method on a rapier managed entity (rigid body, collider, joints...) will fail.
/// Given entity should have a [`RapierContext`].
pub fn try_context_from_entity(
&mut self,
rapier_context_entity: Entity,
) -> Option<Mut<RapierContext>> {
self.rapier_context.get_mut(rapier_context_entity).ok()
}
}

pub fn try_get_default_context(
/// Gets the default RapierContext.
///
/// SAFETY: Panics if no entity with [`DefaultRapierContext`] exist.
pub fn get_single_context(
default_context_access: &Query<Entity, With<DefaultRapierContext>>,
) -> Option<Entity> {
let context_entity = match default_context_access.iter().next() {
Some(it) => it,
None => {
log::error!(
"No entity with `DefaultRapierContext` found.\
) -> Entity {
default_context_access.get_single().expect(
"No entity with `DefaultRapierContext` found.\
Please add a default `RapierContext` or a `RapierContextEntityLink`\
on the new rapier-managed entity."
);
return None;
}
};
Some(context_entity)
on the new rapier-managed entity.",
)
}
26 changes: 6 additions & 20 deletions src/plugin/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ where
.in_set(RapierTransformPropagateSet),
systems::on_add_entity_with_parent,
systems::on_change_world,
//
systems::sync_removals,
#[cfg(all(feature = "dim3", feature = "async-collider"))]
systems::init_async_scene_colliders,
Expand All @@ -118,8 +117,6 @@ where
systems::init_rigid_bodies,
systems::init_colliders,
systems::init_joints,
//systems::sync_removals,
// Run this here so the following systems do not have a 1 frame delay.
systems::apply_scale,
systems::apply_collider_user_changes,
systems::apply_rigid_body_user_changes,
Expand Down Expand Up @@ -246,20 +243,6 @@ where
.before(TransformSystem::TransformPropagate),
);

// These *must* be in the main schedule currently so that they do not miss events.
/*app.add_systems(
PostUpdate,
(
// Change any worlds needed before doing any calculations
systems::on_add_entity_with_parent,
systems::on_change_world,
// Make sure to remove any dead bodies after changing_worlds but before everything else
// to avoid it deleting something right after adding it
//systems::sync_removals,
)
.chain(),
);*/

app.add_systems(
self.schedule,
(
Expand Down Expand Up @@ -287,12 +270,15 @@ where

/// Specifies a default configuration for the default `RapierContext`
///
/// If [`None`], no world will be initialized, you are responsible of creating and maintaining
/// a [`RapierContext`] before creating any rapier entities (rigidbodies, colliders, joints),
/// and as long as any [`RapierContextEntityLink`] has a reference to its [`RapierContext`].
/// If [`NoAutomaticRapierContext`],
#[derive(Resource, Debug, Reflect, Clone)]
pub enum RapierContextInitialization {
/// [`RapierPhysicsPlugin`] will not spawn any entity containing [`RapierContext`] automatically.
///
/// You are responsible for creating a [`RapierContext`],
/// before spawning any rapier entities (rigidbodies, colliders, joints).
///
/// You might be interested in adding [`DefaultRapierContext`] to the created world.
NoAutomaticRapierContext,
/// [`RapierPhysicsPlugin`] will spawn an entity containing a [`RapierContext`]
/// automatically during [`PreStartup`], with the [`DefaultRapierContext`] marker component.
Expand Down
6 changes: 4 additions & 2 deletions src/plugin/systems/character_controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,13 @@ pub fn update_character_controls(
glob_transform,
) in character_controllers.iter_mut()
{
let context = context_access.context(*rapier_context_link);
let config = config.get(rapier_context_link.0).unwrap();
if let (Some(raw_controller), Some(translation)) =
(controller.to_raw(), controller.translation)
{
let config = config
.get(rapier_context_link.0)
.expect("Could not get [`RapierConfiguration`]");
let context = context_access.context(rapier_context_link).into_inner();
let scaled_custom_shape =
controller
.custom_shape
Expand Down
Loading

0 comments on commit f4643ef

Please sign in to comment.