Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.
  2. We have updated the language to the Editor Terms based on feedback from our employees and community. Learn more.
    Dismiss Notice

Question Nested loops in coroutine

Discussion in 'Scripting' started by Sakurinh, Aug 7, 2023.

  1. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    Code (CSharp):
    1. IEnumerator ModifyAttributesWhenKeyPressed()
    2.     {    
    3.         while(unspentSkillPoint>0)
    4.         {
    5.             bool pressed = false;
    6.             while(!pressed)
    7.             {
    8.                 if(Input.instance.RiseAttribute_1||Input.instance.RiseAttribute_2||
    9.                       Input.instance.RiseAttribute_3||Input.instance.RiseAttribute_4||
    10.                       Input.instance.RiseDeffensiveAttributes||  Input.instance.RiseDeffensiveAttributes)
    11.                 {
    12.                     pressed=true;
    13.                 }
    14.                 yield return null;
    15.             }
    16.             if(DeffensiveAttributesPanel.activeInHierarchy)
    17.             {
    18.                 if(Input.instance.RiseAttribute_1)
    19.                 {
    20.                     //+att here
    21.                     unspentSkillPoint--;
    22.                 }
    23.                 else if(Input.instance.RiseAttribute_2)
    24.                 {
    25.                     //+att here
    26.                     unspentSkillPoint--;
    27.                 }
    28.                 else if(Input.instance.RiseAttribute_3)
    29.                 {
    30.                     //+att here
    31.                     unspentSkillPoint--;
    32.                 }
    33.                 else if(Input.instance.RiseAttribute_4)
    34.                 {
    35.                     //+att here
    36.                     unspentSkillPoint--;
    37.                 }
    38.                 else break;
    39.             }
    40.             else if(OffensiveAttributesPanel.activeInHierarchy)
    41.             {
    42.                 if(Input.instance.RiseAttribute_1)
    43.                 {
    44.                     //+att here
    45.                     unspentSkillPoint--;
    46.                 }
    47.                 else if(Input.instance.RiseAttribute_2)
    48.                 {
    49.                     //+att here
    50.                     unspentSkillPoint--;
    51.                 }
    52.                 else if(nput.instance.RiseAttribute_3)
    53.                 {
    54.                     //+att here
    55.                     unspentSkillPoint--;
    56.                 }
    57.                 else if(Input.instance.RiseAttribute_4)
    58.                 {
    59.                     //+att here
    60.                     unspentSkillPoint--;
    61.                 }
    62.                 else break;
    63.             }
    64.         }
    65.         print("end ;oop");
    66.         DeffensiveAttributesPanel.SetActive(false);
    67.         OffensiveAttributesPanel.SetActive(false);
    68.         AttributesPanel.SetActive(true);
    69.     }
    This is my coroutine. All I want is these 4:
    Code (CSharp):
    1. print("end ;oop");
    2.         DeffensiveAttributesPanel.SetActive(false);
    3.         OffensiveAttributesPanel.SetActive(false);
    4.         AttributesPanel.SetActive(true);
    keep standing still until unspentSkillPoints reaches 0 or the menus are closed, but instead, they start even when unspentSkillPoint is >0. The question is: do I misunderstand the coroutine- instead of the normal order in nested loops, it ends the outside loop too? Thank you for your time, I really appreciate it.
     
  2. halley

    halley

    Joined:
    Aug 26, 2013
    Posts:
    1,905
    When the code executes
    yield return ...;
    then no more code in the entire function will run, and then on the next game loop Update cycle, code will continue immediately after that point. If your yield is inside twelve for loops, a switch, six while loops, two ifs and an else, it will still do the same thing: pause execution of this function until the next Update cycle.

    I really don't see why you'd design this kind of feature in a coroutine but that's a separate topic.
     
    Ryiah and Sakurinh like this.
  3. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    36,962
    Yeah, definitely not a useful place for a coroutine. Yes you can use one but it doesn't improve the problem space and actually tangles it up a bit.

    Just make a regular old event-driven UI with spend / assign buttons.

    Have outstanding unspent tokens, let players spend them.

    Bonus: have "revert all" to unspend all and restart.

    Bonus bonus: have "unspend" buttons on individual attributes.

    When all unspent tokens are spent, continue. This is easiest accomplished by having a "Continue" button and set it not interactable while any points re unspent.

    Otherwise if you want to debug any code, use this approach:

    Time to start debugging! Here is how you can begin your exciting new debugging adventures:

    You must find a way to get the information you need in order to reason about what the problem is.

    Once you understand what the problem is, you may begin to reason about a solution to the problem.

    What is often happening in these cases is one of the following:

    - the code you think is executing is not actually executing at all
    - the code is executing far EARLIER or LATER than you think
    - the code is executing far LESS OFTEN than you think
    - the code is executing far MORE OFTEN than you think
    - the code is executing on another GameObject than you think it is
    - you're getting an error or warning and you haven't noticed it in the console window

    To help gain more insight into your problem, I recommend liberally sprinkling
    Debug.Log()
    statements through your code to display information in realtime.

    Doing this should help you answer these types of questions:

    - is this code even running? which parts are running? how often does it run? what order does it run in?
    - what are the names of the GameObjects or Components involved?
    - what are the values of the variables involved? Are they initialized? Are the values reasonable?
    - are you meeting ALL the requirements to receive callbacks such as triggers / colliders (review the documentation)

    Knowing this information will help you reason about the behavior you are seeing.

    You can also supply a second argument to Debug.Log() and when you click the message, it will highlight the object in scene, such as
    Debug.Log("Problem!",this);


    If your problem would benefit from in-scene or in-game visualization, Debug.DrawRay() or Debug.DrawLine() can help you visualize things like rays (used in raycasting) or distances.

    You can also call Debug.Break() to pause the Editor when certain interesting pieces of code run, and then study the scene manually, looking for all the parts, where they are, what scripts are on them, etc.

    You can also call GameObject.CreatePrimitive() to emplace debug-marker-ish objects in the scene at runtime.

    You could also just display various important quantities in UI Text elements to watch them change as you play the game.

    Visit Google for how to see console output from builds. If you are running a mobile device you can also view the console output. Google for how on your particular mobile target, such as this answer for iOS: https://forum.unity.com/threads/how-to-capturing-device-logs-on-ios.529920/ or this answer for Android: https://forum.unity.com/threads/how-to-capturing-device-logs-on-android.528680/

    If you are working in VR, it might be useful to make your on onscreen log output, or integrate one from the asset store, so you can see what is happening as you operate your software.

    Another useful approach is to temporarily strip out everything besides what is necessary to prove your issue. This can simplify and isolate compounding effects of other items in your scene or prefab.

    If your problem is with OnCollision-type functions, print the name of what is passed in!

    Here's an example of putting in a laser-focused Debug.Log() and how that can save you a TON of time wallowing around speculating what might be going wrong:

    https://forum.unity.com/threads/coroutine-missing-hint-and-error.1103197/#post-7100494

    "When in doubt, print it out!(tm)" - Kurt Dekker (and many others)

    Note: the
    print()
    function is an alias for Debug.Log() provided by the MonoBehaviour class.
     
    Yoreki and Sakurinh like this.
  4. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    Sorry if I misunderstood you, but this is what I intended to do.
    I'm pretty new to Unity, then this is what I wanna try, of course, I can do this with some scripts on the menus, just I'm learning anyway so I wanna understand the above case. Through your explanation - sorry if I misunderstood something - I still can't see why the 4 lines of code still execute. As your saying, after the line 64, the code must return to the first while loop, but it wouldn't.
    Btw, could you please explain to me why shouldn't we design this kind of feature in a coroutine? Thank you for your time.
     
  5. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    6,015
    Coroutines are for fire-and-forget stuff that takes place over a number of frames. Not stuff that requires stopping and starting, or this kind of UI management.

    Basically, it doesn't need to be a co-routine. It just needs to be normal code that runs when invoked by user input.

    If you want to let the player hold a key down to steadily assign points, this can be done via normal Update means.
     
    Sakurinh likes this.
  6. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    Thank you for your time!
    This is what I do every time. In this case, as you can see, I put the print() line first, and they write line as I expected (If the misunderstanding is the case). That's the reason I - after 2 hours of testing with every kind of print() I can think of- decided to ask this noob question: does startcoroutine() make the yield return works differently from itself in the original.
     
  7. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    Thank you so much, clear answer.
     
  8. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    36,962
    See how in your code you ask the same question twice? How you spin waiting for anything, then once anything happens you set pressed, exit and go decide again what to do? You want to alway avoid repeating yourself.

    The best way to structure this is to have two panel scripts, one for each: offensive, defensive, etc.

    Display the appropriate panel (perhaps have left/right buttons to switch)

    When on a panel, respond to the user intents and update the data.

    When a panel is inactive, its script should ignore all input intents.

    The usual way to do this is to enable and disable input contexts in the OnEnable() / OnDisable() methods of each panel's script. That way only one panel is listening at a time, if any.

    Otherwise, coroutines work like any other code. Check this out:

    Coroutines in a nutshell:

    https://forum.unity.com/threads/des...en-restarting-the-scene.1044247/#post-6758470

    https://forum.unity.com/threads/proper-way-to-stop-coroutine.704210/#post-4707821

    Splitting up larger tasks in coroutines:

    https://forum.unity.com/threads/bes...ion-so-its-non-blocking.1108901/#post-7135529

    Coroutines are NOT always an appropriate solution: know when to use them!

    "Why not simply stop having so many coroutines ffs." - orionsyndrome on Unity3D forums

    https://forum.unity.com/threads/things-to-check-before-starting-a-coroutine.1177055/#post-7538972

    https://forum.unity.com/threads/heartbeat-courutine-fx.1146062/#post-7358312

    Our very own Bunny83 has also provided a Coroutine Crash Course:

    https://discussions.unity.com/t/coroutines-ienumerator-not-working-as-expected/237287/3
     
    Bunny83 and Sakurinh like this.
  9. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    Thank you, this is what I think of when the first time I was designing this. I tried the above approach because I don't want to make more code, besides, I wanna understand the coroutine more, that's why I have made the above question.
    And for this, thank you so much, I'm gonna read them asap.
    Thank you so much again for your attention.
     
  10. Bunny83

    Bunny83

    Joined:
    Oct 18, 2010
    Posts:
    3,572
    You have 6 conditions in your if statement which makes your statemachine advance outside your nested loop (though the last two are actually the same!!!). If any of those conditions is true, the nested loop would finish the next frame. I'm not sure why you structured your code like this. Currently when in this frame one of the conditions is true, you still have that yield return after your if statement which would make the coroutine wait for another frame before continuing with the rest of the code. This has a lot potential for a "logical" race condition in case your condition(s) is (are) only true for one frame.

    If your coroutine advances right the next frame when it's started, it just means one of those 6 (actually 5) conditions are already true when you start it.

    Waiting for input inside a coroutine should be done like this as your additional bool variable just makes the code more complicated and introduces the potential race condition I mentioned.

    Code (CSharp):
    1.  
    2.             while(!(
    3.                       Input.instance.RiseAttribute_1||Input.instance.RiseAttribute_2||
    4.                       Input.instance.RiseAttribute_3||Input.instance.RiseAttribute_4||
    5.                       Input.instance.RiseDeffensiveAttributes
    6.             )) {
    7.                 yield return null;
    8.             }
    9.            
    If you really want to keep the if statement, the yield should either come before the if statement or make the if actually
    break;
    out of the while loop instead of setting that boolean to true.

    Now in order to debug your actual issue, just make a Debug.Log right after the nested while loop and print out your 5 conditions with it. So you know which one was the reason the while loop was exited.

    Note that in your current code those two logs may not print the same thing due to the yield in between:

    Code (CSharp):
    1.             while(!pressed)
    2.             {
    3.                 if(Input.instance.RiseAttribute_1||Input.instance.RiseAttribute_2||
    4.                       Input.instance.RiseAttribute_3||Input.instance.RiseAttribute_4||
    5.                       Input.instance.RiseDeffensiveAttributes||  Input.instance.RiseDeffensiveAttributes)
    6.                 {
    7.                     pressed=true;
    8.                     Debug.Log("Input1: " + Input.instance.RiseAttribute_1 +" " + Input.instance.RiseAttribute_2+" " +
    9.                         Input.instance.RiseAttribute_3 +" " + Input.instance.RiseAttribute_4 +" " +
    10.                         Input.instance.RiseDeffensiveAttributes
    11.                     );
    12.                 }
    13.                 yield return null;
    14.             }
    15.             Debug.Log("Input2: " + Input.instance.RiseAttribute_1 +" " + Input.instance.RiseAttribute_2+" " +
    16.                Input.instance.RiseAttribute_3 +" " + Input.instance.RiseAttribute_4 +" " +
    17.                Input.instance.RiseDeffensiveAttributes
    18.             );
    19.  
     
    Sakurinh likes this.
  11. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    Thank you so much. These are actually great advices. I've learned so much from your answer. God Blesses You.
     
  12. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    @Bunny83 T'm sorry, may I ask you one more question: in play mode - or testing mode- my Unity freeze randomly. Could you brighten me up with a way to find out where is the problem? Like, which is the most potential function, or how to catch the running call. I'm using visual studio for student, and when attach to unity, VS's call stack shows up nothing. Thank you for your time.
     
  13. Bunny83

    Bunny83

    Joined:
    Oct 18, 2010
    Posts:
    3,572
    That usually happens when you have an infinite loop that doesn't have a yield statement inside. The yield return makes it leave the coroutine so other things can happen. When there's no yield inside a loop, NOTHING outside that loop can ever happen. That includes new user input. The whole process does nothing else except running your loop. So when you have any kind of loop, make sure it's not infinite. The only place where an infinite loop "can be" not a problem is inside a coroutine but only when you have a yield instruction in it.
     
    Yoreki likes this.
  14. Yoreki

    Yoreki

    Joined:
    Apr 10, 2019
    Posts:
    2,590
    Since it hasnt been mentioned yet here, Unity is single threaded, which is why the above is true and important. Even async code execution, like coroutines, are handled on the same thread. Just to clear up potential misunderstandings.

    On another note, when you think you should solve something using a Coroutine: think twice. I personally think they are beginner traps. While im not saying there is no usecase for them whatsoever, im fine without ever having used one outside of prototyping. Usually when you solve something simple with a coroutine, you may aswell handle it manually. And if you handle something complex with a coroutine, they simply take control away from you, reduce maintainability and readability and thus oftentimes cause bugs people cant solve on their own, which then end up on this forum.
    "Set and forget", as spiney described it, is a good rule of thumb. If you ever need to stop, interact or micromanage the code in that coroutine againa after it started, coroutines make it a hassle instead.
     
  15. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    Thank you so much.
     
  16. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    Wise advice! Can you suggest me 1 example? Like I need one action to be delayed some seconds, would I have another way? I'm pretty new to unity and C#, so...Of course, if you have time.
     
  17. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    By using Time.Deltatime, right?
     
  18. Yoreki

    Yoreki

    Joined:
    Apr 10, 2019
    Posts:
    2,590
    If it's a set-and-forget kind of thing, you can get away with using a WaitForSeconds Coroutine. Usually what was once set-and-forget has a tendency to evolve into something that is not tho.

    For most things, such as cooldowns, you want to either have a timer running (increment or decrement some value by Time.deltaTime each frame), or memorize the last time it was activated and check the current time and compare it to some value to determine whether a sufficient amount of time has passed.

    If the action you want to delay is the SetActive art from your first post (closing the skill menu), this sounds like a set-and-forget case on the surface. However, imagine you make it close 1 second after the skillpoints were spend, and the player is quicker and closes, but also reopens the menu during that duration. Suddenly you wanted to implement a convenience for the player, but actively worked against them in this case.
    Personally i would argue that you have to be careful whenever you take the control from the player. It is oftentimes just better to offer the player a convenient way of dealing with things themselfes. For example by supporting hotkeys, like ESC. Or by letting RMB close the menu once all skillpoints are spent. Something quick but intuitive to use like that. If the menu pops up or otherwise becomes relevant so frequently that you are worried that it not closing on its own is negatively affecting the player experience, i would dare to say that mechanic itself needs to be reworked.

    If you absolutely must automate this, you could handle it any given way (coroutine, timer, ..). But make sure you keep track of further player interaction to then stop this timer or kill the coroutine, if you have to.
     
    Sakurinh and Chubzdoomer like this.
  19. Sakurinh

    Sakurinh

    Joined:
    Jul 26, 2017
    Posts:
    15
    Wise talking. Thank you so much for your advise, and your time too. Gods bless you.