Description
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.
Activity
Barsonax commentedon May 12, 2020
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 commentedon May 18, 2020
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 commentedon May 18, 2020
While writing the coroutine doc page, I read the definition on Wikipedia, and at the beginning there is this sentence (bold is mine):
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 commentedon May 19, 2020
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 aIEnumerable<WaitUntil>
to aWaitUntil
so now we do need aFunc<int>
field onWaitUntil
so you can do this: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 callingGetEnumerator()
will allocate a object but no way around that)With the above change you should be able to wait for other coroutines:
Which inlined would be equivalent to:
SirePi commentedon May 19, 2020
Would a restructuring of WaitUntil like this be bad? (comments removed)
Barsonax commentedon May 19, 2020
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 commentedon May 24, 2020
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 commentedon May 24, 2020
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.