Application.isPlaying causing exception about unirx HOT 26 CLOSED

The "How To Use for Multithreading" example in README.md is also affected by this issue.

from unirx.

We've found a workaround for Application.isPlaying. We need to test it a bit and clean it up, but the gist of it is this:

#if UNITY_EDITOR

[InitializeOnLoad]
public class ScenePlaybackDetector
{
    // This callback is notified just before building the scene, before Start().
    [PostProcessScene]
    public static void OnPostprocessScene()
    {   
        IsPlaying = true;         
    } 

    static ScenePlaybackDetector()
    {
        // This callback comes after Start(), it's too late. But it's useful for detecting playback stop.
        EditorApplication.playmodeStateChanged += () => { IsPlaying = EditorApplication.isPlayingOrWillChangePlaymode; }; 
    }

from unirx.

Thanks, I'll check quickly.

from unirx.

When is EditorThreadDispatcher.Dispose() supposed to be called? I added
a Debug.Log in there, but it never shows up in the log.

from unirx.

Sorry for delay response.
You're right, it's my mistake and your solution is great!
I'm merged from your branch, thanks.

When is EditorThreadDispatcher.Dispose()

EditorThreadDispatcher.Dispose is never called.
It's singleton and not influence performance in editor.
Hmn, is remove IDisposable better?

from unirx.

Edit: Oops, I just saw you already added the work-around to UniRx 5 hours ago :)

We have found another related issue: when subscribing for the first time from a worker thread, MainThreadDispatcher.Initialize(), which wants to create a GameObject, is called from the worker thread also.

A quick but dirty work-around is:

ScenePlaybackDetector.cs

    [PostProcessScene]
    public static void OnPostprocessScene()
    {
        IsPlaying = true;

        // ensure the dispatcher GameObject is created by the main thread
        // adds a GameObject to every scene that is started
        MainThreadDispatcher.Initialize();
    } 

Downside: A MainDispatcherThread GameObject is added to every scene that is started, instead of only when UniRx is used for the first time.

Hmn, is remove IDisposable better?

Maybe so. I can't think of a situation where one wants to dispose the EditorThreadDispatcher.

from unirx.

When Awake ScenePlaybackDetector.IsPlaying is false unfortunaly.
following code cause null exception.

void Awake()
{
    MainThreadDispatcher.OnApplicationQuitAsObservable().Subscribe();
}

I changed in 1bcb411

