Coroutines evolution

[SirePi]

Summary

This thread is to continue the discussions going on in the Coroutines' implementation PR #801, and to avoid cluttering the PR itself.

In particular, it emerged that further down the road, it could be worthy to investigate the following two aspects that are currently not managed, related to the management aspects of the new Coroutines system.

Saving/Loading Coroutines

As it is now, the system does not support saving and loading of Coroutines; it might be possible that during de/serialization the status is preserved "automagically", were the coroutineManager be serializable in the Scene, but testing is definitely required to be sure of the results of this operation.
From a quick glance at Unity's references, it appears that this topic still doesn't have an out-of-the-box solution, and relies on the user's management to determine which variables need to be saved and how to restore the Coroutine to the desired state.
https://docs.unity3d.com/Manual/Coroutines.html
https://docs.unity3d.com/ScriptReference/Coroutine.html
https://answers.unity.com/questions/1433878/how-to-store-the-state-of-ienumertor-and-resume-th.html
https://forum.unity.com/threads/how-can-i-save-and-load-a-coroutine-state.796401/

Automatically clearing up Coroutines when the spawning GameObject gets removed

Currently the system doesn't track, and is not interested in, the context in which a Coroutine has been spawned; the Coroutine has the capability to affect any number of Entities in any Scene, and its execution is anyway managed so that an error would not result in an unrecoverable state of the engine.

A possible solution, as proposed by @Barsonax, would be to assign to each GameObject (and/or Component) a list of Coroutines that would be automatically Cancelled once their respective "container" is being removed from the Scene. (Bonus: they could be paused/resumed automatically on de/activation). This would be an extra facility, in addition to the current way to start Coroutines.

[Barsonax]

A possible solution, as proposed by @Barsonax, would be to assign to each GameObject (and/or Component) a list of Coroutines that would be automatically Cancelled once their respective "container" is being removed from the Scene. (Bonus: they could be paused/resumed automatically on de/activation). This would be an extra facility, in addition to the current way to start Coroutines.

I think we should do this on component level and generalize it so it will work for more than just coroutines. In the future we might also have a job system which makes use of this and the enduser might also want to use this API.

[Barsonax]

Another thing we might want to look into in the future:
https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.iasyncenumerable-1?view=dotnet-plat-ext-3.1

Basically the combination of a itterator and async

[SirePi]

While writing the coroutine doc page, I read the definition on Wikipedia, and at the beginning there is this sentence (bold is mine):

Subroutines are special cases of coroutines.[3] When subroutines are invoked, execution begins at the start, and once a subroutine exits, it is finished; an instance of a subroutine only returns once, and does not hold state between invocations. By contrast, coroutines can exit by calling other coroutines, which may later return to the point where they were invoked in the original coroutine; from the coroutine's point of view, it is not exiting but calling another coroutine.[3] Thus, a coroutine instance holds state, and varies between invocations; there can be multiple instances of a given coroutine at once. The difference between calling another coroutine by means of "yielding" to it and simply calling another routine (which then, also, would return to the original point), is that the relationship between two coroutines which yield to each other is not that of caller-callee, but instead symmetric.

So, I was thinking.. would it be interesting to add in v4 a
WaitUntil.CoroutineEnds(IEnumerable<WaitUntil> coroutine)
that would yield until the sub-coroutine ends? (IsAlive is true)

[Barsonax]

So, I was thinking.. would it be interesting to add in v4 a
WaitUntil.CoroutineEnds(IEnumerable coroutine)
that would yield until the sub-coroutine ends? (IsAlive is true)

Thats a neat idea. I don't think thats currently possible though with how the WaitUntil is now. In order to do this we basically have to be able to convert a IEnumerable<WaitUntil> to a WaitUntil so now we do need a Func<int> field on WaitUntil so you can do this:

		public static WaitUntil CoroutineEnds(IEnumerable<WaitUntil> coroutine)
		{
			var enumerator = coroutine.GetEnumerator();
			enumerator.MoveNext();
			var currentCondition = enumerator.Current;
			return new WaitUntil(() =>
			{
				currentCondition.Update();
				if (currentCondition.IsComplete)
				{
					if (enumerator.MoveNext())
					{
						currentCondition = enumerator.Current;
					}
					else
					{
						return 0;
					}
				}
				return 1;
			});
		}

