-
Notifications
You must be signed in to change notification settings - Fork 180
Allow LogSerializer to be disabled to facilitate usage in Unity iOS #67
Allow LogSerializer to be disabled to facilitate usage in Unity iOS #67
Conversation
…pps, which cannot JSON serialize objects with properties.
But that means that logging is broken in that situation. Any other ideas? |
Probably the best alternative would be to remove the {get;set;} from AuthorizeState and OidcClientOptions classes (since these are the only two classes that get passed to LogSerializer.Serialize()). That will allow the logging to work properly on iOS. Or we could override ToString() on those classes and manually construct the log message instead of using JsonConvert.SerializeObject. Sadly fixing the root cause I believe requires a changes to the IL2CPP code generation that Unity employs for iOS builds, which seems to be stripping required code from the generated properties, or changes to the Unity-compatible Json.Net library to handle properties differently. We've had this problem with our own code, it's not something specific to this library, and our solution was to use fields instead of properties on classes that need to be serialized. There are two threads with info on this here and here. What do you think? |
@@ -27,7 +29,7 @@ static LogSerializer() | |||
/// <returns></returns> | |||
public static string Serialize(object logObject) | |||
{ | |||
return JsonConvert.SerializeObject(logObject, jsonSettings); | |||
return Enabled ? JsonConvert.SerializeObject(logObject, jsonSettings) : ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change that to "Logging has been disabled" - and put a comment on the Enabled property to describe why it is there.
thanks! |
thank you! i'm thinking to write a sample unity app showing how to use this with unity for android/ios. |
Put it somewhere on github - i can link to it from the readme. |
Sample code added here: Videos showing the sample running: If you'd prefer I can make a separate repo for this to keep it dis-associated. Or if you are happy to bring Unity into the list of platform samples I can raise a PR on the samples repo. |
separate repo with a readme would be cool - so you can link to the videos. I will link to your repo from my readme. thanks! |
ok i put it here: https://github.com/peterhorsley/Unity3D.Authentication.Example |
Thanks! |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue. |
Unity iOS apps using the Unity equivalent of NewtonSoft.Json library crash when trying to JSON serialize objects with properties. This PR is to add a mechanism to disable the serialization of arbitrary objects for logging purposes, which is the only code change required to have this library working in Unity iOS apps.