EmbarkStudios / physx-rs

🎳 Rust binding for NVIDIA PhysX 🦀
http://embark.rs
Apache License 2.0
656 stars 42 forks source link

ActorMap is missing read-only API #205

Closed rlidwka closed 1 year ago

rlidwka commented 1 year ago

Description of the problem

I'm trying to register on_wake_sleep callback, which has the following signature:

pub trait WakeSleepCallback<L: ArticulationLink, S: RigidStatic, D: RigidDynamic>: Sized {
    fn on_wake_sleep(&mut self, actors: &[&ActorMap<L, S, D>], is_waking: bool);
}

ActorMap as of now has the following implementation (link):

impl<L, S, D> ActorMap<L, S, D>
where
    L: ArticulationLink,
    S: RigidStatic,
    D: RigidDynamic,
{
    pub fn cast_map<'a, Ret, ALFn, RSFn, RDFn>(
        &'a mut self,
        mut articulation_link_fn: ALFn,
        mut rigid_static_fn: RSFn,
        mut rigid_dynamic_fn: RDFn,
    ) -> Ret {/*...*/}

    pub fn as_rigid_dynamic(&mut self) -> Option<&mut D> {/*...*/}
    pub fn as_rigid_static(&mut self) -> Option<&mut S> {/*...*/}
    pub fn as_articulation_link(&mut self) -> Option<&mut L> {/*...*/}
}

Please notice that every single method in ActorMap requires &mut self, but WakeSleepCallback only gives access to &self, thus making wake_sleep callback unusable as is.

Any physx user can of course cast pointers by herself similar to how ActorMap does it internally, but that's not ideal.

Proposed solution

  1. rename existing cast_map and as_* to cast_map_mut and as_*_mut respectively.
  2. add cast_map and as_* functions which take &self and return Option<&self>

For example:

// instead of this
pub fn as_rigid_dynamic(&mut self) -> Option<&mut D> {/*...*/}

// suggesting to do this
pub fn as_rigid_dynamic(&self) -> Option<&D> {/*...*/}
pub fn as_rigid_dynamic_mut(&mut self) -> Option<&mut D> {/*...*/}
tgolsson commented 1 year ago

Seems sensible for me as described here -- feel free to open a PR with the changes.

I noticed this callback is set up in the examples but never actually used. If you have a use-case, can you maybe extend the examples (ball_physx.rs)? Would help catch regressions in the future too.

rlidwka commented 1 year ago

Seems sensible for me as described here -- feel free to open a PR with the changes.

done

I noticed this callback is set up in the examples but never actually used. If you have a use-case, can you maybe extend the examples (ball_physx.rs)?

There is no actual use-case yet.

I can put a debug-print whenever ball is put to sleep, but I'm not sure how useful it would be.