FlexBE / flexbe_app

The classic user interface (editor + runtime control) for the FlexBE behavior engine. See the flexbe_webui for latest
BSD 3-Clause "New" or "Revised" License
48 stars 49 forks source link

Record and log errors when parsing states with Python #57

Closed LoyVanBeek closed 1 year ago

LoyVanBeek commented 4 years ago

I made a typo while testing the subclassing of EventState (which works great in itself, saving a lot of code duplication!), causing an error to be thrown during the Python parsing

This caused the state not to be shown and some other neither.

This PR adds some basic logging for that but is lacking a way to tell the (superclass)state definition has an error. Currently the state doesn't appear in the menu but that is not ideal I guess.

TODO:

LoyVanBeek commented 4 years ago

@pschillinger what would be an appropriate way to indicate to a user that there is an issue with a state definition?

pschillinger commented 4 years ago

Thanks for working on this!

For communicating issues to the user, the global symbol T refers to the built-in terminal. You can use T.logWarn(text) for warnings that are logged in the background and can be seen whenever the user actively checks for it or T.logError(text) for errors that open the terminal themselves so that the user will see it.

There is also a way to show small pop-up messages (like the one telling you that there is no behaviors package), but I try to avoid using it in most cases.

LoyVanBeek commented 4 years ago

Hi @pschillinger, this PR has a merge conflict and I'm not sure which to pick.

The main conflict is that my PR catches exceptions and puts them in a new 'error' field for later display and the version now on master re-raises exceptions.

dcconner commented 1 year ago

I'm planning to have some students work on FlexBE in the coming weeks. It looks like this is reasonable , but will require some clean up. @LoyVanBeek - Have you been using this addition recently? Any status updates?

LoyVanBeek commented 1 year ago

Have not been using this, it's apparently not even merged into my own develop branch.

dcconner commented 1 year ago

At this point, it seems the current approach of passing of exception upward will trigger notification. I'm going to close this for now