search

dotnet-state-machine / stateless

A simple library for creating state machines in C# code
Other
5.43k stars 749 forks source link

PermitIf == false should not trigger OnUnhandledTrigger exception. #412

Open agaertner opened 3 years ago

agaertner commented 3 years ago

Apparently, when the condition in PermitIf returns false and does not allow transition it throws an OnUnhandledTrigger exception.

you can see this in my code here:

https://github.com/TybaIt/Blish-HUD-Modules/blob/5d1265dc5519263263bccd9b6f3c993cd67bb129/Music%20Mixer%20Module/Gw2StateService.cs#L153 The logs were getting spammed when the _toggle was false (and thus PermitIf didn't allow transition) by doing a switch case in the override of OnUnhandledTrigger

HenningNT commented 3 years ago

I agree, this is annoying.

If I find the time I might change the behaviour, but not in the near future.

DeepakParamkusam commented 3 years ago

I disagree.

Correct me if I am wrong, but how else can we know during runtime that a valid trigger was received but a guard condition failed? We use this feature extensively in our application where silently failing is not an option.

HenningNT commented 3 years ago

Of course, there are many use cases, and many ways to use the library. If it's an requirement to keep track of failed guard condition, then the OnUnhandledTrigger event is very convenient.

As a compromise, we could maybe add a OnGuardFailed in addition to OnUnhandledTrigger? One would fire if there are no transitions at all, and the other if there are no valid (i.e. all guards fail) transitions available.

But I don't see this as the most important matter right now...

DeepakParamkusam commented 3 years ago

That would be nice, but it would be a breaking change and I'm always wary of those.

agaertner commented 3 years ago

The compromise is a great idea but it does not solve my initial issue with the throw definition of said exception. If you declare any Permits it should not throw for the corresponding trigger regardless of the returning state of the guard. With a Permit they are handled, not unhandled as the exception suggest.. It's not "unhandled" just because it wasn't allowed on runtime. The wording makes no sense and OnUnhandledException scope is much bigger than its name suggests.

Best Regards

HenningNT commented 3 years ago

I agree, the name of the exception is somewhat misleading, when the trigger has been "handled", but the guard function prevents the transition from completing.

gusbzs commented 2 years ago

How about allowing to complement "PermitIf" with "IgnoreIf" mutually exclusive configurations? For example:

    stateMachine.Configure(State.FileLoaded)
        .PermitIf(Trigger.StartReload, State.LoadingFile, () => File.Exists(filename))
        .IgnoreIf(Trigger.StartReload, () => ! File.Exists(filename));