Search Unity

Resolved How do I Remove Button Listeners - and should I?

Discussion in 'Scripting' started by drakedragoon, Aug 5, 2022.

  1. drakedragoon

    drakedragoon

    Joined:
    Sep 26, 2018
    Posts:
    26
    Hello World!

    I have added a Button Listener when my script is Awake:

    void Awake() {

    settingsButton1.onClick.AddListener(() => PlayMusicTrack());

    }

    Then I (should?) disable the listener here:

    private void OnDisable() {

    Debug.Log("remove listener");

    }

    Should I? And how should I do it using RemoveListener?

    Newbie in training says: THANK YOU!
     
    melvinfrza likes this.
  2. drakedragoon

    drakedragoon

    Joined:
    Sep 26, 2018
    Posts:
    26
    I'm assuming that every time I open the settings and this script becomes 'Active', another Listener get's piled up. But I could be wrong. I'm hoping I don't have garbage collecting in my scripts. =P
     
  3. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    7,859
    Please consider using code tags in future.

    Anyway, if both the button and the subscriber are leaving the scene/being destroed at the same time (such as a scene being unloaded or exiting playmode), then it shouldn't be a problem.

    If the subscriber is leaving/destroyed while the button remains, then it should unsubscribe, same as with any delegate.

    Also you don't need to do this:
    Code (CSharp):
    1. settingsButton1.onClick.AddListener(() => PlayMusicTrack());
    There's no point doing an anonymous method when you're only using it to call a parameterless method. You can just do:
    Code (CSharp):
    1. settingsButton1.onClick.AddListener(PlayMusicTrack);
    Then you can just
    RemoveListener(PlayMusicTrack);
    to unsubscribe.
     
    melvinfrza and drakedragoon like this.
  4. drakedragoon

    drakedragoon

    Joined:
    Sep 26, 2018
    Posts:
    26
    Okay, I just saw how to do that (use code tag), as per your instruction. =)

    I should have used this example as it actually is applied to Next and Previous buttons which navigate through Sound Tracks:
    Code (CSharp):
    1. next.onClick.AddListener(() => Select(1));
    2. previous.onClick.AddListener(() => Select(-1));
    The UI page gets hidden and unhidden (active, inactive) as needed while navigating the UI.
    Each time it is 'awake' it adds the listeners.
    I just wanted to make sure they aren't sticking around and piling up.

    So I thought maybe it would look something like this:
    Code (CSharp):
    1. private void OnDisable()
    2. {
    3.     next.onClick.RemoveAllListeners();
    4.     previous.onClick.RemoveAllListeners();
    5. }
     
    Last edited: Aug 5, 2022
  5. spiney199

    spiney199

    Joined:
    Feb 11, 2021
    Posts:
    7,859
    If you keep adding listeners without removing them, yes I believe they will continue to pile up causing unexpected behaviour.

    Like any delegate you need to unsubscribe when needed, otherwise you'll get unexpected behaviour, or null-reference errors if the listener no longer exists.
     
    drakedragoon likes this.
  6. drakedragoon

    drakedragoon

    Joined:
    Sep 26, 2018
    Posts:
    26
    Awesome, then I think I will add the code in OnDisable() as I wrote above.
    Only thing I will change is:
    Instead of adding the listeners on Awake(), I'll be adding them OnEnable().
    So my final result looks like this:
    Code (CSharp):
    1.     private void OnEnable()
    2.     {
    3.         next.onClick.AddListener(() => Select(1));
    4.         previous.onClick.AddListener(() => Select(-1));
    5.     }
    6.  
    7.     private void OnDisable()
    8.     {
    9.         next.onClick.RemoveAllListeners();
    10.         previous.onClick.RemoveAllListeners();
    11.     }
    So far I have only asked a couple questions on here, and you've been very helpful each time.
    Thanks Spiney199!
     
    Last edited: Aug 5, 2022
  7. Kurt-Dekker

    Kurt-Dekker

    Joined:
    Mar 16, 2013
    Posts:
    38,689
    That's the pattern I use, never had any issues with it: OnEnable / OnDisable is solid for MonoBehaviours.

    I think it's just Good Practice(tm) doing the remove as well: you never know if years down the line you'll change the behaviour or lifecycle slightly and violate your "add once" assumptions.
     
    drakedragoon and spiney199 like this.
  8. Madgvox

    Madgvox

    Joined:
    Apr 13, 2014
    Posts:
    1,317
    As a rule, I would avoid using RemoveAllListeners when in effect you are trying to remove one listener. This is a good way for obfuscated bugs to occur. Instead, I'd format this using methods that I can directly put into the appropriate method:

    Code (CSharp):
    1. private void SelectNext () {
    2.   Select(1);
    3. }
    4.  
    5. private void SelectPrevious () {
    6.   Select(-1);
    7. }
    8.  
    9. private void OnDisable () {
    10.   next.onClick.RemoveListener(SelectNext);
    11.   next.onClick.RemoveListener(SelectPrevious);
    12. }
    13.  
    14. private void OnEnable () {
    15.   next.onClick.AddListener(SelectNext);
    16.   next.onClick.AddListener(SelectPrevious);
    17. }
     
    Kurt-Dekker and drakedragoon like this.
  9. drakedragoon

    drakedragoon

    Joined:
    Sep 26, 2018
    Posts:
    26
    :cool: TY
     
    Last edited: Aug 5, 2022
  10. Bunny83

    Bunny83

    Joined:
    Oct 18, 2010
    Posts:
    3,992
    I just like to add. OnEnable and OnDisable are the respective counterparts as it was already established in this thread. The counterpart of Awake is OnDestroy. So don't mix them up :) OnEnable / OnDisable are called every time the gameobject / component changes its activated / enabled state. Awake and OnDestroy are one time events in the lifetime of the object. Awake will only be called once and so is OnDestroy. So things done in Awake that needs to be "undone" have to be reverted in OnDestroy. Though, especially when it comes to event subscriptions using OnEnable / OnDisable usually makes more sense as in 99% of all cases you would expect a component to not react to events when you disable it. Though there are always exceptions of course :)
     
    drakedragoon likes this.