Auto-debug ExtCitizenInstanceManager.StartPathFind() #834
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If an exception occurs in `StartPathFind()` we don't get much information in the logs to help us determine what caused it. This commit tries to remedy that by intercepting the exception then checking a `TernaryBool` (TB): * `Undefined` = Set the TB to `True` * `True` = Set the TB to `False` * `False` = Do nothing The original exception is then rethrown. When the TB is `True`, the internal debug logging (all of it) is enabled for the next invocation of `StartPathFind()` which will dump all sorts of useful info to the `TMPE.log` file. When an exception occurs, it's usually recurrent on subsequent calls to `StartPathFind()` so hopefully the debug logging will give us insight in to what specifically caused the problem.
originalfoo
added
technical
|
Actually, I can just repeat the call to the internal method to debug the specific params that caused the error... update incoming... |
Rather than debugging the subsequent invocation, just repeat the current one (the one that failed) but with debugging enabled.
Most of the logging was previously filtered in non-`DEBUG` builds. This commit changes that to ensure the logging is always present, even in `RELEASE` builds, but only occurs if the relevant logging toggles are `true`. Downside is that there are now lots of checks for those toggles.
Make sure only one set of trace goes in to log, and reduce amount of stack tracing. Log should be a bit easier to digest now.
User will see error dialog per error anyway, so might as well do the full debug log per error to get more info.
|
I've updated this so now it does the logging on each occurrence of the error, simplifying state management in the process. User has to click on-screen error box anyway so they won't be caring about split second it takes to do the full debug log. |
krzychu124
requested changes
Apr 13, 2020
Unity has weird `==` operator, that returns fake null - see: https://blogs.unity3d.com/2014/05/16/custom-operator-should-we-keep-it/ More details in #834 (review)
|
@krzychu124 is this ok for merge? |
krzychu124
approved these changes
Apr 14, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #840
If an exception occurs in
StartPathFind()we don't get much information in the logs to help us determine what specifically caused it.This commit tries to remedy that by intercepting the exception, then repeating the
StartPathFind()only this time with full debug logging. The original exception is then rethrown.The only performance overheads during normal operation should be:
try .. catchtrue|falseon each invocation (previously they wereconstandfalseinRELEASEbuilds.)if (logParkingAi) { ... }), one for each log callInternalStartPathFind()only a few will be encountered.Hopefully the performance impacts will be negligible, but that remains to be tested.
This PR is result of by Nijamods (ninjanoobslayer) issue report in #825 (comment) and subsequent log files in #825 (comment) . I've sent a build of TMPE with this modification to Ninja to test so we should know if it works with followup in #825.