Skip to content

Investigate support for Unity #644

Closed

Description

@jtattermusch

quoting @jskeet:
"There's suspicion the current version of C# protobufs won't work with Unity, but we can add another build target for that later when we find a need - we can create a new minor version of the NuGet package for that with no problems.

My experience with the "old" protobuf-csharp-port is that Unity on iOS has various issues, so I think we'll want to investigate them pretty thoroughly before trying to support it anyway - and we may need to conditionally compile out some features. We'll see... but I'd like to avoid having Unity support block a dotnetcore release."

also see #638

Activity

  1. dreis2211 commented on Sep 25, 2015

    @dreis2211

    +1

  2. Lyve1981 commented on Sep 25, 2015

    @Lyve1981

    +1

  3. jay-allan commented on Sep 25, 2015

    @jay-allan

    +1

  4. jskeet commented on Sep 25, 2015

    @jskeet
    Contributor

    Three +1s in the space of a few minutes suggests this is being linked to from somewhere - it would be nice if someone could provide the context. It would be even nicer if someone with Unity support could try this and inform us where there are problems :)

  5. jay-allan commented on Sep 25, 2015

    @jay-allan

    I can only speak for myself, but in my case we are developing a mobile game with Unity and a Java backend. In order to keep our client compatible with iOS, we are using protobuf-net. Unfortunately, protobuf-net has some known bugs and does not support the latest Protobuf versions, so an official solution from Google would be very interesting for us.

  6. jskeet commented on Sep 25, 2015

    @jskeet
    Contributor

    @jayallanjethwa: But how did you happen to reach this issue? It seems unlikely that three people would find an issue raised nearly two months ago entirely independently but within the same 5 minute window. I'm only asking so that I can try to get more context that might help resolve the issue. (I'd love to be able to support Unity - but I foresee some tricky issues, based on previous experience of bug reports.)

  7. asuuma commented on Sep 25, 2015

    @asuuma

    @jskeet I started to use protobuf v3 in Unity, and I found two problems.
    First of all, Unity is using .NET 3.5 and there were some compile errors when I copied to Unity project, so I had to modify some code with .NET 4.5 specific. This made me working protobuf v3 with Unity for simple case. Secondly, when I trying JsonFormatter.Format(IMessage) in iOS, Unity will threw exception of System.Reflection.Emit is not permitted., because iOS doesn't allow JIT.
    So far I found these problems, but there may be other problems because I am not using full functions of v3.

  8. jskeet commented on Sep 25, 2015

    @jskeet
    Contributor

    @asuuma: Right. Supporting both .NET 3.5 and modern PCLs is painful. I'm hoping that it won't be too long before Unity moves on in terms of runtime - after all, .NET 4.0 was released a long time ago. Could you give the details of the stack trace around Reflection.Emit? I suspect the problem is the use of expression trees, which would be feasible to fix, though annoying.

    (I'm going to need to balance the importance of supporting a wide variety of platforms with the additional code maintenance headaches and possible performance degredations of doing so. The more I know before making this judgement call, the better...)

  9. asuuma commented on Sep 28, 2015

    @asuuma

    @jskeet Yeah, everyone are hoping to move on newer runtime in near future... They opened roadmap of Unity and it says they are researching about upgrading .NET profile.
    Here is the stack trace.

    NotSupportedException: /Users/builduser/buildslave/unity/build/Tools/il2cpp/il2cpp/libil2cpp/icalls/mscorlib/System.Reflection.Emit/DynamicMethod.cpp(21) : Unsupported internal call for IL2CPP:DynamicMethod::create_dynamic_method - System.Reflection.Emit is not supported.
      at System.Reflection.Emit.DynamicMethod.CreateDelegate (System.Type delegateType, System.Object target) [0x00000] in <filename unknown>:0 
      at System.Linq.Expressions.Expression`1[TDelegate].Compile () [0x00000] in <filename unknown>:0 
      at Google.Protobuf.Reflection.FieldAccessorBase..ctor (System.Reflection.PropertyInfo property, Google.Protobuf.Reflection.FieldDescriptor descriptor) [0x00000] in <filename unknown>:0 
      at Google.Protobuf.Reflection.SingleFieldAccessor..ctor (System.Reflection.PropertyInfo property, Google.Protobuf.Reflection.FieldDescriptor descriptor) [0x00000] in <filename unknown>:0 
      at Google.Protobuf.Reflection.FieldDescriptor.CreateAccessor (System.String propertyName) [0x00000] in <filename unknown>:0 
      at Google.Protobuf.Reflection.FieldDescriptor.CrossLink () [0x00000] in <filename unknown>:0 
      at Google.Protobuf.Reflection.MessageDescriptor.CrossLink () [0x00000] in <filename unknown>:0 
      at Google.Protobuf.Reflection.FileDescriptor.CrossLink () [0x00000] in <filename unknown>:0 
      at Google.Protobuf.Reflection.FileDescriptor.BuildFrom (Google.Protobuf.ByteString descriptorData, Google.Protobuf.Reflection.FileDescriptorProto proto, Google.Protobuf.Reflection.FileDescriptor[] dependencies, Boolean allowUnknownDependencies, Google.Protobuf.Reflection.GeneratedCodeInfo generatedCodeInfo) [0x00000] in <filename unknown>:0 
      at Google.Protobuf.Reflection.FileDescriptor.InternalBuildGeneratedFileFrom (System.Byte[] descriptorData, Google.Protobuf.Reflection.FileDescriptor[] dependencies, Google.Protobuf.Reflection.GeneratedCodeInfo generatedCodeInfo) [0x00000] in <filename unknown>:0 
      at Test..cctor () [0x00000] in <filename unknown>:0 
      at Test.get_Descriptor () [0x00000] in <filename unknown>:0 
      at Google.Protobuf.JsonFormatter.Format (IMessage message) [0x00000] in <filename unknown>:0 
    
  10. jskeet commented on Sep 28, 2015

    @jskeet
    Contributor

    Right, thanks - so that is expression tree compilation. It's definitely feasible to write a workaround for that particular one.

  11. dreis2211 commented on Oct 8, 2015

    @dreis2211

    @asuuma Could you elaborate on what you did to workaround the .NET version mismatch?

  12. asuuma commented on Oct 23, 2015

    @asuuma

    @dreis2211 Oh very sorry, I missed your message.
    Here is diff of what I changed, just fixing compile errors.

    diff -u -r Google.Protobuf/Compatibility/PropertyInfoExtensions.cs protubuf-unity/Compatibility/PropertyInfoExtensions.cs
    --- Google.Protobuf/Compatibility/PropertyInfoExtensions.cs 2015-09-10 20:05:51.000000000 +0900
    +++ protubuf-unity/Compatibility/PropertyInfoExtensions.cs  2015-10-06 12:36:43.000000000 +0900
    @@ -47,7 +47,7 @@
             /// </summary>
             internal static MethodInfo GetGetMethod(this PropertyInfo target)
             {
    -            var method = target.GetMethod;
    +            var method = target.GetGetMethod();
                 return method != null && method.IsPublic ? method : null;
             }
    
    @@ -57,7 +57,7 @@
             /// </summary>
             internal static MethodInfo GetSetMethod(this PropertyInfo target)
             {
    -            var method = target.SetMethod;
    +            var method = target.GetSetMethod();
                 return method != null && method.IsPublic ? method : null;
             }
         }
    diff -u -r Google.Protobuf/Compatibility/TypeExtensions.cs protubuf-unity/Compatibility/TypeExtensions.cs
    --- Google.Protobuf/Compatibility/TypeExtensions.cs 2015-09-10 20:05:51.000000000 +0900
    +++ protubuf-unity/Compatibility/TypeExtensions.cs  2015-10-06 12:36:43.000000000 +0900
    @@ -51,7 +51,8 @@
             /// </summary>
             internal static bool IsValueType(this Type target)
             {
    -            return target.GetTypeInfo().IsValueType;
    +//            return target.GetTypeInfo().IsValueType;
    +           return target.GetType().IsValueType;
             }
    
             /// <summary>
    @@ -59,7 +60,8 @@
             /// </summary>
             internal static bool IsAssignableFrom(this Type target, Type c)
             {
    -            return target.GetTypeInfo().IsAssignableFrom(c.GetTypeInfo());
    +//            return target.GetTypeInfo().IsAssignableFrom(c.GetTypeInfo());
    +           return target.GetType().IsAssignableFrom(c.GetType());
             }
    
             /// <summary>
    @@ -72,9 +74,9 @@
                 // GetDeclaredProperty only returns properties declared in the given type, so we need to recurse.
                 while (target != null)
                 {
    -                var typeInfo = target.GetTypeInfo();
    -                var ret = typeInfo.GetDeclaredProperty(name);
    -                if (ret != null && ((ret.CanRead && ret.GetMethod.IsPublic) || (ret.CanWrite && ret.SetMethod.IsPublic)))
    +                var typeInfo = target.GetType();
    +                var ret = typeInfo.GetProperty(name);
    +                if (ret != null && ((ret.CanRead && ret.GetGetMethod().IsPublic) || (ret.CanWrite && ret.GetSetMethod().IsPublic)))
                     {
                         return ret;
                     }
    @@ -99,8 +101,8 @@
                 // GetDeclaredMethod only returns methods declared in the given type, so we need to recurse.
                 while (target != null)
                 {
    -                var typeInfo = target.GetTypeInfo();
    -                var ret = typeInfo.GetDeclaredMethod(name);
    +                var typeInfo = target.GetType();
    +                var ret = typeInfo.GetMethod(name);
                     if (ret != null && ret.IsPublic)
                     {
                         return ret;
    diff -u -r Google.Protobuf/JsonFormatter.cs protubuf-unity/JsonFormatter.cs
    --- Google.Protobuf/JsonFormatter.cs    2015-09-10 20:05:51.000000000 +0900
    +++ protubuf-unity/JsonFormatter.cs 2015-10-23 10:14:41.000000000 +0900
    @@ -496,7 +496,7 @@
             private void WriteFieldMask(StringBuilder builder, IMessage value)
             {
                 IList paths = (IList) value.Descriptor.Fields[FieldMask.PathsFieldNumber].Accessor.GetValue(value);
    -            AppendEscapedString(builder, string.Join(",", paths.Cast<string>().Select(ToCamelCase)));
    +            AppendEscapedString(builder, string.Join(",", paths.Cast<string>().Select(s => ToCamelCase(s)).ToArray()));
             }
    
             /// <summary>
    diff -u -r Google.Protobuf/WellKnownTypes/TimeExtensions.cs protubuf-unity/WellKnownTypes/TimeExtensions.cs
    --- Google.Protobuf/WellKnownTypes/TimeExtensions.cs    2015-09-10 20:05:51.000000000 +0900
    +++ protubuf-unity/WellKnownTypes/TimeExtensions.cs 2015-10-23 10:13:48.000000000 +0900
    @@ -34,7 +34,6 @@
     using System.Collections.Generic;
     using System.Linq;
     using System.Text;
    -using System.Threading.Tasks;
    
     namespace Google.Protobuf.WellKnownTypes
     {
    
  13. zeroZshadow commented on Nov 2, 2015

    @zeroZshadow

    I've applied this patch (without the commented out code) and can confirm this work in unity3d 5.2.2f
    This also keeps working when used in a WebGL export. Although i am not using any fancy protobuf features (only string, int, etc and repeated)

  14. jskeet commented on Nov 2, 2015

    @jskeet
    Contributor

    Note that with this patch, it won't work with CoreCLR, which is a more important initial target for us. I don't think it's feasible to support both platforms with a single binary.

  15. 64 remaining items

  16. jtattermusch commented on Mar 27, 2018

    @jtattermusch
    ContributorAuthor

    #3794 has been merged.

  17. zaneclaes commented on Mar 27, 2018

    @zaneclaes

    Excellent, thanks @jtattermusch and @jskeet ! Sorry I didn't get to test this before it got merged; this isn't my main work and I have limited time each day ;) Look forward to NuGet being updated!

  18. haberman commented on Jun 8, 2018

    @haberman
    Member

    Closing as this appears to be fixed.

  19. zaneclaes commented on Jun 11, 2018

    @zaneclaes

    @haberman I don't think this is really "fixed." There is limited experimental support for Android alone. It still doesn't work on many platforms, and even those it does work on require a lot of voodoo and undocumented tweaking to use.

  20. xfxyjwf commented on Jun 11, 2018

    @xfxyjwf
    Contributor

    @zaneclaes Can you help give a summary on what's working and what's not working?

  21. zaneclaes commented on Jun 11, 2018

    @zaneclaes

    I can try. The core challenge is that grpc_csharp_ext needs to compiled natively for each platform, and placed into the Assets/Plugins directory of Unity.

    • If you use the NuGet package, you can dig into the compiled binaries and copy grpc_csharp_ext.dll and libgrpc_csharp_ext.so into this directory to make Windows and Linux work.
    • To make OSX work, you need to rename libgrpc_csharp_ext.dylib to a bundle before copying.
    • To make Android work, you need to use the special compilation implemented in this thread and copy its libgrpc_csharp_ext.so
    • There is a partially working iOS solution I've detailed on my blog (it doesn't support bitcode yet).
    • There is no working WebAssembly solution yet.
    • Other platforms may also be broken (I've only enumerated the "big ones.")

    I may have also made some additional small tweaks to GRPC core, which I'll have to dig a little deeper on. But that covers the main points.

    FWIW, Unity has its own distribution channel (the asset store), which is probably the better route for distributing a working GRPC build to Unity users, which I'd be very interested in building/maintaining, if I could get some strong support from the rest of the community.

  22. jskeet commented on Jun 11, 2018

    @jskeet
    Contributor

    Those sound like gRPC issues - this issue is about just protobuf, which I believe is in a rather better state for Unity now, although it still requires some code to get reflection working correctly.

  23. zaneclaes commented on Jun 11, 2018

    @zaneclaes

    Oh, gosh. So sorry; I spent much more time interacting with the gRPC side of this and totally overlooked that this issue was in Protobufs. It seems that this can be closed! Apologies.

  24. xfxyjwf commented on Jun 11, 2018

    @xfxyjwf
    Contributor

    Thanks for the updates!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions