Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.
  2. Dismiss Notice

Showcase I'm open sourcing a small library I made a while ago to replace common uses of coroutines

Discussion in 'Scripting' started by MartinIsla, Aug 31, 2020.

  1. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    Hey everyone,

    A while ago I wrote some code because I was tired of using coroutines for simple things. It was meant to be for myself and for one project specifically, but I ended up using it for every single project I worked on since then, so I wrote a README, commented the code, wrote a few examples and it's now open source and free for everyone.

    Maybe it's not production-ready and the code quality is definitely not the best, but it's really good in terms of performance and it saved me a lot of time during game jams or when prototyping.

    Visit the GitHub page for a better explanation of what it can do and a few examples: https://github.com/MartinIsla/STasks/

    You're encouraged to submit pull requests if you come up with new features or are feeling like improving the code readibility. You can also submit requests if you'd like me to add features!

    I truly hope this makes your life easier! Oh, and if you do use this, please contact me if you have questions (or maybe just say "hey! i'm using your thing!", that would make me happy).

    Happy development!
     
    Ian094, jamespaterson, Vryken and 5 others like this.
  2. Vryken

    Vryken

    Joined:
    Jan 23, 2018
    Posts:
    2,106
    Code (CSharp):
    1. Debug.LogError("Max task capacity reached. What are you doing.");
    Got a chuckle out of me.
     
  3. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    lol I totally forgot to edit that one! I'll leave it like that.

    Since we're here: MAX_CAPACITY is a constant that can be modified. 256 should be more than enough, but maybe there's some really weird case where it's necessary to increase that number. Performance junkies like me should try to keep that number as low as possible!
     
  4. Joe-Censored

    Joe-Censored

    Joined:
    Mar 26, 2013
    Posts:
    11,847
    Looks interesting. What happens to an STask when the GameObject or component is disabled? What happens when the component is destroyed?
     
  5. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    Great question!
    I actually touched this topic in one of the examples included with the package (see OnDestroy in this file).
    The task isn't attached in any way to a GameObject. Because of that and in order to prioritize performance over usability, you need to save tasks (especially those that depend on the GameObject not being null) and manually kill them before the GameObject is destroyed.

    Disabled GameObjects shouldn't give you errors because the references won't become null. However, the resulting behaviour is probably not what you'd expect. Maybe I could add an option to "pause" the tasks? That way you could do something like this:

    Code (CSharp):
    1. private void OnDisable ()
    2. {
    3.     task?.Pause();
    4. }
    5.  
    6. private void OnEnable ()
    7. {
    8.     task?.Resume();
    9. }
    This way you don't need to kill the task and don't lose the progress the task has done so far.
     
  6. gaglabs

    gaglabs

    Joined:
    Oct 17, 2019
    Posts:
    185
    This looks great. Awesome job!
     
    MartinIsla likes this.
  7. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    Thank you! If you use this, feel free to contact me anytime if you have questions, feature requests or need help with anything :)
     
    gaglabs likes this.
  8. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    If anyone is interested, I just updated the package following some feedback I received. Performance is way better now!
     
    gaglabs likes this.
  9. gaglabs

    gaglabs

    Joined:
    Oct 17, 2019
    Posts:
    185
    So im redoing my scrollviewcontroller.cs that pulls feeds from a loader script which in return pulls data from firebase. The loader script pushes the data to the scrollviewcontroller and that in return runs a coroutine that pulls images, video, and text from the server. Does your package deal with unity web requests?
     
  10. gaglabs

    gaglabs

    Joined:
    Oct 17, 2019
    Posts:
    185
    I'm assuming it doesn't affect the usage of unity web requests correct? It only handles the initial coroutine function?
     
  11. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    Sadly, you're correct. STasks is meant to help with gameplay functionality mostly (like "play a cutscene, disable input, enable it whenever the cutscene is over"). They use Update() under the hood, so web requests, which need to be async, aren't possible. They could probably work for checking whether a web request is complete, but you'd still have to use a classic coroutine to start the web request itself, so it wouldn't make too much sense.

    However, you're not the first person to ask if doing things like this is possible. I'll consider adding this kind of functionality in the near future!
     
    gaglabs likes this.
  12. SlimeProphet

    SlimeProphet

    Joined:
    Sep 30, 2019
    Posts:
    50
    This is really cool. It's helpful for learning to read through it, too; not too complex, not too simple. Thanks!
     
  13. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    I'm glad you find it useful for learning too! I apologize for the bad quality of the code in the STask class, especially the infinitely nested if-statements in the Update() method. I promise I'll clean it up some day.

    Also, feel free to contact me if there's anything I can help with!
     
  14. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    Update:
    I just finished refactoring the whole thing. Not only the code is easier to understand now, but also performance saw a huge improvement. Also, now the code is more clear, anyone should be able to create their own, more specific tasks (please, share your creations if you do!).
     
    Munchy2007 likes this.
  15. Bunny83

    Bunny83

    Joined:
    Oct 18, 2010
    Posts:
    3,527
    I've seen your original post on monday but hadn't time to comment yet :) When I saw it, it reminded me a bit of my CoroutineHelper which, instead of your approach, is essentially just a monobehaviour singleton host for coroutines with some added utility stuff. One of the main points of my helper was to be able to stop an arbitrary coroutine / task which wasn't possible back then. Now we have the 3 StopCoroutine versions which cover pretty much all needs for stopping.

    I'd like to point out some issues, especially with your latest refactoring. What immediately struck me was your added "_tasksAdded" variable to your STasksCollection. You only ever increase it but you never decrease or reset it. I'm sure you wanted to set to to 0 inside AddTask before you actually "count" the active tasks. So this is currently an actual bug.

    Apart from that it seems you reinvented the wheel with your STasksCollection and most you're doing isn't really great for performance. I would highly recommend to just use a generic List internally and just set the capacity to your wanted max size. If you want to have a max task count (for whatever reason, I don't really get why you would implement that) there's no point if starting with a smaller capacity. We're moving in a range where the consumed memory is neglectable. So if you want to have 1024 or even 10k max tasks, just go for it. Those 40k or 80k of memory just disapear in the noise.

    Next thing is you always iterate over the whole capacity, no matter what. Pretty much any operation you perfom on your collection has a complexity of O(n) where n is your capacity. The main point of a generic List is that we have a larger array with a certain capacity and a seperate count variable and we condense the active tasks to the beginning of the array. That way we will only ever iterate over the active tasks without any unnecessary gaps. Of course the naive approach to remove a task would be to use RemoveAt. This will move all following elements one index down. However since the order of the tasks is not guaranteed at the moment, we could simply fill a gap with the last element in the list so removing an arbitrary task becomes an O(1) operation, no matter the capacity or task count.


    Code (CSharp):
    1.  
    2.     public class STasksCollection
    3.     {      
    4.         private List<STask> tasks;
    5.         private readonly int maxSize;
    6.  
    7.         public STasksCollection(int initialSize, int maxSize = 0)
    8.         {
    9.             this.maxSize = maxSize;
    10.             if (maxSize > 0)
    11.                 initialSize = maxSize;
    12.             tasks = new List<STask>(initialSize);
    13.         }
    14.  
    15.         public void Update (float deltaTime)
    16.         {
    17.             for (int i = 0; i < tasks.Count; i++)
    18.             {
    19.                 STask task = tasks[i];
    20.                 while(task.isDone && i < tasks.Count)
    21.                 {
    22.                         task = tasks[i] = tasks[tasks.Count - 1];
    23.                         tasks.RemoveAt(tasks.Count - 1);
    24.                 }
    25.                 if (!task.isDone)
    26.                     task.Update(deltaTime);
    27.             }
    28.         }
    29.        
    30.         public void AddTask(STask task)
    31.         {
    32.             if (maxSize > 0 && tasks.Count >= maxSize)
    33.             {
    34.                 Debug.LogError("AddTask failed! Reached maxSize: " + maxSize);
    35.                 return;
    36.             }
    37.             tasks.Add(task);
    38.         }
    39.        
    40.         public void RemoveTask(STask task)
    41.         {
    42.             int index = tasks.IndexOf(task);
    43.             if (index < 0)
    44.                 return;
    45.             tasks[index] = tasks[tasks.Count - 1];
    46.             tasks.RemoveAt(tasks.Count - 1);
    47.         }
    48.     }
    This will have O(1) AddTask complexity and O(1) auto task remove complexity. The Update complexity is of course O(n) but "n" is the number of active tasks, not the capacity. With a large enough initialSize or a given maxSize we don't have any additional memory allocations from this collection besides the initial one.

    The while loop inside Update essentially fills gaps in the list with elements from the end of the list as long as the current task is dead / done. When an Update occurs there could be multiple tasks that are "done". So when we encounter a finished task we simply fill the element with the last one from the list and remove the last one. This is an O(1) operation since there are no further elements behind the last item. Of course the last item could also be done already. That's why we have a while loop that runs until we find an active task or we reached the end of the list. So we can clean up the task list as we update the tasks with no extra work necessary.

    Note that I made the "tasks" list private since that's an implementation detail and no one outside the class should mess with it. I've added a RemoveTask method (which has O(n) complexity since it has to find the task in the list) in order to remove a task from the outside. Though this usually isn't necessary since we can simply set isDone to true and it will be removed. However there might be cases where we want to temporarily remove a task and re-add it later, so it might be useful, at least from this class's perspective.

    The final point I want to make is about your UpdateHelper. While multicast delegates and C# events are quite nice to work with, they have a nasty add and remove habit. Whenever you add or remove a listener from a multicast delegate, you actually create an entire new delegate and essentially copy the internal list of listeners every time. So the more listeners are subscribed to a delegate, the heavier adding and removing listeners gets. Have a look at the MultiCastDelegate implementation. You always have a memory allocation when you do this. Of course in your case you only subscribe a single method to each event / delegate. However since it's a standalone singleton like class users would be tempted to use it for other things. It might be better to just use a List of delegates and iterate through them manually when you want to invoke them. I'm just thinking about reducing / avoiding any garbage allocation that isn't necessary.
     
  16. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    First of all, you're awesome and the first person to make a code review who actually wants to help and not just demonstrate Programmer Superiority™. I definitely learned a few things from your comment. Thank you so much.
    I hadn't heard about CoroutineHelper, but STasks looks (mostly in terms of how it's used) a lot like yours. I swear I didn't copy!

    That part is what I was the most insecure about. Here's the thing. I tested lists before doing my own implementation with array resizing and such, because of course I'm not better than the people at Microsoft who created generic lists. I carefully profiled both solutions and it turned out searching and adding/removing elements from the list generates more garbage and is more expensive on the CPU than iterating a whole array and checking for nulls. That's why I went with an array. However, I see a few optimizations in your code I hadn't considered, so I'll give it another shot and report back!

    The max capacity isn't a performance thing but a quality of life thing. 1024 is an arbitrary number I came up with (which could be bigger, to be fair) because I don't want people to create a billion tasks (say by creating tasks every frame on Update()) either because they misunderstood how to use this thing or by accident.


    It's funny because UpdateHelper actually was a separate thing! I used it for classes that needed an update without being MonoBehaviours. I only learned about how bad delegates are in terms of performance this week, when I thought "hey what if I don't have a collection of tasks, but subscribe tasks directly to the UpdateHelper". I was more shocked than I should've.
    A list of delegates could work great for that one! I'll implement it.

    Expect news and a huge thank you in the github page!
     
  17. Bunny83

    Bunny83

    Joined:
    Oct 18, 2010
    Posts:
    3,527
    Don't worry, that's not what I had in mind at all. Your solution is completely focused on simple events / tasks while my helper is completely focused on coroutines. Also I have developed alot in the past that already existed before.

    I highly doubt that. A generic list does not allocate any memory at all unless it has to resize it's internal array. The thing is most of what you're doing in your STaskCollection class is pretty much the same or similar what the generic List class does.For example this is what the Add method actually does. So if the internal array is still large enough (given enough capacity) the Add method just stores the new item in the internal array at the latest unused spot and just increases the internal counter. This is (given no resize is necessary) an O(1) operation because the complexity is constant.

    Of course when a resize is necessary the List does pretty much the same that your script does. It creates a new array with the new, larger size and copies the old elements into the new array. This is what Array.Resize does as well. Though resizing is something that should never happen since we should pick a proper capacity to avoid any allocations at normal runtime. RemoveAt also just copies all elements behind the one that should be removed one element down to fill the gap and clears the last element (with default(T) which is just "null" for reference types). Though when removing the last element, only the last element will be affected. So removing the last element is also O(1). Again, a List is just a class wrapping a normal array, just like your own class.

    Well, don't put too much constraints on the usage of your classes. Especially when the class or system is a generic task system. The proper usage of a class is up to the user. The more you try to think for them, the less versatile and flexible a system gets. Having the ability to set a max value is nice, especially for debugging, however enforcing it is something I wouldn't recommend unless there's a really good reason for it.
     
  18. PraetorBlue

    PraetorBlue

    Joined:
    Dec 13, 2012
    Posts:
    7,722
  19. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    You're amazing. Thank you.

    You're right, it's a good idea to switch to Lists in this case. I'm testing now, new version should be up by tomorrow!
     
  20. MartinIsla

    MartinIsla

    Joined:
    Sep 18, 2013
    Posts:
    104
    I just tried and this is amazingly better in many ways. Thanks, I learned a lot from this discussion. How would you prefer to be credited?