OSVR / OSVR-Unity

Package for authoring OSVR experiences with Unity.
Apache License 2.0
99 stars 38 forks source link

Better error handling #124

Closed DuFF14 closed 8 years ago

DuFF14 commented 8 years ago

Stops spamming of error messages when an HMD is not connected or the server has not started, by creating flags that are set when those errors are detected, and only printing each error message once. If those errors are resolved, another message is printed indicating the previous errors can be ignored.

Also includes some indentation and spacing issues in DisplayController.cs, and adds [OSVR] prefix to most debug log messages which aren't obvious that they are coming from OSVR, so it's immediately clear which messages relate to the OSVR-Unity plugin.

DuFF14 commented 8 years ago

Also added a try/catch around CreateRenderManager(). If it fails due to not being able to load the DLL, we fallback to working with non-RenderManager functionality.

JeroMiya commented 8 years ago

Could we use "[OSVR-Unity]" for messages from the plugin itself? Eventually I'd like to be able to implement a proper logging system for OSVR-Core so we can redirect those messages to the game-engine-specific or OS-specific logging API.

DuFF14 commented 8 years ago

Sure. When you say "from the plugin itself", do you mean everything in the repo, or only in the main scripts (not the samples)? For example, should messages in SampleAnalog.cs or ColorManager.cs have this prefix?

JeroMiya commented 8 years ago

Not sure, maybe OSVR-Unity-Samples for the sample code?

DuFF14 commented 8 years ago

That works. Updated with those changes.

JeroMiya commented 8 years ago

Sorry for the delay. This looks good. Feel free to merge.

russell-taylor commented 8 years ago

Is RenderManager throwing on failure? I thought it would return false...

DuFF14 commented 8 years ago

No, Unity throws a DLLNotFoundException. This is for cases when the server is configured for RenderManager (now the default) but the osvrUnityRenderingPlugin.dll isn't in the Plugins folder of the Unity project (also the default for now).

DuFF14 commented 8 years ago

@demonixis I'd prefer not to allow developers to be able to toggle whether a server error was detected or not, as this should be determined by the checkStatus() API call. The flag is only there to ensure the message prints once. What would be the benefit of toggling it?