Issue with context lifetimes

[DylanRJohnston]

Hey all, I've been trying to integrate this library into Bevy for helping managing entity states. However I've encountered some issues with the borrow checker and passing in mutable references via Context to the state machine. I believe the root of the issue is that in IntoStateMachine type Ctx<'ctx> only has a single lifetime, which is not expressive enough to pass around mutable references to structs that contain mutable borrows themselves.

I've tried to reduce the problem to some minimal examples

mod example_broken {
    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    struct Context<'a, 'b> {
        event: &'a mut Event<'b>,
    }

    type Ctx<'a> = Context<'a, 'a>;

    fn takes_context(context: &mut Ctx<'_>) {
        unimplemented!()
    }

    fn example(mut event: Event) {
        // Error: Event does not live long enough
        takes_context(&mut Context { event: &mut event })
    }
}

This example I think shows the root of the problem, the type alias type Ctx<'a> = Context<'a, 'a> forces the lifetime of the borrow 'a in event: &'a mut Event<'b> to be the same as the data borrowed by Event 'b. In this case, Event contains data borrowed from the ECS system, whereas 'a only lives for the lifetime of example. Hence the problem, it wants event in example to life as long as the data borrowed from the ECS system.

If we allow type Ctx<'a> to have two lifetime parameters type Ctx<'a, 'b> this is enough to allow the borrow checker to track the lifetimes independently.

mod example_working {
    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    struct Context<'a, 'b> {
        event: &'a mut Event<'b>,
    }

    type Ctx<'a, 'b> = Context<'a, 'b>;

    fn takes_context(context: &mut Ctx<'_, '_>) {
        unimplemented!()
    }

    fn example(mut event: Event) {
        takes_context(&mut Context { event: &mut event })
    }
}

Another solution I explored was adding lifetime constraints to the function itself, however this makes the function longer a valid system for bevy.

mod example_lifetime_constraints {
    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    struct Context<'a, 'b> {
        event: &'a mut Event<'b>,
    }

    type Ctx<'a> = Context<'a, 'a>;

    fn takes_context(context: &mut Ctx<'_>) {
        unimplemented!()
    }

    fn example<'event, 'value>(event: &'event mut Event<'value>)
    where
        'event: 'value,
    {
        takes_context(&mut Context { event: event })
    }
}

Another approach is to use Rc<RefCell<Event>> which lets us eliminate the compile checking of the problematic borrow of event so that Context only has a single lifetime and is thus compatible with the type Ctx<'ctx> definition.

mod example_ref_cell {
    use std::{cell::RefCell, rc::Rc};

    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    impl<'a> Event<'a> {
        fn mutate(&mut self) {
            unimplemented!()
        }
    }

    struct Context<'a> {
        event: Rc<RefCell<Event<'a>>>,
    }

    type Ctx<'a> = Context<'a>;

    fn takes_context(context: &mut Ctx<'_>) {
        context.event.borrow_mut().mutate()
    }

    fn example(mut event: Event) {
        let rc = Rc::new(RefCell::new(event));

        for i in 1..3 {
            takes_context(&mut Context { event: rc.clone() })
        }
    }
}

Curiously, if you pass the &mut event directly i.e. Context is just a type alias, it works?

mod example_bare {
    struct Foo;

    struct Event<'a> {
        value: &'a mut Foo,
    }

    impl<'a> Event<'a> {
        fn mutate(&mut self) {
            unimplemented!()
        }
    }

    type Context<'a> = Event<'a>;

    type Ctx<'a> = Context<'a>;

    fn takes_context(context: &mut Ctx<'_>) {
        context.mutate()
    }

    fn example(mut event: Event) {
        for i in 1..3 {
            takes_context(&mut event);
        }
    }
}```

[DylanRJohnston]

Given this, would you be open to a Pull Request that changed type Ctx<'ctx> to have two lifetime parameters instead? Given the stated purpose of Ctx is to allow the passing of mutable references into the State Machine it would seem two lifetime parameters are required to do so safely.

[mdeloof]

Hi Dylan

Would reborrowing value be an option for you?

mod example_reborrow {
    struct Foo;
    
    struct Event<'a> {
        value: &'a mut Foo,
    }
    
    struct Context<'a, 'b> {
        event: &'a mut Event<'b>,
    }
    
    type Ctx<'a> = Context<'a, 'a>;
    
    fn takes_context(context: &mut Ctx<'_>) {
        unimplemented!()
    }
    
    fn example(mut event: Event<'_>) {
        // Reborrow `value`
        let mut event = Event { value: &mut event.value };
        takes_context(&mut Context { event: &mut event })
    }
}

I must admit support for Bevy was a bit of an afterthought. I've been following its development from a distance, and don't have much personal experience with integrating statig with Bevy. I only added the bevy feature when I saw people were interested in using it.

[DylanRJohnston]

Unfortunately the fields of EventWriter are private so I don't think I can re-borrow it but that's a neat trick I'll definitely use in the future.

I understand that Bevy support is not priority for this library, but wouldn't the lifetime issue occur whenever Context involves borrowed mutable data?

[DylanRJohnston]

My plan was to have the StateMachine be pure / effectless (for easier testing, e.g. property / model testing) and emit an event stream that other systems could subscribe to, to perform mutations on the game state. The Rc<RefCell<EventWriter<'_, _>>> works well enough for now. I don't think it carries much overhead.

Also trying to figure out the best way to model TimeoutEvent's, as the StateMachine can't manage the timers itself as it only runs on events. A naive approach of using a single TimeoutEvent could lead to some weird race conditions where TimeoutEvents get mixed up when a state transition occurs for another reason before the Timer triggers. I need some way to associate a timeout event with a given timer. It'd be nice if the StateMachine.handle(..) could more easily return some data from the state machine itself.

[DylanRJohnston]

I could also use some kind of intermediary event stream that I can give to the StateMachine and then pipe it into the Bevy ECS EventWriter.