Fix logging in Unity Editor (closes #66)

[bawahakim]

As described in the issue #66, Logging does not work as expected in Unity Editor when using the binary dll file. It seems the cause is that we check for the pragma #if UNITY_EDITOR, however the dll file is built by dotnet and does not have this pragma attached, so it compiles with Console.WriteLine()

Changing the pragma to #if RELEASE and setting the build configuration to Release should fix the problem. Can only be tested in the next release.

[matus-tomlein]

Thanks for the contribution @bawahakim and sorry for getting to you a bit late.

One thing that I'm struggling to understand is whether we should limit the use of the UnityEngine.Debug.LogError API only to release builds and if so, why only release builds?

To simplify the behaviour, I think we could just always use the LogError API. As far as I could test, this also outputs to the standard console if Unity Editor is not used (although it's more verbose). Do you see any issues with that?

If we choose to use it only conditionally, I'm not sure if we should do so under release builds – I would expect that one wants to get the logs when debugging their app. On the other hand, they might not want to get such logs when publishing a release build for users.

[bawahakim]

Welcome @matus-tomlein !

To clarify, the conditional for release is only to build the DLL, not for any consumer projects that uses the DLL. That means it doesn't matter in Unity if it's release or debug. I would imagine the release here on GitHub should always be production release, not a debug one (debug should be for local development of the C# project). As I understand, the reason that there is a condition to use Console.Writeline is when working locally on the C# project (without Unity).

Have you tried importing the latest main release DLL (not the source code) into a Unity project? The logging doesn't work in the editor (at least for me). As I understand, it's because the DLL was built with a pragma that wasn't there to enable Unity.Debug.Log, so Unity only outputs to Console.Writeline (which doesn't work in the editor). In other words #if UNITY_EDITOR is not defined when we build the DLL, so the code within it gets stripped, and Unity never knows about it.

On a different topic, would it make more sense to release the source code instead of a DLL, so people can do their local changes there, instead of a fork/edit/build process? What is the reason to ship a DLL?

[matus-tomlein]

That's actually a very good point – the problem comes from the fact that we distribute the package with a precompiled DLL that was built without the proper UNITY_EDITOR macro configuration. The ideal solution would be to change the way we distribute the package.

This is actually something that we have been considering to address in the next releases of the Unity tracker. We would like to adopt the package structure used in Unity Package Manager and distribute the package using a registry such as OpenUPM. I have created an issue to follow up on that work.

I don't see a good solution for this issue without taking that step. If we decide on the logging output based the RELEASE macro as you have suggested, it will mean that we use the Unity.Debug.Log output in all cases, also when the package is distributed in an app without Unity Editor connected. One option would be to have two DLL variants of the package – one compiled with UNITY_EDITOR and one without and choose which DLL to use based on the current build configuration. But I think that would be an overkill for the problem.

I would suggest that instead of doing a partial solution, we do this right and resolve this issue as part of issue #75. That will ensure that the logging output is selected based on the presence of the UNITY_EDITOR macro in the app build configuration as intended.

[bawahakim]

Got it. I had assumed that the SDK was Unity specific (some Unity code is required here and there like in the EventStore). If it's meant to also work outside of Unity, then yes it's probably better to wait for #75

[matus-tomlein]

@bawahakim Sorry maybe I'm misunderstanding something key here. My only concern with using RELEASE in the macro is that it doesn't allow us to distinguish between these two cases:

1. The tracker is used by app developer and connected to the Unity Editor so we want the logs to show up in the console in the editor – that's when we should use Unity.Debug.Log
2. The tracker is used in an app distributed on an App Store (for example iOS) and used by a user. In that case we should prefer to output to console using Console.WriteLine so that the logs are more readable and can be collected by a logging system such as Sentry to be analysed by developers.

The SDK is Unity specific. Please correct me if I'm missing something here, to be honest, I don't have deep experience with developing in Unity.

[bawahakim]

Ah I see. At least in my experience (also use Sentry in our project) we always use Debug.Log for any logging, which gets added to Sentry breadcrumbs, and also gets logged to the Xcode console. We never use Console.Writeline. Not sure if there's a performance drawback, but considering it's not called in Update loop, it shouldn't really matter

[matus-tomlein]

Yeah, I was just thinking there is a bit more overhead when using Debug.Log – when I tried it in XCode, it was also printing out a short stack trace for each log message (as opposed to Console.WriteLine which just writes a single line).

If it's not a big deal in practice (I'm happy to defer to your experience here), I would suggest that we always use the Debug.Log – remove the #if RELEASE macro and the whole #else case and just keep the Debug.Log to simplify it.