Picking out order (#16231)

Tweaks picking docs slightly for formatting and to add additional
context about the ordering of `Over` and `Out` events. Also shifts `Out`
to trigger before `Over` in the global event ordering.

Because of how focus is tracked, we must send all `Over` and `Out`
events at the same time, in a block. Originally I had `Over` precede
`Out` in the global event order, because this seemed natural. However,
the effect of this, when a pointer moves between entities, is to have
the new entity receive `Over` before the old entity received `Out`,
which several users found confusing.

The new ordering (out before over globally, over before out locally per
entity) should make it much easier to write hover state cleanup code.

Author: NthTensor

crates/bevy_picking/src/events.rs

@@ -321,28 +321,51 @@ pub struct PickingEventWriters<'w> {
 /// Dispatches interaction events to the target entities.
 ///
 /// Within a single frame, events are dispatched in the following order:
-/// + The sequence [`DragEnter`], [`Over`].
+/// + [`Out`] → [`DragLeave`].
+/// + [`DragEnter`] → [`Over`].
 /// + Any number of any of the following:
-///   + For each movement: The sequence [`DragStart`], [`Drag`], [`DragOver`], [`Move`].
-///   + For each button press: Either [`Down`], or the sequence [`Click`], [`Up`], [`DragDrop`], [`DragEnd`], [`DragLeave`].
-///   + For each pointer cancellation: Simply [`Cancel`].
-/// + Finally the sequence  [`Out`], [`DragLeave`].
+///   + For each movement: [`DragStart`] → [`Drag`] → [`DragOver`] → [`Move`].
+///   + For each button press: [`Down`] or [`Click`] → [`Up`] → [`DragDrop`] → [`DragEnd`] → [`DragLeave`].
+///   + For each pointer cancellation: [`Cancel`].
 ///
-/// Only the last event in a given sequence is garenteed to be present.
+/// Additionally, across multiple frames, the following are also strictly
+/// ordered by the interaction state machine:
+/// + When a pointer moves over the target:
+///   [`Over`], [`Move`], [`Out`].
+/// + When a pointer presses buttons on the target:
+///   [`Down`], [`Click`], [`Up`].
+/// + When a pointer drags the target:
+///   [`DragStart`], [`Drag`], [`DragEnd`].
+/// + When a pointer drags something over the target:
+///   [`DragEnter`], [`DragOver`], [`DragDrop`], [`DragLeave`].
+/// + When a pointer is canceled:
+///   No other events will follow the [`Cancel`] event for that pointer.
 ///
-/// Additionally, across multiple frames, the following are also strictly ordered by the interaction state machine:
-/// + When a pointer moves over the target: [`Over`], [`Move`], [`Out`].
-/// + When a pointer presses buttons on the target: [`Down`], [`Up`], [`Click`].
-/// + When a pointer drags the target: [`DragStart`], [`Drag`], [`DragEnd`].
-/// + When a pointer drags something over the target: [`DragEnter`], [`DragOver`], [`DragDrop`], [`DragLeave`].
-/// + When a pointer is canceled: No other events will follow the [`Cancel`] event for that pointer.
+/// Two events -- [`Over`] and [`Out`] -- are driven only by the [`HoverMap`].
+/// The rest rely on additional data from the [`PointerInput`] event stream. To
+/// receive these events for a custom pointer, you must add [`PointerInput`]
+/// events.
 ///
-/// Two events -- [`Over`] and [`Out`] -- are driven only by the [`HoverMap`]. The rest rely on additional data from the
-/// [`PointerInput`] event stream. To receive these events for a custom pointer, you must add [`PointerInput`] events.
+/// When the pointer goes from hovering entity A to entity B, entity A will
+/// receive [`Out`] and then entity B will receive [`Over`]. No entity will ever
+/// receive both an [`Over`] and and a [`Out`] event during the same frame.
 ///
-/// Note: Though it is common for the [`PointerInput`] stream may contain multiple pointer movements and presses each frame,
-/// the hover state is determined only by the pointer's *final position*. Since the hover state ultimately determines which
-/// entities receive events, this may mean that an entity can receive events which occurred before it was actually hovered.
+/// When we account for event bubbling, this is no longer true. When focus shifts
+/// between children, parent entities may receive redundant [`Out`] → [`Over`] pairs.
+/// In the context of UI, this is especially problematic. Additional hierarchy-aware
+/// events will be added in a future release.
+///
+/// Both [`Click`] and [`Up`] target the entity hovered in the *previous frame*,
+/// rather than the current frame. This is because touch pointers hover nothing
+/// on the frame they are released. The end effect is that these two events can
+/// be received sequentally after an [`Out`] event (but always on the same frame
+/// as the [`Out`] event).
+///
+/// Note: Though it is common for the [`PointerInput`] stream may contain
+/// multiple pointer movements and presses each frame, the hover state is
+/// determined only by the pointer's *final position*. Since the hover state
+/// ultimately determines which entities receive events, this may mean that an
+/// entity can receive events from before or after it was actually hovered.
 #[allow(clippy::too_many_arguments)]
 pub fn pointer_events(
     // Input
@@ -366,6 +389,57 @@ pub fn pointer_events(
             .and_then(|pointer| pointer.location.clone())
     };
 
+    // If the entity was hovered by a specific pointer last frame...
+    for (pointer_id, hovered_entity, hit) in previous_hover_map
+        .iter()
+        .flat_map(|(id, hashmap)| hashmap.iter().map(|data| (*id, *data.0, data.1.clone())))
+    {
+        // ...but is now not being hovered by that same pointer...
+        if !hover_map
+            .get(&pointer_id)
+            .iter()
+            .any(|e| e.contains_key(&hovered_entity))
+        {
+            let Some(location) = pointer_location(pointer_id) else {
+                debug!(
+                    "Unable to get location for pointer {:?} during pointer out",
+                    pointer_id
+                );
+                continue;
+            };
+
+            // Always send Out events
+            let out_event = Pointer::new(
+                hovered_entity,
+                pointer_id,
+                location.clone(),
+                Out { hit: hit.clone() },
+            );
+            commands.trigger_targets(out_event.clone(), hovered_entity);
+            event_writers.out_events.send(out_event);
+
+            // Possibly send DragLeave events
+            for button in PointerButton::iter() {
+                let state = pointer_state.get_mut(pointer_id, button);
+                state.dragging_over.remove(&hovered_entity);
+                for drag_target in state.dragging.keys() {
+                    let drag_leave_event = Pointer::new(
+                        hovered_entity,
+                        pointer_id,
+                        location.clone(),
+                        DragLeave {
+                            button,
+                            dragged: *drag_target,
+                            hit: hit.clone(),
+                        },
+                    );
+                    commands.trigger_targets(drag_leave_event.clone(), hovered_entity);
+                    event_writers.drag_leave_events.send(drag_leave_event);
+                }
+            }
+        }
+    }
+
     // If the entity is hovered...
     for (pointer_id, hovered_entity, hit) in hover_map
         .iter()
@@ -659,55 +733,4 @@ pub fn pointer_events(
             }
         }
     }
