Michael-F-Bryan / arcs

A Rust CAD System
https://michael-f-bryan.github.io/arcs
Apache License 2.0
253 stars 22 forks source link

Implement add line mode #20

Open DerLando opened 4 years ago

DerLando commented 4 years ago

Super wordy PR, I would love to get some Opinions on the use of States

Also tests fail for me on master branch because impl ApplicationContext for DummyContext does not implement all trait bounds.

Michael-F-Bryan commented 4 years ago

Should we re-use states more? F.e. WaitingToPlaceStart and WaitingToPlaceEnd do similar things

A lot of this is straight copied from AddPointMode maybe there is some layer of abstraction we could implement

After writing up Creating Interactive Applications While Maintaining Your Sanity I'm contemplating whether to switch from nested state machines to a proper pushdown automata model (i.e. stack of state machines where events are sent to the top-most state). That would allow us to reuse a lot of code and massively reduce the amount of boilerplate needed to propagate events to inner state machines, because the propagation is handled by the pushdown automata.

The State trait would be quite similar, except we'd give it on_enter() and on_exit() methods to let it update the world when the state gets pushed/popped from the State stack. We'd also give Transition some Push and Pop variants.

Is it clean to pass around multiple temporary entities between states?

I think that's the best way to do it.

When you're in the middle of an interactive operation there's often a lot of temporary data which needs to persist between intermediate states and the alternative to threading temporaries through the states which use them would be to store them in globals or use some sort of convention for finding the entity in the World (which is just globals in disguise).

I don't think we can really avoid the need for temporary entities. For example, you might be drawing a spline and every time the user clicks on the canvas you transition to PlacingSplineVertex then back to WaitingToPlaceSplineVertex, and both states need to be displaying a preview of the spline being created.

The on_enter() and on_exit() methods from the pushdown automata model can help manage the lifetime of these temporaries though, allowing us to ensure we don't accidentally forget to delete a temporary. A state which needs to transition to another state that uses the same temporary entity might store a Option<Entity> and do self.intermediate.take() when constructing the new state that gets returned with Transition::ChangeState. Then in on_exit() our state would use if let to delete the temporary entity if it wasn't passed to another state (e.g. when we abort an interaction midway through).

Michael-F-Bryan commented 4 years ago

After writing up Creating Interactive Applications While Maintaining Your Sanity I'm contemplating whether to switch from nested state machines to a proper pushdown automata model

It's ironic that this is exactly what I needed to do in our CAD program at work.

We were previously using nested state machines (although C#'s inheritance made it a lot easier) and I ended up needing to rewrite 6000 lines of interactive modes to use a stack of states (the pushdown automata) because simple state machines weren't powerful enough, and made it too easy to introduce bugs or leave the world in a broken state.

DerLando commented 4 years ago

I'm contemplating whether to switch from nested state machines to a proper pushdown automata model (i.e. stack of state machines where events are sent to the top-most state).

We'd also give Transition some Push and Pop variants.

That sounds kind of similar to the command pattern. I came across this pattern when thinking about how to implement undo for arcs. Maybe the pushdown automata composing a stack of commands could be a viable implementation.

The on_enter() and on_exit() methods from the pushdown automata model can help manage the lifetime of these temporaries though, allowing us to ensure we don't accidentally forget to delete a temporary

I love the idea of using lifetimes and the compiler to force us to remember deleting temporary entities!