Fix dragging-over panic #21999
Merged
Fix dragging-over panic #21999
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…`dragged_over` map in `PointerButtonState` if the drag started with the dragged over entity already hovered. This causes a panic when the pointer is moved and it tries to look up the `dragging_over` state for the hovered entity. This PR changes `picking_events` to update the `dragging_state` for all hovered entities every frame, before handling the `PointerInput`s. It also changes the behaviour of the `DragEnter` event. `DragEnter` now also fires when a drag starts over an already hovered entity.
…` in the frame the drag begins.
ickshonpe
added
A-Picking
ickshonpe
added
the
C-Code-Quality
kfc35
reviewed
Dec 1, 2025
| .flat_map(|h| h.iter().map(|(entity, data)| (*entity, data.to_owned()))) | ||
| .filter(|(hovered_entity, _)| *hovered_entity != *drag_target) | ||
| { | ||
| *state.dragging_over.get_mut(&hovered_entity).unwrap() = hit.clone(); |
Contributor
There was a problem hiding this comment.
So this was the line that caused the panic. Is it valid to delete this line? Is it because it’s redundant now with the logic you put in?
Contributor
Author
There was a problem hiding this comment.
Yep, this line was a fix for another bug: #21849. It's redundant with this PR as the dragging_over states are already updated before the PointerInputs are processed.
Co-authored-by: Kevin Chen <chen.kevin.f@gmail.com>
kfc35
approved these changes
Dec 1, 2025
Contributor
|
I checked out the branch and confirmed that the examples listed in the |
rossleonardy
approved these changes
Dec 2, 2025
ickshonpe
added
S-Ready-For-Final-Review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Problems:
If a drag starts over a hovered entity,
picking_eventsdoesn't add the hovered entity to thedragging_overmap inPointerButtonState. This causes a panic when the pointer is moved andpicking_eventstries to update the non-existantdragging_overstate for the hovered entity.DragEnterisn't dispatched onDragStart. This doesn't seem ideal. The drag isn't crossing a geometric boundary, but it is causing the dragged over entities to enter into a dragged over state.Fixes #21998
Solution
dragging_statefor all hovered entities every frame, before handling anyPointerInputs.DragEnterdispatch logic.DragEnternow also fires when a drag starts over an already hovered entity.Testing
The examples using drag events should no longer panic: