Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Things to check before starting a coroutine?

Discussion in 'Scripting' started by wwu39, Oct 1, 2021.

  1. wwu39

    wwu39

    Joined:
    Nov 21, 2018
    Posts:
    26
    Calling coroutine outside of the gameobject sometimes yields ERROR. I met two yet: 1) the monobehaviour is being destroyed; 2) the gameobject is inactive. So what else should I care about before I start a coroutine?
    Code (CSharp):
    1. public void StartFindPath(Vector3Int startCoords, Vector3Int endCoords)
    2.     {
    3.         if (this && gameObject.activeInHierarchy) // checking if I can legally start a coroutine
    4.         {
    5.             findingPath = true;
    6.             StartCoroutine(FindPath(startCoords, endCoords));
    7.         }
    8.         else
    9.         {
    10.             onPathfindingFinished?.Invoke(this);
    11.             PathfindingManager.FinishProcessingPath(null);
    12.             findingPath = false;
    13.         }
    14.     }
     
  2. dgoyette

    dgoyette

    Joined:
    Jul 1, 2016
    Posts:
    4,193
    That seems pretty good to me. The other thing you might be concerned with is if the coroutine is already running. If it's possible for the coroutine to be started/restarted more than once, you might want to make sure you only have it running once. You can avoid that by calling StopCoroutine() before starting the coroutine again. For example:

    Code (CSharp):
    1. private Coroutine _countdownCoroutine;
    2. public void BeginCountdown(float time)
    3. {
    4.     if (_countdownCoroutine != null)
    5.     {
    6.         StopCoroutine(_countdownCoroutine);
    7.         _countdownCoroutine = null;
    8.     }
    9.     _countdownCoroutine = StartCoroutine(Countdown(time));
    10. }
    There may be cases where having one coroutine running multiple times on the same object is fine, but usually I find I want to interrupt the current routine and start over fresh with the new one.
     
  3. Bunny83

    Bunny83

    Joined:
    Oct 18, 2010
    Posts:
    3,919
    The real question what you want to do otherwise? At the moment you essentially circumvent the issue by not calling StartCoroutine. Though the question is why do you actually call this method in the first place? So there has to be a conceptional issue in your setup when this actually happens. In some rare cases just ignoring such issues may be a solution, especially when you need a quick fix late in development.

    However in a proper designed application this should never happen in the first place and ignoring / swallowing such issues blindly could actually hide other deeper issues. Don't be afraid of errors, they are here to help you.

    Though that said we don't have the full picture of your design, so this may be the right place to check for the state of the object.
     
  4. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,519
    The main thing to check before calling / making a coroutine is to ask yourself,

    "Does this even need to be a coroutine?"

    Generally, if your design requires you to call StopCoroutine(), then don't make it a coroutine.

    If you just need something to happen a bit later, this is a more-robust approach:

    https://gist.github.com/kurtdekker/0da9a9721c15bd3af1d2ced0a367e24e

    If you're flipping from one value to another, such as fading in, fading out, changing speeds slowly, etc., coroutines are an AWFUL solution to that. Instead use a pattern like this:

    Smoothing movement between any two particular values:

    https://forum.unity.com/threads/beginner-need-help-with-smoothdamp.988959/#post-6430100

    You have currentQuantity and desiredQuantity.
    - only set desiredQuantity
    - the code always moves currentQuantity towards desiredQuantity
    - read currentQuantity for the smoothed value

    Works for floats, Vectors, Colors, Quaternions, anything continuous or lerp-able.

    The code: https://gist.github.com/kurtdekker/fb3c33ec6911a1d9bfcb23e9f62adac4

    If you just need cooldown timers for limiting gunfire rates, use a cooldown timer:

    Cooldown timers, gun bullet intervals, shot spacing, rate of fire:

    https://forum.unity.com/threads/fire-rate-issues.1026154/#post-6646297

    GunHeat (gunheat) spawning shooting rate of fire:

    https://forum.unity.com/threads/spawning-after-amount-of-time-without-spamming.1039618/#post-6729841

    Far too often people (including very popular tutorial sites on Youtube) reach for needless coroutines and make a colossal unstable mess of their codebases as a result.

    Also: https://forum.unity.com/threads/heartbeat-courutine-fx.1146062/#post-7358312
     
    A1Qicks, D12294 and oscarAbraham like this.
  5. wwu39

    wwu39

    Joined:
    Nov 21, 2018
    Posts:
    26
    The design is that pathfinding is performance heavy and I'm splitting the work across frames by using yield return null. If a unit needs a path then it informs PathfindingManager and the manager queue up those requests and executes them one after another. The problem comes when a unit requests a path but die/inactive by the time the manager executes its FindPath. In that case the manager should move on to the next request.
     
  6. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,519
    That sounds like a very legitimate coroutine use.

    The method I would do is to have a single pathfinding manager that the units ask for pathfinding, then they get a callback when the pathing is done.

    That way if they have died, the pathing callback just dumps data into a dead object for subsequent consumption during that dead object's Update() loop... which never happens!

    The you can put in an OnDisable() method in each destroy-able object that can call the pathfinding manager and say "hey if you had business for me, forget it, I'm outie..."

    Or it could just let the work be done and discard it.

    The above lets everybody NOT care about what anybody else is doing. Everybody just knows what to do.
     
    Wilhelm_LAS likes this.