#28 Added try-catch clause #29
Conversation
|
@KacperKenjiLesniak thank you for the PR! I'll check out the test and see if it passes - in regards to CI, I have a feeling something is broke there (unrelated to this PR). Also sorry for the delay, it seems that I'm not getting e-mails from GitHub 🤔 |
| } | ||
| catch (Exception e) | ||
| { | ||
| if (debug) |
There was a problem hiding this comment.
I think "debug" here is not necessary because Debug.Log is intended to work in Debug mode.
There was a problem hiding this comment.
Hmm, I removed it, but all other Debug.Log are surrounded with this if, so I thought I was following a convention.
There was a problem hiding this comment.
Hmm, I removed it, but all other
Debug.Logare surrounded with this if, so I thought I was following a convention.
Its a bit confusing on how its used now. The initial idea was that those log lines would get replaced with a "stack tracer" which would allow to create an overview of how events connect with each other. Though that never came to fruition :V
| { | ||
| if (debug) | ||
| { | ||
| Debug.Log($"Listener: {listener} of event: {name} has thrown an exception: {e.Message}"); |
There was a problem hiding this comment.
I'd recommend to use Debug.LogException(Exception exception, Object context) (https://docs.unity3d.com/ScriptReference/Debug.LogException.html) as not to consume the stack trace.
There was a problem hiding this comment.
Added Debug.LogException(Exception exception, Object context), but I also left the log informing which listener threw an exception since I think it is very useful for debugging.
|
In regards to failing tests, I think we can ignore them for now and once its merged, I'll run through them and see whats up. |
3 participants
#28