Hi
Why are we iterating through pathprefab.transform? Why is waypoint.transform iteratble through pathprefab.transform, just because waypoint is a child of pathprefab?
Logically I would have thought you’d iterate through pathprefab.waypoints, and get the transform of each gameobjects?
And logically you would be correct.
However, when the Unity programmers wrote the Monobehaviour classes, they specifically made it so that the Transform could be iterated through to return a child transform on each iteration.
You are right about how things normally work but Transform seems to be a special case, with this specific feature of Transform being described in the Unity Scripting Reference.
Yeah. You are absolutely right to ask this question. This is bad code smell, and certainly has been triggering my best practice sensibilities for hours now, so I figured I’d see if anyone else had something to say about this. Here’s the code snippet for anyone else who wants to follow along and understand why this is such a bad thing.
public List<Transform> GetWaypoints()
{
List<Transform> waveWaypoints = new List<Transform>();
foreach (Transform child in pathPrefab.transform)
{
waveWaypoints.Add(child);
}
return waveWaypoints;
}
That foreach is triggering a particularly bad code smell in me. I’ll explain why, so others can comment on this too. It helps to cover some other concepts so anyone not very familiar with c# will know more about what’s going on.
While loops do not necessarily have a preset number of iterations that they will run. They continue to execute until some condition is met, such as a variable changing to a specific value, a resource becoming available, an end of file being reached, etc. We use these loops if the nature of what we are doing doesn’t necessarily lend itself to a fixed number of iterations.
For loops have a fixed number of iterations, and we will naturally know what that count is going to be when we are building the loop. For example, you’ll have a count of enemies to spawn, or a number of elements in an array you need to do something with. When you think “I need to do this operation to all of these items here”, you are going to want a for loop in most cases.
Foreach loops at first SEEM to just be another way to write a for loop, but THAT is where the confusion is coming from here. Whereas a for loop uses a counter to go over a collection of items by index one at a time, a foreach loop does NOT use a counter at all. Instead it relies on a collection implementing the generic interface IEnumerable. What does that mean? Well, I will simply confuse the topic if I explained all of that here fi you don’t already know, but I will refer you to https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.ienumerable-1?view=netframework-4.7.2 where it covers this topic quite thoroughly.
Moving on to the important part here, our foreach loop is going to use the IEnumerator interface on any object passed into it, which must implement the public members Current, MoveNext(), Reset(), and Dispose.
Our GameObject pathPrefab has a public property transform of type Transform. The Transform type implements, you guessed it, IEnumerable, so there is a member public IEnumerator GetEnumerator(). That is what foreach is calling, and that is why this code actually works.
That method actually performs a depth first search to return all Transforms from all children under the GameObject (in a recursive tree search would be my bet). Why did Unity designers decide to implement IEnumerable on a member like this, and make the method name not indicate what it is actually doing? Who knows. I guess brevity, but I would argue strongly that this practice GREATLY reduces readability of the code, and complicates learning the environment for anyone new, and especially for anyone new to c# in general that doesn’t understand the nuances of Interfaces and type inheritance.
</rant over I guess>
I was confused by this as well, weird choice by Unity devs.
If anyone is interested here is a nice shorthand way to do this with
using System.Linq;
we can do
public List<Transform> GetWaypoints()
{
return pathPrefab.transform.Cast<Transform>().ToList();
}
Wow, thanks for digging into that and providing this explanation. That pathPrefab.transform bit had me baffled. Agree about this reducing the readability. At first I tried doing something crazy like
pathPrefab.GetComponentsInChildren<Transform>()
to improve the readability, but that ended up returning the transform from the PARENT as the first element of the list. Sheeeesh! An arguably more readable loop would be:
for (int i = 0; i < pathPrefab.transform.childCount; i++) {
waveWaypoints.Add(pathPrefab.transform.GetChild(i));
}
But I still don’t like the “magic” transform property.