OnDragEnd event sometimes not sent

[Vriisor]

Hi,

There seems to be an issue where OnDragEnd is not sent under some specific circumstances.
I've attached a simple example project with the necessary scene hierarchy to reproduce.
[Does not include NGUI source, need to import version 3.7.7]


Steps to reproduce:
This is a bit hard to explain but hopefully I'll get this across.

Drag the white square back and forth between the containers as quickly as possible.
Basically try hammering the mouse button in rapid succession while also attempting to drag and drop the square. It helps to position the item right on the border between the two containers.


Result:
After doing this for a while (usually takes me between 5-15s) the white square will remain parented to the UIDragDropRoot instead of either of the containers. This is because UIDragDropItem never receives the OnDragEnd event.

I think this might have to do with clicking down during the same frame as you drop the item. In that case slowing down the framerate might make this easier to replicate (I did not test this though).


Environment:
Windows 8.1
Running in Unity Editor

[Vriisor]

Just tested that this can still be replicated with NGUI 3.7.8 (Unity 4.6.1f1).

Here's a video that might illustrate the issue better than my previous description.
http://screencast-o-matic.com/watch/coVQolewjt

[ArenMook]

Looking at the code I don't see anything that would cause it. OnDragEnd is sent on unpress if the drag has started, and the drag start is handled above it. You can watch for UICamera.onPress and if dragging something and you get that delegate call, stop dragging forcefully. How did you even run into this?

[Vriisor]

I looked into this some more. It is in fact caused by managing to click twice in the same frame.
Adding a script to the scene with this update loop makes it easy to repro:
void Update ()
{
    System.Threading.Thread.Sleep(200);
}

UICamera.cs line 1404:
bool pressed = Input.GetMouseButtonDown(i);
bool unpressed = Input.GetMouseButtonUp(i);

Here both 'pressed' and 'unpressed' variables will get set to 'true'.
They are then handed as arguments to ProcessTouch().
Because 'pressed' is true in ProcessTouch(), currentTouch.dragStarted is set to false on line 1709.
If statement on line 1809 then results in OnDragEnd not being sent.

[ArenMook]

It's correct though. One press ended, another is starting. You can't tell if you did this:

- press
- release

or this:

- release
- press

[Vriisor]

I would argue that the processing order of 'press' and 'release' should depend on whether the mouse button was already down before this frame.
Functionally something like this:

if( currentTouch.pressStarted )
{
   if( unpressed ) ...
   if( pressed ) ...
}
else
{
   if( pressed ) ...
   if( unpressed ) ...
}

This would allow the drag operation to complete before another press starts.

[ArenMook]

I think that's a sensible compromise. Try replacing the ProcessTouch function with this:

