Passing mutable references to BroadPhase::update callbacks

[z33ky]

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). 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<ColliderIndex> in World::collide to actually handle collisions afterwards, but I think this could be solved by providing a

BroadPhase::update_with_callback_parameter<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.

[sebcrozet]

I see the limitation here. Filling a Vec<ColliderIndex> seems to be the cleanest way to do this with the current implementation.

The issue with your proposal is that BroadPhase::update_with_callback_parameter<Parameter> will make the BroadPhase impossible to use as a trait-object because it takes a type parameter. I'm not sure what other solution would be viable (besides passing a &mut Box which is ugly).

[Ralith]

This could be solved well by replacing the two callbacks with a trait object:

trait BroadPhaseFilter<T> {
    fn allow_proximity(&mut self, &T, &T) -> bool;
    fn proximity_handler(&mut self, &T, &T, bool);
}

trait BroadPhase<T> {
    fn update(&mut self, &mut BroadPhaseFilter<T>);
}

A mutable reference can safely be stored inside an implementer and used in both methods.

Alternatively, to reduce boilerplate, a single callback could be used, taking a parameter that tells it which behavior is desired for any given invocation. The trait object approach seems less confusing to me, though.

[sebcrozet]

Thanks @Ralith ! The trait-object based approach seems to be the right approach.