Other than making the WaitUntil struct a bit bigger this shouldnt impact performance for the other use cases that much. We do have to prevent allocations if possible (though calling GetEnumerator() will allocate a object but no way around that)

With the above change you should be able to wait for other coroutines:

		private IEnumerable<WaitUntil> CoroutineEnds(CoroutineObject obj)
		{
			obj.Value = 10;
			yield return WaitUntil.NextFrame;
			obj.Value = 20;
			yield return WaitUntil.CoroutineEnds(this.Counter(obj));
		}
		private IEnumerable<WaitUntil> Counter(CoroutineObject obj)
		{
			for (int i = 0; i < 5; i++)
			{
				obj.Value = i;
				yield return WaitUntil.NextFrame;
			}
		}

Which inlined would be equivalent to:

		private IEnumerable<WaitUntil> CoroutineEnds(CoroutineObject obj)
		{
			obj.Value = 10;
			yield return WaitUntil.NextFrame;
			obj.Value = 20;
			for (int i = 0; i < 5; i++)
			{
				obj.Value = i;
				yield return WaitUntil.NextFrame;
			}
		}

[SirePi]

Would a restructuring of WaitUntil like this be bad? (comments removed)

using System;
public struct WaitUntil
{
  public static readonly WaitUntil NextFrame = new WaitUntil(() => true);

  private readonly Func<bool> updater;

  public bool IsComplete { get; private set; }

  private WaitUntil(Func<bool> updater)
  {
    this.updater = updater;
    this.IsComplete = false;
  }

  internal void Update()
  {
    this.IsComplete = this.updater?.Invoke() ?? true;
  }

  public static WaitUntil Frames(int frames)
  {
    int counter = frames;
    return new WaitUntil(() =>
    {
      counter--;
      return counter <= 0;
    });
  }

  public static WaitUntil Seconds(float seconds, bool realTime = false)
  {
    float counter = seconds;
    return new WaitUntil(() =>
    {
      counter -= realTime ? Time.UnscaledDeltaTime : Time.DeltaTime;
      return counter <= 0;
    });
  }

  public static WaitUntil TimeSpan(TimeSpan timeSpan, bool realTime = false)
  {
    return WaitUntil.Seconds((float)timeSpan.TotalSeconds, realTime);
  }

  public static WaitUntil CoroutineEnds(IEnumerable<WaitUntil> coroutine)
  {
    IEnumerator<WaitUntil> enumerator = coroutine.GetEnumerator();
    enumerator.MoveNext();

    WaitUntil currentCondition = enumerator.Current;
    return new WaitUntil(() =>
    {
      CoroutineHelper.AdvanceCoroutine(enumerator, ref currentCondition, out bool isComplete);
      return isComplete;
    });
  }
}

[Barsonax]

Would a restructuring of WaitUntil like this be bad? (comments removed)

I think so. Not because the code is bad but because relying on delegates with a closure for everything means that there will be a allocation even for the common wait x frames/seconds use case.

Though having the possibility of using a delegate does open up some powerful ways to make abstractions around coroutines.

In the end I think we need to go with a hybrid approach and just add a extra switch branch for the delegate variant so we only get the allocation when calling another coroutine for instance (which should happen alot less than calling a API like WaitUntil.NextFrame).

[ilexp]

In the end I think we need to go with a hybrid approach and just add a extra switch branch for the delegate variant so we only get the allocation when calling another coroutine for instance (which should happen alot less than calling a API like WaitUntil.NextFrame).

Yeah, I think this would be a good way to go when extending it. The delegate-by-default thing would throw away some headway you made in the original design.

Edit: Ah, just saw the closed PR with the perf measurements - wouldn't have expected the impact to still be that big and even affect the baseline case that much. Good call to keep it like it is for now, though it would have been neat. Maybe there's a design with a smaller impact (or bigger payoff) some point down the line.

[Barsonax]

Ye the impact was pretty big even in the base case because the struct got bigger and theres quite alot of copying going on under the covers.

I think we need C# to provide something like a yield return all feature that allows you to directly yield a IEnumerable so the compiler can just generate the required code for you. Maybe in the future.