davestewart / javascript-state-machine

An expressive, feature-rich, event-driven JavaScript finite-state machine
http://statemachine.davestewart.io
342 stars 25 forks source link

When using start() to go to the initial state, only the system handlers are triggered #25

Open fieldfoxWim opened 6 years ago

fieldfoxWim commented 6 years ago

Hi - me again

As I am progressing in my project, I really get into the deeps of this project.

My question is: when calling start() on the FSM, it goes to the initial state, but does only trigger system:start and system:change. Is there a reason why not all the other transition handlers are triggered?

Context: I am creating a fsm, where every state is an object. When the state becomes active, the state:enter trigger will cause the task to invoke an action. Registering transitions and handlers is done in the parent class State. So when starting the FSM does not fire the enter handler, it breaks the - for me expected - flow.

An example of state: `class CheckBalance extends State { constructor(fsm) { super(fsm); fsm.config.initial = CheckBalance.id; }

async enter() {
    this.fsm.pause();
    const b = await ex.getBalance('eur');
    this.fsm.resume();
}

leave() {
   // wrap up
}

next(fsm) {
    return Final.id; // id of the next state object
}

}`

(any ideas on this are also welcome )

davestewart commented 6 years ago

Interesting. My to do list has a bunch of notes on potentially using classes for states.

Can you work around it with something like :

start () {
  this.enter()
}

?

fieldfoxWim commented 6 years ago

that would probably work if I -only- let my initial state listen to start.

It works already pretty much out of the box to have classes linked to states. It could be extended with having a before-active-after event loop instead of enter and leave. But for now it is clean code and the complexity is manageable.

FYI this is my parent state class:

class State {
    constructor(fsm) {
        this.fsm = fsm;

        this.fsm.add(this.constructor.name, '*', this.constructor.name);
        this.fsm.on(this.constructor.name, () => { //+ ":enter"
            this.status = 'active';
            this.enter();
        });
        this.fsm.on(this.constructor.name + ":leave", () => {
            this.status = 'inactive';
            this.leave();
        });

        this.fsm.add({action:'next', from:this.constructor.name, to:this.next})

        this.status = "inactive";
    }

    enter() {  throw new Error('You have to implement the method enter!');}
    leave() { throw new Error('You have to implement the method leave!');}
    next(fsm) { throw new Error('You have to implement the method next!');}

    static get id() {
        return this.name;
    }
}
davestewart commented 6 years ago

Hey, I like what you're doing here.

I wrote SM pre-using ES6, so hadn't really considered this kind of usage.

Can you post a JS fiddle, Plunkr or CodeSandbox with working code?

davestewart commented 6 years ago

I've just been testing this with the event handler playground:

image

It makes complete sense to include enter along with start.

I'll look to patch this. It feels like a breaking change, so I may have to bump the major number to 2.

fieldfoxWim commented 6 years ago

may have to bump the major number to 2

Sounds like a good idea

I have spend quite some time on that page :)

davestewart commented 6 years ago

I have spend quite some time on that page :)

I hope it was helpful!

davestewart commented 6 years ago

I'm thinking:

1 system.start
2 state.enter *
3 state.enter a
4 system.change

But, as I said, I've not touched this for a couple of years, nor looked at the code.

Seems logical?

fieldfoxWim commented 6 years ago

yep seems logical, and would solve my problem!

All your examples are very useful. I tested some other fsm frameworks as well, yours was by far the easiest to build a bridge to using objects as states.

Can you post a JS fiddle, Plunkr or CodeSandbox with working code?

I will create an example (I am a fan of gist.run, as I am also en big aurelia.io fan)

davestewart commented 6 years ago

yours was by far the easiest to build a bridge to using objects as states

Thanks!

I'd always hoped to release a version 2 which would use classes to help me bridge to a HFSM!

Its been usurped by other projects though, of course :P

davestewart commented 6 years ago

So I took a look at this last night, and I think it's going to take a bit more work than I expected, mainly due to not looking at the code for so long.

Not sure when I will get to look at it, but the whole project has been on my list to look at for a while. Maybe now is a good time.

I can't see anything being updated in the next few days though, so I'd suggest a workaround in the meantime.

fieldfoxWim commented 6 years ago

Hi Dave

I will also have a look on how I would fix it, matching your coding style. But a workaround would work as well of course. Thanks

I have been trying to create a jsFiddel, but having issues to run ES6 code. I actually use your fsm in a NodeJS server application, and the states are invoked via a websocket in an Aurelia App. I will get it up, just taking more time then I expected.

davestewart commented 6 years ago

I also want to refactor the project to ES6, and probably make a few changes, primarily to extract the state syntax DSL to a plugin, which would allow the core to be much lighter.

I'd forgotten how much is going on under the hood!

fieldfoxWim commented 6 years ago

(off topic: are you actually using this project in a production app?)

Might be a good idea to do that in a different project, in the meanwhile get this version bugfree, as it already fits the purpose.

davestewart commented 6 years ago

off topic: are you actually using this project in a production app?

No. I wrote it and thought I would get way more use out of it than I did

Might be a good idea to do that in a different project, in the meanwhile get this version bugfree, as it already fits the purpose.

It will simply be a major version bump with upgrade docs

The upgrade path will simply be something like:

import StateMachine, { StateDSL } from 'state-machine'

StateMachine.use(StateDSL)
fieldfoxWim commented 6 years ago

Any plans on updating the production NPM package to the latest develop branch? That would make my life a bit easier ;)

thx

davestewart commented 6 years ago

Oh bum, sorry. Will do that now!

davestewart commented 6 years ago

Published :)

fieldfoxWim commented 6 years ago

Published :)

I notice that my pull request was done to the develop. Do I need to do a pull request to the master, or do you merge it. As your master branch does not contain my fix currently

davestewart commented 6 years ago

Not sure to be honest.

I'm using Git Flow, but it seems people are finding it less and less useful these days as work is generally done in a feature branch then pushed to master. So I might just get rid of the Develop branch.

The fixes made it into the NPM release though, right?

fieldfoxWim commented 6 years ago

I am using Git Flow as well, that is why I commit to the dev branch and you still need to merge de dev into the master.

The NPM release does not contain the fix from my pull request

davestewart commented 6 years ago

I've taken a look and I think the .min version should.

I've actually started using a new build process for my other libs, so I'll migrate this to use that, maybe tomorrow on the way to work if I get a seat!

davestewart commented 6 years ago

OK, I've updated the build system with Rollup and the code is in. I'll need to check everything over, but if all is good, no reason why I can't push the updates later today