koobas.hobune.stream
Unparenting a transform in a foreach loop in the parent transform
Hi everyone, This little piece of code has a bug.
using UnityEngine; using System.Collections;
public class TestRemoveChild : MonoBehaviour { public void Update() { if( Input.GetKeyDown( KeyCode.Y ) ) { RemoveChild(); } }
public void RemoveChild()
{
foreach( Transform child in transform )
{
Debug.Log( "child : " + child.name + "\n" );
if( child.name == "Child_1" ) // Or some given condition
child.parent = null;
// Next child won't be enumerated
}
}
}
If we set the parent of a transform in a foreach loop of the parent, the next child is not enumerated.
I know removing an item while enumerating is not generally a good idea. But Unity don't even throw a warning about that, either in the doc or at runtime. On the other hand, C# or Mono throws an exception when we do something like that.
And I just spent an afternoon figuring why some object was correctly suppressed when some other doesn't while both of them pass the condition :-(.
There is no question here. Put the children into a List, first, and then operate on that.
The solution for this bug is obvious. But again it's easy to check if we are in a foreach loop when we access to the parent property. Then we can either throw an exception (simple solution which prevents time loss) or it could be managed (more complex but I think it can be done).
Ok I didn't see there were the GetChild and the GetChildCount methods like Statement says below (well it's not a surprise since they are not documented :-(). With these methods, a for loop can be used and I can manage the index myself.
A simple example depicting the suggestion put by Jessy.
public void RemoveChild() { List<Transform> unparent = new List<Transform>(transform.childCount);
foreach (Transform child in transform)
{
if (child.name == "Child_1")
unparent.Add(child);
}
foreach (Transform child in unparent)
{
child.parent = null;
}
}
Extra reading that is slowly but steadily going off topic ahead:
You can make use of predicates to allow to unparent a child following an arbitrary test:
public void UnparentChildren(System.Func<Transform, bool> predicate) { List<Transform> unparent = new List<Transform>(transform.childCount);
foreach (Transform child in transform)
{
if (predicate(child))
unparent.Add(child);
}
foreach (Transform child in unparent)
{
child.parent = null;
}
}
And use it as such:
// Unparents children named Child_1 // Unparents children tagged Enemy UnparentChildren(t => t.name == "Child_1"); UnparentChildren(t => t.tag == "Enemy");
// Or do it like this, combined, "if name is Child_1 or if tag is Enemy": UnparentChildren(t => t.name == "Child_1" || t.tag == "Enemy");
And you can even make it a bit more reusable if you also provide an action so you can do other things that just unparenting them:
// Assume using System; public void ForEachChildThat(Func<Transform, bool> predicate, Action<Transform> action) { List<Transform> children = new List<Transform>(transform.childCount);
foreach (Transform child in transform)
{
if (predicate(child))
children.Add(child);
}
foreach (Transform child in children)
{
action(child);
}
}
Then you could do:
// Unparens children named Child_1
// Calls OnShakeOff on children tagged Enemy
ForEachChildThat(t => t.name == "Child_1", t => t.parent = null);
ForEachChildThat(t => t.tag == "Enemy", t => t.SendMessage("OnShakeOff"));
I seem to always overcomplicate it. Ugh. Just do with the first example :)
Like you say, you're not supposed to change the object you are enumerating (in this case 'transform').
This should work:
Transform childToRemove;
do
{
childToRemove = null;
foreach (Transform child in transform)
{
if (child.name == "RemoveMe")
{
childToRemove = child;
break;
}
}
if (childToRemove != null)
{
childToRemove.parent = null;
}
}
while (childToRemove != null);
You could use some fancy LINQ stuff if you want for the 'find' bit.
While this probably works, its complexity is quite bad. For example, had you 50 children and the last 25 were the ones to be removed you'd end up iterating 26 * 26 (676) times, or so. $$anonymous$$aybe 675 given the last time it wouldn't find any object.
But a new and interesting approach I've never stumbled across. $$anonymous$$emory usage is very low :)
But practically speaking, I did some performance tests on this approach vs. using a list, and there is no noticable difference at all until you start to reach into the thousands. And if you're dealing with thousands of children chances are you have bigger problems to handle :)
Yeah, it's quadratic, but, like you say, it will only become a problem when the number of children is really high. If you know that the children are 'vector-like', you can always do a single for-loop over the children and only increment the index when the child is not removed.
Or simpler: '...and don't increment the index when the child is removed'
This is an old question, but, here is how I deal with operating on children in a foreach loop a bit cleanly. Just add an extension method for Transform. Drop this class in your project:
using UnityEngine;
using System.Collections.Generic;
public static class TransformExtensions {
public static Transform[] GetChildren(this Transform transform) {
List<Transform> children = new List<Transform>();
foreach (Transform child in transform) {
children.Add(child);
}
return children.ToArray();
}
}
And use it like this:
foreach(Transform child in myTransform.GetChildren()) {
child.SetParent(someOtherTransform);
}
It's pretty much the same thing as the accepted answer, except it eliminates having to write two foreach loops everything you need to operate on children.
