Am I using Coroutines Correctly?

Scripting
1

I decided to take a look at coroutines today because I noticed that my Update callback was getting busy already, and coroutines seemed like a better way to do things. Here’s the problem I tried to solve and how I solved it.

I have a player character which always needs to be facing an object. The object it faces may move at any time, but it may not always be moving. When the player character notices that it is not facing the correct direction, it starts an animation and begins turning toward the object. When it is facing the object again, it stops turning and goes back to waiting until the object has moved again.

This is pretty easy to solve in update. You just check whether you’re turning or not. If you are, you check if you need to stop, and if you’re not, you check if you need to start.

But there’s no need for that conditional check. I know whether I’m turning or not. So I thought it would be better to move to coroutines, where I can only check whichever one I need to check. Here’s my solution:

	// Use this for initialization
	protected void Start()
    {
        SetupCoroutines();
	}

    void SetupCoroutines()
    {
        StartCoroutine(Update_NotTurning());
    }
	

    IEnumerator Update_Turning()
    {
        while (turnState == TurnState.turning)
        {
            transform.rotation = Quaternion.Slerp(transform.rotation, targetOrientation, turnSpeed * Time.deltaTime);
            float dProd = Mathf.Abs(Quaternion.Dot(transform.rotation, targetOrientation));
            if (dProd >= 0.99)
            {
                FinishTurning();
            }
            yield return 0;
        }
    }

    IEnumerator Update_NotTurning()
    {
        while (turnState == TurnState.notturning)
        {
            float dProd = Mathf.Abs(Quaternion.Dot(transform.rotation, targetOrientation));
            if (dProd < 0.99)
            {
                BeginTurning();
            }
            yield return 0;
        }
    }

    void BeginTurning()
    {
        turnState = TurnState.turning;
        BeginTurnAnimation();
        StartCoroutine(Update_Turning());
    }

    void FinishTurning()
    {
        turnState = TurnState.notturning;
        BeginIdleAnimation();
        StartCoroutine(Update_NotTurning());
    }

So a few questions:

  1. Is this a good use for coroutines?
  2. Am I doing everything right here? No leaks, no running both coroutines at once, or not cleaning up after myself?

It seems to work fine, but I just wanted to make sure that I’ve understood it all, so that I can safely do similar things elsewhere instead of a variety of state machines.

2

Personally I would probably prefer the update version, because it keeps the conditional code together, which (for me) would make it easier to understand what’s going on when reading back the code.

Especially if the main reason for doing this is removing a single conditional check. Even if it were for the iPhone it’s extremely unlikely that a conditional check on the player (i.e. happens only once per frame) would have any sort of significant performance impact. However, you can put this down to my personal taste ;).

The fact that you have to ask this question supports my response to the other :P. The update function approach is just less complex.

That being said, as far as I can tell from the code, it looks correct.

3

Thanks Tom, that’s what I was hoping to hear. I converted it as an excuse to learn about coroutines, but I couldn’t really see any value in having done so, even when it was converted. I suppose it will result in the Update function being less cluttered in the long run, but I could have done that by refactoring that chunk of code into a new method called in Update too.

I’m pleased to hear that it looks right though. Thanks again.

4

Any further comments on my use (misuse?abuse?) of Coroutines here? It’s one of the biggest changes for me, coming to Unity now, so if I’m not really getting it, I’d like to know where I’m going wrong.

5

If there are time-specific events which must occur, coroutines are a good way to ensure accuracy. (For example, a count-down).

If there are events which may take an undefined amount of time, and you don’t want to have your game freeze/pause, coroutines are a good way to accomplish this. (For example, getting a texture from a WWW request).

It sounds like you did this because all this code in a single function (your Update) was getting too long. There isn’t really abuse here, but it seems a bit unnecessary. You’re not really using coroutines for what they were designed for, but it’s not going to cause any performance problems. When you’re doing can be done through Update() exactly as you have it now, so it’s really just your preference on whether to use Unity’s method, or create your own.