fix: return current hit for DragDrop event

[moe-moe-pupil]

Objective

• previously, the hit field in DragDrop event is the clone of the DragEnter one.
• Fixes Odd bevy_picking's DragDrop event hit position #21849.

Solution

• Describe the solution used to achieve the objective above.
  • use the hit from hover_map rather than the hit in state.dragging_over

Testing

• Did you test these changes? If so, how?
  • yes, tested this simple demo and examples/picking/debug_picking.rs and ui_drag_and_drop.rs on my mac.
• Are there any parts that need more testing?
  • no, i think it's enough.
• How can other people (reviewers) test your changes? Is there anything specific they need to know?
  • i pasted the code for testing at the bottom
• If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
  • mac, but it doesnt matter

---

Showcase

514759590-6bbf6631-89d6-4211-9078-d09b13fd3980.mp4

fix_DragDrop.mp4

Click to view showcase

use bevy::{
    prelude::*,
    window::WindowMode,
};

#[derive(Component)]
struct Area;

#[derive(Component)]
struct ExampleButton;

#[derive(Component)]
struct GhostElement;

#[derive(Component)]
struct Element;


const AREA_SIZE: f32 = 500.0;
const BUTTON_SIZE: i32 = 50;
const ELEMENT_SIZE: f32 = 25.0;

#[bevy_main]
pub fn main() -> AppExit {
    App::new()
        .add_plugins((
            DefaultPlugins
                .set(WindowPlugin {
                    primary_window: Some(Window {
                        mode: WindowMode::BorderlessFullscreen(MonitorSelection::Primary),
                        ..default()
                    }),
                    ..default()
                }),
            MeshPickingPlugin,
        ))
        .add_systems(Startup, setup)
        .run()
}

fn setup(
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<ColorMaterial>>,
) {
    commands.spawn(Camera2d);

    commands.spawn((
        Node {
            width: percent(100),
            height: percent(100),
            align_items: AlignItems::Center,
            justify_content: JustifyContent::Start,
            ..default()
        },
        Pickable::IGNORE,
    ))
        .with_children(|parent| {
            parent.spawn((
                ExampleButton,
                Button,
                Node {
                    width: px(BUTTON_SIZE),
                    height: px(BUTTON_SIZE),
                    margin: UiRect::all(px(10)),
                    border_radius: BorderRadius::MAX,
                    ..default()
                },
                BackgroundColor(Color::srgb(1.0, 0.0, 0.0)),
            ))
                .observe(|mut event: On<Pointer<DragStart>>, mut button_color: Single<&mut BackgroundColor, With<ExampleButton>>| {
                    button_color.0 = Color::srgb(1.0, 0.5, 0.0);
                    event.propagate(false);
                })
                .observe(|mut event: On<Pointer<DragEnd>>, mut button_color: Single<&mut BackgroundColor, With<ExampleButton>>| {
                    button_color.0 = Color::srgb(1.0, 0.0, 0.0);
                    event.propagate(false);
                });
        });


    commands.spawn((
        Area,
        Mesh2d(meshes.add(Rectangle::new(AREA_SIZE, AREA_SIZE))),
        MeshMaterial2d(materials.add(Color::srgb(0.1, 0.4, 0.1))),
        Transform::IDENTITY,
    ))
        .observe(on_enter)
        .observe(on_over)
        .observe(on_drop)
        .observe(on_leave);
}


fn on_enter(
    mut event: On<Pointer<DragEnter>>,
    button: Single<Entity, With<ExampleButton>>,
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<ColorMaterial>>,
) {
    if event.dragged == *button {
        let Some(position) = event.hit.position else { return; };
        commands.spawn((
            GhostElement,
            Mesh2d(meshes.add(Circle::new(ELEMENT_SIZE))),
            MeshMaterial2d(materials.add(Color::srgba(1.0, 1.0, 0.6, 0.5))),
            Transform::from_translation(position),
            Pickable::IGNORE,
        ));
        event.propagate(false);
    }
}


fn on_over(
    mut event: On<Pointer<DragOver>>,
    button: Single<Entity, With<ExampleButton>>,
    mut ghost_element_transform: Single<&mut Transform, With<GhostElement>>,
) {
    if event.dragged == *button {
        let Some(position) = event.hit.position else { return; };
        ghost_element_transform.translation = position;
        event.propagate(false);
    }
}

 fn on_drop(
    mut event: On<Pointer<DragDrop>>,
    button: Single<Entity, With<ExampleButton>>,
    mut commands: Commands,
    ghost_element: Single<Entity, With<GhostElement>>,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<ColorMaterial>>,
) {
    if event.dropped == *button {
        println!("{:?}", event);
        commands.entity(*ghost_element).despawn();
        let Some(position) = event.hit.position else { return; };
        commands.spawn((
            Element,
            Mesh2d(meshes.add(Circle::new(ELEMENT_SIZE))),
            MeshMaterial2d(materials.add(Color::srgb(1.0, 1.0, 0.6))),
            Transform::from_translation(position),
            Pickable::IGNORE,
        ));
        event.propagate(false);
    }
}

fn on_leave(
    mut event: On<Pointer<DragLeave>>,
    button: Single<Entity, With<ExampleButton>>,
    mut commands: Commands,
    ghost_element: Single<Entity, With<GhostElement>>,
) {
    if event.dragged == *button {
        commands.entity(*ghost_element).despawn();
        event.propagate(false);
    }
}

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[alice-i-cecile]

@MarkGordeychuk can I get your review here please? I'd love a quick test of your reproduction code.

[ickshonpe]

Works correctly, looks good to merge.

Maybe there's an underlying problem with how the hitdata is updated? But that could be looked at in a follow up.

[ickshonpe]

Is the underlying problem further down than this, shouldn't the HitData in the dragging_over map be updated every time the pointer moves?

[moe-moe-pupil]

good point, i checked where we handle the PointerAction::Move, and update the hit in state.dragging_over there

[moe-moe-pupil]

btw, im new to rust, i also noticed this but bc im afraid of calling .clone() many times, i didnt do it 😂

[ickshonpe]

Yep most of the time it's not worth worrying about cloning for simple types like Hitdata where it won't cause any heap allocations. Here in picking, the real work has already been done during hit detection. We should only be dispatching a tiny number of picking events per frame, so any difference isn't going to be measurable.

[ickshonpe]

Also might be useful to add the bug demo here to the picking examples.

[moe-moe-pupil]

Also might be useful to add the bug demo here to the picking examples.

agree, there are not that many examples in /picking, glad to add one, but @MarkGordeychuk made the test demo, i can add one later and add him as co-author for that commit

[ickshonpe]

This makes sense now, this definitely seems like a more complete solution.

It's mostly just cosmetic, but you should be able to remove the second clone below on line 776:

                                DragOver {
                                    button,
                                    dragged: *drag_target,
                                    hit,
                                },

[ickshonpe]

Also might be useful to add the bug demo here to the picking examples.

agree, there are not that many examples in /picking, glad to add one, but @MarkGordeychuk made the test demo, i can add one later and add him as co-author for that commit

Sounds good, I've added an issue about the example: #21894 .