Using PermitIf on a state with substates seems to lead to reentry about stateless HOT 8 CLOSED

I think it's about time to merge in #544, bump the version number to 5.13.1, and make a new release :-)

from stateless.

I think that this should not have happened.

If the state machine is in SubstateA, then the trigger should be handled by the super state handler. So far, so good.
But the state doesn't change, so the state machine should not execute the onEntry action.

So I would say this is a bug.

from stateless.

Thanks

from stateless.

@HenningNT
in an attempt to be helpful I did create this unit test in the Stateless solution. and implemented the fix this way.. .(I'm not super happy with this fix as I'm not sure it is how you might implement this fix. (for example, in the "findhandler" maybe that should somehow return an "ignoretriggerbehaviour" result. trying to have the result.handler.ResultsInTransitionFrom return false cause the "not trigger defined" exception in the default case. Either of those feel better than what I proposed here but I wanted to start the conversation at least"

If you have other suggestions on how this should be addressed I'd be happy to modify my changes and open a PR.
If you like this and would like me to open a PR I'm fine doing that too.

Unit test:

    public void WhenInSubstate_TriggerSuperStateTwiceToSameSubstate_DoesNotReenterSubstate()
    {
        var sm = new StateMachine<State, Trigger>(State.A);
        var eCount = 0;
        sm.Configure(State.B)
            .OnEntry(() => { eCount++;})
            .SubstateOf(State.C);
        sm.Configure(State.A)
            .SubstateOf(State.C);
        sm.Configure(State.C)
            .Permit(Trigger.X, State.B);
        sm.Fire(Trigger.X);
        sm.Fire(Trigger.X);
        Assert.Equal(1, eCount);
    }

code change in statemachine.cs (line 424ish)

             case TransitioningTriggerBehaviour _ when result.Handler.ResultsInTransitionFrom(source, args, out destination):
             {
                 //If a trigger was found on a superstate that would cause unintended reentry, don't trigger.
                 if (source.Equals(destination))
                     break;
                 // Handle transition, and set new state
                 var transition = new Transition(source, destination, trigger, args);
                 HandleTransitioningTrigger(args, representativeState, transition);
                 break;
             }

from stateless.

Hi, I'm just checking in on this and my pull request. Is there anything more I can be doing to help move this forward? Thanks!

from stateless.

Ping! @crozone @mclift

from stateless.

Ping! @crozone @mclift

Thanks for the ping. LGTM and tests are passing. I'll give @mclift a chance to review this but I think I understand what's going on now. If I understand it, the superstate is transitioning to itself with TransitioningTriggerBehaviour because only the primary substate will use the InternalTriggerBehaviour case which has been set up, so this effectively kills that edge case.

from stateless.

It's been a little while since the last release. There was a discussion about bumping the major version number as we've changed the targeted frameworks to the LTS versions, but on reflection, this probably doesn't warrant a major release on its own. So maybe 5.14.0?

Anyway, I'll get on that :)

from stateless.