Iterating through GetTargets() one more time after waiting for destroyDelay

[plant]

I’m trying to set up an effect strategy that will enable/disable a particle effect attached to the target. The idea is that if I cast a fire spell, the targets will activate an “onfire” particle effect and take damage over time, and after the DoT expires deactivate the effect. For the most part this all works, with one issue:

I can not get the “disableFX” portion of the script to work, it doesn’t seem to ever get called.

Here is my effect strategy:

using System;
using System.Collections;
using System.Collections.Generic;
using Tribes.Combat;
using UnityEngine;


namespace Tribes.Abilities.Effects
{
    [CreateAssetMenu(fileName = "Spawn Status FX", menuName = "Abilities/Effects/Spawn Status FX", order = 0)]
    public class SpawnStatusFX : EffectStrategy
    {
        [SerializeField] float destroyDelay = -1;

        public override void StartEffect(AbilityData data, Action finished)
        {
            data.StartCoroutine(Effect(data, finished));  
        }

        private IEnumerator Effect(AbilityData data, Action finished)
        {

            IEnumerable<GameObject> targets = data.GetTargets();

            Debug.Log(targets);
            foreach (var target in targets)
            {
                target.GetComponentInChildren<StatusVFX>().EnableFireVFX();
            }

            if (destroyDelay > 0)
            {
                yield return new WaitForSeconds(destroyDelay);
                Debug.Log("Waited for: " + destroyDelay);
                foreach (var target in targets)
                {
                    target.GetComponentInChildren<StatusVFX>().DisableFireVFX();
                    Debug.Log("called disable fire ");
                }
            }

            finished();
        }
    }
}

and here is the component it calls, which is attached to an object childed under both the player and npc gameobjects.

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

namespace Tribes.Combat
{
    public class StatusVFX : MonoBehaviour
    {

        public void EnableFireVFX()
        {
            transform.GetChild(0).gameObject.SetActive(true);
            Debug.Log(gameObject.transform.parent + " set on fire ");

        }

        public void DisableFireVFX()
        {
            transform.GetChild(0).gameObject.SetActive(false);
            Debug.Log(gameObject.transform.parent + " extinguished ");
        }
    }
}

Based on the debug logs, it looks like its working up until the foreach loop after the WaitForSeconds line.

[plant]

 if (destroyDelay > 0)
            {
                foreach (var target in targets)
                {
                    yield return new WaitForSeconds(destroyDelay);
                    target.GetComponentInChildren<StatusVFX>().DisableFireVFX();
                    Debug.Log("called disable fire ");
                }
            }

If I move the yield return into the foreach loop then it kind of works, but it will delete the effects one by one, waiting every time.

[Brian_Trotter]

While still only being a bandaid (I’m not sure why it’s not working in the first place), Try moving the WaitForSeconds back out of the foreach loop, but including yield return null;` within the foreach loop.

[plant]

if (destroyDelay > 0)
            {
                yield return new WaitForSeconds(destroyDelay);

                foreach (var target in targets)
                {
                    yield return null;
                    target.GetComponentInChildren<StatusVFX>().DisableFireVFX();
                    Debug.Log("called disable fire ");
                }

Just tried running with the above, but I still does not work. I also tried WaitForSeconds(0), but that did not work either. It seems like if I wait before the foreach loop, then the loop never gets triggered.

[Brian_Trotter]

One more thing to try, as now I’m thinking that it’s possible that the targets IEnumerable is re-evaluating and coming up blank. (It’s a quirk of IEnumerables… as long as you keep them as IEnumerable, every time you reference them, like in a new foreach loop, they re-evaluate the query that made them in the first place – in something like a DelayedClickTargeting, this can mean that the targets left the circle during the delay…

Try changing

 IEnumerable<GameObject> targets = data.GetTargets();

to

List<GameObject> targets = data.GetTargets().ToList();

(You’ll have to add using System.Linq; to your usings clauses.)

What this does is lock in the targets collection as a list, it isn’t re-evaluated the next time hit a foreach.

Alternate Solution
Consider passing the destroyDelay on to the EnableFireVFX() method:

public void EnableFireVFX(float disableAfterSeconds=-1)
{
     transform.GetChild(0).gameObject.SetActive(true);
     if(disableAfterSeconds>0) Invoke(nameof(DisableFireVFX), disableAfterSeconds);
}

Then you only need one foreach loop in Effect:

foreach(var target in targets)
{
    target.GetComponentInChildren<StatusVFX>().EnableFireVFX(destroyDelay);
}

[plant]

Thank you for your help as always Brian! I tried changing the IEnumarable to a list, and now it is working.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using Tribes.Combat;
using UnityEngine;


namespace Tribes.Abilities.Effects
{
    [CreateAssetMenu(fileName = "Spawn Status FX", menuName = "Abilities/Effects/Spawn Status FX", order = 0)]
    public class SpawnStatusFX : EffectStrategy
    {
        [SerializeField] float destroyDelay = -1;

        public override void StartEffect(AbilityData data, Action finished)
        {
            data.StartCoroutine(Effect(data, finished));  
        }

        private IEnumerator Effect(AbilityData data, Action finished)
        {

            List<GameObject> targets = data.GetTargets().ToList();

            foreach (var target in targets)
            {
                target.GetComponentInChildren<StatusVFX>().EnableFireVFX();
            }

            if (destroyDelay > 0)
            {

                yield return new WaitForSeconds(destroyDelay);
                foreach (var target in targets)
                {

                    target.GetComponentInChildren<StatusVFX>().DisableFireVFX();
                }
            }

            finished();
        }
    }
}

[Brian_Trotter]

Glad to hear it!