Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SetupHttpCapsClient fails in Unity #82

Closed
zangetsu-oe opened this issue Mar 5, 2023 · 3 comments · Fixed by #84
Closed

SetupHttpCapsClient fails in Unity #82

zangetsu-oe opened this issue Mar 5, 2023 · 3 comments · Fixed by #84

Comments

@zangetsu-oe
Copy link

We have a Unity project that using the Mono Scripting Backend, and the .Net Framework API Compatibility Level. We are referencing the netstandard2.1 version of the compiled assembly in the Unity Project.

We have found that the SetupHttpCapsClient code in GridClient, attempts to configure MaxConnectionsPerServer. The MaxConnectionsPerServer property is not supported on all platforms. We have found that if we take out the code to set that on the HttpClientHandler, then the HttpClientHandler works just fine.

See:
zangetsu-oe@72c1620

I'm not sure what's the correct way to go about making this particular line conditional, otherwise I would have and included a pull request.

Advice is appreciated.

@github-actions
Copy link

github-actions bot commented Mar 5, 2023

Thank you for taking the time to improve LibreMetaverse! We will review your contribution shortly.

@cinderblocks
Copy link
Owner

Potentially fixed in 2f743ad

@zangetsu-oe
Copy link
Author

I tried out the changes, and it seems that #if NET_4_6 doesn't seem to be a working solution. I'm working towards using the pre-compiled net48 version of LibreMetaverse, which means that by the time Unity sees the .dll the #if NET_4_6 code has already been removed. I don't think we can rely on Unity's Compiler directives to solve this.

Let me add more information about what I have tried today.
Scenario One - Basic Attempt:

  1. Synchronized my fork with your latest changes. Built my fork.
  2. Copied net48 binaries into the Unity Project.
  3. Live Debug in the Unity Editor and observed the exception:
    NotImplementedException: The method or operation is not implemented.
    System.Net.Http.MonoWebRequestHandler.set_MaxConnectionsPerServer (System.Int32 value)
  4. Made a Build of the Unity project, Ran the resulting .exe. Observed same exception in log.

Scenario Two - Remove the NET_4_6 directive:

  1. Edited GridClient.cs:
#if NETSTANDARD2_1_OR_GREATER || NET48_OR_GREATER || NETCOREAPP3_1_OR_GREATER
            if (Utils.GetRunningRuntime() != Utils.Runtime.Mono)
                handler.MaxConnectionsPerServer = Settings.MAX_HTTP_CONNECTIONS;
#endif
  1. Build my fork, copied net48 binaries into the Unity Project.
  2. Live Debug in the Unity Editor, Success!
  3. Made a Build of the Unity project, Ran the resulting .exe, Success!

Side note: I ended up trying a bunch of different stuff because I was getting crash to desktop from the compiled unity exe under several different scenarios, but that turned out to be something crufty with my environment. When I forced Unity to do a 'Clean' build, I got the above described behavior.

So my experimental observations are:
Setting .MaxConnectionsPer Server does not work when LibreMetaverse is Compiled for .Net Framework 4.8 and the 4.8 assemblies are executed using the Mono Runtime.

This got me thinking about the System.Net.Http.dll. There are two different implementations of this, and I went looking at the source code for both trying to understand more:
https://github.com/mono/mono/tree/main/mcs/class/System.Net.Http
https://github.com/microsoft/referencesource/tree/master/System/net/System/Net/Http

And then I remembered that at some point I had delibrately added the Microsoft System.Net.Http implementation to the unity project, and went and looked and its not there anymore. I added it back to the unity project, but the Mono Runtime seems to have ignored its presence. So my best guess is that i knows better than we do that it wants to use its own internal implementation. That's fine. I don't need to argue with them.

At this point my full understanding is that the code probably doesn't even need the

#if NETSTANDARD2_1_OR_GREATER || NET48_OR_GREATER || NETCOREAPP3_1_OR_GREATER

since that will be true for all the LibreMetaverse.csproj Target Frameworks:

<TargetFrameworks>net48;netstandard2.1;net6.0;net7.0</TargetFrameworks>

As such, this revision seems to be working just fine for me:
zangetsu-oe@95457c1

I'm going to go ahead and make a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants