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. Dismiss Notice

Stopping Coroutine (Dictionary Reference)

Discussion in 'Scripting' started by HomeBearJosh, Jan 28, 2019.

  1. HomeBearJosh

    HomeBearJosh

    Joined:
    Dec 13, 2017
    Posts:
    32
    I'm trying to write some simple code that stores coroutines for easy access.
    In this case, I need one specific coroutine to stop.

    I store the coroutine like this:

    Code (CSharp):
    1. public Coroutine Set(IEnumerator IEnumerator, string ID = "")
    2.     {
    3.         coroutine = StartCoroutine(IEnumerator);
    4.  
    5.         if (!string.IsNullOrEmpty(ID))
    6.         {
    7.             if (!coroutines.ContainsKey(ID)) coroutines.Add(ID, coroutine);
    8.             else Debug.LogWarning("Coroutine ID not unique: " + ID);
    9.         }
    10.  
    11.         return coroutine;
    12.     }
    And am trying to stop it like this:


    Code (CSharp):
    1.         public void DeactivateChoiceTimer()
    2.         {
    3.             App.coroutine.Stop("choiceTimer");
    4.         }
    Code (CSharp):
    1. public void Stop(string ID = "")
    2.     {
    3.         if (coroutines.ContainsKey(ID))
    4.         {
    5.             Coroutine value;
    6.             if (coroutines.TryGetValue(ID, out value))
    7.             {
    8.                 coroutine = coroutines[ID];
    9.                 StopCoroutine(coroutine);
    10.                 coroutines.Remove(ID);
    11.             }
    12.         }
    13.         else Debug.LogWarning("Could not stop Coroutine; ID '" + ID + "' not found.");
    14.     }
    Grabbing dictionary values doesn't seem to work, though.
    I suspect it's because the dictionary just stores a reference, but I'm not certain.
    Could anyone help me a bit with trying to get the coroutine to stop based on a stored reference to it?
    Thanks a lot!
     
  2. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,380
    1) What exactly is your problem? You say:

    What do you mean it doesn't work? Is the dictionary reporting no coroutine exists with that id?

    What exactly is going wrong?

    2) On a side note, in relation to your Dictionary. In your Stop method you effectively look-up your coroutine 3 times. TryGetValue should handle both checking if the key exists and retrieving it...

    Code (csharp):
    1.  
    2.     public void Stop(string id = "")
    3.     {
    4.         Coroutine c;
    5.         if(coroutines.TryGetValue(id, out c))
    6.         {
    7.             coroutines.Remove(id);
    8.             StopCoroutine(c);
    9.         }
    10.         else
    11.         {
    12.             Debug.LogWarning("Could not stop Coroutine; ID '" + ID + "' not found.");
    13.         }
    14.     }
    15.  
    3) Mind you that 'StartCoroutine' and 'StopCoroutine' associate that coroutine with the 'this' object in the scope you called 'StartCoroutine'. Are you sure you're calling Stop on the correct MonoBehaviour?

    This also means that all your coroutines are being ran on some global MonoBehaviour (which you have a reference to via 'All.coroutine'). Usually a corouting is associated with the MonoBehaviour that is running it so that way destroy/disable events work correctly with it. Less you end up with a component getting destroyed but its coroutine still running and attempting to access it.

    4) How do your coroutines get removed if they end cleanly?
     
  3. HomeBearJosh

    HomeBearJosh

    Joined:
    Dec 13, 2017
    Posts:
    32
    Thanks a lot for the help!

    1) The coroutine is present in the dictionary, but it won't stop when I call StopCoroutine() on it.

    2) Thanks for that tip - I presumed the check was separate from the retrieval.

    3) All coroutines are called from Set(), indeed from a global monobehaviour which never gets altered/destroyed.

    4) I thought coroutines clean themselves after successful execution - I typically don't keep track of coroutines (usually simple animations/lerps). In this case I wish to stop a coroutine prematurely via the Stop() script in my global coroutine manager; the idea is to first call 'StopCoroutine()' and to then clear said coroutine from the dictionary.
     
  4. lordofduct

    lordofduct

    Joined:
    Oct 3, 2011
    Posts:
    8,380
    Sure they clean themselves up, but your Dictionary is still holding a reference to a now dead coroutine and the key is taken up. If you call 'Set' in the future it'll find that key still in there and throw your warning.

    Also... 'Set' isn't really a descriptive name for what you're doing... rather maybe something like 'Start', or 'Begin'. Also 'Set' is very similar to the keyword 'set' (it's why it shows up blue in the code tags)... it's also considered bad to have member names similar to keywords.

    ...

    As for the coroutine not stopping. That's odd.

    I'm assuming you've stepped through the code and checked to make sure StopCoroutine is getting called and a coroutine is passed in. I also assume that since it's a global non-destroyed monobehaviour than it's definitely the same one that StartCoroutine was called on.

    Are there any warnings/exceptions/errors thrown?
     
  5. lowbl_dan

    lowbl_dan

    Joined:
    Jan 22, 2019
    Posts:
    34
    Ensure that the gameObject that started the coroutine is also the same object that stops the coroutine. Stop coroutine will not work if it is being invoked on a different object.
     
  6. HomeBearJosh

    HomeBearJosh

    Joined:
    Dec 13, 2017
    Posts:
    32
    It has to be the same object, as I only start/stop coroutines from 1 monobehaviour (the global coroutine manager).
    Thanks for the suggestion!
     
  7. HomeBearJosh

    HomeBearJosh

    Joined:
    Dec 13, 2017
    Posts:
    32
    Wouldn't removing the completed coroutine from the dictionary take care of that?
    That's what Stop() (seen above) is currently doing.
    It's just that Monobehaviour.StopCoroutine() doesn't actually stop the coroutine before discarding it from the dictionary.

    The code does execute without any errors.
    It also shows the (single) dictionary coroutine entry:



    That coroutine was added with the Set() method in my first post (you're right; I should alter the name).

    My suspicion is that adding the coroutine to the dictionary creates a copy rather than a reference, and I'm stopping the copy instead of the actual coroutine. But a dictionary is convenient as I'm using strings to ID specific coroutines...
    Even if there is another solution I'd like to know why this isn't working just out of curiosity.
     
  8. lowbl_dan

    lowbl_dan

    Joined:
    Jan 22, 2019
    Posts:
    34
  9. HomeBearJosh

    HomeBearJosh

    Joined:
    Dec 13, 2017
    Posts:
    32
    Thanks, I believe I've seen that before.
    Thing is, this covers stopping coroutines stored in a single IEnumerator/Coroutine variable.
    I'm dealing with a Dictionary<string, Coroutine>.

    I've already tried storing my coroutine as a single variable and using that to stop it.
    This works fine - my coroutine manager is a single monobehavior which both starts and stops it.
    However, when I try to retrieve the same coroutine from a dictionary and stop that, it doesn't work.
     
  10. lowbl_dan

    lowbl_dan

    Joined:
    Jan 22, 2019
    Posts:
    34
    i can verify that your code works when it is being executed in a single class, so its probably not a pass by value issue.

    Code (csharp):
    1.  
    2. using System.Collections;
    3. using System.Collections.Generic;
    4. using UnityEngine;
    5. public class ManagerA : MonoBehaviour
    6. {
    7.     Coroutine coroutine;
    8.     Dictionary<string, Coroutine> coroutines = new Dictionary<string, Coroutine>();
    9.     private void Start()
    10.     {
    11.         for (int i = 0; i < 3; ++i)
    12.         {
    13.             IEnumerator running = runningCoroutine(i);
    14.             Set(running, "choiceTimer" + i);
    15.         }
    16.     }
    17.     IEnumerator runningCoroutine(int index)
    18.     {
    19.         while(true)
    20.         {
    21.             Debug.Log("Coroutine" + index + " is running");
    22.             yield return null;
    23.         }
    24.     }
    25.     public Coroutine Set(IEnumerator IEnumerator, string ID = "")
    26.     {
    27.         coroutine = StartCoroutine(IEnumerator);
    28.         if (!string.IsNullOrEmpty(ID))
    29.         {
    30.             if (!coroutines.ContainsKey(ID)) coroutines.Add(ID, coroutine);
    31.             else Debug.LogWarning("Coroutine ID not unique: " + ID);
    32.         }
    33.         return coroutine;
    34.     }
    35.     int index = 0;
    36.     private void Update()
    37.     {
    38.         if (Input.GetKeyDown(KeyCode.A))
    39.         {
    40.             DeactivateChoiceTimer();
    41.             ++index;
    42.         }
    43.     }
    44.     public void DeactivateChoiceTimer()
    45.     {
    46.         Stop("choiceTimer" + index);
    47.     }
    48.     public void Stop(string ID = "")
    49.     {
    50.         if (coroutines.ContainsKey(ID))
    51.         {
    52.             Debug.Log("------------" + ID + " is stopped-------------" );
    53.             Coroutine value;
    54.             if (coroutines.TryGetValue(ID, out value))
    55.             {
    56.                 coroutine = coroutines[ID];
    57.                 StopCoroutine(coroutine);
    58.                 coroutines.Remove(ID);
    59.             }
    60.         }
    61.         else Debug.LogWarning("Could not stop Coroutine; ID '" + ID + "' not found.");
    62.     }
    63. }
    64.  
    65.  
    66.  
     
  11. HomeBearJosh

    HomeBearJosh

    Joined:
    Dec 13, 2017
    Posts:
    32
    Interesting, thanks!
    So the Coroutine method is inside the manager itself, and you call it from the manager as well.
    In my case, I call Set() from a different class like so:

    Code (CSharp):
    1.             App.coroutineManager.Set(Animation.SomeCoroutine),"choiceTimer");
    2.  
    So the issue is that the coroutine exists in a different class (Animation) instead of the manager itself?
    That seems a bit strange.