Skip to content

Commit

Permalink
Rename the Pickable component and fix incorrect documentation (#15707)
Browse files Browse the repository at this point in the history
# 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
(7c2e75f)

Closes #15632
  • Loading branch information
tim-blackbird authored Oct 7, 2024
1 parent 8adc9e9 commit d454db8
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 46 deletions.
4 changes: 2 additions & 2 deletions crates/bevy_picking/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -42,7 +42,7 @@ pub mod prelude {
pub use super::{ray::RayMap, HitData, PointerHits};
pub use crate::{
pointer::{PointerId, PointerLocation},
PickSet, Pickable,
PickSet, PickingBehavior,
};
}

Expand Down
26 changes: 13 additions & 13 deletions crates/bevy_picking/src/focus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -43,8 +43,8 @@ type OverMap = HashMap<PointerId, LayerMap>;
/// 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
///
Expand All @@ -64,7 +64,7 @@ pub struct PreviousHoverMap(pub HashMap<PointerId, HashMap<Entity, HitData>>);
/// 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<backend::PointerHits>,
mut pointer_input: EventReader<PointerInput>,
Expand All @@ -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.
Expand Down Expand Up @@ -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()));
}
}
Expand All @@ -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<OverMap>,
// 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 {
Expand Down
18 changes: 9 additions & 9 deletions crates/bevy_picking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`])
//!
Expand Down Expand Up @@ -169,7 +169,7 @@ pub mod prelude {
#[doc(hidden)]
pub use crate::{
events::*, input::PointerInputPlugin, pointer::PointerButton, DefaultPickingPlugins,
InteractionPlugin, Pickable, PickingPlugin,
InteractionPlugin, PickingBehavior, PickingPlugin,
};
}

Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -227,7 +227,7 @@ impl Pickable {
};
}

impl Default for Pickable {
impl Default for PickingBehavior {
fn default() -> Self {
Self {
should_block_lower: true,
Expand Down Expand Up @@ -354,7 +354,7 @@ impl Plugin for PickingPlugin {
.chain(),
)
.register_type::<Self>()
.register_type::<Pickable>()
.register_type::<PickingBehavior>()
.register_type::<pointer::PointerId>()
.register_type::<pointer::PointerLocation>()
.register_type::<pointer::PointerPress>()
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_sprite/src/picking_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub fn sprite_picking(
Option<&TextureAtlas>,
&Handle<Image>,
&GlobalTransform,
Option<&Pickable>,
Option<&PickingBehavior>,
&ViewVisibility,
)>,
mut output: EventWriter<PointerHits>,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 =
Expand Down
16 changes: 8 additions & 8 deletions crates/bevy_ui/src/picking_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
//!
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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;
}

Expand Down
16 changes: 8 additions & 8 deletions examples/ui/scroll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
},
..default()
})
.insert(Pickable::IGNORE)
.insert(PickingBehavior::IGNORE)
.with_children(|parent| {
// horizontal scroll example
parent
Expand Down Expand Up @@ -98,7 +98,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
align_content: AlignContent::Center,
..default()
})
.insert(Pickable {
.insert(PickingBehavior {
should_block_lower: false,
..default()
})
Expand Down Expand Up @@ -177,7 +177,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
},
..default()
})
.insert(Pickable {
.insert(PickingBehavior {
should_block_lower: false,
..default()
})
Expand All @@ -198,7 +198,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
Role::ListItem,
)),
))
.insert(Pickable {
.insert(PickingBehavior {
should_block_lower: false,
..default()
});
Expand Down Expand Up @@ -256,7 +256,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
},
..default()
})
.insert(Pickable::IGNORE)
.insert(PickingBehavior::IGNORE)
.with_children(|parent| {
// Elements in each row
for i in 0..25 {
Expand All @@ -276,7 +276,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
Role::ListItem,
)),
))
.insert(Pickable {
.insert(PickingBehavior {
should_block_lower: false,
..default()
});
Expand Down Expand Up @@ -340,7 +340,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
.into(),
..default()
})
.insert(Pickable {
.insert(PickingBehavior {
should_block_lower: false,
..default()
})
Expand All @@ -362,7 +362,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
Role::ListItem,
)),
))
.insert(Pickable {
.insert(PickingBehavior {
should_block_lower: false,
..default()
});
Expand Down
6 changes: 3 additions & 3 deletions examples/ui/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
},
..default()
})
.insert(Pickable::IGNORE)
.insert(PickingBehavior::IGNORE)
.with_children(|parent| {
// left vertical fill (border)
parent
Expand Down Expand Up @@ -167,7 +167,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
Label,
AccessibilityNode(NodeBuilder::new(Role::ListItem)),
))
.insert(Pickable {
.insert(PickingBehavior {
should_block_lower: false,
..default()
});
Expand Down Expand Up @@ -214,7 +214,7 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
},
..default()
})
.insert(Pickable::IGNORE)
.insert(PickingBehavior::IGNORE)
.with_children(|parent| {
parent
.spawn(NodeBundle {
Expand Down

0 comments on commit d454db8

Please sign in to comment.