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

Discussion: Changing SchedulerDefaults in a 3rd party lib to support Unity Engine #1628

Open
Fijo opened this issue Oct 30, 2021 · 3 comments

Comments

@Fijo
Copy link
Contributor

Fijo commented Oct 30, 2021

Motivation:

I'm currently attempting to create a lightweight extension on top of System.Reactive for use with the latest version 2021.2 of the Unity Game engine. The existing adoption of System.Reactive for Unity called UniRx has lacked maintenance in a big way for years hence a growing community interest in the ability to benefit from the improvements made to System.Reactive in recent years.
Sources:

UniRx is a System.Reactive fork and that's why it's maintenance is quite a bit of work. Also it makes it so you can't just consume a library that depends on System.Reactive when using UniRx inside unity. (This alone has lead to some creative solutions/ points of pain in the past)

To make System.Reactive work well within Unity one key thing has to be changed though and that's unfortunately currently not possible as far I was able to tell from the research I did. It's also something that probably everyone switching over from UniRx will rely on to make a switch without having to add unreasonable 'hacks' all over their project.
I'm talking about being able to change the deafult scheduler behaviour to accommodate for unities unique multithreading considerations.
Thus it's required for TimeBasedOperations to be scheduled on the main unity thread.
Also if a build is for WebGL multithreading support isn't a thing altogether hence why all AsyncConversions need to also be scheduled on the main unity thread.

From my reading it looks to me like in the past it was possible to make these behaviour changes from a third party library by implementing an IPlatformEnlightenmentProvider. However as of version 5 of System.Reactive it looks like it's no longer possible to influence the behaviour sufficiently when using that method. If feels like that concept has been partially retired.

As an alternative the easiest way that I'd at least have the option (though it wouldn't be entirely clean yet) would be to change SchedulerDefaults.cs to look like this:

namespace System.Reactive.Concurrency
{
    internal static class SchedulerDefaults
    {
        private static IScheduler _timeBasedOperations;
        private static IScheduler _asyncConversions;

        internal static IScheduler ConstantTimeOperations => ImmediateScheduler.Instance;
        internal static IScheduler TailRecursion => ImmediateScheduler.Instance;
        internal static IScheduler Iteration => CurrentThreadScheduler.Instance;

        internal static IScheduler TimeBasedOperations
        {
            get => _timeBasedOperations ??= DefaultScheduler.Instance;
            set => _timeBasedOperations = value;
        }

        internal static IScheduler AsyncConversions
        {
            get => _asyncConversions ??= DefaultScheduler.Instance;
            set => _asyncConversions = value;
        }
    }
}

Using reflection I could then make the following calls in my 3rd party library. (Reflection omit for ease of readability)

            SchedulerDefaults.TimeBasedOperations = UnityMainThreadScheduler.Instance;
#if UNITY_WEBGL
            SchedulerDefaults.AsyncConversions = UnityMainThreadScheduler.Instance;
#else
            SchedulerDefaults.AsyncConversions = ThreadPoolOnlyScheduler.Instance; // Possibly subject to change in 3rd party lib still
#endif

Of course it would be a lot cleaner be able to make those changes without using internal code and I would hands down prefer that. But this is where I feel like I'd hear an opinion from you as to a possible approach that'd fit well into the current philosophy about doing this. Would it be a good idea to make use of the IPlatformEnlightenmentProvider again for this or would you rather just have that be gone?

@Fijo
Copy link
Contributor Author

Fijo commented Nov 19, 2021

I still think the cleanest way to do this would be by using the IPlatformEnlightenmentProvider here. Again would you be okay with not retiring that?
If you agree I'll create a PR making this change using the IPlatformEnlightenmentProvider so you can review it.

As this is the only thing keeping this library from being used with the NuGet Version from System.Reactive getting this change in sooner rather than later is quite important to me. Sorry for bothering you again and thanks in advance.

@Fijo Fijo closed this as completed Nov 19, 2021
@Fijo Fijo reopened this Nov 19, 2021
@Thaina
Copy link

Thaina commented Feb 19, 2022

I would like to add my supporting to any system that allow changing default scheduler for some specific platform. In fact instead of SchedulerDefaults static class, we should have instance or factory that we could set to override the scheduler instance

@Thaina
Copy link

Thaina commented Dec 7, 2022

Is it possible to made pull request for this?

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

No branches or pull requests

3 participants