Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream CorePlugin from bevy_mod_picking #13677

Merged
merged 6 commits into from
Jun 15, 2024

Conversation

NthTensor
Copy link
Contributor

@NthTensor NthTensor commented Jun 5, 2024

Objective

This is the first of a series of PRs intended to begin the upstreaming process for bevy_mod_picking. The purpose of this PR is to:

  • Create the new bevy_picking crate
  • Upstream CorePlugin as PickingPlugin
  • Upstream the core pointer and backend abstractions.

This code has been ported verbatim from the corresponding files in bevy_picking_core with a few tiny naming and docs tweaks.

The work here is only an initial foothold to get the up-streaming process started in earnest. We can do refactoring and improvements once this is in-tree.

@alice-i-cecile alice-i-cecile added S-Blocked This cannot move forward until something else changes X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers D-Straightforward Simple bug fixes and API improvements, docs, test and examples A-Picking Pointing at and selecting objects of all sorts labels Jun 5, 2024
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jun 5, 2024

Part of #12365. Merging this is blocked until after the 0.14 release is cut: it's not useful on its own.

Copy link
Contributor

github-actions bot commented Jun 5, 2024

You added a new feature but didn't add a description for it. Please update the root Cargo.toml file.

@NthTensor NthTensor marked this pull request as ready for review June 5, 2024 02:00
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good; the mechanical integration looks correct and the copy-pasta seems al dente to me.

Copy link
Contributor

@mgi388 mgi388 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a reverse copy paste between the 3 source files in this PR and aevyrie/bevy_mod_picking@d846416 and this is the diff, which looks good to me, and CI builds the PR, so giving an approval if it's helpful (I note this won't be merged for 0.14).

`git diff` (+s are things in mod_picking not included in this PR)
diff --git a/crates/bevy_picking/src/backend.rs b/crates/bevy_picking/src/backend.rs
index 606459b18..fe4b910c8 100644
--- a/crates/bevy_picking/src/backend.rs
+++ b/crates/bevy_picking/src/backend.rs
@@ -1,11 +1,11 @@
 //! This module provides a simple interface for implementing a picking backend.
 //!
-//! Don't be dissuaded by terminology like "backend"; the idea is dead simple. `bevy_picking`
+//! Don't be dissuaded by terminology like "backend"; the idea is dead simple. `bevy_picking_core`
 //! will tell you where pointers are, all you have to do is send an event if the pointers are
 //! hitting something. That's it. The rest of this documentation explains the requirements in more
 //! detail.
 //!
-//! Because `bevy_picking` is very loosely coupled with its backends, you can mix and match as
+//! Because `bevy_picking_core` is very loosely coupled with its backends, you can mix and match as
 //! many backends as you want. For example, You could use the `rapier` backend to raycast against
 //! physics objects, a picking shader backend to pick non-physics meshes, and the `bevy_ui` backend
 //! for your UI. The [`PointerHits`]s produced by these various backends will be combined, sorted,
@@ -71,7 +71,7 @@ pub struct PointerHits {
     /// ### Why is this an `f32`???
     ///
     /// Bevy UI is special in that it can share a camera with other things being rendered. in order
-    /// to properly sort them, we need a way to make `bevy_ui`'s order a tiny bit higher, like adding
+    /// to properly sort them, we need a way to make bevy_ui's order a tiny bit higher, like adding
     /// 0.5 to the order. We can't use integers, and we want users to be using camera.order by
     /// default, so this is the best solution at the moment.
     pub order: f32,
@@ -158,8 +158,8 @@ pub mod ray {
     ///
     /// ```
     /// # use bevy_ecs::prelude::*;
-    /// # use bevy_picking::backend::ray::RayMap;
-    /// # use bevy_picking::backend::PointerHits;
+    /// # use bevy_picking_core::backend::ray::RayMap;
+    /// # use bevy_picking_core::backend::PointerHits;
     /// // My raycasting backend
     /// pub fn update_hits(ray_map: Res<RayMap>, mut output_events: EventWriter<PointerHits>,) {
     ///     for (&ray_id, &ray) in ray_map.iter() {
diff --git a/crates/bevy_picking/src/lib.rs b/crates/bevy_picking/src/lib.rs
index e19b9ce41..0f3bce8f4 100644
--- a/crates/bevy_picking/src/lib.rs
+++ b/crates/bevy_picking/src/lib.rs
@@ -1,14 +1,19 @@
-//! TODO, write module doc
+//! Core functionality and types required for `bevy_mod_picking` to function.
 
+#![allow(clippy::type_complexity)]
+#![allow(clippy::too_many_arguments)]
 #![deny(missing_docs)]
 
 pub mod backend;
+pub mod events;
+pub mod focus;
 pub mod pointer;
 
 use bevy_app::prelude::*;
 use bevy_ecs::prelude::*;
 use bevy_reflect::prelude::*;
 
+use bevy_eventlistener::{prelude::*, EventListenerSet};
 /// Used to globally toggle picking features at runtime.
 #[derive(Clone, Debug, Resource, Reflect)]
 #[reflect(Resource, Default)]
@@ -83,7 +88,7 @@ pub struct Pickable {
     /// For example, if a pointer is over a UI element, and this field is set to `false`, it will
     /// not be marked as hovered, and consequently will not emit events nor will any picking
     /// components mark it as hovered. This can be combined with the other field
-    /// [`Self::should_block_lower`], which is orthogonal to this one.
+    /// [Self::should_block_lower], which is orthogonal to this one.
     ///
     /// Entities without the [`Pickable`] component are hoverable by default.
     pub is_hoverable: bool,
@@ -115,7 +120,7 @@ impl Default for Pickable {
 /// spawning a custom `PointerId::Custom` pointer, either for testing, as a software controlled
 /// pointer, or if you are replacing the default touch and mouse inputs.
 #[derive(Bundle)]
-pub struct PointerBundle {
+pub struct PointerCoreBundle {
     /// The pointer's unique [`PointerId`](pointer::PointerId).
     pub id: pointer::PointerId,
     /// Tracks the pointer's location.
@@ -126,22 +131,24 @@ pub struct PointerBundle {
     pub interaction: pointer::PointerInteraction,
 }
 
-impl PointerBundle {
+impl PointerCoreBundle {
+    /// Sets the location of the pointer bundle
+    pub fn with_location(mut self, location: pointer::Location) -> Self {
+        self.location.location = Some(location);
+        self
+    }
+}
+
+impl PointerCoreBundle {
     /// Create a new pointer with the provided [`PointerId`](pointer::PointerId).
     pub fn new(id: pointer::PointerId) -> Self {
-        PointerBundle {
+        PointerCoreBundle {
             id,
             location: pointer::PointerLocation::default(),
             click: pointer::PointerPress::default(),
             interaction: pointer::PointerInteraction::default(),
         }
     }
-
-    /// Sets the location of the pointer bundle
-    pub fn with_location(mut self, location: pointer::Location) -> Self {
-        self.location.location = Some(location);
-        self
-    }
 }
 
 /// Groups the stages of the picking process under shared labels.
@@ -166,11 +173,9 @@ pub enum PickSet {
     Last,
 }
 
-/// This plugin sets up the core picking infrastructure. It receives input events, and provides the shared
-/// types used by other picking plugins.
-pub struct PickingPlugin;
-
-impl Plugin for PickingPlugin {
+/// Receives input events, and provides the shared types used by other picking plugins.
+pub struct CorePlugin;
+impl Plugin for CorePlugin {
     fn build(&self, app: &mut App) {
         app.init_resource::<PickingPluginsSettings>()
             .init_resource::<pointer::PointerMap>()
@@ -196,7 +201,7 @@ impl Plugin for PickingPlugin {
                     PickSet::Backend,
                     PickSet::Focus.run_if(PickingPluginsSettings::focus_should_run),
                     PickSet::PostFocus,
-                    // Eventually events will need to be dispatched here
+                    EventListenerSet,
                     PickSet::Last,
                 )
                     .chain(),
@@ -210,3 +215,44 @@ impl Plugin for PickingPlugin {
             .register_type::<backend::ray::RayId>();
     }
 }
+
+/// Generates [`Pointer`](events::Pointer) events and handles event bubbling.
+pub struct InteractionPlugin;
+impl Plugin for InteractionPlugin {
+    fn build(&self, app: &mut App) {
+        use events::*;
+        use focus::{update_focus, update_interactions};
+
+        app.init_resource::<focus::HoverMap>()
+            .init_resource::<focus::PreviousHoverMap>()
+            .init_resource::<DragMap>()
+            .add_event::<PointerCancel>()
+            .add_systems(
+                PreUpdate,
+                (
+                    update_focus,
+                    pointer_events,
+                    update_interactions,
+                    send_click_and_drag_events,
+                    send_drag_over_events,
+                )
+                    .chain()
+                    .in_set(PickSet::Focus),
+            )
+            .add_plugins((
+                EventListenerPlugin::<Pointer<Over>>::default(),
+                EventListenerPlugin::<Pointer<Out>>::default(),
+                EventListenerPlugin::<Pointer<Down>>::default(),
+                EventListenerPlugin::<Pointer<Up>>::default(),
+                EventListenerPlugin::<Pointer<Click>>::default(),
+                EventListenerPlugin::<Pointer<Move>>::default(),
+                EventListenerPlugin::<Pointer<DragStart>>::default(),
+                EventListenerPlugin::<Pointer<Drag>>::default(),
+                EventListenerPlugin::<Pointer<DragEnd>>::default(),
+                EventListenerPlugin::<Pointer<DragEnter>>::default(),
+                EventListenerPlugin::<Pointer<DragOver>>::default(),
+                EventListenerPlugin::<Pointer<DragLeave>>::default(),
+                EventListenerPlugin::<Pointer<Drop>>::default(),
+            ));
+    }
+}
diff --git a/crates/bevy_picking/src/pointer.rs b/crates/bevy_picking/src/pointer.rs
index 3d1991c1e..2d4fbb3c4 100644
--- a/crates/bevy_picking/src/pointer.rs
+++ b/crates/bevy_picking/src/pointer.rs
@@ -4,12 +4,10 @@ use bevy_ecs::prelude::*;
 use bevy_math::{Rect, Vec2};
 use bevy_reflect::prelude::*;
 use bevy_render::camera::{Camera, NormalizedRenderTarget};
-use bevy_utils::HashMap;
+use bevy_utils::{HashMap, Uuid};
 use bevy_window::PrimaryWindow;
 
-use uuid::Uuid;
-
-use std::fmt::Debug;
+use std::{fmt::Debug, ops::Deref};
 
 use crate::backend::HitData;
 
@@ -30,7 +28,6 @@ pub enum PointerId {
     #[reflect(ignore)]
     Custom(Uuid),
 }
-
 impl PointerId {
     /// Returns true if the pointer is a touch input.
     pub fn is_touch(&self) -> bool {
@@ -62,6 +59,21 @@ pub struct PointerInteraction {
     pub(crate) sorted_entities: Vec<(Entity, HitData)>,
 }
 
+impl PointerInteraction {
+    /// Returns the nearest hit entity and data about that intersection.
+    pub fn get_nearest_hit(&self) -> Option<&(Entity, HitData)> {
+        self.sorted_entities.first()
+    }
+}
+
+impl Deref for PointerInteraction {
+    type Target = Vec<(Entity, HitData)>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.sorted_entities
+    }
+}
+
 /// A resource that maps each [`PointerId`] to their [`Entity`] for easy lookups.
 #[derive(Debug, Clone, Default, Resource)]
 pub struct PointerMap {
@@ -91,7 +103,6 @@ pub struct PointerPress {
     secondary: bool,
     middle: bool,
 }
-
 impl PointerPress {
     /// Returns true if the primary pointer button is pressed.
     #[inline]
@@ -128,7 +139,6 @@ pub struct InputPress {
     /// Identifies the pointer button changing in this event.
     pub button: PointerButton,
 }
-
 impl InputPress {
     /// Create a new pointer button down event.
     pub fn new_down(id: PointerId, button: PointerButton) -> InputPress {
@@ -175,7 +185,7 @@ impl InputPress {
                         PointerButton::Middle => pointer.middle = is_down,
                     }
                 }
-            });
+            })
         }
     }
 }
@@ -216,7 +226,6 @@ pub struct PointerLocation {
     #[reflect(ignore)]
     pub location: Option<Location>,
 }
-
 impl PointerLocation {
     /// Returns `Some(&`[`Location`]`)` if the pointer is active, or `None` if the pointer is
     /// inactive.
@@ -235,7 +244,6 @@ pub struct InputMove {
     /// The distance moved (change in `position`) since the last event.
     pub delta: Vec2,
 }
-
 impl InputMove {
     /// Create a new [`InputMove`] event.
     pub fn new(id: PointerId, location: Location, delta: Vec2) -> InputMove {
@@ -256,7 +264,7 @@ impl InputMove {
                 if *id == event_pointer.pointer_id {
                     pointer.location = Some(event_pointer.location.to_owned());
                 }
-            });
+            })
         }
     }
 }
@@ -276,7 +284,6 @@ pub struct Location {
     /// The position of the pointer in the `target`.
     pub position: Vec2,
 }
-
 impl Location {
     /// Returns `true` if this pointer's [`Location`] is within the [`Camera`]'s viewport.
     ///

Copy link
Member

@aevyrie aevyrie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Noted some non-blocking observations in the process. We should probably minimize any changes during this process, and do those in tracked PRs so we can keep track of breakages for users migrating from the plugin. Added them as a means of thinking out loud.

Thank you for doing this. 🙂

Can I get co-author credit on this please?

/// [`Self::should_block_lower`], which is orthogonal to this one.
///
/// Entities without the [`Pickable`] component are hoverable by default.
pub is_hoverable: bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name should probably be changed. Something like should_emit_events.

///
/// Note that the word "lower" here refers to entities that have been reported as hit by any
/// picking backend, but are at a lower depth than the current one. This is different from the
/// concept of event bubbling, as it works irrespective of the entity hierarchy.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove references to bubbling, as that is an eventlistener concept.

Comment on lines +15 to +22
pub struct PickingPluginsSettings {
/// Enables and disables all picking features.
pub is_enabled: bool,
/// Enables and disables input collection.
pub is_input_enabled: bool,
/// Enables and disables updating interaction states of entities.
pub is_focus_enabled: bool,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit ancient, we should revisit if this pattern is still best practice for plugin config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move into the plugin struct probably?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! Cart mentioned in the preferences PR that plugin structs are now accessible at runtime? I'm not really sure how that works though.

#[derive(Debug, Default, Clone, Component, Reflect)]
#[reflect(Component, Default)]
pub struct PointerInteraction {
pub(crate) sorted_entities: Vec<(Entity, HitData)>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this accessible to users somehow?

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jun 5, 2024
@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Jun 5, 2024
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jun 5, 2024

@aevyrie during the release note process I'm listing your name first on the list in that section :D Even if you do nothing to help with the port at all.

@NthTensor
Copy link
Contributor Author

But I will make you a git co-author on these comments too. Force push imminent later today.

@alice-i-cecile alice-i-cecile removed the S-Blocked This cannot move forward until something else changes label Jun 8, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 10, 2024
@alice-i-cecile alice-i-cecile removed this pull request from the merge queue due to a manual request Jun 10, 2024
@alice-i-cecile
Copy link
Member

Waiting on said co-authoring!

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 10, 2024
@alice-i-cecile
Copy link
Member

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Jun 10, 2024
@NthTensor
Copy link
Contributor Author

Mm :( will look tonight

crates/bevy_internal/Cargo.toml Outdated Show resolved Hide resolved
crates/bevy_internal/src/lib.rs Show resolved Hide resolved
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 15, 2024
@NthTensor
Copy link
Contributor Author

This finally seems to be passing CI!

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 15, 2024
Merged via the queue into bevyengine:main with commit aaccbe8 Jun 15, 2024
32 checks passed
knutsoned pushed a commit to knutsoned/bevy that referenced this pull request Jun 25, 2024
This is the first of a series of PRs intended to begin the upstreaming
process for `bevy_mod_picking`. The purpose of this PR is to:
+ Create the new `bevy_picking` crate
+ Upstream `CorePlugin` as `PickingPlugin`
+ Upstream the core pointer and backend abstractions.

This code has been ported verbatim from the corresponding files in
[bevy_picking_core](https://github.com/aevyrie/bevy_mod_picking/tree/main/crates/bevy_picking_core/src)
with a few tiny naming and docs tweaks.

The work here is only an initial foothold to get the up-streaming
process started in earnest. We can do refactoring and improvements once
this is in-tree.

---------

Co-authored-by: Aevyrie <aevyrie@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Picking Pointing at and selecting objects of all sorts D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Blessed Has a large architectural impact or tradeoffs, but the design has been endorsed by decision makers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants