[$50 Bounty] RP completely broken with il2cpp (Unity 2018.2 Stable): Needs dll marshaling

Question

[$50 Bounty] RP completely broken with il2cpp (Unity 2018.2 Stable): Needs dll marshaling

Closed this issue 6 years ago · 30 comments

Reviving this issue:

Background

Unity 2018.2 now only builds il2cpp for Windows UWP and now also gives the option for il2cpp builds for Windows & Mac standalone (which is definitely the future for security+performance) -- this version is not a beta. Very soon, you'll see tons of people raising this issue: a permanent fix in a newer version with cross-compatibility would be ideal~

The Issue

The answer within the #186 was not explained in detail for those not too familiar with DLL madness (very little interaction with DLL's from Unity users).

I found where to put this attribute, but don't know what to do with the errors that follow.

@mdsitton was the original person that answered #186 and @franckmolto seemed to resolve it from that answer: Any of you guys up to sharing your result?

Bugs reported from logs

[Discord] init @ OnEnable
UnityEngine.DebugLogHandler:LogFormat(LogType, Object, String, Object[])
UnityEngine.Logger:Log(LogType, Object)
UnityEngine.Debug:Log(Object)
DiscordController:OnEnable()

(Filename: C:\buildslave\unity\build\Runtime/Export/Debug.bindings.h Line: 43)

NotSupportedException: IL2CPP does not support marshaling delegates that point to instance methods to native code.
  at DiscordController.OnEnable () [0x00000] in <00000000000000000000000000000000>:0

Original Code

[MonoPInvokeCallback(typeof(DiscordRpc.ErrorCallback))]

So you have this @ DiscordRpc:

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
public delegate void ErrorCallback(int errorCode, string message);

public struct EventHandlers
{
    public ErrorCallback errorCallback;
    [...]
}

Suggested Change from #186

If you change it to this:

using AOT; // Parent of MonoPInvokeCallback

[MonoPInvokeCallback(typeof(DiscordRpc.ErrorCallback))]

So you have this @ DiscordRpc:

[MonoPInvokeCallback(typeof(DiscordRpc.ErrorCallback))] // 
public static void ErrorCallback(int errorCode, string message);

public struct EventHandlers
{
    public ErrorCallback errorCallback;
    [...]
}

It starts getting confusing, starting with ErrorCallback not being a type

Cross-Compatibility

From my research (someone else can confirm), converting to dll marshaling using the [MonoPInvokeCallback] attribute and static callbacks has claims of cross-compatibility, exampled at steamworks.net and facepunch.steamworks repo's (or a quick Google search).

Answer 1 · 2018-10-20T13:47:03.000Z

The keyword is AOT (Ahead of Time) - we need AOT-friendly delivery of the dll reads.

Answer 2 · 2018-10-22T00:07:48.000Z

Responded to the closed issue without seeing this open first. Sure, if il2cpp is the new standard in Unity, we definitely need to come up with a fix and ship a new version. No problem.

If the static class method implementation in the closed issue is a valid fix, I'm more than happy to accept a PR that does as much. It won't break any existing implementations, as the native stuff here is pretty well abstracted from what actually "happens" in JS land in Discord, but I do sense some unknowns around how this plays with older Unity versions, which many folks will still use.

Plenty of folks asking questions around VS2010 oddities when it comes to our newer game SDK 😛

Answer 3 · 2018-10-23T04:51:44.000Z

Thread Merge

Responded to the closed issue without seeing this open first
To Consolidate:
I'm perfectly happy accepting a PR for the static class member changes if it won't break existing implementations (and it shouldn't?).

Unless you have suggestions for a more "real" solution. As mentioned above, neither myself nor snowyote are well-versed in this il2cpp stuff, so we're going a bit on community knowledge here.

Alas, I'm not good, either. I tried a few things, but failed. Something to do with turning the callbacks into static callbacks tagged with (for ErrorCallback's example) [MonoPInvokeCallback(typeof(DiscordRpc.ErrorCallback))]

Cross Compatible? Yes!

I read that it's cross-compatible. Steamworks.net had to do the same thing and confirms cross-compatability: Perhaps see here for clues @ rlabrecque/Steamworks.NET#227 and https://gist.github.com/GMMan/7a8a475b703363d2911fd20f238caef0

Then the importdll[] tags probably have to be changed to something similar, too. Maybe? I tried researching, but I wasn't even familiar with dll madness to begin with :P I mostly use native C# libraries, personally.

Anyone able to come to the rescue?

Answer 4 · 2018-10-23T07:16:34.000Z

💰 Bounty!

$50 PayPal to whoever first posts the correct il2cpp-friendly solution below (that I can confirm works).

🔍 Hints:

There are tons of links and examples in this issue that may show possible patterns that would work.
The solution has something to do with turning a dll callback delegate into a static one and with use of the [[MonoPInvokeCallback(typeof(DiscordRpc.ErrorCallback))]] MonoPInvokeCallback attribute (exampled for the ErrorCallback).
If you use that attribute above, it's said that the statement below needs to be a full() function (not end with a ; like a delegate)
Importing the importdll()'s may need something extra too.
All of the changes should be found at this single file: https://github.com/discordapp/discord-rpc/blob/master/examples/button-clicker/Assets/DiscordRpc.cs
You may find further hints at steamworks.net's WIP implementation that uses similar methods. Their open issue also has hints.

💡 If you complete the bounty...

Reply here with the solution before someone else does!
Reach out to me @ https://discord.gg/tol as i42-Xblade with your email

Answer 5 · 2018-10-24T14:52:46.000Z

I'd like to point out that IL2CPP is the new standard for UWP builds only - "standalone" windows builds using mono/.net are not being deprecated.

See the section about it at https://blogs.unity3d.com/2018/07/10/2018-2-is-now-available/

Answer 6 · 2018-10-24T17:05:09.000Z

@pipliz thank you for that documentation. That's mainly what I'm referring to. Unity deprecating .NET altogether seemed a bit fishy to me :p I think we can definitely come up with a solve for folks who want to make the switch, and just ensure it doesn't break other stuff.

Answer 7 · 2018-10-25T06:29:03.000Z

Ah, my bad -- I updated OP

Answer 8 · 2018-11-01T12:26:58.000Z

More leads:

I still can't figure it out. I've never directly dealt with dlls. I've tried several combos, but I can't get it to work. I'm just doing trial+error because I don't understand dll importing to begin with, so it's very foreign to me. I'm willing to bet an experienced dev would only need 15 minutes to fix this, from what I researched.

Answer 9 · 2018-11-01T20:31:38.000Z

Based on the solution offered in #186, this seems like something that is handled more in the use of the methods, rather than the underlying source code. That is to say, handled in DiscordController.cs which is an example, rather than DiscordRpc.cs which is the "header" file.

Perhaps @mdsitton would be able to share with us a bit more of his code, but in the meantime, I am trying to finagle the example to get it working.

EDIT: I do not know that for sure, but trying to parse his short response, that's what I am assuming. I could be, and most certainly am, wrong.

Answer 10 · 2018-11-02T02:25:53.000Z

It has to do with the initial DLL callback -- it must be a static callback and handled as a non-delegate (ending with () instead of ;). Then swap an attribute tag.

This file should be the culprit: https://github.com/discordapp/discord-rpc/blob/master/examples/button-clicker/Assets/DiscordRpc.cs

So we have this (of 7 funcs); I'm just going to use ready examples:

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
public delegate void ReadyCallback(ref DiscordUser connectedUser);

public struct EventHandlers
{
	public ReadyCallback readyCallback;
}

[DllImport("discord-rpc", EntryPoint = "Discord_Initialize", CallingConvention = CallingConvention.Cdecl)]
public static extern void Initialize(string applicationId, ref EventHandlers handlers, bool autoRegister, string optionalSteamId);

This needs to changed to look something like this (again, I'm still getting that error, but for directions sake):

// DiscordRpc.cs
using AOT;

public delegate void OnReadyInfo(ref DiscordUser connectedUser);

[MonoPInvokeCallback(typeof(OnReadyInfo))]
public static void ReadyCallback(ref DiscordUser connectedUser)
{
      UnityEngine.Debug.Log("[DiscordRpc] @ ReadyCallback");
}

public struct EventHandlers
{
	public ReadyCallback readyCallback; // Unchanged?
}

// From the examples I've seen, I don't think this line is the issue -- but the callback, rather.
[DllImport("discord-rpc", EntryPoint = "Discord_Initialize", CallingConvention = CallingConvention.Cdecl)]
public static extern void Initialize(string applicationId, ref EventHandlers handlers, bool autoRegister, string optionalSteamId);

The actual err does occur at DiscordController.cs, however, the true error comes from DiscordRpc: Here is what is happening @ DiscordController -- after OnEnable, it goes straight to DiscordRpc.cs before the err

// DiscordController.cs
void OnEnable()
{
        handlers = new DiscordRpc.EventHandlers(); // << This is where the il2cpp err happens: Inside DiscordRpc. Stacktrace will only show it's from DiscordController because the parent starts at OnEnable.

        handlers.onReadyInfo = ReadyCallback;
        [...]
}

AOT Requirements TL;DR Seem to Be:

When you import a DLL, the callback must be static (original ReadyCallback was not static)
The initial callback must not be a delegate and must end with (), not ; (ReadyCallback was a delegate)
The callback must have the attribute [MonoPInvokeCallback] (via using AOT;), recommended to include the typeof (ReadyCallback used [UnmanagedFunctionPointer])

Answer 11 · 2018-11-02T03:42:37.000Z

Porting a recent comment over from #186 from @joshpeterson

Hey, I'm a dev at Unity working on IL2CPP, so I thought I might help here.

The use of the MonoPInvokeCallback attribute for cases where a managed delegate is marshaled to native code is required for both IL2CPP and Mono AOT. As it turns out, Unity won't be supporting Mono AOT on many platforms soon (and currently we don't support too many), but this is a more general AOT issue.

The AOT code generator needs to know which managed methods could be called from native code, as it needs to generate marshaling wrappers. Doing this for all managed methods is not feasible, so the MonoPInvokeCallback attribute guides the AOT code generator.

While IL2CPP is now an option for desktop standalone player builds in Unity 2018.2 and later, we plan to continue to support Mono JIT for those platforms. We don't plan to require IL2CPP as is the case on iOS, for example.

Still, using MonoPInvokeCallback is the correct solution here, and should have no impact for other cases. The chances of it breaking something else are pretty low (or zero).

Answer 12 · 2018-11-02T12:13:08.000Z

I think this is on the right track.

When you import a DLL, the callback must be static (original ReadyCallback was not static)

The callback must have the attribute [MonoPInvokeCallback] (via using AOT;), recommended to include the typeof (ReadyCallback used [UnmanagedFunctionPointer])

This is correct. This code is trying to get a function pointer to the managed method ReadyCallback, and give it is native code via the Initialize method, so the native code can call back into the managed code at some point later.

These two requirements are correct. The method which will be called back from native code needs to be a static method and it needs to have the MonoPInvokeCallback attribute.

The initial callback must not be a delegate and must end with (), not ; (ReadyCallback was a delegate)

I'm not entirely sure what this requirement means.

Based on the code above though, I think you need one modification:

// DiscordRpc.cs
using AOT;

public delegate void OnReadyInfo(ref DiscordUser connectedUser);

[MonoPInvokeCallback(typeof(OnReadyInfo))]
public static void ReadyCallback(ref DiscordUser connectedUser)
{
      UnityEngine.Debug.Log("[DiscordRpc] @ ReadyCallback");
}

public struct EventHandlers
{
        // I think this is the change we need.
	// public ReadyCallback readyCallback; // Unchanged?
        public OnReadyInfo readyCallback; // The type of this field should be OnReadyInfo
}

// From the examples I've seen, I don't think this line is the issue -- but the callback, rather.
[DllImport("discord-rpc", EntryPoint = "Discord_Initialize", CallingConvention = CallingConvention.Cdecl)]
public static extern void Initialize(string applicationId, ref EventHandlers handlers, bool autoRegister, string optionalSteamId);

If this does not fix the issue, can you provide more details about what the error is? Thanks!

Answer 13 · 2018-11-02T17:15:30.000Z

Hi josh! Thanks a ton for this helpful info and for chiming in. Based on your and Dylan's notes this definitely seems like a change we need to make. No problem at all there.

The second piece will be updating the example code in DiscordController.cs to work with the new "header" file. As we swap these callbacks to statics, the code example throws a number of errors about requiring object references when Invoke()ing the UnityEvents. I've refactored them into a singleton GameManager:

public class GameManager {
  private static GameManager instance = null;
  public static GameManager Instance {
    get
    {
      if (instance == null)
      {
        instance = new GameManager();
      }
      return instance;
    }
  }
  public UnityEngine.Events.UnityEvent onConnect;
  public UnityEngine.Events.UnityEvent onDisconnect;
  public UnityEngine.Events.UnityEvent hasResponded;
}

And changing the invokes to:

public static void ReadyCallback(ref DiscordRpc.DiscordUser connectedUser)
  {
    Debug.Log(string.Format("Discord: connected to {0}#{1}: {2}", connectedUser.username, connectedUser.discriminator, connectedUser.userId));
    GameManager.Instance.onConnect.Invoke();
  }

This makes the compiler happy, but seems to cause some runtime crashes in the player. Starting the player will result in a freeze after Discord: init without any errors thrown. So, that's where I am at as of this morning 😄

EDIT: I realize this is now an implementation issue rather than a native code issue, but if we've got the example up, we need to make sure it's as easy as Open -> Play for people learning.

Answer 14 · 2018-11-02T17:48:33.000Z

Can confirm that UnityEvent.Invoke() seems to be the culprit. This freezes the player:

public static void ReadyCallback(ref DiscordRpc.DiscordUser connectedUser)
  {
    Debug.Log(string.Format("Discord: connected to {0}#{1}: {2}", connectedUser.username, connectedUser.discriminator, connectedUser.userId));
    GameManager.Instance.onConnect.Invoke();
  }

This works successfully and the Debug Log is printed:

public static void ReadyCallback(ref DiscordRpc.DiscordUser connectedUser)
  {
    Debug.Log(string.Format("Discord: connected to {0}#{1}: {2}", connectedUser.username, connectedUser.discriminator, connectedUser.userId));
    // GameManager.Instance.onConnect.Invoke();
  }

Answer 15 · 2018-11-02T18:32:06.000Z

I'm curious about the specific errors you get when calling Invoke here. Can you provide more details?

Answer 16 · 2018-11-02T20:25:34.000Z

It actually isn't throwing any errors on my end; the Unity player freezes up and needs to be force quit. Perhaps there's some logging I can attach?

Answer 17 · 2018-11-02T20:51:31.000Z

Is there any information in the Unity player log?

Answer 18 · 2018-11-02T21:10:19.000Z

Not much from what I've seen, but here's a couple videos.

Not working when invoking events: https://gfycat.com/ShrillDigitalAnglerfish
Working when removing the Invoke(): https://gfycat.com/QuickDearDogwoodtwigborer

Sorry for the gfycat links; github doesn't like mp4 embeds apparently!

If there's some logging I can attach that I might be missing, would love to know. I'm a Unity newbie for the most part. My changes are also reflected in #249 if anyone would like to pull it down themself.

Answer 19 · 2018-11-02T21:55:18.000Z

Think I found my own issue. The callbacks in DiscordController.cs did not need to be static; that was my misunderstanding. So there's no need for a singleton and the "normal" code works just fine. Updating PR.

Answer 20 · 2018-11-02T22:30:52.000Z

@dylanh724 could you pull down my PR and see if it fixes the issue for you? It works on my machine for il2cpp and Mono, but would love a second confirmation before merging.

Answer 21 · 2018-11-04T11:19:14.000Z

@msciotti Thanks for progressing on this! I copy+pasted your DiscordRpc changes without changing it. I then moved my custom logic into its own file, leaving DiscordController a 99% copy+paste for init sequence:

Works fine in editor,

However, I was unable to get this working in a standalone build, getting the same error:

NotSupportedException: IL2CPP does not support marshaling delegates that point to instance methods to native code.
  at DiscordController.OnEnable () [0x00000] in <00000000000000000000000000000000>:0

What's your build info? Mine is:

Unity 2018.2.12f1
il2cpp (not Mono)
.NET 4.x
Windows Standalone x64 build

Answer 22 · 2018-11-05T18:31:47.000Z

We can continue tracking this over in the PR. But my settings are as follows:

I'm testing over on macOS. Unity build 2018.3.0b8

Answer 23 · 2018-11-28T21:08:38.000Z

This didn't get auto-closed with the PR, so closing.

