Benedicht / BestHTTP-Issues

Issue tracking repo for the Best HTTP and all related Unity plugins.
https://assetstore.unity.com/publishers/4137
11 stars 1 forks source link

Websocket Delegates Run Outside Unity Context - Unable to Call Unity APIs #112

Closed RMichaelPickering closed 1 year ago

RMichaelPickering commented 2 years ago

It appears that all the BestHTTP Websocket client delegates (e.g. OnOpen, OnMessage, etc.) run in the context of the Websocket thread, which means that Unity APIs fail silently, and in a way that's quite hard to debug, whenever they're called inside the application delegates. This should be fixed by ensuring the Websocket library runs the delegates using the Unity context. There are some details in this blog: https://devblogs.microsoft.com/dotnet/configureawait-faq/

As an interim workaround, I will attempt to get and reset the context manually.

Benedicht commented 2 years ago

With default settings, callbacks are called from a Unity Update functions, so users don't have to deal with threading.

You can set the plugin's logging level to verbose by adding the following line somewhere in your startup code:

BestHTTP.HTTPManager.Logger.Level = BestHTTP.Logger.Loglevels.All;

Replicate your issue and send all log entries over to me (here or in mail: besthttp@gmail.com). From the logs i might have know more what's happening.

You can read more about logging in the plugin here: https://benedicht.github.io/BestHTTP-Documentation/pages/best_http2/global_topics/Logging.html

RMichaelPickering commented 2 years ago

Thanks for your quick response!

I've already attempted to implement my workaround, and had also set BestHTTP logging level to all in my code. I've attached the log from a test session WITH my workaround, and will deactivate the workaround and capture another log to share later today.

Thanks and regards,

Mike

R. Michael Pickering CBIP President & CEO CloudConstable Incorporated

On Mon, Aug 8, 2022 at 3:02 AM Tivadar György Nagy @.***> wrote:

With default settings, callbacks are called from a Unity Update functions, so users don't have to deal with threading.

You can set the plugin's logging level to verbose by adding the following line somewhere in your startup code:

BestHTTP.HTTPManager.Logger.Level = BestHTTP.Logger.Loglevels.All;

Replicate your issue and send all log entries over to me (here or in mail: @.***). From the logs i might have know more what's happening.

You can read more about logging in the plugin here: https://benedicht.github.io/BestHTTP-Documentation/pages/best_http2/global_topics/Logging.html

— Reply to this email directly, view it on GitHub https://github.com/Benedicht/BestHTTP-Issues/issues/112#issuecomment-1207744336, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEABETMAYYW4OANAXALE55TVYCWIZANCNFSM55WZZSHQ . You are receiving this because you authored the thread.Message ID: @.***>

Benedicht commented 2 years ago

Hey, couldn't find any attached log. Could you send it over to my email address (besthttp@gmail.com)?

RMichaelPickering commented 2 years ago

Just to provide a further update, I'm still working on this issue. I just noticed that I don't have the latest version of BestHTTP/2 installed in my project and have pulled in the latest release for retesting.

RMichaelPickering commented 2 years ago

@Benedicht After updating to the latest version and tweaking the BestHTTP settings I still find the same issue with callbacks not running on the Unity main loop. How is your code meant to accomplish this please? I keep seeing information that suggests that Unity uses its own, custom synchronization context that replaces the standard C# context, and its thread IDs use a different namespace from the thread IDs used by regular C# threads. I did find some ideas about using UnityEngine.LowLevel.SetPlayerLoop to allow callbacks to be injected into and run on the Update loop...

Benedicht commented 2 years ago

@RMichaelPickering

How is your code meant to accomplish this please?

The HTTPUpdateDelegator derives from MonoBehaviour and implements the regular Update function called by Unity. From this Update the plugin calls HTTPManager.OnUpdate(); that maintains the plugin and dispatches all the callbacks. It's plain and simple.

Even the plugin's samples are interacting with Unity, without any problem.

I keep seeing information that suggests that Unity uses its own, custom synchronization context that replaces the standard C# context

It's in their documentation (see the Limitations of async and await tasks section), but even their custom context was implemented to run tasks on the Unity main thread.

Their implementation also stores and later checks the threads ManagedThreadId, so i wouldn't think unity's main thread id changes.

If you could send a minimal repro project so i could inspect and debug your issue, i could help more, but at this point I think you might tring to find the cause and solution in the wrong place

RMichaelPickering commented 2 years ago

Hi Tivadar,

Thanks for your answer! I've done some more testing, and I'd like to clarify that I'm having an issue with the very first step in the Websocket process, which from my application's perspective is on receiving the "OnOpen" event from the BestHTTP Websocket instance. At that time, my application needs to make a Unity API call to a Unity UI object (which is a GameObject) so that it's clear that the connection is open and the app should be working. Based on that, other processes are started to respond to user requests made in user interactions that would all be communicated (to/from our node.js server) through the opened Websocket. However, if the websocket doesn't get opened successfully, the entire application won't work, so we try to fail somewhat gracefully and show an error message.

If the OnOpen delegate didn't run on the Unity main (Update) loop, the API call would certainly fail, but the subsequent behaviour of my app would be somewhat undefined. Certainly the UI object that I'm looking for will not appear in my app, and this is indeed what I'm seeing.

I find it VERY interesting that your WebSocketResponse.cs code includes Actions that correspond to OnText (line 35), OnBinary (line 40), OnIncompleteFrame (line 45), and OnClosed (line 50). I can't find any Action for OnOpen (which might be derived from the 'switching protocols', status 101, message sent from the Websocket server after the websocket negotiation has completed successfully.

Is it possible that the WebSocket OnOpen handler isn't being properly dispatched so that its callback runs on the Unity main loop? Would defining an Action for it in the WebSocketResponse code mentioned above help to fix this issue? And, at the same time, should the OnError handler not be dispatched in a similar way, in case any Unity API call(s) need to be run there, too?

I will try to implement my own temporary workaround to handle the OnOpen event shortly, and will let you know if this approach proves fruitful in due course...

Thanks and regards,

Mike

R. Michael Pickering CBIP President & CEO CloudConstable Incorporated 10271 Yonge Street, Suite 321 Richmond Hill, ON L4C 3B5 Canada www.cloudconstable.com @.*** Toronto: 416-721-1826 Orlando: 321-337-2824

On Thu, Aug 25, 2022 at 3:19 AM Tivadar György Nagy < @.***> wrote:

@RMichaelPickering https://github.com/RMichaelPickering

How is your code meant to accomplish this please?

The HTTPUpdateDelegator derives from MonoBehaviour and implements the regular Update https://docs.unity3d.com/ScriptReference/MonoBehaviour.Update.html function called by Unity. From this Update the plugin calls HTTPManager.OnUpdate(); that maintains the plugin and dispatches all the callbacks. It's plain and simple.

Even the plugin's samples are interacting with Unity, without any problem.

I keep seeing information that suggests that Unity uses its own, custom synchronization context that replaces the standard C# context

It's in their documentation https://docs.unity3d.com/Manual/overview-of-dot-net-in-unity.html (see the Limitations of async and await tasks section), but even their custom context was implemented to run tasks on the Unity main thread.

Their implementation also stores and later checks the threads ManagedThreadId https://github.com/Unity-Technologies/UnityCsReference/blob/master/Runtime/Export/Scripting/UnitySynchronizationContext.cs#L33, so i wouldn't think unity's main thread id changes.

If you could send a minimal repro project so i could inspect and debug your issue, i could help more, but at this point I think you might tring to find the cause and solution in the wrong place

— Reply to this email directly, view it on GitHub https://github.com/Benedicht/BestHTTP-Issues/issues/112#issuecomment-1226872653, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEABETNKVSSHW6MT7LQ3WKLV24M6JANCNFSM55WZZSHQ . You are receiving this because you were mentioned.Message ID: @.***>

Benedicht commented 2 years ago

OnOpen called from OverHTTP1.cs in the OnInternalRequestUpgraded callback (subscribed to HTTPRequest's OnUpgraded event). OnUpgraded is called from RequestEventHelper.ProcessQueue, and RequestEventHelper.ProcessQueue called from HTTPManager.OnUpdate and as we already know HTTPManager.OnUpdate is called from HTTPUpdateDelegator's Update function that's presumably called by Unity from its main thread.

You can attach a debugger when Script Debugging enabled, or you can place a Debug.log(...) call that prints out the stack trace to confirm these chain of calls.

RMichaelPickering commented 2 years ago

Hi Tivadar,

Thanks for this information! It's weird but I confirmed that the OnOpen call works as expected (it sets a UI object to active) in the Editor (when I press "Play") but doesn't work in a release build. (Whether it works in debug mode or not is irrelevant to me, as I'm building production software!)

Could this be a Unity bug?

Best regards,

Mike

R. Michael Pickering CBIP President & CEO CloudConstable Incorporated 10271 Yonge Street, Suite 321 Richmond Hill, ON L4C 3B5 Canada www.cloudconstable.com @.*** Toronto: 416-721-1826 Orlando: 321-337-2824

On Sat, Aug 27, 2022 at 11:15 AM Tivadar György Nagy < @.***> wrote:

OnOpen called from OverHTTP1.cs in the OnInternalRequestUpgraded callback (subscribed to HTTPRequest's OnUpgraded event). OnUpgraded is called from RequestEventHelper.ProcessQueue, and RequestEventHelper.ProcessQueue called from HTTPManager.OnUpdate and as we already know HTTPManager.OnUpdate is called from HTTPUpdateDelegator's Update function that's presumably called by Unity from its main thread.

You can attach a debugger when Script Debugging https://docs.unity3d.com/Manual/ManagedCodeDebugging.html enabled, or you can place a Debug.log(...) call that prints out the stack trace to confirm these chain of calls.

— Reply to this email directly, view it on GitHub https://github.com/Benedicht/BestHTTP-Issues/issues/112#issuecomment-1229211320, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEABETNEOTJUCZULA23X45DV3IWJ5ANCNFSM55WZZSHQ . You are receiving this because you were mentioned.Message ID: @.***>

Benedicht commented 2 years ago

I don't know what code do you have in th OnOpen callback so i can't comment on what it could be.

Can we agree on closing this issue? I see no reason keeping it open as it seems the callbacks are dispatched on the unity main thread as it's expected. I'm still happy to help, however without seeing any code or repro project i'm not sure i can be any helpful.

RMichaelPickering commented 2 years ago

Hi Tivadar,

Can we please agree that this issue should be closed only once I've got your asset entirely integrated and working in my app?

I've been very patient because clearly your asset is quite comprehensive and generally very robust, which is exactly what I want. However, I'm sure you more than anyone can appreciate that implementing it for 'real world' use cases is inherently complex due to the wide range of supported options and implementation targets. I've now been working on debugging my implementation for more than two months, and it is causing significant impact to my application development roadmap, not to mention long delays on two different, dependent projects!

Certainly my intended use of your asset is quite different from even your own included Websocket sample, which itself works essentially using a fullscreen UI in Unity, under user control, to demonstrate some simple text-based message-passing functionality. I would point out that this is fairly different from what I'm trying to accomplish, which is to use your Websocket asset to communicate in the background from my Unity app to a cloud-based node.js server -- and it needs to do this without direct end-user intervention. Where your sample uses the 'legacy' Unity input system, my app uses the 'new' Unity input system. Also, my app is targeted for Windows standalone where your sample runs in WebGL -- I'm not sure if it's been tested on any other platform.

In fact, I'm not even sure that the method being called to add the received message from the Websocket to the UI is a MonoBehaviour -- I don't believe it would be able to call Unity APIs either, nor could the class be attached to a GameObject, which begs the question -- how could it cause ANY code to run on the Unity main loop? Have you got any Websocket example that uses messages received from a Websocket to interact with Unity APIs? For example, setting a GameObject to active and changing its colour?

Thanks and regards,

Mike

R. Michael Pickering CBIP President & CEO CloudConstable Incorporated 10271 Yonge Street, Suite 321 Richmond Hill, ON L4C 3B5 Canada www.cloudconstable.com @.*** Toronto: 416-721-1826 Orlando: 321-337-2824

On Sun, Aug 28, 2022 at 6:51 AM Tivadar György Nagy < @.***> wrote:

I don't know what code do you have in th OnOpen callback so i can't comment on what it could be.

Can we agree on closing this issue? I see no reason keeping it open as it seems the callbacks are dispatched on the unity main thread as it's expected. I'm still happy to help, however without seeing any code or repro project i'm not sure i can be any helpful.

— Reply to this email directly, view it on GitHub https://github.com/Benedicht/BestHTTP-Issues/issues/112#issuecomment-1229431416, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEABETPFVRLSZQ4IAQQUYG3V3NABVANCNFSM55WZZSHQ . You are receiving this because you were mentioned.Message ID: @.***>

Benedicht commented 2 years ago

Can we please agree that this issue should be closed only once I've got your asset entirely integrated and working in my app?

While i think new issues should get their own report and even this one can be reopened, as you wish, i leave it open. But as you know, you can reach me over email and i'm available on discord too, where discussing various topics and/or issues might be more convenient.

I've now been working on debugging my implementation for more than two months, and it is causing significant impact to my application development roadmap, not to mention long delays on two different, dependent projects!

On top of the general support of the plugin, as a more direct help, i'm available as a contractor to help debugging (with 15+ years of experience) and to help integrating the plugin.

Certainly my intended use of your asset is quite different from even your own included Websocket sample, which itself works essentially using a fullscreen UI in Unity, under user control, to demonstrate some simple text-based message-passing functionality.

Capability of the websocket api and my sample are two different things. Websocket is plain and simple on the surface, making an overly complex sample would be overkill and confusing, but I'm happy to receive feedback and suggestions for additional samples. Websocket also used in other parts of the plugin as the main transport layer, so even viewing a SignalR Core or SocketIO sample are running on top of websockets.

Certainly my intended use of your asset is quite different from even your own included Websocket sample, which itself works essentially using a fullscreen UI in Unity, under user control, to demonstrate some simple text-based message-passing functionality.

I would point out that this is fairly different from what I'm trying to accomplish, which is to use your Websocket asset to communicate in the background from my Unity app to a cloud-based node.js server -- and it needs to do this without direct end-user intervention.

Where your sample uses the 'legacy' Unity input system, my app uses the 'new' Unity input system.

From the plugin's perspective? No, they are not any different.

  1. The 'new' and 'legacy' input systems still operate on the very same unity main thread. (And even if they wouldn't, websocket's Send implementations are thread safe!)
  2. Whether the server is in the cloud, hosted locally or somewhere else, doesn't matter either. All the networking and websocket are using and implementing protocols, until the other peer communicates with the same and proper implementation of the protocol, it doesn't matter where and how they are, what programming language they are written in.
  3. And finally, it doesn't matter who and why calls the websocket's Send. It's called from a function that triggered by user interaction or from an other function that called from somewhere else? It doesn't matter and the plugin doesn't care (and i don't know why would it be different).

Also, my app is targeted for Windows standalone where your sample runs in WebGL -- I'm not sure if it's been tested on any other platform.

The WebGL sample is there to show off for potential users and they can try out a few features of the plugin before buying it. It also demonstrates that it can run under and supports WebGL too (supporting the other platforms are easy, one of the plugin's unique selling point is WeGL).

In fact, I'm not even sure that the method being called to add the received message from the Websocket to the UI is a MonoBehaviour

I'm not sure what you are referring here? If you're referring to the websocket sample, then fortunately, you have handful of ways to find it out. For example, you can just check the source code of the samples! The WebSocketSample derived from the BestHTTP.Examples.Helpers.SampleBase class. And SampleBase is a MonoBehaviour:

using UnityEngine;

namespace BestHTTP.Examples.Helpers
{
    public abstract class SampleBase : MonoBehaviour
    {
        [Header("Common Properties")]
        public string Category;
        public string DisplayName;

        [TextArea]
        public string Description;

        public RuntimePlatform[] BannedPlatforms = new RuntimePlatform[0];

        protected SampleRoot sampleSelector;

        protected virtual void Start()
        {
            this.sampleSelector = FindObjectOfType<SampleRoot>();
        }
    }
}

-- I don't believe it would be able to call Unity APIs either, nor could the class be attached to a GameObject, which begs the question -- how could it cause ANY code to run on the Unity main loop?

Fortunately, you don't have to believe, every bits needed to confirm it are on your computer too. ;) For example, you can start the SampleSelector scene, start the WebSocketSample and search for the WebSocketSample type in the Hierarchy (type "t:WebSocketSample" in the search box), and you can see it attached to a gameobject.

Have you got any Websocket example that uses messages received from a Websocket to interact with Unity APIs? For example, setting a GameObject to active and changing its colour?

The WebSocketSample interacts with the Unity yes. Set's the UI component's interactable property, calls GameObject.instantiate. But yeah, it doesn't changing colors.

All in all, i see it as that you are trying to find the issue in the wrong place and picking fights where you shouldn't, wasting both of our precious times.

Benedicht commented 2 years ago

communicate in the background from my Unity app

If your application is in the background, so it's not in focus all the time, you have to set Application.runInBackground to true. Otherwise no update functions are called when the application isn't the focused one.

You can also set it through the Project Settings/Resolution and Presentation/Run In Background checkbox.

RMichaelPickering commented 2 years ago

Hi Tivadar,

Thanks again for this additional information.

It is now clear that the issue is in your Websocket sample implementation itself, which differs from the other 'long lived' protocols such as SocketIO and SignalR in that the Websocket sample needs to provide its own handler as opposed to using ones already provided by the higher level protocols. In particular, you're using C# Actions that need to be subscribed to like C# events, but they aren't declared as events as shown in the following pattern: http://i.stack.imgur.com/LeUSA.jpg

I personally think that the approach shown on the link is a good and elegant C# pattern, though other good options may exist. For example, if it would be easier, you could try invoking or running the existing Actions more directly in the Websocket sample, but this wouldn't use the event subscription syntax of "+=" for each event type as you currently have in the sample code.

As to why this sample works in Editor (play mode) and WebGL, I'm not certain, but generally it seems that the Editor is more forgiving as compared to a full release build for the Windows target platform using IL2CPP. I did follow your existing pattern in my application code, and it certainly doesn't fully work there, although it does seem like the Actions are getting triggered -- they just don't seem to be running on the main Unity thread as required.

So how can I help you to fix the Websocket sample code so that it works properly across all supported target platforms, especially Windows? I can't provide you with all my application's source code, but I might be able to provide an excerpt of the key logic that runs the websocket to control my app from a remote server -- you could use any simple text messages to trigger simple Unity API calls such as setting objects active or triggering animation on them.

Please let me know how you'd like to proceed.

Best regards,

Mike

R. Michael Pickering CBIP President & CEO CloudConstable Incorporated 10271 Yonge Street, Suite 321 Richmond Hill, ON L4C 3B5 Canada www.cloudconstable.com @.*** Toronto: 416-721-1826 Orlando: 321-337-2824

On Mon, Aug 29, 2022 at 8:10 AM Tivadar György Nagy < @.***> wrote:

Can we please agree that this issue should be closed only once I've got your asset entirely integrated and working in my app?

While i think new issues should get their own report and even this one can be reopened, as you wish, i leave it open. But as you know, you can reach me over email and i'm available on discord too, where discussing various topics and/or issues might be more convenient.

I've now been working on debugging my implementation for more than two months, and it is causing significant impact to my application development roadmap, not to mention long delays on two different, dependent projects!

On top of the general support of the plugin, as a more direct help, i'm available as a contractor to help debugging (with 15+ years of experience) and to help integrating the plugin.

Certainly my intended use of your asset is quite different from even your own included Websocket sample, which itself works essentially using a fullscreen UI in Unity, under user control, to demonstrate some simple text-based message-passing functionality.

Capability of the websocket api and my sample are two different things. Websocket is plain and simple on the surface, making an overly complex sample would be overkill and confusing, but I'm happy to receive feedback and suggestions for additional samples. Websocket also used in other parts of the plugin as the main transport layer, so even viewing a SignalR Core or SocketIO sample are running on top of websockets.

Certainly my intended use of your asset is quite different from even your own included Websocket sample, which itself works essentially using a fullscreen UI in Unity, under user control, to demonstrate some simple text-based message-passing functionality.

I would point out that this is fairly different from what I'm trying to accomplish, which is to use your Websocket asset to communicate in the background from my Unity app to a cloud-based node.js server -- and it needs to do this without direct end-user intervention.

Where your sample uses the 'legacy' Unity input system, my app uses the 'new' Unity input system.

From the plugin's perspective? No, they are not any different.

  1. The 'new' and 'legacy' input systems still operate on the very same unity main thread. (And even if they wouldn't, websocket's Send implementations are thread safe!)
  2. Whether the server is in the cloud, hosted locally or somewhere else, doesn't matter either. All the networking and websocket are using and implementing protocols, until the other peer communicates with the same and proper implementation of the protocol, it doesn't matter where and how they are, what programming language they are written in.
  3. And finally, it doesn't matter who and why calls the websocket's Send. It's called from a function that triggered by user interaction or from an other function that called from somewhere else? It doesn't matter and the plugin doesn't care (and i don't know why would it be different).

Also, my app is targeted for Windows standalone where your sample runs in WebGL -- I'm not sure if it's been tested on any other platform.

The WebGL sample is there to show off for potential users and they can try out a few features of the plugin before buying it. It also demonstrates that it can run under and supports WebGL too (supporting the other platforms are easy, one of the plugin's unique selling point is WeGL).

In fact, I'm not even sure that the method being called to add the received message from the Websocket to the UI is a MonoBehaviour

I'm not sure what you are referring here? If you're referring to the websocket sample, then fortunately, you have handful of ways to find it out. For example, you can just check the source code of the samples! The WebSocketSample https://github.com/Benedicht/BestHTTP_Examples/blob/master/Websocket/WebSocketSample.cs#L12 derived from the BestHTTP.Examples.Helpers.SampleBase https://github.com/Benedicht/BestHTTP_Examples/blob/master/Helpers/SampleBase.cs#L5 class. And SampleBase is a MonoBehaviour:

using UnityEngine; namespace BestHTTP.Examples.Helpers { public abstract class SampleBase : MonoBehaviour { [Header("Common Properties")] public string Category; public string DisplayName;

    [TextArea]
    public string Description;

    public RuntimePlatform[] BannedPlatforms = new RuntimePlatform[0];

    protected SampleRoot sampleSelector;

    protected virtual void Start()
    {
        this.sampleSelector = FindObjectOfType<SampleRoot>();
    }
}

}

-- I don't believe it would be able to call Unity APIs either, nor could the class be attached to a GameObject, which begs the question -- how could it cause ANY code to run on the Unity main loop?

Fortunately, you don't have to believe, every bits needed to confirm it are on your computer too. ;) For example, you can start the SampleSelector scene, start the WebSocketSample and search for the WebSocketSample type in the Hierarchy (type "t:WebSocketSample" in the search box), and you can see it attached to a gameobject.

Have you got any Websocket example that uses messages received from a Websocket to interact with Unity APIs? For example, setting a GameObject to active and changing its colour?

The WebSocketSample interacts with the Unity yes. Set's the UI component's interactable property, calls GameObject.instantiate. But yeah, it doesn't changing colors.

All in all, i see it as that you are trying to find the issue in the wrong place and picking fights where you shouldn't, wasting both of our precious times.

— Reply to this email directly, view it on GitHub https://github.com/Benedicht/BestHTTP-Issues/issues/112#issuecomment-1230200330, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEABETPXMWTLWVTGUTUGFTDV3SSEDANCNFSM55WZZSHQ . You are receiving this because you were mentioned.Message ID: @.***>

RMichaelPickering commented 2 years ago

Hi Tivadar,

I just sent another note on this topic. My application is in fact running in the foreground -- I consider the Websocket communications (and other communications using ZeroMQ/NetMQ) to be running in the background. The trickiest part of the ZeroMQ/NetMQ implementation was how to get results from the background thread into Unity's main thread!

Best regards,

Mike

R. Michael Pickering CBIP President & CEO CloudConstable Incorporated 10271 Yonge Street, Suite 321 Richmond Hill, ON L4C 3B5 Canada www.cloudconstable.com @.*** Toronto: 416-721-1826 Orlando: 321-337-2824

On Mon, Aug 29, 2022 at 1:27 PM Tivadar György Nagy < @.***> wrote:

communicate in the background from my Unity app

If your application is in the background, so it's not in focus all the time, you have to set Application.runInBackground https://docs.unity3d.com/ScriptReference/Application-runInBackground.html to true. Otherwise no update functions are called when the application isn't the focused one.

You can also set it through the Project Settings/Resolution and Presentation/Run In Background https://docs.unity3d.com/2022.2/Documentation/Manual/playersettings-windows.html checkbox.

— Reply to this email directly, view it on GitHub https://github.com/Benedicht/BestHTTP-Issues/issues/112#issuecomment-1230616658, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEABETN54ZWHMQ2VXCLMLRDV3TXHXANCNFSM55WZZSHQ . You are receiving this because you were mentioned.Message ID: @.***>

Benedicht commented 2 years ago

In particular, you're using C# Actions that need to be subscribed to like C# events

No, using events instead of delegates doesn't cause any issues. An event declaration is more restricted from the outside (only allows += and -= operators to add/remove subscription), while a plain delegate allows full access and control.

although it does seem like the Actions are getting triggered

So, the plugin works, period.

they just don't seem to be running on the main Unity thread as required.

There's a few ways to check whether it is the main thread or not and i already mentioned at least one in this issue or in mail.

As to why this sample works in Editor (play mode) and WebGL, I'm not certain, but generally it seems that the Editor is more forgiving as compared to a full release build for the Windows target platform using IL2CPP.

You can do a windows build to see the samples working there too.

I can't provide you with all my application's source code

The devil is in the details. While an excerpt of the key logic could look fine, the implementation of that logic can have serious bugs.

you could use any simple text messages to trigger simple Unity API calls such as setting objects active or triggering animation on them

Then it looks like it's quite easy to replicate this issue in a separate project too to show it's not working. Then you can share that projects. (You can use the websocket url from my websocket sample, it just echoes back the text/binary it receives.)

RMichaelPickering commented 2 years ago

"No, using events instead of delegates doesn't cause any issues. An event declaration is more restricted from the outside (only allows += and -= operators to add/remove subscription), while a plain delegate allows full access and control."

So, in your sample, why do you use += to subscribe, like C# events, when only plain action delegates are implemented?

Are you saying that you're unwilling to spend any time trying to ACTUALLY fix a legitimate issue reported by a paying customer?

Best regards,

Mike

R. Michael Pickering CBIP President & CEO CloudConstable Incorporated 10271 Yonge Street, Suite 321 Richmond Hill, ON L4C 3B5 Canada www.cloudconstable.com @.*** Toronto: 416-721-1826 Orlando: 321-337-2824

On Mon, Aug 29, 2022 at 2:03 PM Tivadar György Nagy < @.***> wrote:

In particular, you're using C# Actions that need to be subscribed to like C# events

No, using events instead of delegates doesn't cause any issues. An event declaration is more restricted from the outside (only allows += and -= operators to add/remove subscription), while a plain delegate allows full access and control.

although it does seem like the Actions are getting triggered

So, the plugin works, period.

they just don't seem to be running on the main Unity thread as required.

There's a few ways to check whether it is the main thread or not and i already mentioned at least one in this issue or in mail.

As to why this sample works in Editor (play mode) and WebGL, I'm not certain, but generally it seems that the Editor is more forgiving as compared to a full release build for the Windows target platform using IL2CPP.

You can do a windows build to see the samples working there too.

I can't provide you with all my application's source code

The devil is in the details. While an excerpt of the key logic could look fine, the implementation of that logic can have serious bugs.

you could use any simple text messages to trigger simple Unity API calls such as setting objects active or triggering animation on them

Then it looks like it's quite easy to replicate this issue in a separate project too to show it's not working. Then you can share that projects. (You can use the websocket url from my websocket sample, it just echoes back the text/binary it receives.)

— Reply to this email directly, view it on GitHub https://github.com/Benedicht/BestHTTP-Issues/issues/112#issuecomment-1230663051, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEABETPWOZSDVGZSU7VME7TV3T3QRANCNFSM55WZZSHQ . You are receiving this because you were mentioned.Message ID: @.***>

Benedicht commented 2 years ago

So, in your sample, why do you use += to subscribe, like C# events, when only plain action delegates are implemented?

Habits. But, it doesn't matter how you subscribe to an event/delegate, the method is subscribed, and how it will be called doesn't change (the subscribed method will not be called on a different thread just because you use += instead of =, or something like that.). But, you can try out changing it, or changing the fields to events (all source code is included in the package), then make a windows build, they will still work the same way.

Are you saying that you're unwilling to spend any time trying to ACTUALLY fix a legitimate issue reported by a paying customer?

You are still failed to ACTUALLY prove anything. You wrote wall of texts and always something else the issue (first threads, then events, and then how in my samples i subscribe to events), but you are not trying out any of my suggestions, no repro project. Nothing! Just accusitions. If the plugin doesn't work and you couldn't enable/disable a gameobject from the callbacks, you could already make a simple repro project to show it!

This conversation became very heated, let's talk again when you have any proof to back your claims.

RMichaelPickering commented 2 years ago

Tivadar, I agree that we should take this discussion down a couple of notches. Please do understand that just because I haven't been able to share 'proof' that your asset isn't working, doesn't mean that I have no proof, it just means that I have a fairly complex application that was previously using an open source, unsupported C# wrapper of a native C/C++ Websocket library as a native plugin -- that worked with some limitations, especially around security and encryption -- which has been replaced by your asset, but is not yet fully working. On the plus side, your asset is much better for managing the HTTP upgrade process, including negotiating for https then on to wss. However, once the Websocket has been opened, the actual message passing functionality isn't working very well at all. Honestly, I've been working on trying to get your asset fully working for so long that I don't even remember how the events or delegates were implemented in that library -- or if in fact it was using some other approach.

Can you please explain why your Websocket sample attempts to subscribe, using +=, to the events/delegates called "OnOpen", "OnMessage", "OnClosed" and "OnError" but your WebSocketResponse script only defines "OnClosed"? It does define the following as part of its public interface: Action... OnText Action... OnBinary Action... OnIncompleteFrame

It seems like the OnOpen condition is being triggered twice, probably first from the http to https upgrade event, then again from https to wss in my case. But really, in attempting to open a Websocket, it should only be considered open after the final upgrade. Still, is the Action being triggered twice? I shall try invoking it and the OnText using = instead of +=...

Regards,

Mike

R. Michael Pickering CBIP President & CEO CloudConstable Incorporated 10271 Yonge Street, Suite 321 Richmond Hill, ON L4C 3B5 Canada www.cloudconstable.com @.*** Toronto: 416-721-1826 Orlando: 321-337-2824

On Mon, Aug 29, 2022 at 2:39 PM Tivadar György Nagy < @.***> wrote:

So, in your sample, why do you use += to subscribe, like C# events, when only plain action delegates are implemented?

Habits. But, it doesn't matter how you subscribe to an event/delegate, the method is subscribed, and how it will be called doesn't change (the subscribed method will not be called on a different thread just because you use += instead of =, or something like that.). But, you can try out changing it, or changing the fields to events (all source code is included in the package), then make a windows build, they will still work the same way.

Are you saying that you're unwilling to spend any time trying to ACTUALLY fix a legitimate issue reported by a paying customer?

You are still failed to ACTUALLY prove anything. You wrote wall of texts and always something else the issue (first threads, then events, and then how in my samples i subscribe to events), but you are not trying out any of my suggestions, no repro project. Nothing! Just accusitions. If the plugin doesn't work and you couldn't enable/disable a gameobject from the callbacks, you could already make a simple repro project to show it!

This conversation became very heated, let's talk again when you have any proof to back your claims.

— Reply to this email directly, view it on GitHub https://github.com/Benedicht/BestHTTP-Issues/issues/112#issuecomment-1230709593, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEABETKAV2UKBFDUVP76YZLV3T7WHANCNFSM55WZZSHQ . You are receiving this because you were mentioned.Message ID: @.***>

Benedicht commented 2 years ago

The public facing WebSocket class is just a wrapper, a facet to hide the underlying implementations. For ages there were only two, a http/1 based one and one to support WebGL.

The base request that goes out to the server is created in OverHTTP1.cs's CreateInternalRequest. A WebSocketResponse is created and used instead of the regular HTTPResponse when the plugin starts receiving back the server's response to its (websocket) upgrade. In CreateInternalRequest it also subscribes to an OnUpgraded event. OnUpgraded called when HTTPResponse (WebSocketResponse's base class) detects a status code of 101 (upgrade) and a connection header with the "upgrade" value. But OnUpgraded isn't called directly, because reading the server's response is done on a thread, so it instead queues up a request-event:

RequestEventHelper.EnqueueRequestEvent(new RequestEventInfo(this.baseRequest, RequestEvents.Upgraded));

So, when RequestEventHelper's ProcessQueue is called the next time (from HTTPManager.OnUpdate, and that's as we already know is called from HTTPUpdateDelegator's Update function that's called by unity from the unity main thread), the OnUpgrade callback is called (OnInternalRequestUpgraded in OverHTTP1). In OnInternalRequestUpgraded, if everything goes well, the plugin considers the websocket connection open and calls the parent WebSocket's OnOpen event:

this.State = WebSocketStates.Open;
if (this.Parent.OnOpen != null)
{
    try
    {
        this.Parent.OnOpen(this.Parent);
    }
    catch (Exception ex)
    {
        HTTPManager.Logger.Exception("OverHTTP1", "OnOpen", ex, this.Parent.Context);
    }
}

The Parent is the WebSocket instance you created and used to subscribe to various events. So when you do something like this:

var webSocket = new WebSocket.WebSocket(new Uri(address));
webSocket.OnOpen += OnOpenCallback;

void OnOpenCallback(WebSocket ws)
{
    Debug.Log("WebSocket connection is open!");
}

OnOpenCallback will be called from that OnInternalRequestUpgraded.

Something similar happens with the other events too. Because the WebSocket class is just a facet, it doesn't know when to dispatch its events. WebSocketResponse however knows, because that's where the websocket frames are read. Frame reading happens on an other thread, so in the ReceiveThreadFunc it's only queues up the text and binary frames in the CompletedFrames queue. WebSocketResponse implements the IProtocol interface and its HandleEvents method is called periodically from ProtocolEventHelper.ProcessQueue (and it's called from HTTPManager.OnUpdate that called from the Unity main thread through HTTPUpdateDelegator's Update). So, WebSocketResponse fires its own OnText event:

case WebSocketFrameTypes.Text:
    // Any not Final frame is handled as a fragment
    if (!frame.IsFinal)
        goto case WebSocketFrameTypes.Continuation;

    HTTPManager.Logger.Verbose("WebSocketResponse", "HandleEvents - OnText", this.Context);
    if (OnText != null)
        OnText(this, frame.DataAsText);
    break;

That will call the function subscribed in OverHTTP1(note that webSocket here is a WebSocketResponse instance):

webSocket.OnText = (ws, msg) =>
{
    if (this.Parent.OnMessage != null)
        this.Parent.OnMessage(this.Parent, msg);
};

And Parent is the public facing WebSocket, so the this.Parent.OnMessage(this.Parent, msg); line will finally call the OnOpenCallback method.

It seems like the OnOpen condition is being triggered twice, probably first from the http to https upgrade event, then again from https to wss in my case. But really, in attempting to open a Websocket, it should only be considered open after the final upgrade. Still, is the Action being triggered twice? I shall try invoking it and the OnText using = instead of +=...

And again, you are writing a bunch of nonsense that i'm tired to refute. Please, before you write out your theories, fire up your favorite debugger, place a few breakpoints and check out what happens by real.

RMichaelPickering commented 2 years ago

Hi Tivadar,

Thanks for all this additional information -- except for the last part which was a bit rude, everything was most helpful!

Indeed, your example was done using the += event subscription approach as you detailed, but when I looked into the code to double check that I was using the correct definitions, I found simple delegate declarations that didn't have events. I agree that it shouldn't be my responsibility to dig through your code, but it seems to me that there should be some better way to clearly delineate between the code for your provided sample and the code of your asset itself. For example, not declaring a namespace for examples that's part of the asset namespace would be a good start -- a good sample should show how any Unity dev can use the asset in their own app so it should only need to be built USING the appropriate module(s) of that asset. I did check your Websocket documentation and it clearly lists all the events and shows how to subscribe to each. When these events are triggered are there log entries added automatically to the BestHTTP log file?

I did find the definition for each of the listed delegates in the script WebSocketBaseImplementation, where each one is declared as "public delegate void..." but that led to a bit more confusion on my part. May I please ask how these delegate definitions are in fact the same as C# event definitions? I don't understand why the "event" keyword isn't used instead of simply "delegate" as shown in the code. I refer you to an article here: https://docs.microsoft.com/en-us/dotnet/csharp/distinguish-delegates-events

Based on the article, it seems clear that C# events or delegates are to some extent interchangeable, but they aren't really the same thing. As an example, in your documentation the delegates are all listed as "Events" which should be corrected. Even in your helpful description above, you refer to events but I think you actually mean delegates and callbacks. I'm not trying to nit-pick, I'm just trying to avoid getting further confused if I need a bit of extra context to understand how to make this work, and need to look up additional documentation online, if I look up "C# events" I'm going to get led to the wrong (though similar) pattern.

I finally would like to point out that in a WebGL Unity app, the underlying browser supplies the actual Websocket implementation. Thus, my Windows standalone app is using features that aren't tested in, for example, your WebGL sample, and if no Windows (or other standalone platform) sample has been built and tested by you, then you haven't actually tested your own Websocket implementation code at all.I do think it really does work, but there may well be some unexpected glitches...

Best regards,

Mike

R. Michael Pickering CBIP President & CEO CloudConstable Incorporated 10271 Yonge Street, Suite 321 Richmond Hill, ON L4C 3B5 Canada www.cloudconstable.com @.*** Toronto: 416-721-1826 Orlando: 321-337-2824

On Tue, Aug 30, 2022 at 3:52 AM Tivadar György Nagy < @.***> wrote:

The public facing WebSocket class is just a wrapper, a facet to hide the underlying implementations. For ages there were only two, a http/1 based one and one to support WebGL.

The base request that goes out to the server is created in OverHTTP1.cs's CreateInternalRequest. A WebSocketResponse is created and used instead of the regular HTTPResponse when the plugin starts receiving back the server's response to its (websocket) upgrade. In CreateInternalRequest it also subscribes to an OnUpgraded event. OnUpgraded called when HTTPResponse (WebSocketResponse's base class) detects a status code of 101 (upgrade) and a connection header with the "upgrade" value. But OnUpgraded isn't called directly, because reading the server's response is done on a thread, so it instead queues up a request-event:

RequestEventHelper.EnqueueRequestEvent(new RequestEventInfo(this.baseRequest, RequestEvents.Upgraded));

So, when RequestEventHelper's ProcessQueue is called the next time (from HTTPManager.OnUpdate, and that's as we already know is called from HTTPUpdateDelegator's Update function that's called by unity from the unity main thread), the OnUpgrade callback is called ( OnInternalRequestUpgraded in OverHTTP1). In OnInternalRequestUpgraded, if everything goes well, the plugin considers the websocket connection open and calls the parent WebSocket's OnOpen event:

this.State = WebSocketStates.Open;if (this.Parent.OnOpen != null) { try { this.Parent.OnOpen(this.Parent); } catch (Exception ex) { HTTPManager.Logger.Exception("OverHTTP1", "OnOpen", ex, this.Parent.Context); } }

The Parent is the WebSocket instance you created and used to subscribe to various events. So when you do something like this:

var webSocket = new WebSocket.WebSocket(new Uri(address));webSocket.OnOpen += OnOpenCallback; void OnOpenCallback(WebSocket ws) { Debug.Log("WebSocket connection is open!"); }

OnOpenCallback will be called from that OnInternalRequestUpgraded.

Something similar happens with the other events too. Because the WebSocket class is just a facet, it doesn't know when to dispatch its events. WebSocketResponse however knows, because that's where the websocket frames are read. Frame reading happens on an other thread, so in the ReceiveThreadFunc it's only queues up the text and binary frames in the CompletedFrames queue. WebSocketResponse implements the IProtocol interface and its HandleEvents method is called periodically from ProtocolEventHelper.ProcessQueue (and it's called from HTTPManager.OnUpdate that called from the Unity main thread through HTTPUpdateDelegator's Update). So, WebSocketResponse fires its own OnText event:

case WebSocketFrameTypes.Text: // Any not Final frame is handled as a fragment if (!frame.IsFinal) goto case WebSocketFrameTypes.Continuation;

HTTPManager.Logger.Verbose("WebSocketResponse", "HandleEvents - OnText", this.Context);
if (OnText != null)
    OnText(this, frame.DataAsText);
break;

That will call the function subscribed in OverHTTP1(note that webSocket here is a WebSocketResponse instance):

webSocket.OnText = (ws, msg) => { if (this.Parent.OnMessage != null) this.Parent.OnMessage(this.Parent, msg); };

And Parent is the public facing WebSocket, so the this.Parent.OnMessage(this.Parent, msg); line will finally call the OnOpenCallback method.

It seems like the OnOpen condition is being triggered twice, probably first from the http to https upgrade event, then again from https to wss in my case. But really, in attempting to open a Websocket, it should only be considered open after the final upgrade. Still, is the Action being triggered twice? I shall try invoking it and the OnText using = instead of +=...

And again, you are writing a bunch of nonsense that i'm tired to refute. Please, before you write out your theories, fire up your favorite debugger, place a few breakpoints and check out what happens by real.

— Reply to this email directly, view it on GitHub https://github.com/Benedicht/BestHTTP-Issues/issues/112#issuecomment-1231283722, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEABETOQD4KM3KKSCV2TBBTV3W4UFANCNFSM55WZZSHQ . You are receiving this because you were mentioned.Message ID: @.***>

Benedicht commented 2 years ago

I don't understand why the "event" keyword isn't used instead of simply "delegate" as shown in the code. I refer you to an article here: https://docs.microsoft.com/en-us/dotnet/csharp/distinguish-delegates-events

Good article, i would suggest to read the previous ones too, like the 'System.Delegate and the delegate keyword', 'Strongly Typed Delegates' and 'Introduction to events' topics, they will give you a good basic overview over c# delegates and events.

An event keyword can be used on delegate fields to restrict access outside from the class declaration to the base delegate field. So, otside of the class only the += and -= operators can be used on them.

dotnetfiddle.net is an online .net playground that you can play around with c#. Here's an example i made for you: https://dotnetfiddle.net/oaIiuL.

There's two classes, DelegateTest and EventTest. Both are declareing a MyAction field and Action<> is a strongly typed delegate (declared as public delegate void Action<in T>(T arg); by the .net framework, see the Strongly Typed Delegates topic linked above).

    class DelegateTest
    {
        public Action<string> MyAction;

        public void DoMyAction()
        {
            MyAction("From DelegateTest - DoMyAction!");
        }
    }

    class EventTest
    {
        public event Action<string> MyAction;

        public void DoMyAction()
        {
            MyAction("From EventTest - DoMyAction!");
        }
    }

And there are two functions TestDelegate and TestEvent. This is the TestDelegate function:

    private static void TestDelegate()
    {
        DelegateTest test = new DelegateTest();

        test.MyAction = (str) => Console.WriteLine("[TestDelegate - First callback] " + str);

        // this will overwrite the previous subscription
        test.MyAction = (str) => Console.WriteLine("[TestDelegate - Second callback] " + str);

        // += will add the subscription and not overwrite
        test.MyAction += (str) => Console.WriteLine("[TestDelegate - Third callback] " + str);      

        test.DoMyAction();
    }

In TestDelegate we can manipulate MyAction however we want. For example with the = operator, we can overwrite any previously set subscribers. In the example above, executing TestDelegate will produce the following output:

[TestDelegate - Second callback] From DelegateTest - DoMyAction! [TestDelegate - Third callback] From DelegateTest - DoMyAction!

As you can see, "[TestDelegate - First callback]" wasn't printed, because test.MyAction = (str) => Console.WriteLine("[TestDelegate - Second callback] " + str); overwrote the callback function. But, "[TestDelegate - Third callback]" was printed because += subscribed that method too. (For example you could use test.MyAction = null; to remove all subscribing methods.)

So let's see TestEvent:

    private static void TestEvent()
    {
        EventTest test = new EventTest();

        test.MyAction = (str) => Console.WriteLine("[TestEvent - First callback] " + str);

        // this will overwrite the previous subscription
        test.MyAction = (str) => Console.WriteLine("[TestEvent - Second callback] " + str);

        // += will add the subscription and not overwrite
        test.MyAction += (str) => Console.WriteLine("[TestEvent - Third callback] " + str);     

        test.DoMyAction();
    }

As you can see, TestEvent is almost the same as TestDelegate, but it creates an EventTest object instead of DelegateTest.

Try to press the play button now. Oh no, it doesn't even compile!

Compilation error (line 51, col 8): The event 'Program.EventTest.MyAction' can only appear on the left hand side of += or -= (except when used from within the type 'Program.EventTest') Compilation error (line 54, col 8): The event 'Program.EventTest.MyAction' can only appear on the left hand side of += or -= (except when used from within the type 'Program.EventTest')

Because the event keyword is used, it hides and allows only the += and -= operators on the MyAction field. You can comment out the two misbehaving lines, or change them to use +=:

    private static void TestEvent()
    {
        EventTest test = new EventTest();

        test.MyAction += (str) => Console.WriteLine("[TestEvent - First callback] " + str);

        // this will overwrite the previous subscription
        test.MyAction += (str) => Console.WriteLine("[TestEvent - Second callback] " + str);

        // += will add the subscription and not overwrite
        test.MyAction += (str) => Console.WriteLine("[TestEvent - Third callback] " + str);     

        test.DoMyAction();
    }

The TestEvent now produces the following output:

[TestEvent - First callback] From EventTest - DoMyAction! [TestEvent - Second callback] From EventTest - DoMyAction! [TestEvent - Third callback] From EventTest - DoMyAction!

See? Using an event keyword on a delegate field only changes the what it's allowed to do on that field, but it still has the same behavior. And this is why this whole topic around wheter i declare my fields that you can subscribe to are with or without the event keyword: it just doesn't matter in your case!

I do think it really does work, but there may well be some unexpected glitches...

Delegates and events are c# features, if you experience glitches with them, please contact the unity support.

Benedicht commented 1 year ago

Any update about your issue?