Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fieldfoxWim opened this issue Sep 25, 2018 · 22 comments

Comments

@fieldfoxWim
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

fieldfoxWim commented Sep 25, 2018

may have to bump the major number to 2

Sounds like a good idea

I have spend quite some time on that page :)

@davestewart
Copy link
Owner

I have spend quite some time on that page :)

I hope it was helpful!

@davestewart
Copy link
Owner

davestewart commented Sep 25, 2018

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
Copy link
Author

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
Copy link
Owner

davestewart commented Sep 25, 2018

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

(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
Copy link
Owner

davestewart commented Sep 27, 2018

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
Copy link
Author

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

thx

@davestewart
Copy link
Owner

Oh bum, sorry. Will do that now!

@davestewart
Copy link
Owner

Published :)

@fieldfoxWim
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants