Community

Question

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 :-(.

Answer 1

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"));

Answer 2

0

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.

answered Mar 11 '11 at 05:08 PM

MvD
149 ● 6 ● 7 ● 15

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. Maybe 675 given the last time it wouldn't find any object.

Mar 11 '11 at 05:26 PM Statement ♦♦

But a new and interesting approach I've never stumbled across. Memory usage is very low :)

Mar 11 '11 at 05:49 PM Statement ♦♦

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 :)

Mar 11 '11 at 06:13 PM Statement ♦♦

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.

Community

Unparenting a transform in a foreach loop in the parent transform

Extra reading that is slowly but steadily going off topic ahead:

Follow this question

Welcome to Unity Answers

Unity Answers tutorial video!

Related Questions