Answer 24 · 2018-12-06T07:42:56.000Z

I'm a little late to the party, but glad this issue was figured out eventually 👍

Answer 25 · 2018-12-07T14:02:25.000Z

Is there an official solution here now? I am still getting the IL2CPP does not support marshaling delegates that point to instance methods to native code. error in the build when compiling the button-clicker example with IL2CPP:

My solution after trying around a bit was the following (not sure if that all makes sense but it seems to work):

In DiscordRpc.cs:

// public EventHandlers are set from the DiscordController.cs instead of passing in the handlers when initializing:
public static EventHandlers Callbacks;

[MonoPInvokeCallback(typeof(OnReadyInfo))]
public static void ReadyCallback(ref DiscordUser connectedUser) { Callbacks.readyCallback(ref connectedUser); }
public delegate void OnReadyInfo(ref DiscordUser connectedUser);

[ ... ]

public static void Initialize(string applicationId, bool autoRegister, string optionalSteamId)
{
	EventHandlers eventHandlers = new EventHandlers();
	// asign the static methods above, which invoke the Callbacks
	eventHandlers.readyCallback += DiscordRpc.ReadyCallback;
	eventHandlers.disconnectedCallback += DiscordRpc.DisconnectedCallback;
	eventHandlers.errorCallback += DiscordRpc.ErrorCallback;
	eventHandlers.joinCallback += DiscordRpc.JoinCallback;
	eventHandlers.spectateCallback += DiscordRpc.SpectateCallback;
	eventHandlers.requestCallback += DiscordRpc.RequestCallback;
	InitializeInternal(applicationId, ref eventHandlers, autoRegister, optionalSteamId);
}
[ ... ]

In DiscordController.cs I replaced the OnEnable() with this:

void OnEnable()
{
	Debug.Log("Discord: init");
	DiscordRpc.Callbacks.readyCallback = ReadyCallback;
	DiscordRpc.Callbacks.disconnectedCallback = DisconnectedCallback;
	DiscordRpc.Callbacks.errorCallback = ErrorCallback;
	DiscordRpc.Callbacks.joinCallback = JoinCallback;
	DiscordRpc.Callbacks.spectateCallback = SpectateCallback;
	DiscordRpc.Callbacks.requestCallback = RequestCallback;
        DiscordRpc.Initialize(applicationId, true, optionalSteamId);
}

Answer 26 · 2018-12-07T18:48:09.000Z

@derwodaso Hm, the example as-is in the repo works fine for me? Here are my steps:

Get the repo fresh (git reset --hard HEAD && git pull && git clean -xdf)
python build.py
Open unity -> button-clicker
Swap to il2cpp compiler -> restart
Press play

I get my ready callback with my userid in it:

This does not work for you?

Answer 27 · 2018-12-07T20:16:15.000Z

You must make a build with il2cpp. It stays c# in the editor Provenance : Courrier pour Windows 10 De : Mason Sciotti Envoyé le :vendredi 7 décembre 2018 19:48 À : discordapp/discord-rpc Cc : franckmolto; Mention Objet :Re: [discordapp/discord-rpc] [$50 Bounty] RP completely broken withil2cpp (Unity 2018.2 Stable): Needs dll marshaling (#243) @derwodaso Hm, the example as-is in the repo works fine for me? Here are my steps: 1. Get the repo fresh (git reset --hard HEAD && git pull && git clean -xdf) 2. python build.py 3. Open unity -> button-clicker 4. Swap to il2cpp compiler -> restart 5. Press play I get my ready callback with my userid in it: This does not work for you? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread. Garanti sans virus. www.avg.com

Answer 28 · 2018-12-08T10:17:02.000Z

@franckmolto is correct, the problem only occurs in the build. The editor worked right away.

Also I got the relevant DLL's directly (most recent), not building them. Might that be another issue @msciotti?

Answer 29 · 2018-12-11T18:37:03.000Z

Hm, OK can confirm, I didn't realize it was failing on a built app. @derwodaso if your changes work and you'd like to PR them, I'm happy to review and test and merge.

Answer 30 · 2018-12-12T21:56:10.000Z

@msciotti done! Did some minor changes to my previous version to keep the signature of the Initialize method the same. So no changes are required in DiscordController.cs.