Potentially useful pattern for working with complex state machines with shipyard ECS

[snyball]

I've been working on a simulation project that heavily utilizes shipyard (great project!) and have come to a point where I struggle a little bit with code organization in quickly evolving complicated state machines that access ECS storages. For splitting up the code into more easily readable bits, I previously simply used functions, like so:

fn florb(ctrl: &mut Ctrl) {
    // florbing ...
}
fn flirp(thingy_two: &mut ThingyTwo, b: &mut PhysicsBody, s: &mut RigidBodySet) {
    // flirping ...
}
fn frobnicate(ctrl: &mut Ctrl, thing: &mut thing) {
    // frobnicating
}

pub trait ViewBundle<T: EntityView> {
    fn update_view(view: Self);
}

pub fn update_state_machine_view(ctrl: ViewMut<'a, Ctrl>,
                                 thing: ViewMut<'a, Thing>,
                                 rigid_set: UniqueViewMut<'a, RigidBodySet>,
                                 b: View<'a, PhysicsBody>,
                                 thingy_two: View<'a, ThingyTwo>)
{
    for (ctrl, thing, thingy_two, b) in (&mut ctrl, &mut thing, &thingy_two, &b).iter() {
        match ctrl.state {
            Ctrl::State1 => {
                florp(ctrl);
                flirp(thingy_two, b, s);
            }
            Ctrl::State2 => {
                frobnicate(ctrl, thing);
                flirp(thingy_two, b, s);
                thingy_two.take_damage(1);
            }
        }
    }
}

This can work fine, but it introduces some annoying problems. First off there's a lot of particularly annoying (to me) boilerplate with the loop and placing the &mut and & borrows correctly. Secondly, and most importantly, it requires manually passing each borrow to the various helper functions, introducing more boilerplate again. It also becomes more difficult when the system gets big, and you suddenly realize that flirp, frobnicate, and florp all need to suddenly access 2-3 new storages, and you need to go over it again. Even more annoying, if for example frobnicate calls frobnicate_flarp, which calls frobnicate_larp and suddenly you need to add new arguments to all previous functions in the chain when frobnicate_larp needs to access a new storage, this creates a lot of noise.

Granted, this isn't an enormous issue, but it annoys me enough personally that I wanted a solution, and this is the pattern I suggest (pseudocode):

#[derive(Borrow, BorrowInfo)]
pub struct StateMachineView<'a> {
    ctrl: ViewMut<'a, Ctrl>,
    thing: ViewMut<'a, Thing>,
    rigid_set: UniqueViewMut<'a, RigidBodySet>,
    b: View<'a, PhysicsBody>,
    thingy_two: View<'a, ThingyTwo>,
}

struct StateMachine<'a> {
    ctrl: &'a mut Ctrl,
    thing: &'a mut Thing,
    rigid_set: &'a mut RigidBodySet,
    thingy_two: &'a ThingyTwo,
    b: &'a PhysicsBody,
}

pub trait EntityView {
    fn update_entity(&mut self);
}

impl<'a> EntityView for StateMachine<'a> {
    // Entry point for stepping the state machine
    fn update_entity(&mut self) {
        match self.ctrl.state {
            Ctrl::State1 => {
                self.florp();
                self.flirp();
            }
            Ctrl::State2 => {
                self.frobnicate();
                self.flirp();
                self.thingy_two.take_damage(1);
            }
        }
    }
}

// Helper methods that all have access to the storage for this entity, to
// organize code better
impl<'a> StateMachine<'a> {
    fn florb(&mut self) {
        // florbing ...
    }
    fn flirp(&mut self) {
        // flirping ...
    }
    fn frobnicate(&mut self) {
        // frobnicating
    }
}

pub trait MagicView<T: EntityView> {
    fn update_view(view: Self);
}

impl<'a> MagicView<StateMachine<'a>> for StateMachineView<'a> {
    fn update_view(state_machine_view: StateMachineView) {
        let StateMachineView { mut ctrl, mut thing, mut rigid_set, thingy_two, b } = state_machine_view;
        for (ctrl, thing, thingy_two, b) in (&mut ctrl, &mut thing, &thingy_two, &b).iter() {
            StateMachine { ctrl, thing, thingy_two, b, rigid_set: &mut rigid_set }.update_entity();
        }
    }
}

I am thinking of writing a proc-macro that automates this to something like

#[shipyard::magic_view(StateMachine)]
#[derive(Borrow, BorrowInfo)]
pub struct StateMachineView<'a> {
    ctrl: ViewMut<'a, Ctrl>,
    thing: ViewMut<'a, Thing>,
    rigid_set: UniqueViewMut<'a, RigidBodySet>,
    b: View<'a, PhysicsBody>,
    thingy_two: View<'a, ThingyTwo>,
}

impl<'a> EntityView for StateMachine<'a> {
    // Entry point for stepping the state machine
    fn update_entity(&mut self) {
        match self.ctrl.state {
            Ctrl::State1 => {
                self.florp();
                self.flirp();
            }
            Ctrl::State2 => {
                self.frobnicate();
                self.flirp();
                self.thingy_two.take_damage(1);
            }
        }
    }
}

// Helper methods that all have access to the storage for this entity, to
// organize code better
impl<'a> StateMachine<'a> {
    fn florb(&mut self) {
        // florbing ...
    }
    fn flirp(&mut self) {
        // flirping ...
    }
    fn frobnicate(&mut self) {
        // frobnicating
    }
}

Again, this issue is perfectly solvable without resorting to such a pattern, and a new proc-macro, but I feel like this pattern allows for much faster and easier iteration on systems, especially when you know that they can get quite large and will change a lot.

Let me know if this is something you're interested in having in shipyard, and I'll keep that in mind when creating the proc-macro so that I can create a pull request against master later.

Do also let me know if you are aware of a better and easier pattern that accomplishes the same goal, which would save me some work :)

Unresolved issues

• Naming bikeshedding
• Common processing that happens before the loop, sometimes you want to do something like this:

pub fn update_state_machine_view(ctrl: ViewMut<'a, Ctrl>,
                                 thing: ViewMut<'a, Thing>,
                                 rigid_set: UniqueViewMut<'a, RigidBodySet>,
                                 b: View<'a, PhysicsBody>,
                                 thingy_two: View<'a, ThingyTwo>)
{
    let common = rigid_set.expensive_operation();
    for (ctrl, thing, thingy_two, b) in (&mut ctrl, &mut thing, &thingy_two, &b).iter() {
        match ctrl.state {
            Ctrl::State1 => {
                florp(common, ctrl);
                flirp(common, thingy_two, b, s);
            }
            Ctrl::State2 => {
                frobnicate(common, ctrl, thing);
                flirp(common, thingy_two, b, s);
                thingy_two.take_damage(1);
            }
        }
    }
}

• It is possible to allow for this, but I need to do some thinking about exactly how that should work
• EDIT: On second thought, this kind of stuff belongs in a UniqueStorage

[colelawrence]

@leudz could respond to this more eloquently, but I have an idea that might work or it might be an antipattern.

For what you are doing, I would consider simply making each helper system function a separate system fn that has an initial guard check to check if Ctrl is the right state.

So, now, all your enum values effectively act as a tag-like/state-like component (e.g. in Minecraft you could have a 'Following { None, Player(PlayerId) }')

Alternatively, though, perhaps the variants of this enum are promoted to individual components, so the actual system definition filters to the right entities automatically.

I think the approach to implementing methods into a borrow structure looks convenient, but I wonder if there are negative implications to not keeping all the systems flattened more.