C# delegate call to an instance method sometimes loses the instance (this == null)

Hey all, so I needed a way to call Invoke() with a time delay that isn't affected by Time.timeScale. To do this, I wrote the following helper class:

Code (csharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4.  
5. public delegate void Invokable();
6.  
7. public static class Common {
8.     private struct InvokableItem
9.     {
10.         public Invokable func;
11.         public float executeAtTime;
12.        
13.         public InvokableItem(Invokable func, float delaySeconds)
14.         {
15.             this.func = func;
16.             this.executeAtTime = Time.realtimeSinceStartup + delaySeconds;
17.         }
18.     }  
19.    
20.     static List<InvokableItem> invokeList = new List<InvokableItem>();
21.     static List<InvokableItem> invokeListPendingAddition = new List<InvokableItem>();
22.     static List<InvokableItem> invokeListExecuted = new List<InvokableItem>();
23.    
24.     /// <summary>
25.     /// Invokes the function with a time delay.  This is NOT
26.     /// affected by timeScale like the Invoke function in Unity.
27.     /// </summary>
28.     /// <param name='func'>
29.     /// Function to invoke
30.     /// </param>
31.     /// <param name='delaySeconds'>
32.     /// Delay in seconds.
33.     /// </param>
34.     public static void InvokeDelayed(Invokable func, float delaySeconds)
35.     {
36.         invokeListPendingAddition.Add(new InvokableItem(func, delaySeconds));
37.     }
38.    
39.     // must be maanually called from a game controller or something similar every frame
40.     public static void Update()
41.     {
42.         // Copy pending additions into the list (Pending addition list
43.         // is used because some invokes add a recurring invoke, and
44.         // this would modify the collection in the next loop,
45.         // generating errors)
46.         foreach(InvokableItem item in invokeListPendingAddition)
47.         {
48.             invokeList.Add(item);  
49.         }
50.         invokeListPendingAddition.Clear();
51.        
52.        
53.         // Invoke all items whose time is up
54.         foreach(InvokableItem item in invokeList)
55.         {
56.             if(item.executeAtTime <= Time.realtimeSinceStartup)
57.             {
58.                 if(item.func != null)
59.                     item.func();
60.                
61.                 invokeListExecuted.Add(item);
62.             }
63.         }
64.        
65.         // Remove invoked items from the list.
66.         foreach(InvokableItem item in invokeListExecuted)
67.         {
68.             invokeList.Remove(item);
69.         }
70.         invokeListExecuted.Clear();
71.     }
72. }
73.  

In my game controller script (a monoBehavior), I call the following code in the update function to process any pending invokes:

Code (csharp):

1. Common.Update()

My game controller script also calls the InvokeDelayed() function in one of its instance methods, reinvoking itself:

Code (csharp):

1. function FindNearestDock()
2. {
3. // this == null here, but only sometimes, typically a few seconds after reloading the game
4.    
5.     Common.InvokeDelayed(new Invokable(this.FindNearestDock), 1); // recall this function every second
6.     //Invoke("FindNearestDock", 1); // old code that works fine!
7. }

Does anyone know why FindNearestDock() is occasionally being called on a non-instance object, meaning that "this" is null, like its trying to call it as a static function?

 

You are talking about reloading. Do you clear your invoke lists when you reload? That doesn't seem to be the case right now.

 

Does it happen when youve made no code changes?

Im assuming you arnt changing code, having unity re-compile while game is running.

 

Thank you for the quick reply. Clearing the static lists on startup did seem to fix the issue!!! But, now I'm confused I also tried using a singleton version of this script where the lists were not static, and I had the same issues, so i didn't suspect the static lists were a problem. Here's my class in its singleton form:

Code (csharp):

1. using UnityEngine;
2. using System.Collections;
3. using System.Collections.Generic;
4.  
5. public delegate void Invokable();
6.  
7. public class Common {
8.     private struct InvokableItem
9.     {
10.         public Invokable func;
11.         public float executeAtTime;
12.        
13.         public InvokableItem(Invokable func, float delaySeconds)
14.         {
15.             this.func = func;
16.             this.executeAtTime = Time.realtimeSinceStartup + delaySeconds;
17.         }
18.     }
19.    
20.     private static Common _instance = null;
21.     public static Common Instance
22.     {
23.         get
24.         {
25.             if( _instance == null )
26.                 _instance = new Common();
27.  
28.             return _instance;
29.         }
30.     }
31.    
32.     List<InvokableItem> invokeList = new List<InvokableItem>();
33.     List<InvokableItem> invokeListPendingAddition = new List<InvokableItem>();
34.     List<InvokableItem> invokeListExecuted = new List<InvokableItem>();
35.    
36.     /// <summary>
37.     /// Invokes the function with a time delay.  This is NOT
38.     /// affected by timeScale like the Invoke function in Unity.
39.     /// </summary>
40.     /// <param name='func'>
41.     /// Function to invoke
42.     /// </param>
43.     /// <param name='delaySeconds'>
44.     /// Delay in seconds.
45.     /// </param>
46.     public void InvokeDelayed(Invokable func, float delaySeconds)
47.     {
48.         invokeListPendingAddition.Add(new InvokableItem(func, delaySeconds));
49.     }
50.    
51.     // must be maanually called from a game controller or something similar every frame
52.     public void Update()
53.     {
54.         // Copy pending additions into the list (Pending addition list
55.         // is used because some invokes add a recurring invoke, and
56.         // this would modify the collection in the next loop,
57.         // generating errors)
58.         foreach(InvokableItem item in invokeListPendingAddition)
59.         {
60.             invokeList.Add(item);  
61.         }
62.         invokeListPendingAddition.Clear();
63.        
64.        
65.         // Invoke all items whose time is up
66.         foreach(InvokableItem item in invokeList)
67.         {
68.             if(item.executeAtTime <= Time.realtimeSinceStartup)
69.             {
70.                 if(item.func != null)
71.                     item.func();
72.                
73.                 invokeListExecuted.Add(item);
74.             }
75.         }
76.        
77.         // Remove invoked items from the list.
78.         foreach(InvokableItem item in invokeListExecuted)
79.         {
80.             invokeList.Remove(item);
81.         }
82.         invokeListExecuted.Clear();
83.     }
84. }

I was then calling Common.Instance.InvokeDelayed(...), and Common.Instance.Update() to use it as a singleton.

Is this possibly a problem because _instance itself is a static and needs to be nullified or something? If nothing else, you helped me get up and running, so thanks for that!! I'm just trying to learn what was wrong with my singleton now

 

Correct. I have a lot of things in my project that go very unhappy when i let unity recompile at runtime, so I just don't do that. I always restart the game after code changes.

 

At the moment you only check if the delegate object is not null before calling it. I think you should also be checking that the delegate's target is not null because that may have been destroyed since the delegate was added to your list.

Code (csharp):

1.  
2. if(item.func != null  item.func.Target != null)
3.   item.func();
4.  

 

I usually make my singletons inherit MonoBehaviour and place them in a GameObject. Like that I have a little more direct control. E.g. in your scenario, you would only need to implement OnLevelWasLoaded and clear the lists in there.

 

You are right that this would work. But with a good design this won't and shouldn't be necessary.

 

Possibly. What happens when a component issues an invoke and then something else happens that means the component should be destroyed (eg. something based on user input). Is it up to the component to make sure it doesn't destroy itself until the invoke happens? Should there be a way for the component to remove it's invoke from the list before it's destroyed? Should these components only be destroyed at fixed times when it's safe? IMHO all of these would make the system harder to use.

 

For me there is no question that there should be a manual deletion of all pending invokes when a component is destroyed. That's pretty much a no brainer. You have to add this deletion in OnDestroy. This avoids lots of maintenance in the long run.
Otherwise you may be in the situation that you know, I started this invoke, why did nothing happen? You may look at many different places why it is not executed. Whereas if you use the deletion function in OnDestroy, you will directly notice that this is related to the invoke and clears everything because it is destroyed.

 

The check for null was just there because I had read about doing that, not necessarily because it was needed by me. As far as checking the target, with the singleton method, the target should always be valid. Its a game controller script monobehavior in my scene, and it was never destroyed since the singleton instance was created (so the list in the instance should never have a null target).

I do keep a static var instance : GameControllerScript in my game controller for easy access everywhere else in code, is this possibly still running after a reload (along with a new instance that overwrites the variable?) Maybe that old instance is adding it's invokes, then being garbage collected?

I guess I don't know much about statics and how they survive across launches.

 

Your singleton will persist across play sessions (until Unity recompiles the assemblies) just like the static lists would unless you let Unity manage it (by turning it into a MonoBehaviour or ScriptableObject). Personally, I use MonoBehaviours so that I can easily visualize "Oh, holy crap - there are two WhateverManager's in the scene... that shouldn't happen!"

Generally, from what I can tell, Unity only reinitializes static variables/objects when it recompiles (or when the app is relaunched outside of the editor).

 

Okay then, thanks for the info! That explains a LOT.