1.         /// <summary>
2.         /// Process the press part of a touch.
3.         /// </summary>
4.  
5.         void ProcessPress (bool pressed, float click, float drag)
6.         {
7.                 // Send out the press message
8.                 if (pressed)
9.                 {
10.                         if (mTooltip != null) ShowTooltip(false);
11.  
12.                         currentTouch.pressStarted = true;
13.                         if (onPress != null && currentTouch.pressed)
14.                                 onPress(currentTouch.pressed, false);
15.  
16.                         Notify(currentTouch.pressed, "OnPress", false);
17.  
18.                         currentTouch.pressed = currentTouch.current;
19.                         currentTouch.dragged = currentTouch.current;
20.                         currentTouch.clickNotification = ClickNotification.BasedOnDelta;
21.                         currentTouch.totalDelta = Vector2.zero;
22.                         currentTouch.dragStarted = false;
23.  
24.                         if (onPress != null && currentTouch.pressed)
25.                                 onPress(currentTouch.pressed, true);
26.  
27.                         Notify(currentTouch.pressed, "OnPress", true);
28.  
29.                         // Update the selection
30.                         if (currentTouch.pressed != mCurrentSelection)
31.                         {
32.                                 if (mTooltip != null) ShowTooltip(false);
33.                                 currentScheme = ControlScheme.Touch;
34.                                 selectedObject = currentTouch.pressed;
35.                         }
36.                 }
37.                 else if (currentTouch.pressed != null && (currentTouch.delta.sqrMagnitude != 0f || currentTouch.current != currentTouch.last))
38.                 {
39.                         // Keep track of the total movement
40.                         currentTouch.totalDelta += currentTouch.delta;
41.                         float mag = currentTouch.totalDelta.sqrMagnitude;
42.                         bool justStarted = false;
43.  
44.                         // If the drag process hasn't started yet but we've already moved off the object, start it immediately
45.                         if (!currentTouch.dragStarted && currentTouch.last != currentTouch.current)
46.                         {
47.                                 currentTouch.dragStarted = true;
48.                                 currentTouch.delta = currentTouch.totalDelta;
49.  
50.                                 // OnDragOver is sent for consistency, so that OnDragOut is always preceded by OnDragOver
51.                                 isDragging = true;
52.  
53.                                 if (onDragStart != null) onDragStart(currentTouch.dragged);
54.                                 Notify(currentTouch.dragged, "OnDragStart", null);
55.  
56.                                 if (onDragOver != null) onDragOver(currentTouch.last, currentTouch.dragged);
57.                                 Notify(currentTouch.last, "OnDragOver", currentTouch.dragged);
58.  
59.                                 isDragging = false;
60.                         }
61.                         else if (!currentTouch.dragStarted && drag < mag)
62.                         {
63.                                 // If the drag event has not yet started, see if we've dragged the touch far enough to start it
64.                                 justStarted = true;
65.                                 currentTouch.dragStarted = true;
66.                                 currentTouch.delta = currentTouch.totalDelta;
67.                         }
68.  
69.                         // If we're dragging the touch, send out drag events
70.                         if (currentTouch.dragStarted)
71.                         {
72.                                 if (mTooltip != null) ShowTooltip(false);
73.  
74.                                 isDragging = true;
75.                                 bool isDisabled = (currentTouch.clickNotification == ClickNotification.None);
76.  
77.                                 if (justStarted)
78.                                 {
79.                                         if (onDragStart != null) onDragStart(currentTouch.dragged);
80.                                         Notify(currentTouch.dragged, "OnDragStart", null);
81.  
82.                                         if (onDragOver != null) onDragOver(currentTouch.last, currentTouch.dragged);
83.                                         Notify(currentTouch.current, "OnDragOver", currentTouch.dragged);
84.                                 }
85.                                 else if (currentTouch.last != currentTouch.current)
86.                                 {
87.                                         if (onDragStart != null) onDragStart(currentTouch.dragged);
88.                                         Notify(currentTouch.last, "OnDragOut", currentTouch.dragged);
89.  
90.                                         if (onDragOver != null) onDragOver(currentTouch.last, currentTouch.dragged);
91.                                         Notify(currentTouch.current, "OnDragOver", currentTouch.dragged);
92.                                 }
93.  
94.                                 if (onDrag != null) onDrag(currentTouch.dragged, currentTouch.delta);
95.                                 Notify(currentTouch.dragged, "OnDrag", currentTouch.delta);
96.  
97.                                 currentTouch.last = currentTouch.current;
98.                                 isDragging = false;
99.  
100.                                 if (isDisabled)
101.                                 {
102.                                         // If the notification status has already been disabled, keep it as such
103.                                         currentTouch.clickNotification = ClickNotification.None;
104.                                 }
105.                                 else if (currentTouch.clickNotification == ClickNotification.BasedOnDelta && click < mag)
106.                                 {
107.                                         // We've dragged far enough to cancel the click
108.                                         currentTouch.clickNotification = ClickNotification.None;
109.                                 }
110.                         }
111.                 }
112.         }
113.  
114.         /// <summary>
115.         /// Process the release part of a touch.
116.         /// </summary>
117.  
118.         void ProcessRelease (bool isMouse, float drag)
119.         {
120.                 // Send out the unpress message
121.                 currentTouch.pressStarted = false;
122.                 if (mTooltip != null) ShowTooltip(false);
123.  
124.                 if (currentTouch.pressed != null)
125.                 {
126.                         // If there was a drag event in progress, make sure OnDragOut gets sent
127.                         if (currentTouch.dragStarted)
128.                         {
129.                                 if (onDragOut != null) onDragOut(currentTouch.last, currentTouch.dragged);
130.                                 Notify(currentTouch.last, "OnDragOut", currentTouch.dragged);
131.  
132.                                 if (onDragEnd != null) onDragEnd(currentTouch.dragged);
133.                                 Notify(currentTouch.dragged, "OnDragEnd", null);
134.                         }
135.  
136.                         // Send the notification of a touch ending
137.                         if (onPress != null) onPress(currentTouch.pressed, false);
138.                         Notify(currentTouch.pressed, "OnPress", false);
139.  
140.                         // Send a hover message to the object
141.                         if (isMouse)
142.                         {
143.                                 if (onHover != null) onHover(currentTouch.current, true);
144.                                 Notify(currentTouch.current, "OnHover", true);
145.                         }
146.                         mHover = currentTouch.current;
147.  
148.                         // If the button/touch was released on the same object, consider it a click and select it
149.                         if (currentTouch.dragged == currentTouch.current ||
150.                                 (currentScheme != ControlScheme.Controller &&
151.                                 currentTouch.clickNotification != ClickNotification.None &&
152.                                 currentTouch.totalDelta.sqrMagnitude < drag))
153.                         {
154.                                 if (currentTouch.pressed != mCurrentSelection)
155.                                 {
156.                                         mNextSelection = null;
157.                                         mCurrentSelection = currentTouch.pressed;
158.                                         if (onSelect != null) onSelect(currentTouch.pressed, true);
159.                                         Notify(currentTouch.pressed, "OnSelect", true);
160.                                 }
161.                                 else
162.                                 {
163.                                         mNextSelection = null;
164.                                         mCurrentSelection = currentTouch.pressed;
165.                                 }
166.  
167.                                 // If the touch should consider clicks, send out an OnClick notification
168.                                 if (currentTouch.clickNotification != ClickNotification.None && currentTouch.pressed == currentTouch.current)
169.                                 {
170.                                         float time = RealTime.time;
171.  
172.                                         if (onClick != null) onClick(currentTouch.pressed);
173.                                         Notify(currentTouch.pressed, "OnClick", null);
174.  
175.                                         if (currentTouch.clickTime + 0.35f > time)
176.                                         {
177.                                                 if (onDoubleClick != null) onDoubleClick(currentTouch.pressed);
178.                                                 Notify(currentTouch.pressed, "OnDoubleClick", null);
179.                                         }
180.                                         currentTouch.clickTime = time;
181.                                 }
182.                         }
183.                         else if (currentTouch.dragStarted) // The button/touch was released on a different object
184.                         {
185.                                 // Send a drop notification (for drag & drop)
186.                                 if (onDrop != null) onDrop(currentTouch.current, currentTouch.dragged);
187.                                 Notify(currentTouch.current, "OnDrop", currentTouch.dragged);
188.                         }
189.                 }
190.                 currentTouch.dragStarted = false;
191.                 currentTouch.pressed = null;
192.                 currentTouch.dragged = null;
193.         }
194.  
195.         /// <summary>
196.         /// Process the events of the specified touch.
197.         /// </summary>
198.  
199.         public void ProcessTouch (bool pressed, bool released)
200.         {
201.                 // Whether we're using the mouse
202.                 bool isMouse = (currentScheme == ControlScheme.Mouse);
203.                 float drag   = isMouse ? mouseDragThreshold : touchDragThreshold;
204.                 float click  = isMouse ? mouseClickThreshold : touchClickThreshold;
205.  
206.                 // So we can use sqrMagnitude below
207.                 drag *= drag;
208.                 click *= click;
209.  
210.                 if (currentTouch.pressed != null)
211.                 {
212.                         if (released) ProcessRelease(isMouse, drag);
213.                         ProcessPress(pressed, click, drag);
214.                 }
215.                 else
216.                 {
217.                         ProcessPress(pressed, click, drag);
218.                         if (released) ProcessRelease(isMouse, drag);
219.                 }
220.         }

[Vriisor]

Thank you. This resolved the issue.