Multiple coroutines problem

Hello,

I’m making a script which is a powerup that, when collected, the player will become invisible and enemies’ bullets will not collide with it during 5 seconds.

As I have multiple powerup that do different things, I found it easier to create a delegate on the player that receives the effects from the powerups which in turn are coroutines. I needed to do it because when the powerup is collected, it is destroyed, therefore I need to run the coroutine in the player script in order to not destroy the coroutine with the powerup. Also every powerup inherits from a Powerup parent class which handles the things every powerup should do.

Every powerup is working well, except for the one I described, the ghost thing, because it works different from the others. The problem happens when the player collects 2 of this powerup, the second one being collected before the effect of the first one ended.

I found that it happens because the first coroutine is still running when the second one starts, and then there is a conflict because while the first coroutine tries to make the player visible again by increasing it color’s alpha overtime, the second one tries to make the player invisible by decreasing said value. This same thing happens with the bullets not colliding thing. After this, the player remains kinda invisible forever, but not the way it would be, and in Layer 0 (which collides with bullets; therefore, the functional effect isn’t active, just the bugged visual effect). When the player collects 1 of this powerup at a time, however, it works just fine.

Here is my code for the powerup:

    [SerializeField] float duration = 7f;
    [SerializeField] float opacity = 0.25f;
    [SerializeField] Material playerGhostMaterial; // Material with Rendering Mode == Transparent.
    [SerializeField] Material playerMaterial; // Material with Rendering Mode == Opaque.

    protected override void Awake()
    {
        base.Awake(); // Calling Awake method from Powerup parent class
        playerGhostMaterial = new Material(playerGhostMaterial);
        playerMaterial = new Material(playerMaterial);
    }

    protected override IEnumerator Effect(PlayerController player)
    {
        MeshRenderer[] meshRenderers = player.gameObject.GetComponentsInChildren<MeshRenderer>();
        playerGhostMaterial.color = player.gameObject.GetComponent<MeshRenderer>().material.color; // Because each player has different colors from each other.
        player.gameObject.GetComponent<MeshRenderer>().material = playerGhostMaterial;
        while (System.Math.Round(meshRenderers[0].material.color.a, 2) > opacity) // Fading to invisible.
        {
            foreach (MeshRenderer meshRenderer in meshRenderers)
                meshRenderer.material.color = new Color(meshRenderer.material.color.r, meshRenderer.material.color.g, meshRenderer.material.color.b, Mathf.Lerp(meshRenderer.material.color.a, opacity, 0.1f));
            yield return null;
        }
        player.gameObject.layer = 8; // Layer 8 doesn't collide with bullets.
        yield return new WaitForSeconds(duration); // Wait for the duration before the effect ends.
        while (System.Math.Round(meshRenderers[0].material.color.a, 2) < 1.0f) // Fading to visible.
        {
            foreach (MeshRenderer meshRenderer in meshRenderers)
                meshRenderer.material.color = new Color(meshRenderer.material.color.r, meshRenderer.material.color.g, meshRenderer.material.color.b, Mathf.Lerp(meshRenderer.material.color.a, 1.0f, 0.1f));
            yield return null;
        }
        player.gameObject.layer = 0; // Layer 0 is default, collides with bullets.
        playerMaterial.color = playerGhostMaterial.color;
        player.gameObject.GetComponent<MeshRenderer>().material = playerMaterial;
        yield break;
    }

Can anybody help me?

Also, I’m kinda new to Unity coding, so if there’s a better way to do something here, please let me know! And just to confirm, if I used Update() to check if the effect should end, the performance would be worse, wouldn’t it?

Thank you!

Not really no.

Honestly I’d go this route and take it once step further. Have each powerup add a component to the player and have that component’s Update monitor the state of the powerup and destroy itself when it’s done. Then you can check for redundant powerups by just using GetComponent.

Thank you, I’ll try to do this.
When should I use Update(), Coroutines and InvokeRepeating()? When is one better than the other in terms of performance? I mean, doesn’t adding too much things to Update() compromise performance as it’s called like so much times per second? I was trying to avoid it haha

You should just have one coroutine for any active effect. If you collect a second one for example you just add the time to the duration of that effect type. So the original coroutine will keep running longer.
There are various ways to track if a coroutine is running or not.

You have other options such as stopping the current coroutine and the starting another.

I use update when I know I will be doing something every frame for long periods of time.
I will use coroutines even if it will run every frame sometimes if I want to execute long running tasks over many frames.

I never use invoke repeating.

Coroutines really are just update loops that unity tracks the delay times for you. Every frame it’s having to ask does this coroutine need to be executed.

If you know that is true every frame an update can be a little better because it doesn’t check.
But readability and the ability to easily write sequential code across many frames coroutines are worth paying the tiny extra costs.

But these are both on resonable scales.
You should not have 10000 updates. Should use one update and a list of things.
You should not be starting lots of coroutines every frame. Start it once and have it do work from a queue etc. and yield the rest of the time.

Write the code so you can understand it and it works. Once it’s working then just profile it.
And if it’s slow change it.
Most things are ok. A lot of times it just comes down to how many times and how often it is executed.
A little slower but easier to understand is better if it’s only executed on time per game.

Coroutines are evaluated every frame as well. The performance difference typically isn’t worth worrying about. The key differences are that coroutines will continue to run on disabled components whereas Update will not; and if your code throws an exception in a coroutine it kills the entire coroutine. You also, of course, have to manage exactly the sort of thing you’re talking about now - making sure you don’t run the same coroutine multiple times.

And yeah, don’t use InvokeRepeating :slight_smile:

How would I do this “add the time” thing when collecting a second one? I’m using WaitForSeconds() to keep track of when the effect should end…

Subtract delta time from an initial duration.

public class SomePowerup : MonoBehaviour
{
    float duration;

    void Update()
    {
        duration -= Time.deltaTime;
        if (duration <= 0) Destroy(this);
    }
}

I was talking about Coroutines haha how can I check if there’s a running coroutine of this powerup and then, if there is, make it last longer on the WaitForSeconds()?

StartCoroutine gives you back the coroutine you started so you’d have to keep each one in some kind of lookup - maybe a dictionary with a string key that is the name of the powerup. In terms of modifying the existing one’s duration - you can’t do that.

Don’t use WaitForSeconds(). Make your own loop to check the duration time, something like this:

while (duration > 0f)
    yield return null;

That way, you’re free to just extend the duration amount while the coroutine is still running.

Also, you can store a reference to the coroutine when you create one using StartCoroutine. You can use this reference to check if a coroutine’s already running or not. (Which KelsoMRK just pointed out while I was typing this :slight_smile: )

EDIT: Just don’t forget to subtract deltaTime from duration! It’s up to you whether you want to do that in the coroutine, or in player’s Update().

Sorry, I’m not quite understanding… How would I create this reference and then get it back later and modify the duration value?

It’s not very straight forward. You’d have to keep the initial duration somewhere (whatever is wrapping the coroutine?) and then keep another value that tracks the actual duration of the effect and subtract delta time from it and then provide a method for resetting the tracked duration back to the initial duration.

Ultimately, you’ll have to make a class for each unique coroutine anyway so I’d suggest again going the component route.

Here’s an idea of how it would work with a single powerup:

    public class Player : MonoBehaviour
   {
       private Coroutine _coroutine;
       private float _duration;

       private void Update()
       {
           if (Input.GetKeyDown(KeyCode.Space))
           {
               _duration = 5f;

               if (_coroutine == null)
                   _coroutine = StartCoroutine(Powerup());
           }
       }

       private IEnumerator Powerup()
       {
           while (_duration > 0)
           {
               _duration -= Time.deltaTime;

               Debug.Log("Time remaining: " + _duration);

               yield return null;  // or use WaitForTime with some small value (maybe 1 sec) so you don't spam Debug.Log every frame :smile:
           }

           _coroutine = null;
       }
   }

Here you can check whether _coroutine is null or not, which tells you if the coroutine is still running or not. If it’s still running, don’t create a new one. Simply reset/extend _duration.

To support multiple coroutines… well, that’s going to have to be up to you. Like KelsoMRK suggested, you could make a dictionary of powerups, where you use the type as the key, and a struct to contain the coroutine, duration, and whatever other data is needed.

I guess you could get away with passing in the initial duration and manually stopping the coroutine if it’s running

if (cor != null)
{
    StopCoroutine(cor);
}
cor = StartCoroutine(PowerUp(5));

IEnumerator PowerUp(float duration)
{
    var d = duration;
    while (d < duration)
    {
        // ...
    }
}

Another suggestion is to wrap all of this up into a PowerUp ScripableObject that you could drag-drop into a field on whatever class is handling the collider for the powerup. This gives you a little more power in terms of type checking because SO’s are strongly typed and then you can use your coroutines and skip the component adding and destroying part. The upshot is - don’t try to handle all of this on the Player itself :slight_smile:

1 Like

Thank you guys!
I tried the add component thing but coudn’t make it work the way I wanted. So I changed the coroutine and made it work, so when the player pickups a new powerup of this type, the old one is stopped.
Here’s the final code:

    public override IEnumerator Effect(PlayerController player)
    {
        MeshRenderer[] meshRenderers = player.gameObject.GetComponentsInChildren<MeshRenderer>();
        playerGhostMaterial.color = player.gameObject.GetComponent<MeshRenderer>().material.color;
        player.gameObject.GetComponent<MeshRenderer>().material = playerGhostMaterial;
        player.gameObject.layer = 8;
        while (System.Math.Round(meshRenderers[0].material.color.a, 2) > opacity)
        {
            if(player.gameObject.layer == 0) // This will be true if there was already a running coroutine of this powerup.
                player.gameObject.layer = 8;
            foreach (MeshRenderer meshRenderer in meshRenderers)
                meshRenderer.material.color = new Color(meshRenderer.material.color.r, meshRenderer.material.color.g, meshRenderer.material.color.b, Mathf.Lerp(meshRenderer.material.color.a, opacity, 0.1f));
            yield return null;
        }
        while (duration > 0f)
        {
            if (player.gameObject.layer == 0) // This will be true if there was already a running coroutine of this powerup.
                player.gameObject.layer = 8;
            duration -= Time.deltaTime;
            yield return null;
        }
        player.gameObject.layer = 0;
        while (System.Math.Round(meshRenderers[0].material.color.a, 2) < 1.0f)
        {
            if (player.gameObject.layer == 8) // This will be true if a new coroutine of this powerup started after the running one.
                yield break;
            foreach (MeshRenderer meshRenderer in meshRenderers)
                meshRenderer.material.color = new Color(meshRenderer.material.color.r, meshRenderer.material.color.g, meshRenderer.material.color.b, Mathf.Lerp(meshRenderer.material.color.a, 1.0f, 0.1f));
            yield return null;
        }
        playerMaterial.color = new Color(playerGhostMaterial.color.r, playerGhostMaterial.color.g, playerGhostMaterial.color.b);
        player.gameObject.GetComponent<MeshRenderer>().material = playerMaterial;
        yield break;
    }

Thanks for the help, guys!