Coroutine stored in a variable class seperated from the Mono Behavior

General run down of what is going on.
I am attempting to crate a modifier building block system using class, inheritance, virtual + override. It should be faster if I just show the codes.

Code (CSharp):

1. public class ModifiersBrain : MonoBehaviour
2. {
3.     public List<Modifier> modifiers;
4.  
5.     public void AddModifiers(Modifier Type)
6.     {
7.         Debug.Log(Type.Name + " Added");
8.         modifiers.Add(Type);
9.         Type.OnSet();
10.     }
11.  
12.     public void DestroyModifiers(Modifier Type)
13.     {
14.         Type.OnExpire();
15.         modifiers.Remove(Type);
16.         Debug.Log(Type.Name + " Removed");
17.     }
18.     public void UpdateModifiers()
19.     {
20.         foreach (Modifier modifier in modifiers)
21.         {
22.             modifier.UpdateMod();
23.         }
24.     }
25.     public Coroutine StartUpCoroutine(IEnumerator coroutine)
26.     {
27.         Coroutine co = StartCoroutine(coroutine);
28.         return co;
29.     }
30.     public void ShutDownCoroutine(Coroutine coroutine)
31.     {
32.         StopCoroutine(coroutine);
33.     }
34.  
35. }

Code (CSharp):

1. [System.Serializable]
2. public class Modifier
3. {
4.     public ModifiersBrain Host { get; set; }
5.     public string Name { get; set; }
6.     public int Stacks { get; set; }
7.  
8.     public virtual void OnSet()
9.     {
10.  
11.     }
12.     public virtual void UpdateMod()
13.     {
14.  
15.     }
16.     public virtual void OnExpire()
17.     {
18.  
19.     }
20. }

Code (CSharp):

1. namespace Modify
2. {
3.     public class Burning : Modifier
4.     {
5.         public Enemies OnEnemy { get; set; }
6.         private WaitForSeconds wait = new WaitForSeconds(1);
7.  
8.         public Coroutine DPS;
9.  
10.         public Burning(int i, Enemies enemy, ModifiersBrain host) //This modifier is only enemy exclusive
11.         {
12.             Name = "Burning";
13.             Stacks = i;
14.             OnEnemy = enemy;
15.             Host = host;
16.         }
17.         public override void OnSet()
18.         {
19.             Debug.Log("Burning On Set");
20.             //When set, will check to see if the modifier holder actually already have one with the same name, if so, increase their stack by this and destroy this modifier.
21.             for (int i = 0; i < Host.modifiers.Count; i++)
22.             {
23.                 if (Host.modifiers[i].Name == Name && Host.modifiers[i] != this)
24.                 {
25.                     Debug.Log("Burning On Set Found");
26.                     Host.modifiers[i].Stacks += Stacks;
27.                     Host.DestroyModifiers(this);
28.                     break;
29.                 }
30.             }
31.             DPS = Host.StartUpCoroutine(SetTimerForDamageUpdate());
32.         }
33.  
34.         public override void OnExpire()
35.         {
36.             Host.ShutDownCoroutine(DPS);
37.         }
38.         void DealDamageToEnemy()
39.         {
40.             OnEnemy.PureDamage(Stacks);
41.         }
42.  
43.         IEnumerator SetTimerForDamageUpdate()
44.         {
45.             while (true)
46.             {
47.                 yield return wait;
48.                 DealDamageToEnemy();
49.             }
50.         }
51.     }
52. }

The main problem currently is that I purposely have the Coroutine be set using a method within the brain Mono Behavior which work relatively well, but when I need to remove the Coroutine, I proceed to use the method that removes said Coroutine that is early on set as a variable under Coroutine DPS; When I return to the brain to stop the Coroutine, using the method, it actually works completely correctly and soundly, except for 1 problem.

Unity is throwing this error towards me when I feel like it's actually a false positive.

NullReferenceException: routine is null

This error blocks the remaining code within the DestroyModifier method, and I am unsure if there is a way to basically tell Unity this is actually a false positive. And if it is not a false positive, then what is actually the problem here.

I suspect the problem lies in the fact that Unity is expecting the Coroutine to be in the Mono Behavior but became alarmed when it is not actually there.

 

Well, it can not be a false positive since this exception is most likely thrown here. So the routine you pass most likely is actually null. Now there can be several reasons for this. Though one big issue I already see is that you have a bi-directional communication and manipulation across your modifiers and the host monobehaviour. Especially the modifier you've shown does iterate through all modifiers of the host in his OnSet callback where it actually removes items from the modifiers list. This causes all sorts of issues, especially since you iterate through the list forwards. So when a modifier is actually removed, you would skip the next.