-
-    // If the entity was hovered by a specific pointer last frame...
-    for (pointer_id, hovered_entity, hit) in previous_hover_map
-        .iter()
-        .flat_map(|(id, hashmap)| hashmap.iter().map(|data| (*id, *data.0, data.1.clone())))
-    {
-        // ...but is now not being hovered by that same pointer...
-        if !hover_map
-            .get(&pointer_id)
-            .iter()
-            .any(|e| e.contains_key(&hovered_entity))
-        {
-            let Some(location) = pointer_location(pointer_id) else {
-                debug!(
-                    "Unable to get location for pointer {:?} during pointer out",
-                    pointer_id
-                );
-                continue;
-            };
-
-            // Always send Out events
-            let out_event = Pointer::new(
-                hovered_entity,
-                pointer_id,
-                location.clone(),
-                Out { hit: hit.clone() },
-            );
-            commands.trigger_targets(out_event.clone(), hovered_entity);
-            event_writers.out_events.send(out_event);
-
-            // Possibly send DragLeave events
-            for button in PointerButton::iter() {
-                let state = pointer_state.get_mut(pointer_id, button);
-                state.dragging_over.remove(&hovered_entity);
-                for drag_target in state.dragging.keys() {
-                    let drag_leave_event = Pointer::new(
-                        hovered_entity,
-                        pointer_id,
-                        location.clone(),
-                        DragLeave {
-                            button,
-                            dragged: *drag_target,
-                            hit: hit.clone(),
-                        },
-                    );
-                    commands.trigger_targets(drag_leave_event.clone(), hovered_entity);
-                    event_writers.drag_leave_events.send(drag_leave_event);
-                }
-            }
-        }
-    }
 }