Search Unity

Baffled by foreach loop! Skipping objects?

Discussion in 'Scripting' started by ideaengine, Aug 6, 2019.

  1. ideaengine

    ideaengine

    Joined:
    Jul 8, 2014
    Posts:
    9
    Ok. This is a total head scratcher to me.

    I have the following method in a GameObject...

    Code (CSharp):
    1.     public virtual void Execute()
    2.     {
    3.         Debug.Log("----------------------------");
    4.         foreach (var enableState in enableStates)
    5.         {
    6.             Debug.Log($"Executing Cue: {gameObject.name}\tSetting {enableState.gameObject.name} to {(enableState.enable ? "Enabled" : "Disabled")}");
    7.             enableState.gameObject.SetActive(enableState.enable);
    8.         }
    9.     }
    I hit command key 1
    It enables / disables 5 objects, as expected, and prints this log.


    [Log] ----------------------------
    [Log] Executing Cue: Starting Screen Setting Start to Enabled
    [Log] Executing Cue: Starting Screen Setting Dropdown to Enabled
    [Log] Executing Cue: Starting Screen Setting Option to Disabled
    [Log] Executing Cue: Starting Screen Setting Box to Disabled
    [Log] Executing Cue: Starting Screen Setting Results to Disabled

    I hit command key 2
    It enables / disables the same 5 objects, but a different pattern. (As desired)


    [Log] ----------------------------
    [Log] Executing Cue: Option Selected Setting Start to Disabled
    [Log] Executing Cue: Option Selected Setting Dropdown to Disabled
    [Log] Executing Cue: Option Selected Setting Option to Enabled
    [Log] Executing Cue: Option Selected Setting Box to Disabled
    [Log] Executing Cue: Option Selected Setting Results to Disabled

    I hit command key 1 again.
    It actually skips my first Log statement and 2 of the objects in the enabledStates list!!
    How is this even possible?


    [Log] Executing Cue: Starting Screen Setting Option to Disabled
    [Log] Executing Cue: Starting Screen Setting Box to Disabled
    [Log] Executing Cue: Starting Screen Setting Results to Disabled

    THEN I hit command key 1 again, and...
    It enables / disables 5 objects, as expected, and prints this log.


    [Log] ----------------------------
    [Log] Executing Cue: Starting Screen Setting Start to Enabled
    [Log] Executing Cue: Starting Screen Setting Dropdown to Enabled
    [Log] Executing Cue: Starting Screen Setting Option to Disabled
    [Log] Executing Cue: Starting Screen Setting Box to Disabled
    [Log] Executing Cue: Starting Screen Setting Results to Disabled

    None of the List objects change their contents between calling each version of Execute...

    It literally seems like some sort of compilation issue. Can't for the life of me determine what would cause a foreach loop to start midway through the list.

    No exceptions are thrown, so it's not hitting a null value (because it would hit it the second time through, which it doesn't).

    Completely at a loss...
     
  2. Madgvox

    Madgvox

    Joined:
    Apr 13, 2014
    Posts:
    1,317
    Do you have Collapse logs on in the console?
     
  3. ideaengine

    ideaengine

    Joined:
    Jul 8, 2014
    Posts:
    9
    @Madgvox I checked, but the two objects that appear to be getting skipped in the foreach loop are not getting enabled the first time. The second time I hit the command key, they appear.
     
  4. csofranz

    csofranz

    Joined:
    Apr 29, 2017
    Posts:
    1,556
    Unless, as Madgvox mentioned above, you have 'collapse' log active, the missing 'Log ---------' is a dead giveaway that you have other code that is being executed that you perhaps forgot to delete. Try to recall if you have any other script with similar code hanging around your scene that might accidentally be called. This is not a compiler issue.

    Oh, and why all the Billiard references :)?
     
    Madgvox likes this.
  5. Madgvox

    Madgvox

    Joined:
    Apr 13, 2014
    Posts:
    1,317
    That was going to be my next suggestion. Lines don't just "get skipped".
     
  6. palex-nx

    palex-nx

    Joined:
    Jul 23, 2018
    Posts:
    1,748
    Afaik foreach iteration skips elements what are inactive, does not it?
     
  7. csofranz

    csofranz

    Joined:
    Apr 29, 2017
    Posts:
    1,556
    It shouldn't skip, no [reason being: c# foreach should have no knowledge of Unity particulars like if a GameObject is active or not. They are in the List, and therefore they should be iterated. If you destroy a GameObject that's in a list,I don't know off-Hand what's going to happen - it's either removed (unlikely) or nulled, returning a null reference during iteration].

    But even if forach skipped inactive gameObjects - there is no way the Debug.Log("----") would be skipped, which I believe is a tell-tale that the entire code isn't executing.
     
    Last edited: Aug 6, 2019
  8. Madgvox

    Madgvox

    Joined:
    Apr 13, 2014
    Posts:
    1,317
    GameObjects that get destroyed are put into an invalid null-like state, meaning that if you attempt to do anything with them (other than check if they are
    == null
    ), they will throw an error.
     
  9. Owen-Reynolds

    Owen-Reynolds

    Joined:
    Feb 15, 2012
    Posts:
    1,998
    For a hammer&tongs style test, print enabledStates.count (or Length, depending). If you're not 100% sure foreach isn't doing something funny, also write a standard FOR loop. Print the index and the name.

    My guess is that enabledStates was remade before the third call. "Option" really is the first thing in the list. "Start" and "Dropdown", previously disabled, were excluded when the list was remade (maybe with gameObject.Find?).
     
  10. palex-nx

    palex-nx

    Joined:
    Jul 23, 2018
    Posts:
    1,748
    Operator "foreach" is just syntax sugar for iterators. All the actual work of building and traversing the list is done via collection implementing the iterator. In this case this is Transform class, wich obviously knows everything about game objects, their hierarchy and activity state.
     
  11. csofranz

    csofranz

    Joined:
    Apr 29, 2017
    Posts:
    1,556
    I really don't think so. For a number of reasons:
    1. foreach might be syntactical sugar, but it is implemented in the .Net library (System.Collection.Generic), not the Unity transform class. Dead giveaway: it's not used as transform.foreach(), but rather using a C# language Statement and block construct. Transforms are an iterable type, yes, but it's a rather big leap from that to conclude that an iterable type implements the iteration. A List<string> is a type that can be iterated. The string class, however, does not implement how foreach iterates it.
    2. The script is iterating a List<Component>, Component is implemented in UnityEngine.CoreModule
    3. I'm very, very convinced that the way a Transform is iterated is implemented in C# System.Collection.Generic, not UnityEngine.CoreModule - if it wasn't, Unity's Scripting Manual should be dripping with warnings about that
    4. Hacking the Compiler's Parser/Code Gen to substiture a foreach (Transform) with transform.foreach is entirely possible, but a hack so ugly I (hope) even the Unity devs baulk at it :)
    No, I really don't think Unity knows it's iterating a transform and would then check if the parenting GameObject is active in order to skip inactive ones.

    Belated Addition:
    Oh, and of course I only now remembered that I know that I can iterate a mixed list of active and inactive GameObjects because that's how my GameObject Poolmanager works. If Unity only iterated active GameObjects, my script would never find the first inactive GameObject in the pool to hand out to the requestor...

    Even later Addition:
    Of course, silly me overlooked the possibility that Transform was implemented as an IEnumerable class through implementing GetEnumerator(), invalidating most of my smartassery :)

    Still, pointing at me iterating a list of game objects with active and inactive GameObjects, I know that foreach will not skip inactive GameObjects
     
    Last edited: Aug 7, 2019
  12. palex-nx

    palex-nx

    Joined:
    Jul 23, 2018
    Posts:
    1,748
    Then you should read something about .net in general. Because during compilation foreach is replaced with enumerator traversal loop. It call GetEnumerator implementation from UnityEngine.Transform class wich return some enumerator implemented by unity team and then it call GetNext on that enumerator from Transform class until it reports there's no more elements.
     
  13. csofranz

    csofranz

    Joined:
    Apr 29, 2017
    Posts:
    1,556
    Yes, I remembered that belatedly. It still does not follow, though, that iterating a Component will skip inactive gameobjects (that was the OP's question)
     
    Last edited: Aug 7, 2019
  14. palex-nx

    palex-nx

    Joined:
    Jul 23, 2018
    Posts:
    1,748
    foreach (V v in x) embedded-statement

    is then expanded to:

    Code (csharp):
    1. {
    2.    E e = ((C)(x)).GetEnumerator();
    3.    try {
    4.      V v;
    5.      while (e.MoveNext()) {
    6.         v = (V)(T)e.Current;
    7.         embedded-statement
    8.      }
    9.    }
    10.    finally {
    11.      … // Dispose e
    12.    }
    13. }
    The variable e is not visible to or accessible to the expression x or the embedded statement or any other source code of the program.
     
  15. Suddoha

    Suddoha

    Joined:
    Nov 9, 2013
    Posts:
    2,824
    The latter would apply if "enableStates" were actually a Transform that's iterated, but it could as well be a anything else that is accepted as a valid argument for that loop type, for example a List<Transform>. In that case, type List<T> would be the one that provides the IEnumerator<T>, where T resolves to some Component type. The default enumerator for .Net's List<T> does not care what a specific entry contains, so it returns anything that's next in the list.

    The OP used the word "list" multiple times, so it's more likely System.Generic.List<T> or perhaps an array.

    Few notes:
    regarding 1) That sort of syntactic sugar is not a .Net library thing, but a language feature (C#).
    regarding 3) Given the fact that Transform is not a derived type of any iterable class in that namespace, this wouldn't be possible. What's possible though, it could use an iterator that's already implemented in .NET, but it'd still need to actually return something for its GetEnumerator call, even if it's just a forwarding call to an internally maintained list of Transforms. That's forwarding can also be considered implementation, which as stated above, cannot be defined in that namespace as there's no common iterable base class.

    *Edit*
    "enable" does not exist for components, it's "enabled". This raises some questions.

    1) Is it only a typo in your post?
    I mean, you've produced some logs, so it seems it compiled at some point...
    2) if the code compiles, what's the type of objects you're dealing with (e.g. "enableState")?
    Just asking because if it compiled, it couldn't be a component type, but rather something that attempts to mimic its interface. However, if that was meant to be a component type, it shouldn't even compile and instead, should throw some errors.
    3) what's the type of "enableStates"
     
    Last edited: Aug 7, 2019
  16. Owen-Reynolds

    Owen-Reynolds

    Joined:
    Feb 15, 2012
    Posts:
    1,998
    It is. Remember that "foreach(Transform myChild in transform)" loops through every child. Logically it shouldn't, since transforms aren't lists or arrays. Just looking at it, it makes no sense. But since you're allowed to write an iterator for _anything_, UnityCo decided to write a Transform iterator to hit every child, giving us that cool shortcut.

    I think you're confusing which iterator gets used. List<A> uses the List iterator. It doesn't care whether A has one -- with a list of 10 lists, foreach runs 10 times. It won't dip inside those sublists in a nested loop. For a List<Transform>, foreach won't look at Transform's iterator.

    The confusing thing is C++, which C# was partly based on, can look at the type of the List, called "specializing generics". You could write an iterator for only List<Transform>, which overrides the normal list iterator, that does whatever you want. But C# doesn't allow that (which makes sense, since C# generics are so limited in every other way as well).
     
  17. csofranz

    csofranz

    Joined:
    Apr 29, 2017
    Posts:
    1,556
    Indeed. I dimly remembered (but too late in the discussion) that there is the quirky option (probably a leftover of Unity's earlier -- pre 3.0? -- time when Generic Collections weren't readily available) to iterate the transform itself.

    However, that wasn't how this thread got started. The code isn't entirely clear, but it looks as if the OP is iterating a List<T> or array, not a transform, and in the course of the discussion the question arose if, when iterating GameObjects, inactive GameObjects would be skipped automatically. I doubted that, but screwed up the substantiation big time.

    In a nutshell, I think it would have been better to say that if the OP iterates a List<T> via foreach, the generic iterator will be used, even if the items itrated are a Transform. Also, the items iterated most probably aren't transforms, so I got sidetracked on that one too :)

    It'll be interesting to hear what the OP found out to be the culprit,