Can someone tell me if my code is broken, or Unity is? (C#, serialization, Actions)

Unity Engine
1

I’ve got no clue why this behaves so poorly.

[Serializable] public class State {
	public Mesh mesh;
	public Action assignMesh;
} [SerializeField] State cube, sphere;

void Awake() {
	foreach (State state in new State[]{cube, sphere})
		state.assignMesh = () => GetComponent<MeshFilter>().mesh = state.mesh;	
	cube.assignMesh();	// Mesh is a sphere – WTF?
		
	cube.assignMesh = () => GetComponent<MeshFilter>().mesh = cube.mesh;
	cube.assignMesh();	// Cube, as expected.
		
	var states = new State[]{cube, sphere};
	for (int i = 0; i < states.Length; ++i)
		states[i].assignMesh = () => GetComponent<MeshFilter>().mesh = states[i].mesh;
	cube.assignMesh();  // Causes a crash!
//	IndexOutOfRangeException: Array index is out of range.
//	TestClass+<Awake>c__AnonStorey2.<>m__2 () (at Assets/TestClass.cs:22)
//	TestClass.Awake () (at Assets/TestClass.cs:24)
}
2

Because by the time you reach line 17, the loop variable i has a value of 2 because the loop has concluded. When the lambda function is evaluated, even though i is within scope of the Action, it is no longer within the loop and the loop variable i has reached it terminating value.

You have the same issue at line 9. The lambda function executes after the loop has finished, but you are attempting to retrieve the last value of the IEnumerator state, which in this case, is a sphere.

When you call the lambda functions at line 9 and line 17, they are not executing within the context of the loops they are assigned within, so any loop scope local variables are now invalid. The implementation details of the C# language indicate that the loop variables are still within function scope, but that their values are undefined.

3

Thanks. I get it now. Is there no way to use loops to initialize the assignMesh’s, then? I wanted to use loop variables for that, as shown above, but only for initialization, and I don’t think I know of a way to code that.

4

You need to store a Temporary variable:

foreach (State state in new State[]{cube, sphere})
{
        State tempState = state;
        tempState.assignMesh = () => GetComponent<MeshFilter>().mesh = tempState.mesh;   
}

for (int i = 0; i < states.Length; ++i)
{
        int index = i;
        states[i].assignMesh = () => GetComponent<MeshFilter>().mesh = states[index].mesh;
}

This is because the created delegate doesn’t check what it’s referencing until it’s invoked, meaning you need a different variable than your iterator if you intend to call it after the loop, or otherwise all elements in your loop look at the iterator object rather than an instance of what it was at the time it was set. This property can have some unusual effects like here(I ussually comment the temporary variable for clarity), but you can use it to your advantage as well. One example being you can have one process wait until a delegate returns true, and another that sets a bool to true when it completes, and so long as you pass the same bool reference to each delegate they can manage their interaction from then on without any more managing code.

5

Thanks. Now that I was taught what kind of problem I was seeing, I know what to look for.

So, although I get the idea of why this works, I’m not sure what it all means. Have an example?

Also, Ntero, although your second loop used the copied variable only where necessary, tempState isn’t required at the beginning of this line. (I was confused at first by you handling it two different ways, and while they both work, I guess this is clearer code to someone who really groks it?)

state.assignMesh = () => GetComponent<MeshFilter>().mesh = tempState.mesh;

…but for the for loop, I think this is a better way of doing it, because states has to be defined outside the loop, and could therefore be overwritten. Right?

for (int i = 0; i < states.Length; ++i) {
	var tempState = states[i];
	states[i].assignMesh = () => GetComponent<MeshFilter>().mesh = tempState.mesh;
}
states = null;	// no problem!
cube.assignMesh();
6

You are definitely correct with regards to the the foreach and for loops, where within the foreach both state or tempstate could be used as the reference to .assignMesh. And in the for loop, your idea is a safer bet, as it won’t go haywire if the array gets reorganized, I just wrote it quickly.

I’ll give an example of a good use for that passing references into delegates thing in a few hours. Though short answer is to to treat it like referencing variables in a Lambda or anonymous delegate always is ‘passed in’ via the ref keyword.

7

You only asked why it was broken. You never asked me to fix it. :slight_smile: **

** Okay, technically you didn’t “ask” but its close enough. :slight_smile:

8

No problem, Justin! You didn’t know what I needed to learn to get past the barrier. Both of you got me to the next sequential step in my understanding; I greatly appreciate it.

9

+1 I discovered this oddity just a few days ago, my code is as follows:

  foreach (var chr in characters)
                {
                    ...
                    if (GUILayout.Button(chr.Key))
                    {
                        var tempVar = chr.Key;  //otherwise lambda will default to last value for unknown reason.
                        mainGF.QueuedAction = () => StartCoroutine(SetChar(tempVar));
                    }
                   ...
                }

FYI - I’m now using lambda’s for my GUI rendering - it allows me, as long as I carefully follow the design, to make calls from my GUI to the main thread without the GUI crashing on me as it oft does.

10

You’re dipping your toes in to some very neat but ultimately advanced areas of programming, so few people get this far in their software development skills. You’re probably in the top 2% of software developers the world over based on your posts to these forums. When you start talking about lambda functions, delegates, events, multi-threading and a swathe of other concepts most programmer’s eyes glaze over.