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

Add support for using AsyncAwaitUtil in Editor scripts #18

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

benvvalk
Copy link

@benvvalk benvvalk commented Nov 7, 2019

add support for using AsyncAwaitUtil in Editor scripts

minimum version requirement: Unity 2018.2.0

Please note that in addition to the changes contained in this pull request, safe use of AsyncAwaitUtil in Editor scripts requires Unity 2018.2.0 or newer. Older versions of Unity have fundamental issues with usage of async/await in Editor scripts, as discussed in this thread: https://forum.unity.com/threads/async-await-in-editor-script.481276/

problems solved by this pull request

This pull request adds support for using AsyncAwaitUtil in Editor scripts.

Unity version aside, there are currently two issues regarding usage of AsyncAwaitUtil in Editor scripts:

  1. Editor scripts will throw an NPE, because SyncContextUtil.Install is only invoked when entering Play Mode (see Cannot use for Editor unit testing, as SyncContextUtil.Install is not called except at runtime #9)
  2. Unlike Play Mode (and standalone builds), Unity does not "pump" coroutines in Edit Mode by calling MoveNext() on them at regular intervals.

Issue 1 is minor and is easily fixed in this pull request by adding the [InitializeOnLoad] attribute to SyncContextUtil.Install.

Issue 2 required a bit more work. To solve it, I added a new class called EditModeCoroutineRunner that tracks running coroutines and calls MoveNext() on them after each EditorApplication.update event. In addition, EditModeCoroutineRunner must correctly handle the various types of "yield instructions" (i.e. objects return by yield return) that have specially defined behaviour in Play Mode (e.g. WaitForSeconds).

tests

To test that AsyncAwaitUtil works correctly in Edit Mode, I have added a new editor GUI that runs the existing tests in Edit Mode, and which can be accessed from the Unity menu under Window => AsyncAwaitUtil => "Edit Mode Tests". Similar to the GUI for Play Mode tests in the AsyncTests scene, the user must manually run individual tests by clicking buttons in the GUI, and then interpret resulting output in the console to determine if that test succeeded/failed.

As per my notes about Unity versions above, many of the edit tests fail in Unity versions older than 2018.2.0. I have confirmed that all of the Edit Mode tests behave correctly in Unity versions 2018.2.0f2, 2018.3.0f2, 2019.1.9f1 with the following exceptions:

  1. In Unity 2019.1.9f1, the "Test multiple threads" test doesn't start a background thread as intended (not sure why).
  2. In Unity 2018.2.0f2 I see D3D/shader errors when running the "Load assetbundle" test, but those errors seem to be unrelated to async/await or AsyncAwaitUtil.

It may be possible to convert the Edit Mode tests into normal, automated unit tests that can be run with the Unity Test Runner. I attempted to do this using Unity 2017.1.1f1 but I ran into too many problems caused by poor Editor support for async/await support in that version (see my notes about Unity versions above). However, I haven't attempted to write unit tests in newer Unity versions and it may be worth a try.

As described modesttree#9, using Unity3dAsyncAwaitUtil in
Editor scripts causes an NPE, because `SyncContextUtil.Install` is never called
to initialize global variables `SyncContextUtil.UnitySynchronizationContext` and
`SyncContextUtil.UnityThreadId`.

Fix this by adding the `[InitializeOnLoadMethod()]` attribute to the
`SyncContextUtil.Install` method. Note that the existing
`[RuntimeInitializeOnLoadMethod]` attribute is still needed in order for
`SyncContextUtil.Install` to be correctly invoked in Play Mode.

Fixes modesttree#9
@StephenHodgson
Copy link

See #9 (comment) for a much more minimal change for editor async support

* Copy existing `AsyncCoroutineRunner` -> `PlayModeCoroutineRunner`
* Add new class `EditModeCoroutineRunner` for running coroutines in Edit Mode
* Modify `AsyncCoroutineRunner` to delegate to `PlayModeCoroutineRunner` or
`EditModeCoroutineRunner`, depending on whether Unity is in Play Mode or Edit
Mode

The main purpose of `EditModeCoroutineRunner` is to call `MoveNext()` on
in-flight coroutines at regular intervals. Unlike Play Mode, Unity does *not*
call `MoveNext()` on coroutines at regular intervals in Edit Mode.

In addition to pumping the coroutines by calling `MoveNext()`,
`EditModeCoroutineRunner` must also handle the various types of "yield
instructions" (i.e. `yield return` values) returned by the coroutines, which may
have specially-defined behaviour in Unity. (Two examples of yield instructions
with specially defined behaviour are `WaitForSeconds` and
`WaitForSecondsRealtime`.) The handling for the different types of yield
instructions is performed in `EditorModeCoroutineRunner.Update()`.
Added a new editor window for running tests in Edit Mode. Similar to the
existing tests in Play Mode, the Edit Mode tests are triggered by clicking
buttons in a GUI. The new Edit Mode Tests window can be opened from the Unity
menu by going to: Window => AsyncAwaitUtil => "Edit Mode Tests".

The test methods are shared between the Play Mode GUI and Edit Mode GUI and are
located in `AsyncUtilTests.cs` (as before).

Note 1: In order for all of the Edit Mode tests to run successfully, the Unity
version must be 2018.2.0 or higher! Earlier versions of Unity have fundamental
issues regarding usage of async/await in Editor scripts. See the following Unity
Forum thread for details:

https://forum.unity.com/threads/async-await-in-editor-script.481276/

Note 2: For maintenance purposes, it would be better if the Edit Mode tests were
implemented as normal unit tests that could be run by the Unity Test Runner. I
attempted to do this in Unity 2017.1.1f1, but found it impossible due to poor
async/await support in that version of Unity. However, it may be possible in
newer versions of Unity (I haven't tried).
@benvvalk
Copy link
Author

benvvalk commented Nov 8, 2019

@StephenHodgson

Thanks for the pointer to your patch. I actually didn't see it before I started writing my own solution, and I was a bit depressed to find out that I had spent ~4 days implementing/debugging/testing for nothing. Gah!

However! I tried running my Edit Mode tests with your changes and most of the tests do not run to completion (I tried with Unity 2018.3.0f2 and Unity 2019.1.9f1). It seems to be related to the use of yield new WaitForSeconds, which is used by most of the tests. I guess it isn't surprising that WaitForSeconds causes problems, since it has specially-defined behaviour in Play Mode. But even when I replaced WaitForSeconds with my own SafeWaitForSeconds class, the tests still fail to complete, and I'm not sure why.

In case you (or anyone else) wants to try it, I have combined your changes with my Edit Mode tests on the edit-mode-fixes-hodgson branch of my repo: https://github.com/AwesomesauceLabs/Unity3dAsyncAwaitUtil/tree/edit-mode-fixes-hodgson. The Edit Mode tests can be opened from the Unity menu under Window => AsyncAwaitUtil => Edit Mode Tests.

I would actually prefer use your solution as it is a lot simpler, but I guess I will leave this pull request open for the time being.

@benvvalk
Copy link
Author

benvvalk commented Nov 8, 2019

I forgot to mention: I tried (again) to write proper unit tests, this time with Unity 2018.3.0f2, but it still doesn't seem possible due to poor async/await support in the version of NUnit that ships with Unity. See this thread for further discussion: https://forum.unity.com/threads/async-await-in-unittests.513857/

Fixes compile error when doing standalone builds.
@dubois
Copy link

dubois commented Nov 27, 2019

I spent yesterday adding AsyncAwaitUtil into my project and learning how to unit test async code with Unity's somewhat old version of NUnit and only ran across this when I was thinking of sending a PR. Wish I had checked first!

Anyway, maybe these data points will be useful:

  • Like others, I used [InitializeOnLoad] and added a static constructor to SyncContextUtil
  • For in-editor coroutine support I'm using the package "com.unity.editorcoroutines": "0.0.2-preview.1" and it's been working fine for me so far
  • For unit tests, I'm using this pattern ( @benvvalk )
  [UnityTest] public IEnumerator Test_Something() {
    return Task.Run(async () => {
      /* non-boilerplate code here */
    }).AsIEnumerator();
  }

For example, here's a couple tests I wrote today:

  // A test that our integration of AsyncAwaitUtil and com.unity.editorcoroutines works.
  // Also serves as an example of how to test async code, because the Unity version of NUnit
  // doesn't have direct support for async test methods.
  [UnityTest]
  public IEnumerator TestAsyncAwaitUtilWorksAtEditTime() {
    return Task.Run(async () => {

      const int kNumFrames = 3;
      float dt = 0;
      // This should put us on the Unity thread...
      await Awaiters.NextFrame;
      for (int i = 0; i < kNumFrames; ++i) {
        dt -= Time.realtimeSinceStartup;  // ...and this will throw if we're not.
        await Awaiters.NextFrame;
        dt += Time.realtimeSinceStartup;
      }

      Assert.Less(dt / kNumFrames, 2f);

    }).AsIEnumerator();
  }

  // Checks that Future.Awaiter returns the proper value whether or not
  // the Future has completed at the time of the await.
  [UnityTest]
  public IEnumerator TestAwaitFutureSuccess() {
    return Task.Run(async () => {

      const int kValue = 3;
      var slowFuture = new Future<int>(() => { Thread.Sleep(20); return kValue; });
      Assert.AreEqual(kValue, await slowFuture);
      var fastFuture = new Future<int>(() => kValue);
      Thread.Sleep(50);  // plenty of time for fastFuture to complete
      Assert.AreEqual(kValue, await fastFuture);

    }).AsIEnumerator();
  }

Edit: to use AsIEnumerator() so exceptions propagate properly.

@dubois
Copy link

dubois commented Nov 27, 2019

Oh, I forgot I also had to put this awkward hack into AsyncAwaitUtil to integrate it with com.unity.editorcoroutines.

namespace UnityAsyncAwaitUtil
{
#if UNITY_EDITOR
    public static class AsyncCoroutineRunner {
        public static class Instance {
            public static EditorCoroutine StartCoroutine(IEnumerator routine) {
                return EditorCoroutineUtility.StartCoroutineOwnerless(routine);
            }
        }
    }
#else

@benvvalk
Copy link
Author

benvvalk commented Nov 29, 2019

Thanks very much for sharing your knowledge, @dubois!

I was not aware of the com.unity.editorcoroutines package, and I'm happy to hear that there is already a solution available that is written/supported by Unity themselves! I guess that probably renders this PR obsolete, but as a sanity check I will try out the package with my own project before I close this PR.

The unit test examples are also super helpful, so thank you!

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

Successfully merging this pull request may close these issues.

3 participants