Coroutine WaitForSeconds

Question

I want my player to be immune when he gets the power up "vaccine" but one of my coroutines is being called which is the StopCoroutine but when when the player gets it should stop the "vaccine" after 5 seconds for I have the WaitForSeconds and the player will be immune again but it won't work for some reason. Is there a problem with my code?

public class Vaccine : MonoBehaviour
{
    public GameObject pickupEffect;

    public Score currentScore;
    public float BonusScore;
    private float immuneTime = 5f;
    public PlayerMotor player;
    private IEnumerator tookVaccineCo;

    private void Start()
    {
        currentScore = FindObjectOfType<Score>();
    }

    void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Player"))
        {
            var player = other.GetComponent<PlayerMotor>();
            Pickup();
            TookVaccine(player);
        }
    }


    private void TookVaccine(PlayerMotor playerMotor)
    {
        if (tookVaccineCo != null)
            StopCoroutine(tookVaccineCo);
        tookVaccineCo = TookVaccineEnum(playerMotor);
        StartCoroutine(tookVaccineCo);

    }

    IEnumerator TookVaccineEnum(PlayerMotor playerMotor)
    {
        playerMotor.isImmune = true;
        yield return new WaitForSeconds(immuneTime);
        playerMotor.isImmune = false;
    }

    void Pickup()
    {
        currentScore.CurrentScore = currentScore.CurrentScore + BonusScore;
        //apply effect for player


        //remove power up object
        Destroy(gameObject);
    }
}

Answer 1

you do

Destroy(gameObject);

the object which is responsible for executing the Coroutine -> it will never reach its end.

---

Option A - Move the routine into the Player

you could e.g. have a dedicated component on the player like

public class ImmunityController : MonoBehaviour
{
    [SerializeField] private PlayerMotor _motor;

    private void Awake ()
    {
        if(!_motor) _motor = GetComponent<PlayerMotor>();
    }

    private Coroutine _current;

    public void SetImmune(float immuneTime)
    {
        if(_current) StopCoroutine(_current);

        _current = StartCoroutine (ImmuneRoutine (immuneTime));
    }

    private IEnumerator ImmuneRoutine(float immuneTime)
    {
        _motor.isImmune = true;

        yield return new WaitForSeconds (immuneTime);

        _motor.isImmune = false;
    }
}

And now let your pickup object only start that routine

public class Vaccine : MonoBehaviour
{
    public GameObject pickupEffect;

    public Score currentScore;
    public float BonusScore;
    private const float immuneTime = 5f;

    void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Player") && other.TryGetComponent<ImmuneController>(out var player))
        {
            // only tell the player that he took a vaccine - the rest is his own responsibility
            player.SetImmune(immuneTime);
            currentScore.CurrentScore = currentScore.CurrentScore + BonusScore
            Destroy (gameObject);
        }
    }
}

This way you have decoupled the responsibility - I would prefer this option.

---

Option B - Only move the StartCoroutine to the player

The other option - though a bit more "dirty" in my eyes - would be to let the player execute the Coroutine itself while you still implement it in the Vaccine script!

This only makes sense though if it is really necessary to "obfuscate" a bit what exactly is being executed from the player so the player object shall not know exactly what Vaccine causes.

It would be a few simple changes without the need for a new script.

In the PlayerMotor have one additional field

public Coroutine tookVaccineCo;

and then do

public class Vaccine : MonoBehaviour
{
    public GameObject pickupEffect;

    public Score currentScore;
    public float BonusScore;

    private float immuneTime = 5f;

    public PlayerMotor player;

    private void Start()
    {
        currentScore = FindObjectOfType<Score>();
    }

    void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Player") && other.TryGetComponent<PlayerMotor>(out var player)
        {
            Pickup();
            TookVaccine(player);
        }
    }


    private void TookVaccine(PlayerMotor playerMotor)
    {
        // Magic! Simply let the player execute this routine without the need 
        // for letting him know what he is executing exactly
        // For correctly stopping the routine also let the player itself 
        // store the currently running routine
        if (player.tookVaccineCo != null) player.StopCoroutine(player.tookVaccineCo);

        
        player.tookVaccineCo = player.StartCoroutine(TookVaccineEnum(player, immuneTime)); 
    }

    // Also this is now "shared" between all instances
    // Wouldn't be necessary in the current state BUT if you are ever going to use something 
    // related to a specific instance of this script 
    // then you would have troubles -> now you simply can't do that by accident
    private static IEnumerator TookVaccineEnum(PlayerMotor playerMotor, float time)
    {
        playerMotor.isImmune = true;
        yield return new WaitForSeconds(time);
        playerMotor.isImmune = false;
    }

    void Pickup()
    {
        currentScore.CurrentScore = currentScore.CurrentScore + BonusScore;

        Destroy(gameObject);
    }
}

This way it is no problem either that the Vaccine object is destroyed since the routine itself is actually executed by the player object.

---

Option C - Disable components & Destroy later

Simply first only disable all relevant components. Then wait with the destroy until the Coroutine has finished

public class Vaccine : MonoBehaviour
{
    [SerializeField] private Renderer _renderer;
    [SerializeField] private Collider _collider;

    public GameObject pickupEffect;

    public Score currentScore;
    public float BonusScore;
    private float immuneTime = 5f;
    public PlayerMotor player;

    private Coroutine tookVaccineCo;

    private void Start()
    {
        if(!currentScore)currentScore = FindObjectOfType<Score>();

        if(!_renderer) _renderer = GetComponent<Renderer>();
        if(!_collider) _collider = GetComponent<Collider>();
    }

    private void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Player") && other.TryGetComponent<PlayerMotor>(out var player))
        {
            Pickup();
            TookVaccine(player);
        }
    }

    private void TookVaccine(PlayerMotor playerMotor)
    {
        if (tookVaccineCo != null)                StopCoroutine(tookVaccineCo);

        tookVaccineCo = StartCoroutine(tookVaccineCo);
    }

    private IEnumerator TookVaccineEnum(PlayerMotor playerMotor)
    {
        playerMotor.isImmune = true;
        yield return new WaitForSeconds(immuneTime);
        playerMotor.isImmune = false;

        // Now finally destroy this object
        Destroy(gameObject);
    }

    void Pickup()
    {
        currentScore.CurrentScore = currentScore.CurrentScore + BonusScore;

        // ONLY disable power up object
        _renderer.enabled = false;
        _collider.enabled = false;
    }
}

Answer 2

You are destroying your Vaccine instance gameobject in void Pickup() with Destroy(gameObject);. That does not seem a good idea. If you do Destroy(gameObject); when you enter your trigger:

void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Player"))
        {
            var player = other.GetComponent<PlayerMotor>();
            Pickup(); //this calls Destroy(gameObject);
            TookVaccine(player);
        }
    }

The current gameobject that holds your Vaccine monobehaviour instance gets destroyed, so no logic that was placed in there will work anymore. In case that might be the problem you are having.

I think you need to handle the player immunity from a script whose home gameobject does not get destroyed, the one handling the player behaviour (in your case think it might be PlayerMotor seems a good idea, instead of the Vaccine.