EventDelegate.Remove Failing

[AtomicBob]

I've run into multiple circumstances now where EventDelegate.Remove fails. It returns false and the previously Added callback will continue to get called. I haven't had time to dig down and figure out the problem, but I've noticed the actual reference to the onFinished list has changed and is empty. Explicitly specifying that it's not a one-shot callback doesn't help.

A small code example:

1. TweenPosition registrationTwn;
2. List<EventDelegate> registrationRef;
3.  
4. public void CancelButtonPressed ()
5. {
6.         isCanceled = true;
7.  
8.         registrationTwn = tween;
9.         registrationRef = tween.onFinished;
10.         EventDelegate.Add(tween.onFinished, TweenOutFinished);
11.         //EventDelegate.Add(tween.onFinished, TweenOutFinished, false); //Same results
12.         tween.Play(false);
13. }
14.  
15. private void TweenOutFinished ()
16. {
17.         Debug.Log("tween Equal Original: " + ReferenceEquals(registrationTwn, tween)); //True
18.         Debug.Log("onFinished Equal Original: " + ReferenceEquals(registrationRef, tween.onFinished)); //False
19.  
20.         Debug.Log(
21.         EventDelegate.Remove(tween.onFinished, TweenOutFinished) //Fails
22.         );
23.  
24.         UIManager.ToggleUIElement(UIElement.InputBlocker, false);
25.  
26.         friendList.enabled = false;
27.         root.SetActive(false);
28.  
29.         if ( FriendsSelectedCallback != null )
30.                 FriendsSelectedCallback(( isCanceled ? null : selected ));
31. }
32.  

In this case, it would be sufficient to make it one-shot, but I'm running into the same problem in cases where that's not an option.

[ArenMook]

Yeah... this is a side effect of the fact that when executing delegates we're iterating through a copy of the list, not the list itself. It's done this way so that you can re-activate the same tween from its own finished function.

Even if it wasn't, it's not safe to modify the list while iterating through it. If it worked before, then you were lucky. You should delay your removal by using a coroutine set to yield WaitForEndOfFrame().

[AtomicBob]

That seems like a really inconvenient pattern. Any chance you could be coerced into working around that limitation? Presumably you could just remove from the original list, not the copy.

[ArenMook]

Try this. Modify EventDelegate.Execute to the following:

1.         static public void Execute (List<EventDelegate> list)
2.         {
3.                 if (list != null)
4.                 {
5.                         for (int i = 0; i < list.Count; )
6.                         {
7.                                 EventDelegate del = list[i];
8.  
9.                                 if (del != null)
10.                                 {
11.                                         del.Execute();
12.  
13.                                         if (i < list.Count) break;
14.                                         if (list[i] != del) continue;
15.  
16.                                         if (del.oneShot)
17.                                         {
18.                                                 list.RemoveAt(i);
19.                                                 continue;
20.                                         }
21.                                 }
22.                                 ++i;
23.                         }
24.                 }
25.         }

Now add this to UITweener:

1.         List<EventDelegate> mTemp = null;
2.  
3.         public void RemoveOnFinished (EventDelegate del)
4.         {
5.                 if (onFinished != null) onFinished.Remove(del);
6.                 if (mTemp != null) mTemp.Remove(del);
7.         }

Lastly, change line 216 in UITweener from:

1. List<EventDelegate> mTemp = onFinished;

to just:

1. mTemp = onFinished;

You will still need to use the new RemoveOnFinished instead of modifying 'onFinished', but it should work as you expect.

[AtomicBob]

I finally got around to testing this, and I don't quite understand how it's supposed to be used. I need a reference to the EventDelegate created for the method that is registered, but I don't have that. Here's what I tried:

1. public class PauseWindow : MonoBehaviour
2. {
3.         public void Show ()
4.         {
5.                 //Pause game
6.                 GameManager.TimeManager.PauseApplication(true);
7.  
8.                 //Standard pop-up stuff
9.                 UIManager.ToggleUIElement(UIElement.InputBlocker, true);
10.                 NGUITools.SetActiveChildren(gameObject, true);
11.  
12.                 //Start scale tween transiton of the entire window
13.                 tweener.Play(true);
14.         }
15.  
16.         public void Hide ()
17.         {
18.                 EventDelegate.Add(tweener.onFinished, Deactivate);
19.                 tweener.Play(false);
20.         }
21.  
22.         public void Deactivate ()
23.         {
24.                 //Doesn't work
25.                 //EventDelegate.Remove(tweener.onFinished, Deactivate)
26.  
27.                 //Still doesn't work
28.                 tweener.RemoveOnFinished(tweener.onFinished.Find(del => del.Equals((EventDelegate.Callback)Deactivate)));
29.  
30.                 NGUITools.SetActiveChildren(gameObject, false);
31.  
32.                 GameManager.TimeManager.PauseApplication(false);
33.                 UIManager.ToggleUIElement(UIElement.InputBlocker, false);
34.         }
35. }
36.  

It fails though. In the previous system this sort of behavior was intuitive and simple. This pattern in the new system is really throwing me for a loop.

[ArenMook]

Instead of this:

1. tweener.RemoveOnFinished(tweener.onFinished.Find(del => del.Equals((EventDelegate.Callback)Deactivate)));

You should have this:

1. tweener.RemoveOnFinished(Deactivate);

[AtomicBob]

Assets/Scripts/GameScreens/GameplayScreen/PauseWindow.cs(118,25): error CS1502: The best overloaded method match for `UITweener.RemoveOnFinished(EventDelegate)' has some invalid arguments
Assets/Scripts/GameScreens/GameplayScreen/PauseWindow.cs(118,25): error CS1503: Argument `#1' cannot convert `method group' expression to type `EventDelegate'

[ArenMook]

Alright:

1. tweener.RemoveOnFinished(new EventDelegate(Deactivate));

[AtomicBob]

Thanks, that works.

It gets the job done, but honestly, I was hoping to be able to use the same EventDelegate.Remove method for this. Using RemoveOnFinished isn't terribly intuitive and it's kind of hidden away in the UITweener class instead of EventDelegate. It requires a bit of NGUI tribal knowledge to use, and while I can tell my coworkers about it, it's bound to give new people a problem when they run into it.

Regardless, thanks for the help.

[ArenMook]

I agree, but the way it is now was done to resolve another issue (ability to chain things properly, where one tween's OnFinished function reactivates the same tween with a different event callback). There is no perfect solution here.