Tasharen Entertainment Forum

Support => NGUI 3 Support => Topic started by: AtomicBob on January 22, 2014, 04:31:25 PM

Title: EventDelegate.Remove Failing
Post by: AtomicBob on January 22, 2014, 04:31:25 PM
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.
Title: Re: EventDelegate.Remove Failing
Post by: ArenMook on January 23, 2014, 06:27:28 AM
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().
Title: Re: EventDelegate.Remove Failing
Post by: AtomicBob on January 23, 2014, 09:02:00 AM
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.
Title: Re: EventDelegate.Remove Failing
Post by: ArenMook on January 24, 2014, 07:49:36 AM
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.
Title: Re: EventDelegate.Remove Failing
Post by: AtomicBob on February 04, 2014, 03:54:34 PM
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.
Title: Re: EventDelegate.Remove Failing
Post by: ArenMook on February 05, 2014, 03:27:01 PM
Instead of this:
  1. tweener.RemoveOnFinished(tweener.onFinished.Find(del => del.Equals((EventDelegate.Callback)Deactivate)));
You should have this:
  1. tweener.RemoveOnFinished(Deactivate);
Title: Re: EventDelegate.Remove Failing
Post by: AtomicBob on February 05, 2014, 03:29:31 PM
Quote
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'
Title: Re: EventDelegate.Remove Failing
Post by: ArenMook on February 05, 2014, 04:24:54 PM
Alright:
  1. tweener.RemoveOnFinished(new EventDelegate(Deactivate));
Title: Re: EventDelegate.Remove Failing
Post by: AtomicBob on February 05, 2014, 04:35:31 PM
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.
Title: Re: EventDelegate.Remove Failing
Post by: ArenMook on February 05, 2014, 04:39:22 PM
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.