From d454db8e5803df1e41b961f960b85cbe91e90880 Mon Sep 17 00:00:00 2001 From: Tim Date: Mon, 7 Oct 2024 17:09:57 +0000 Subject: [PATCH] Rename the `Pickable` component and fix incorrect documentation (#15707) # Objective - Rename `Pickable` to `PickingBehavior` to counter the easily-made assumption that the component is required. It is optional - Fix and clarify documentation - The docs in `crates/bevy_ui/src/picking_backend.rs` were incorrect about the necessity of `Pickable` - Plus two minor code quality changes in this commit (7c2e75f48d20387ed27f38b3c9f2d28ae8adf84f) Closes #15632 --- crates/bevy_picking/src/backend.rs | 4 ++-- crates/bevy_picking/src/focus.rs | 26 +++++++++++------------ crates/bevy_picking/src/lib.rs | 18 ++++++++-------- crates/bevy_sprite/src/picking_backend.rs | 6 +++--- crates/bevy_ui/src/picking_backend.rs | 16 +++++++------- examples/ui/scroll.rs | 16 +++++++------- examples/ui/ui.rs | 6 +++--- 7 files changed, 46 insertions(+), 46 deletions(-) diff --git a/crates/bevy_picking/src/backend.rs b/crates/bevy_picking/src/backend.rs index 8f29a5a300323..39c8f00ae9765 100644 --- a/crates/bevy_picking/src/backend.rs +++ b/crates/bevy_picking/src/backend.rs @@ -20,7 +20,7 @@ //! - The [`PointerHits`] events produced by a backend do **not** need to be sorted or filtered, all //! that is needed is an unordered list of entities and their [`HitData`]. //! -//! - Backends do not need to consider the [`Pickable`](crate::Pickable) component, though they may +//! - Backends do not need to consider the [`PickingBehavior`](crate::PickingBehavior) component, though they may //! use it for optimization purposes. For example, a backend that traverses a spatial hierarchy //! may want to early exit if it intersects an entity that blocks lower entities from being //! picked. @@ -42,7 +42,7 @@ pub mod prelude { pub use super::{ray::RayMap, HitData, PointerHits}; pub use crate::{ pointer::{PointerId, PointerLocation}, - PickSet, Pickable, + PickSet, PickingBehavior, }; } diff --git a/crates/bevy_picking/src/focus.rs b/crates/bevy_picking/src/focus.rs index fdb5b862be980..d07c0c095f4bc 100644 --- a/crates/bevy_picking/src/focus.rs +++ b/crates/bevy_picking/src/focus.rs @@ -10,7 +10,7 @@ use std::collections::HashSet; use crate::{ backend::{self, HitData}, pointer::{PointerAction, PointerId, PointerInput, PointerInteraction, PointerPress}, - Pickable, + PickingBehavior, }; use bevy_derive::{Deref, DerefMut}; @@ -43,8 +43,8 @@ type OverMap = HashMap; /// between it and the pointer block interactions. /// /// For example, if a pointer is hitting a UI button and a 3d mesh, but the button is in front of -/// the mesh, and [`Pickable::should_block_lower`], the UI button will be hovered, but the mesh will -/// not. +/// the mesh, the UI button will be hovered, but the mesh will not. Unless, the [`PickingBehavior`] +/// component is present with [`should_block_lower`](PickingBehavior::should_block_lower) set to `false`. /// /// # Advanced Users /// @@ -64,7 +64,7 @@ pub struct PreviousHoverMap(pub HashMap>); /// This is the final focusing step to determine which entity the pointer is hovering over. pub fn update_focus( // Inputs - pickable: Query<&Pickable>, + picking_behavior: Query<&PickingBehavior>, pointers: Query<&PointerId>, mut under_pointer: EventReader, mut pointer_input: EventReader, @@ -81,7 +81,7 @@ pub fn update_focus( &pointers, ); build_over_map(&mut under_pointer, &mut over_map, &mut pointer_input); - build_hover_map(&pointers, pickable, &over_map, &mut hover_map); + build_hover_map(&pointers, picking_behavior, &over_map, &mut hover_map); } /// Clear non-empty local maps, reusing allocated memory. @@ -136,7 +136,7 @@ fn build_over_map( .or_insert_with(BTreeMap::new); for (entity, pick_data) in entities_under_pointer.picks.iter() { let layer = entities_under_pointer.order; - let hits = layer_map.entry(FloatOrd(layer)).or_insert_with(Vec::new); + let hits = layer_map.entry(FloatOrd(layer)).or_default(); hits.push((*entity, pick_data.clone())); } } @@ -148,26 +148,26 @@ fn build_over_map( } } -/// Build an unsorted set of hovered entities, accounting for depth, layer, and [`Pickable`]. Note -/// that unlike the pointer map, this uses [`Pickable`] to determine if lower entities receive hover +/// Build an unsorted set of hovered entities, accounting for depth, layer, and [`PickingBehavior`]. Note +/// that unlike the pointer map, this uses [`PickingBehavior`] to determine if lower entities receive hover /// focus. Often, only a single entity per pointer will be hovered. fn build_hover_map( pointers: &Query<&PointerId>, - pickable: Query<&Pickable>, + picking_behavior: Query<&PickingBehavior>, over_map: &Local, // Output hover_map: &mut HoverMap, ) { for pointer_id in pointers.iter() { - let pointer_entity_set = hover_map.entry(*pointer_id).or_insert_with(HashMap::new); + let pointer_entity_set = hover_map.entry(*pointer_id).or_default(); if let Some(layer_map) = over_map.get(pointer_id) { // Note we reverse here to start from the highest layer first. for (entity, pick_data) in layer_map.values().rev().flatten() { - if let Ok(pickable) = pickable.get(*entity) { - if pickable.is_hoverable { + if let Ok(picking_behavior) = picking_behavior.get(*entity) { + if picking_behavior.is_hoverable { pointer_entity_set.insert(*entity, pick_data.clone()); } - if pickable.should_block_lower { + if picking_behavior.should_block_lower { break; } } else { diff --git a/crates/bevy_picking/src/lib.rs b/crates/bevy_picking/src/lib.rs index ea20dd3b5fddf..c1ec7a49e9d96 100644 --- a/crates/bevy_picking/src/lib.rs +++ b/crates/bevy_picking/src/lib.rs @@ -135,8 +135,8 @@ //! just because a pointer is over an entity, it is not necessarily *hovering* that entity. Although //! multiple backends may be reporting that a pointer is hitting an entity, the focus system needs //! to determine which entities are actually being hovered by this pointer based on the pick depth, -//! order of the backend, and the [`Pickable`] state of the entity. In other words, if one entity is -//! in front of another, usually only the topmost one will be hovered. +//! order of the backend, and the optional [`PickingBehavior`] component of the entity. In other words, +//! if one entity is in front of another, usually only the topmost one will be hovered. //! //! #### Events ([`events`]) //! @@ -169,7 +169,7 @@ pub mod prelude { #[doc(hidden)] pub use crate::{ events::*, input::PointerInputPlugin, pointer::PointerButton, DefaultPickingPlugins, - InteractionPlugin, Pickable, PickingPlugin, + InteractionPlugin, PickingBehavior, PickingPlugin, }; } @@ -178,7 +178,7 @@ pub mod prelude { /// the fields for more details. #[derive(Component, Debug, Clone, Reflect, PartialEq, Eq)] #[reflect(Component, Default, Debug, PartialEq)] -pub struct Pickable { +pub struct PickingBehavior { /// Should this entity block entities below it from being picked? /// /// This is useful if you want picking to continue hitting entities below this one. Normally, @@ -198,7 +198,7 @@ pub struct Pickable { /// element will be marked as hovered. However, if this field is set to `false`, both the UI /// element *and* the mesh will be marked as hovered. /// - /// Entities without the [`Pickable`] component will block by default. + /// Entities without the [`PickingBehavior`] component will block by default. pub should_block_lower: bool, /// If this is set to `false` and `should_block_lower` is set to true, this entity will block @@ -213,11 +213,11 @@ pub struct Pickable { /// components mark it as hovered. This can be combined with the other field /// [`Self::should_block_lower`], which is orthogonal to this one. /// - /// Entities without the [`Pickable`] component are hoverable by default. + /// Entities without the [`PickingBehavior`] component are hoverable by default. pub is_hoverable: bool, } -impl Pickable { +impl PickingBehavior { /// This entity will not block entities beneath it, nor will it emit events. /// /// If a backend reports this entity as being hit, the picking plugin will completely ignore it. @@ -227,7 +227,7 @@ impl Pickable { }; } -impl Default for Pickable { +impl Default for PickingBehavior { fn default() -> Self { Self { should_block_lower: true, @@ -354,7 +354,7 @@ impl Plugin for PickingPlugin { .chain(), ) .register_type::() - .register_type::() + .register_type::() .register_type::() .register_type::() .register_type::() diff --git a/crates/bevy_sprite/src/picking_backend.rs b/crates/bevy_sprite/src/picking_backend.rs index 7199d7875dc6b..f9f906536c722 100644 --- a/crates/bevy_sprite/src/picking_backend.rs +++ b/crates/bevy_sprite/src/picking_backend.rs @@ -35,7 +35,7 @@ pub fn sprite_picking( Option<&TextureAtlas>, &Handle, &GlobalTransform, - Option<&Pickable>, + Option<&PickingBehavior>, &ViewVisibility, )>, mut output: EventWriter, @@ -78,7 +78,7 @@ pub fn sprite_picking( .copied() .filter(|(.., visibility)| visibility.get()) .filter_map( - |(entity, sprite, atlas, image, sprite_transform, pickable, ..)| { + |(entity, sprite, atlas, image, sprite_transform, picking_behavior, ..)| { if blocked { return None; } @@ -129,7 +129,7 @@ pub fn sprite_picking( let is_cursor_in_sprite = rect.contains(cursor_pos_sprite); blocked = is_cursor_in_sprite - && pickable.map(|p| p.should_block_lower) != Some(false); + && picking_behavior.map(|p| p.should_block_lower) != Some(false); is_cursor_in_sprite.then(|| { let hit_pos_world = diff --git a/crates/bevy_ui/src/picking_backend.rs b/crates/bevy_ui/src/picking_backend.rs index da708511bbe9f..500fea213bf0f 100644 --- a/crates/bevy_ui/src/picking_backend.rs +++ b/crates/bevy_ui/src/picking_backend.rs @@ -8,9 +8,9 @@ //! ## Important Note //! //! This backend completely ignores [`FocusPolicy`](crate::FocusPolicy). The design of `bevy_ui`'s -//! focus systems and the picking plugin are not compatible. Instead, use the [`Pickable`] component -//! to customize how an entity responds to picking focus. Nodes without the [`Pickable`] component -//! will not trigger events. +//! focus systems and the picking plugin are not compatible. Instead, use the optional [`PickingBehavior`] component +//! to override how an entity responds to picking focus. Nodes without the [`PickingBehavior`] component +//! will still trigger events and block items below it from being hovered. //! //! ## Implementation Notes //! @@ -50,7 +50,7 @@ pub struct NodeQuery { entity: Entity, node: &'static Node, global_transform: &'static GlobalTransform, - pickable: Option<&'static Pickable>, + picking_behavior: Option<&'static PickingBehavior>, calculated_clip: Option<&'static CalculatedClip>, view_visibility: Option<&'static ViewVisibility>, target_camera: Option<&'static TargetCamera>, @@ -197,13 +197,13 @@ pub fn ui_picking( picks.push((node.entity, HitData::new(camera_entity, depth, None, None))); - if let Some(pickable) = node.pickable { - // If an entity has a `Pickable` component, we will use that as the source of truth. - if pickable.should_block_lower { + if let Some(picking_behavior) = node.picking_behavior { + // If an entity has a `PickingBehavior` component, we will use that as the source of truth. + if picking_behavior.should_block_lower { break; } } else { - // If the Pickable component doesn't exist, default behavior is to block. + // If the PickingBehavior component doesn't exist, default behavior is to block. break; } diff --git a/examples/ui/scroll.rs b/examples/ui/scroll.rs index 9ce6f36337cd1..a8919e598b890 100644 --- a/examples/ui/scroll.rs +++ b/examples/ui/scroll.rs @@ -40,7 +40,7 @@ fn setup(mut commands: Commands, asset_server: Res) { }, ..default() }) - .insert(Pickable::IGNORE) + .insert(PickingBehavior::IGNORE) .with_children(|parent| { // horizontal scroll example parent @@ -98,7 +98,7 @@ fn setup(mut commands: Commands, asset_server: Res) { align_content: AlignContent::Center, ..default() }) - .insert(Pickable { + .insert(PickingBehavior { should_block_lower: false, ..default() }) @@ -177,7 +177,7 @@ fn setup(mut commands: Commands, asset_server: Res) { }, ..default() }) - .insert(Pickable { + .insert(PickingBehavior { should_block_lower: false, ..default() }) @@ -198,7 +198,7 @@ fn setup(mut commands: Commands, asset_server: Res) { Role::ListItem, )), )) - .insert(Pickable { + .insert(PickingBehavior { should_block_lower: false, ..default() }); @@ -256,7 +256,7 @@ fn setup(mut commands: Commands, asset_server: Res) { }, ..default() }) - .insert(Pickable::IGNORE) + .insert(PickingBehavior::IGNORE) .with_children(|parent| { // Elements in each row for i in 0..25 { @@ -276,7 +276,7 @@ fn setup(mut commands: Commands, asset_server: Res) { Role::ListItem, )), )) - .insert(Pickable { + .insert(PickingBehavior { should_block_lower: false, ..default() }); @@ -340,7 +340,7 @@ fn setup(mut commands: Commands, asset_server: Res) { .into(), ..default() }) - .insert(Pickable { + .insert(PickingBehavior { should_block_lower: false, ..default() }) @@ -362,7 +362,7 @@ fn setup(mut commands: Commands, asset_server: Res) { Role::ListItem, )), )) - .insert(Pickable { + .insert(PickingBehavior { should_block_lower: false, ..default() }); diff --git a/examples/ui/ui.rs b/examples/ui/ui.rs index 009354634c179..aa0608029f02f 100644 --- a/examples/ui/ui.rs +++ b/examples/ui/ui.rs @@ -44,7 +44,7 @@ fn setup(mut commands: Commands, asset_server: Res) { }, ..default() }) - .insert(Pickable::IGNORE) + .insert(PickingBehavior::IGNORE) .with_children(|parent| { // left vertical fill (border) parent @@ -167,7 +167,7 @@ fn setup(mut commands: Commands, asset_server: Res) { Label, AccessibilityNode(NodeBuilder::new(Role::ListItem)), )) - .insert(Pickable { + .insert(PickingBehavior { should_block_lower: false, ..default() }); @@ -214,7 +214,7 @@ fn setup(mut commands: Commands, asset_server: Res) { }, ..default() }) - .insert(Pickable::IGNORE) + .insert(PickingBehavior::IGNORE) .with_children(|parent| { parent .spawn(NodeBundle {