magic-wormhole / magic-wormhole.rs

Rust implementation of Magic Wormhole, with new features and enhancements
European Union Public License 1.2
752 stars 80 forks source link

use `machine` crate for state machines? #65

Closed warner closed 5 years ago

warner commented 5 years ago

I just saw the machine crate.. looks like a nice way to build state machines. Maybe we should use it instead of the manually-written ones?

https://github.com/rust-bakery/machine

One hassle with our current implementation is ownership issues when a new state enum is generated using pieces from the previous state's enum. I had to make a lot of those values Copy so they could be copied freely, but it feels like that shouldn't be necessary (we drop the old state in the process of building the new one, so maybe we could use move instead or something?). I don't know if machine would affect this or not.

warner commented 5 years ago

I also saw references to macro_machine and macro_machines.

machine itself gave me problems because I want to return a list of events from each transition (those events are then dispatched to other state machines, or they cause IO to happen), and machine didn't have a way to support this: the only return value was the new state instance.

I'm moderately happy with our current implementation: we use an Option<State> in each state machine, and remove the State object from the machine (with let oldstate = self.state.take().unwrap()) at the start of the process() method. Then a big match statement switches on the old state and returns the new state (along with pushing any actions/events that need to be returned to a nearby Vec). This way, we can consume the old State, along with anything it contained, and use those pieces to create the new State object. I'm still cloning things like Nameplates left and right, but maybe we can turn those into Rcs or something to reduce the cost a bit.

warner commented 5 years ago

I guess I'll close this for now. If anyone finds a cleaner way to implement our state machines, feel free to reopen it.