dimforge / dimforge/ncollide

Passing mutable references to BroadPhase::update callbacks

Open
#224 3 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
926
Forks
104
PR merge metrics
No merged PRs in 30d

Description

`BroadPhase::update` takes two callback-functions, `allow_proximity` and `proximity_handler`.
In my code I am not storing the objects themselves in the `BroadPhase`, but indices to them (I have a minified version [here](https://gist.github.com/z33ky/4031121d0a95f9190e831d179c32c2fb)).
I am unable to obtain the actual objects from the indices in both callbacks if one is mutable, since Rust only allows a singular mutable reference. The problem in the minified code is at line 81, where `entities` is borrowed mutably for the `proximity_handler` and immutably for the `allow_proximity`.
I could use a `RefCell`, `unsafe` or pass an immutable reference to both and fill a `Vec` in `World::collide` to actually handle collisions afterwards, but I think this could be solved by providing a

```
BroadPhase::update_with_callback_parameter(
&mut self,
allow_proximity: &mut FnMut(&Parameter, &T, &T) -> bool,
proximity_handler: &mut FnMut(&mut Parameter, &T, &T, bool),
param: &mut Parameter
)
```

method, which passes `param` to both callbacks. Line 81 in the minified example would become
```
broad.update_with_callback_parameter(
&mut |entities, c0, c1| Self::collides_with(entities, c0, c1),
&mut |entities, c0, c1, started| Self::collide(entities, c0, c1, started),
entities
);
```
without violating borrow checks.
`BroadPhase::update` can simply be implemented via
```
self.update_with_callback_parameter(&mut |(), a, b| allow_proximity(a, b), &mut |(), a, b, started| proximity_handler(a, b, started), &mut ())
```
.

Now when three circles collide as the same time, it will depend on the evaluation order which two circles die and which is kept alive, but this is just some example code anyways.

`allow_proximity` could also take a `&mut Parameter` instead of `&Parameter`, just for my specific use-case, `allow_proximity` only needs a immutable reference. It would likely depend on what `Parameter` actually is to determine whether it should be mutable or not, though a `FnMut(&Parameter, &T, &T)` can be implicitly converted to a `FnMut(&mut Parameter, &T, &T)`.
I like to prevent "accidental mutability" though; A separate `update_with_callback_parameter_mutable_allow`-method can be added and `update_with_callback_parameter` can call that.
A better name would be desirable.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.