@@ -266,7 +253,7 @@ public static void Initialize()
             {
 #if UNITY_EDITOR
                 // Don't try to add a GameObject when the scene is not playing. Only valid in the Editor, EditorView.
-                if (!ScenePlaybackDetector.IsPlaying) return;
+                if (!Application.isPlaying) return; // Initialize must call from MainThread
 #endif

maybe works fine.

from unirx.

I want to call Initialize automatically before Awake ideally.
But I can't find the way.

from unirx.

You are right, the order is like this:

Press Play button

EditorApplication.playmodeStateChanged (isPlaying = false; isPlayingOrWillChangePlaymode= true)
...
OnValidate (sometimes after OnDidReloadScripts)
OnDidReloadScripts (isPlaying = false; isPlayingOrWillChangePlaymode= false)
OnValidate (it's called twice)
Awake
OnEnable
OnPostProcessScene
OnApplicationPause
OnApplicationFocus
Start
FixedUpdate
Update
LateUpdate
OnPreCull
OnPreRender
OnPostRender
OnRenderObject
EditorApplication.update (isPlaying = true; isPlayingOrWillChangePlaymode= true)

I will test more callbacks. If I can't find a callback for the exact moment between pressing the Play Scene button and before any scripts on game objects are handled, I will ask the Unity Team for help. I think EditorApplication.playmodeStateChanged should be called before Awake, but it's called after Start!

from unirx.

Alright, instead of

    [PostProcessScene]
    public static void OnPostprocessScene()
    {
        IsPlaying = true;
        MainThreadDispatcher.Initialize();
    }

it seems we can use

    [DidReloadScripts]
    public static void OnDidReloadScripts()
    {
        IsPlaying = true;
        MainThreadDispatcher.Initialize();
    }

DidReloadScripts is not only triggered on scene start, but also for example when editing and creating new files. Therefore it's possible that multiple MainThreadDispatchers are added to the same scene. So we need to polish this approach a bit more.

from unirx.

I thought I had a solution with the code below. Unfortunately when OnDidReloadScripts is called, all variables are reset, including static variables in static classes.

private static bool _isPlaying = false;

// InitializeOnLoad ensures that this constructor is called when the Unity Editor is started.
static ScenePlaybackDetector()
{
    EditorApplication.playmodeStateChanged += () =>
    {
        // Before scene start:          isPlayingOrWillChangePlaymode = false;  isPlaying = false
        // Pressed Playback button:     isPlayingOrWillChangePlaymode = true;   isPlaying = false
        // Playing (after Start()):     isPlayingOrWillChangePlaymode = false;  isPlaying = true
        // Pressed stop button:         isPlayingOrWillChangePlaymode = true;   isPlaying = true
        if (EditorApplication.isPlayingOrWillChangePlaymode && !EditorApplication.isPlaying)
        {
            // If you try to create the MainThreadDispatcher GameObject in this time, you will get the following error:
            // "Component MonoBehaviour could not be loaded when loading game object. Cleaning up!"
            // Fortunately we can move that job to OnDidReloadScripts(), which is called a bit later, but still before Awake().
            //MainThreadDispatcher.Initialize();

            _aboutToStartScene = true;
        }
        else
        {
            _aboutToStartScene = false;
        }

        // Detect when playback is stopped.
        if (!EditorApplication.isPlaying)
        {
            IsPlaying = false;
        }
    };
}

// This callback is notified after scripts have been reloaded.
[DidReloadScripts]
public static void OnDidReloadScripts()
{
    // Filter DidReloadScripts callbacks to the moment where playmodeState transitions into isPlaying.
    if (_aboutToStartScene)
    {
        IsPlaying = true;

        // Ensures the dispatcher GameObject is created by the main thread
        MainThreadDispatcher.Initialize();
    }
}

Looking for a new solution. :(

from unirx.

Thank you for an investigation.
In conclusion, is solution of initialize before start nothing?

If initialized = true, MainThreadDispatcher exists.
I add if(!initialized) before every check. b548ee6
(I removed UnsafeInitialize after it's commit)

If initialize on application entry point, this solution is fine.
But not yet settled problem

• don't call initialze and use Rx on Awake
• in editor mode(entry point is nothing)

Slightly serious.

from unirx.

If isPlaying = true is default

private static bool _isPlaying = true;

failed is only inEditor and Awake, it's still better.

from unirx.

I will test it with all use-cases:

• In the build, subscribing in Awake()
• In the Editor, when scene is playing, subscribing in Awake()
• In the Editor, when scene is not playing: ExecuteInEditMode, Editor script

In my opinion it should be possible to solve this issue for all use-cases, without requiring the user to manually initialize MainThreadDispatcher.

By the way, Mike Talbot aka WhyDoIDoIt has created a similar dispatcher for Unity called Loom. And users have reported similar problems with get_isPlaying as in this issue.

https://www.google.com/search?q=loom+get_isplaying
http://answers.unity3d.com/questions/662891/loom-initialise-per-app-per-scene-or-.html

from unirx.

After exploring many options, I think the best way is this:

Summary

In the Editor: MainThreadDispatcher will always run on EditorApplication.update.
For the Build: user has to put a MainThreadDispatcher into the scene.
Execution order: MainThreadDispatcher needs a .meta file with a negative value for Script Execution Order.

I will create a commit with those changes soon after cleaning up.

In the Editor

I think we can change MainThreadDispatcher to always run on EditorApplication.update in the Unity Editor. No need to switch to MonoBehaviour.Update() when the scene starts. No need to check Application.isPlaying. That simplifies the logic of MainThreadDispatcher and MainThreadScheduler. We can also remove ScenePlaybackDetector.

(I have an ugly backup plan if MonoBehaviour.Update() turns out to be absolutely necessary. We can save _aboutToStartScene in EditorPrefs and read it back in OnDidReloadScripts.)

In the Build

Outside the Unity Editor there is no notification that the game has started. There is no such thing as [InitializeOnLoad] for builds.

The dispatcher has to be on the main thread before Awake(). The only way to guarantee that is when the MainThreadDispatcher is in the scene before building the game.

We could make a script that automatically adds a MainThreadDispatcher to the scene. But we don't know whether the scene uses UniRx. MainThreadDispatcher would have to be added to every scene. I do not favor that approach, because it adds a dependency to UniRx.

Therefore I suggest MainThreadDispatcher must be added to the scene explicitly. Either:

• the user has to add MainThreadDispatcher manually as a prefab or component, or
• a framework that uses Rx internally (for example uFrame) adds MainThreadDispatcher automatically when creating a new scene.

If UniRx is called and there is no MainThreadDispatcher, show console error "UniRx requires a MainThreadDispatcher in the scene".

Execution order

To make sure MainThreadDispatcher.Awake() is run before other scripts' Awake(), we need to change its Script Order Execution. There are several ways:

• user needs to change Project Settings > Script Execution Order manually
• MonoImporter.SetExecutionOrder(script, order) in a static constructor
• .meta files with modified executionOrder value: probably the best way

from unirx.

Thanks.

I think always use EditorApplication.update in the editor is not good idea.
in EditorApplication.update, storange behaviour in Input.mousePosition.
for example

void Awake()
{
    Observable.EveryUpdate()
        .Where(_ => Input.GetMouseButtonDown(0))
        .Subscribe(x => Debug.Log(Input.mousePosition));
}

Sometimes output point is diffrent(maybe take from editor point?).
And use StartCoroutine can't return Coroutine.

Threfore, In the Edit(isPlaying) and In the Build are diferrent environment is not desirable.

//

In the build, currently MainThreadDispatcher is loaded automatically(when first call MainThreadDispatcher.Instance).
It works fine in most cases.
Exception is only when called the first from another thread but it's very rare.
(in UniRx, use Scheduler.ThreadPool explicitly only)
At the rare cases, users call explicitly Initialize is not bad.
Explicitly added to the Scene as an option is good but it's I want to be option.

from unirx.

I think the latest commit 8707f01 addresses your concerns. ScenePlayDetector is now using EditorPrefs. ScenePlayDetector.IsPlaying will now be true before Awake().

But I just found a bug related to an earlier commit:

If initialized = true, MainThreadDispatcher exists.
I add if(!initialized) before every check. b548ee6

When you stop the scene in the editor, the static variable MainThreadDispatcher.initialized is still true. This will cause editor scripts to look for a MainThreadDispatcher that's already removed.

To fix it we could do something like this:

    void OnApplicationQuit()
    {
        instance = null;
        initialized = false;
    }

or, since we can now determine when the scene has started, maybe we don't need if(!initialized) anymore.

from unirx.

Awesome!
Your idea is perfect!

Oh, It's a bug < initialized is true in the editor(editor).
We don't need if(!initialized) but we should fix it.
Is if UNITY_EDITOR necessary just to make sure ?

from unirx.

Thanks, I'm glad that we are close to a resolution to this issue!

I added some fixes. Yesterday I accidentally added the wrong .meta file for example, so the WorkerThreadTest scene was probably broken.

Also, I created a new example scene called MultipleDispatchersTest. It demonstrates a rare case where there are multiple dispatchers in the scene. I was wondering if a running timer-based Observable such as an Interval could be moved from a destroyed MainThreadDispatcher to another one. But I have a feeling I'm moving way too far into theoretical situations rather than practical ones.

However, there is one reasonable situation: scene switching. What if you add MainThreadDispatcher to a scene and LoadLevel another scene during run-time? Awake() does not include DontDestroyOnLoad(instance), so the dispatcher will be destroyed. In the loaded scene there will be a new dispatcher or none at all. Since co-routines are bound to the game object, any previously running Observables depending on co-routines will be stopped when the scene switches. Maybe that's a good thing. I'm not sure what the expected behavior is for this situation. The author of Loom had this to say about this topic:

So it "depends" if you ask me. I put a Loom initialization in each scene to stop there being any extraneous calls that get processed from the previous scene. If you DontDestroyOnLoad the component it's attached to (and you actually bother to attach it yourself to a scene component, so you are fully in control) then you can have it run once for the entire application - bearing in mind that it's possible queued actions will attempt to access things that no longer exist.

I should probably move this to a new issue :P

Is if UNITY_EDITOR necessary just to make sure ?

Which one do you mean?

from unirx.

I checked your MultipleDispatchersTest, CullingMode and any other test scenes.
I agree with your thoughts.
When Timer is spinning, multiple MainThreadDispatcher will cause unexpected results.
I think for this reason, there is a need it is only the first one.

Is if UNITY_EDITOR necessary just to make sure ?
Which one do you mean?

Ah, I thought Destroy dispatcher for only Editor is unnatural behaviour.
However it is not related anymore.

from unirx.

I have one question.
Is guranteed Thread.CurrentThread.ManagedThreadId == 0 on mainthread all platfrom?

from unirx.

Good question. In the Unity Editor Thread.CurrentThread.ManagedThreadId is often 1, even worker threads. But in a Windows build the threads' ManagedThreadIds are different. I'm not sure what causes that difference. I haven't tested other platforms. But I think the main thread ManagedThreadId is guaranteed to be 1.

You are asking because of this, right?

static void CheckForMainThread()
{
    if (Thread.CurrentThread.GetApartmentState() == ApartmentState.MTA || 
        Thread.CurrentThread.ManagedThreadId > 1 ||
        Thread.CurrentThread.IsBackground || 
        Thread.CurrentThread.IsThreadPoolThread)

It was based on this: http://stackoverflow.com/questions/2374451/how-to-tell-if-a-thread-is-the-main-thread-in-c-sharp but the accepted answer's code didn't work in Unity. Maybe checking for ApartmentState alone is enough.

If it's a problem we can remove CheckForMainThread() and replace it with a try/catch block. The only downside is that it's not certain the exception comes from accessing Unity API from worker thread, but it's highly likely. Anyway, it's only for displaying a nice message.

from unirx.

Yes, I mentioned about CheckForMainThread. (Sorry, it's 1 rather than 0)

Unity's ManagedThreadId is different from .NET CLR and let alone do not know the behavior of the other platforms.
So I used ThreadStatic.

[ThreadStatic]
static object mainThreadToken;

But ofcourse it can use after initialize.
I think remove CheckForMainThread() and replace it with a try/catch block is good.
We need error message.

from unirx.

Done.

Note that when a game is built and run with the "Development Build" setting disabled, exceptions are skipped and the application will attempt to continue.

For example, the WorkerThreadTest that was designed to throw an exception will run as if the MainThreadDispatcher was added. That surprised me. I'm curious how it's possible that the dispatcher is created from a worker thread.

from unirx.

Sorry for late reply.
(Personal talk: My compnay release Unity game on iOS Appstore soon(today, I submit App Review) of course it use UniRx)

I thank for your hard work .
I'll merge it.

I'm curious how it's possible that the dispatcher is created from a worker thread.

Only way have possibilities, pass MonoBhaviour from the outside?

from unirx.

You're welcome :)

(Congratulations on your upcoming release!)

I'm curious how it's possible that the dispatcher is created from a worker thread.

Only way have possibilities, pass MonoBhaviour from the outside?

I found a possibly related question: Uncaught exception doesn't kill the application

I think Unity will throw errors when attempting to do thread-unsafe things in the editor and in the development build. Such as adding MonoBehaviour from worker threads. That way the developer can fix it.

However, in the release build you don't want to annoy the player with errors. Therefore Unity will continue to do unsafe things hoping for the best, although it might eventually lead to unexpected behavior or even a crash.

from unirx.