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

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

Open
agaertner opened this issue Dec 26, 2020 · 7 comments
Open

Comments

@agaertner
Copy link

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

I agree, this is annoying.

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

@DeepakParamkusam
Copy link
Contributor

DeepakParamkusam commented Jan 5, 2021

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

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

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

@agaertner
Copy link
Author

agaertner commented Jan 6, 2021

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

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

gusbzs commented Oct 26, 2021

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));

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

4 participants