Reasons why your DPS variable is null would be quite simple. Imagine you have an instance of your modifier attached and active. Now you try to attach the same modifer again. Just look at what happens. Your "AddModifiers" method (which should be singular, not plural) first adds the new modifer to the end of the list, then it calls the OnSet method of the modifier. Now what happens inside that method? Right we iterate through all modifiers looking for other modifiers that have the same name but is not this one. If we find one, we just add our value to the existing one and call "DestroyModifiers" on ourselfs. Keep in mind we are still inside OnSet and we haven't started our coroutine yet. That means we destroy our new instance while it hasn't been started properly yet. So of course the DPS variable would be null since it is set when the coroutine is started after the while loop. Though we haven't left the while loop yet.

So either add a null check to your OnExpire callback, or rethink your flow of control.

 

Ah yes, I've finally noticed the problem. Must have missed it and thought it was some other problem.

 

I've been looking into things and I've realized the problem, the object variable is likely still running after the DestroyModifiers is called, which ends up starting up the DPS Coroutine after that part, hard to explain, but it was causing there to be a ghost modifier to stay, I fixed it by forcing it to start the DPS Coroutine, which I feel like that is not a great idea to start a Coroutine just to stop it immediately.

Code (CSharp):

1.  
2.         public override void OnSet()
3.         {
4.             Debug.Log("Burning On Set");
5.  
6.             DPS = Host.StartUpCoroutine(SetTimerForDamageUpdate());
7.             //When set, will check to see if the modifier holder actually already have one with the same name, if so, increase their stack by this and destroy this modifier.
8.             for (int i = 0; i < Host.modifiers.Count; i++)
9.             {
10.                 if (Host.modifiers[i].Name == Name && Host.modifiers[i] != this)
11.                 {
12.                     Debug.Log("Burning On Set Found");
13.                     Host.modifiers[i].Stacks += Stacks;
14.                     Host.DestroyModifiers(this);
15.                     break;
16.                 }
17.             }
18.             Debug.Log("Burning On Set After");
19.         }

Edit: I've used a return; Should have done this sooner, I am a noob.

 

Why do you start a coroutine that you don't want to run in the first place? As I said, the main issue is that you try to stop a coroutine that you haven't started yet. So a null check inside OnExpire would be the most logical approach. Of course when you decide that you don't want this instance to actually run, so you call "DestroyModifiers" on it, you should of course exit the "OnSet" method on the spot by using

return;

. So you never start the coroutine for the modifier that you just removed anyways.

 

That is the face value problem, but the actual problem is that I assumed that when I used remove modifier from the list, it would actually also clear the memory that stop the whole process but that is actually not the case, instead, because the GC have not kicked in yet, the rest of the code gets called and so causing the coroutine to start in the lower part of the method.

 

Uhhh, you have the complete wrong idea about managed memory and code execution if that was your assumption. You do know about the call stack? When you call a sub routine / method / function, you will aways return to the caller when the method has finished or is aborted for some reason. The only two reasons why methods may be aborted are: You explicitly leve the method by calling "return" which brings you back to the caller and resumes execution after the method call. The other case is that an exception is thrown due to an error in which case the execution can not continue. If an exception is thrown, the execution will continue in the closest exception handler as you go up the call stack. If there is no try catch block anywhere the error will bubble up to the root which in case of Unity is the Unity engine itself which will catch any error caused in your code and display an error. In a "normal" application this would actually cause the application to be terminated by the OS if it's not catched inside the application.

The GC can only collect memory that is no longer referenced. When something is referenced from the currently executing stack it will never be collected since it's still referenced by the process itself. The CPU / thread is currently executing code inside that object, therefore there is still a reference to it. Apart from that the GC only runs rarely, sometimes even never if no new garbage is generated. It only runs when the system think its necessary. So loosing a reference to an object does not guarantee that the object is actually removed. However once you lost all referenced to such an object, you don't have to care about it anymore since there is no way you can access this object anymore (since you don't have a reference anymore...). Eventually the GC may clean up the object when the memory is needed. However the GC will not and can not clean up memory that is in use in any way.

 

That sounds like a record number of buzzwords to put into a single project. Are you sure that's a good idea? Nothing about making such a system requires any of that noise. You may wish to consider just getting something working first before going crazy with the Resume-Driven Development.

Unity doesn't "expect" anything about coroutines.

Coroutines in a nutshell:

https://forum.unity.com/threads/des...en-restarting-the-scene.1044247/#post-6758470

https://forum.unity.com/threads/proper-way-to-stop-coroutine.704210/#post-4707821

Splitting up larger tasks in coroutines:

https://forum.unity.com/threads/bes...ion-so-its-non-blocking.1108901/#post-7135529

Coroutines are NOT always an appropriate solution: know when to use them!

https://forum.unity.com/threads/things-to-check-before-starting-a-coroutine.1177055/#post-7538972

https://forum.unity.com/threads/heartbeat-courutine-fx.1146062/#post-7358312

 

What are you talking about? I am just asking about a problem and I want to avoid not giving enough information about what I am doing, Resume Driven Development? Are you high?

Thanks for the links, I will probably, look though them.