Ellpeck / Coroutine

A simple implementation of Unity's Coroutines to be used for any C# project
https://www.nuget.org/packages/Coroutine
MIT License
37 stars 3 forks source link

[Feature Request] Co-routine Priority #6

Closed zaafar closed 3 years ago

zaafar commented 3 years ago

Let's say there are 5 co-routines yielding for a event called FOO. How do you make sure 2 of them are executed before rest of the 3. or 1 of them is executed first and then 2 and then rest of the 3.

This does not apply to time based co-routines since time-based co-routine can easily raise an event, under which the priority can be configured.

zaafar commented 3 years ago

Creating more than 1 event for the same task/event (e.g. FOO1 & FOO2 .... FOO100) is not performance efficient and scalable.

Ellpeck commented 3 years ago

This library was never intended to be used in a priority-sensitive context like this, to be honest. What is the use case that you need this behavior for? Maybe I can implement something like coroutine priority, though.

zaafar commented 3 years ago

The framework I am creating is monitoring a process in a time-based co-routine that is executed every 1 second (i.e. new wait(1d)). If the process window moves on the monitor I raise an event called OnProcessWindowMoved. I have a lot of co-routines that are yielding for the OnProcessWindowMoved event and some of those co-routines are dependent on one and the other.

One option is to implement priority in co-routines (implementation detail 1: default/unknown priority can be assumed as a minimum priority. implementation detail 2: priority is configured when adding event-based co-routines rather than while executing it. implementation detail 3: priority won't/can't be changed during co-routine execution).

Another option is to merge all that (multiple-co-routine) logic that's dependent on each-other into 1 big sequential logic. Over here I can use try to use events/delegate (to keep things modular).

Ellpeck commented 3 years ago

It would be great if you could try this prerelease and see if it works to your liking. If it does, I will release a proper update later tonight!

zaafar commented 3 years ago

Wow, that was fast!! I will test it and get back to you. Thanks!

zaafar commented 3 years ago

something wrong with this.tickingCoroutines.RemoveAll in public void Tick(double deltaSeconds). it's removing more than what it is suppose to remove.

Doing some further investigation.

Ellpeck commented 3 years ago

Are you sure? I tested with my own implementations (including the included example) and everything seemed to work fine. RemoveAll should have the same behavior as the loop that was there previously, so it would be weird if there was an error there.

zaafar commented 3 years ago

https://gist.github.com/zaafar/0a4fa1a711e9e7bfb92fc7c4d83ec912 bug ^

(calling it a bug because this feature was working before ~ not sure if this feature was intentional or not)

Ellpeck commented 3 years ago

Ah, interesting. I've never thought of the ability to start coroutines in other coroutines being a thing, but I assume RemoveAll doesn't really play well with that sort of behavior. I'll revert back to using the for loop then.

Ellpeck commented 3 years ago

oh, that didn't seem to have fixed it after all. Will do some more investigating.

zaafar commented 3 years ago

Worst case scenario if I have to choose between Priority feature and Nested Co-routine. I choose priority feature but do write in the readme the limitation of Nested-Coroutine (for others).

Ellpeck commented 3 years ago

All fixed! I'll do a release in a few minutes.