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

Add Disabled marker #12928

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/bevy_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub struct TypeRegistrationPlugin;

impl Plugin for TypeRegistrationPlugin {
fn build(&self, app: &mut App) {
app.register_type::<Name>();
app.register_type::<Name>().register_type::<Disabled>();
}
}

Expand Down
8 changes: 6 additions & 2 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ pub mod prelude {
component::Component,
entity::{Entity, EntityMapper},
event::{Event, EventReader, EventWriter, Events},
query::{Added, AnyOf, Changed, Has, Or, QueryBuilder, QueryState, With, Without},
query::{
Added, AnyOf, Changed, Disabled, Has, Or, QueryBuilder, QueryState, With, Without,
},
removal_detection::RemovedComponents,
schedule::{
apply_deferred, apply_state_transition, common_conditions::*, Condition,
Expand All @@ -69,7 +71,7 @@ mod tests {
change_detection::Ref,
component::{Component, ComponentId},
entity::Entity,
query::{Added, Changed, FilteredAccess, QueryFilter, With, Without},
query::{Added, Changed, Disabled, FilteredAccess, QueryFilter, With, Without},
system::Resource,
world::{EntityRef, Mut, World},
};
Expand Down Expand Up @@ -1379,13 +1381,15 @@ mod tests {
#[test]
fn filtered_query_access() {
let mut world = World::new();
let disabled_id = world.init_component::<Disabled>();
let query = world.query_filtered::<&mut A, Changed<B>>();

let mut expected = FilteredAccess::<ComponentId>::default();
let a_id = world.components.get_id(TypeId::of::<A>()).unwrap();
let b_id = world.components.get_id(TypeId::of::<B>()).unwrap();
expected.add_write(a_id);
expected.add_read(b_id);
expected.and_without(disabled_id);
assert!(
query.component_access.eq(&expected),
"ComponentId access from query fetch and query filter should be combined"
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/query/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub enum QueryEntityError {

/// An error that occurs when evaluating a [`Query`](crate::system::Query) or [`QueryState`](crate::query::QueryState) as a single expected result via
/// [`get_single`](crate::system::Query::get_single) or [`get_single_mut`](crate::system::Query::get_single_mut).
#[derive(Debug, Error)]
#[derive(Debug, PartialEq, Eq, Error)]
pub enum QuerySingleError {
/// No entity fits the query.
#[error("No entities fit the query {0}")]
Expand Down
46 changes: 46 additions & 0 deletions crates/bevy_ecs/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,52 @@ pub use par_iter::*;
pub use state::*;
pub use world_query::*;

use crate as bevy_ecs;
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved

/// A special marker component to disable an entity.
///
/// Disabled entities do not show up in most queries, unless the query mentions Disabled.
Copy link
Member

@alice-i-cecile alice-i-cecile Apr 13, 2024

Choose a reason for hiding this comment

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

Suggested change
/// Disabled entities do not show up in most queries, unless the query mentions Disabled.
/// Disabled entities do not show up in queries unless the query mentions [`Disabled`] in some way.

///
/// ## Example
///
/// ```rust
/// # use bevy_ecs::prelude::*;
/// # let mut world = World::new();
/// #
/// // This entity is enabled, since all entities are enabled by default
/// let entity_a = world.spawn_empty().id();
///
/// // We disable this entity using a helper method
/// let entity_b = world.spawn_empty().id();
/// world.disable(entity_b);
///
/// // It can also be inserted like other component
/// let entity_c = world.spawn(Disabled).id();
///
/// // This query does not mention Disabled, so disabled entities are hidden
/// let mut query = world.query::<Entity>();
/// assert_eq!(1, query.iter(&world).count());
/// assert_eq!(Ok(entity_a), query.get_single(&world));
///
/// // If our query mentions Disabled, we can find disabled entities like normal
/// // Here we query for only the disabled entities
/// let mut query = world.query_filtered::<(), With<Disabled>>();
/// assert!(query.get(&world, entity_a).is_err());
/// assert!(query.get_many(&world, [entity_b, entity_c]).is_ok());
///
/// // It also works as part of the query data
/// let mut query = world.query::<Has<Disabled>>();
/// assert_eq!(Ok([false, true, true]), query.get_many(&world, [entity_a, entity_b, entity_c]));
///
/// // If we exclude Disabled, it functions the same as the default behavior
/// let mut query = world.query_filtered::<Entity, Without<Disabled>>();
/// assert_eq!(1, query.iter(&world).count());
/// assert_eq!(Ok(entity_a), query.get_single(&world));
/// ```
#[derive(bevy_ecs_macros::Component, Clone, Copy)]
#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
pub struct Disabled;
Comment on lines +67 to +68
Copy link
Contributor

Choose a reason for hiding this comment

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

This should reflect Component, and maybe also implement and reflect Serialize and Deserialize.

Copy link
Member

Choose a reason for hiding this comment

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

Yes to both.


/// A debug checked version of [`Option::unwrap_unchecked`]. Will panic in
/// debug modes if unwrapping a `None` or `Err` value in debug mode, but is
/// equivalent to `Option::unwrap_unchecked` or `Result::unwrap_unchecked`
Expand Down
72 changes: 69 additions & 3 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{
archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId},
component::{ComponentId, Tick},
entity::Entity,
prelude::FromWorld,
prelude::{Disabled, FromWorld},
query::{
Access, BatchingStrategy, DebugCheckedUnwrap, FilteredAccess, QueryCombinationIter,
QueryIter, QueryParIter,
Expand Down Expand Up @@ -146,6 +146,17 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
}
}

fn contains_disabled(
component_access: &FilteredAccess<ComponentId>,
disabled_id: ComponentId,
) -> bool {
component_access.access().has_read(disabled_id)
|| component_access.access().has_archetypal(disabled_id)
|| component_access.filter_sets.iter().any(|f| {
f.with.contains(disabled_id.index()) || f.without.contains(disabled_id.index())
})
}

impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
/// Creates a new [`QueryState`] from a given [`World`] and inherits the result of `world.id()`.
pub fn new(world: &mut World) -> Self {
Expand Down Expand Up @@ -190,6 +201,12 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
// properly considered in a global "cross-query" context (both within systems and across systems).
component_access.extend(&filter_component_access);

let disabled_id = world.init_component::<Disabled>();
let allow_disabled = contains_disabled(&component_access, disabled_id);
if !allow_disabled {
component_access.and_without(disabled_id);
}

Self {
world_id: world.id(),
archetype_generation: ArchetypeGeneration::initial(),
Expand All @@ -214,13 +231,20 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
let filter_state = F::init_state(builder.world_mut());
D::set_access(&mut fetch_state, builder.access());

let mut component_access = builder.access().clone();
let disabled_id = builder.world_mut().init_component::<Disabled>();
let allow_disabled = contains_disabled(&component_access, disabled_id);
if !allow_disabled {
component_access.and_without(disabled_id);
}

let mut state = Self {
world_id: builder.world().id(),
archetype_generation: ArchetypeGeneration::initial(),
matched_storage_ids: Vec::new(),
fetch_state,
filter_state,
component_access: builder.access().clone(),
component_access,
matched_tables: Default::default(),
matched_archetypes: Default::default(),
#[cfg(feature = "trace")]
Expand Down Expand Up @@ -1627,7 +1651,7 @@ impl<D: QueryData, F: QueryFilter> From<QueryBuilder<'_, D, F>> for QueryState<D
mod tests {
use crate as bevy_ecs;
use crate::world::FilteredEntityRef;
use crate::{component::Component, prelude::*, query::QueryEntityError};
use crate::{prelude::*, query::QueryEntityError};

#[test]
fn get_many_unchecked_manual_uniqueness() {
Expand Down Expand Up @@ -1999,4 +2023,46 @@ mod tests {
let query_2 = QueryState::<&B, Without<C>>::new(&mut world);
let _: QueryState<Entity, Changed<C>> = query_1.join_filtered(&world, &query_2);
}

#[test]
fn does_exclude_disabled() {
let mut world = World::new();
let entity_a = world.spawn(A(0)).id();
let _ = world.spawn((A(1), Disabled)).id();
let entity_c = world.spawn(A(1)).id();
world.disable(entity_c);

let mut query = QueryState::<Entity>::new(&mut world);
assert_eq!(entity_a, query.single(&world));

let mut query = QueryState::<Entity, Without<Disabled>>::new(&mut world);
assert_eq!(entity_a, query.single(&world));
}

#[test]
fn can_request_disabled() {
let mut world = World::new();
let _ = world.spawn(A(0)).id();
let _ = world.spawn((A(1), Disabled)).id();
let entity_c = world.spawn(A(1)).id();
world.disable(entity_c);

let mut query = QueryState::<Entity, With<Disabled>>::new(&mut world);
assert_eq!(2, query.iter(&world).count());

let mut query = QueryState::<(Entity, &Disabled)>::new(&mut world);
assert_eq!(2, query.iter(&world).count());

let mut query = QueryState::<(Entity, &mut Disabled)>::new(&mut world);
assert_eq!(2, query.iter(&world).count());

let mut query = QueryState::<(Entity, Ref<Disabled>)>::new(&mut world);
assert_eq!(2, query.iter(&world).count());

let mut query = QueryState::<(Entity, Option<&Disabled>)>::new(&mut world);
assert_eq!(3, query.iter(&world).count());

let mut query = QueryState::<(Entity, Has<Disabled>)>::new(&mut world);
assert_eq!(3, query.iter(&world).count());
}
}
22 changes: 22 additions & 0 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,16 @@ impl EntityCommands<'_> {
self.add(despawn);
}

/// Enable this entity, see [`Disabled`](crate::prelude::Disabled) for more information.
pub fn enable(&mut self) {
self.add(enable);
}

/// Disable this entity, see [`Disabled`](crate::prelude::Disabled) for more information.
pub fn disable(&mut self) {
self.add(disable);
}

/// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`].
///
/// # Examples
Expand Down Expand Up @@ -991,6 +1001,10 @@ impl EntityCommands<'_> {
/// }
/// # bevy_ecs::system::assert_is_system(remove_combat_stats_system);
/// ```
///
/// # Disabled
///
/// Calling retain does not remove the [`Disabled`](crate::prelude::Disabled) marker even if it is present.
pub fn retain<T>(&mut self) -> &mut Self
where
T: Bundle,
Expand Down Expand Up @@ -1084,6 +1098,14 @@ fn despawn(entity: Entity, world: &mut World) {
world.despawn(entity);
}

fn enable(entity: Entity, world: &mut World) {
world.enable(entity);
}

fn disable(entity: Entity, world: &mut World) {
world.disable(entity);
}

/// An [`EntityCommand`] that adds the components in a [`Bundle`] to an entity.
fn insert<T: Bundle>(bundle: T) -> impl EntityCommand {
move |entity: Entity, world: &mut World| {
Expand Down
35 changes: 33 additions & 2 deletions crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
change_detection::MutUntyped,
component::{Component, ComponentId, ComponentTicks, Components, StorageType},
entity::{Entities, Entity, EntityLocation},
query::{Access, DebugCheckedUnwrap},
query::{Access, DebugCheckedUnwrap, Disabled},
removal_detection::RemovedComponentEvents,
storage::Storages,
world::{Mut, World},
Expand Down Expand Up @@ -1136,6 +1136,8 @@ impl<'w> EntityWorldMut<'w> {
///
/// See [`EntityCommands::retain`](crate::system::EntityCommands::retain) for more details.
pub fn retain<T: Bundle>(&mut self) -> &mut Self {
let disabled_id = self.world.init_component::<Disabled>();
Copy link
Member

Choose a reason for hiding this comment

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

This behavior needs to be documented. It's a nice bit of design though!


let archetypes = &mut self.world.archetypes;
let storages = &mut self.world.storages;
let components = &mut self.world.components;
Expand All @@ -1148,7 +1150,7 @@ impl<'w> EntityWorldMut<'w> {

let to_remove = &old_archetype
.components()
.filter(|c| !retained_bundle_info.components().contains(c))
.filter(|c| !retained_bundle_info.components().contains(c) && *c != disabled_id)
.collect::<Vec<_>>();
let remove_bundle = self.world.bundles.init_dynamic_info(components, to_remove);

Expand Down Expand Up @@ -1269,6 +1271,16 @@ impl<'w> EntityWorldMut<'w> {
self.entity
}

/// Enables the current entity, see [`Disabled`] for more information.
pub fn enable(&mut self) {
self.remove::<Disabled>();
}

/// Disables the current entity, see [`Disabled`] for more information.
pub fn disable(&mut self) {
self.insert(Disabled);
}

/// Gets read-only access to the world that the current entity belongs to.
#[inline]
pub fn world(&self) -> &World {
Expand Down Expand Up @@ -2583,6 +2595,25 @@ mod tests {
assert_eq!(world.entity(ent).archetype().components().next(), None);
}

// Test that calling retain retains `Disabled`.
#[test]
fn retain_disabled() {
#[derive(Component)]
struct Marker<const N: usize>;

let mut world = World::new();
let ent = world
.spawn((Marker::<1>, Marker::<2>, Marker::<3>, Disabled))
.id();

let disabled_id = world.init_component::<Disabled>();
world.entity_mut(ent).retain::<Marker<1>>();
assert!(world.entity(ent).contains_id(disabled_id));

world.entity_mut(ent).retain::<()>();
assert!(world.entity(ent).contains_id(disabled_id));
}

// Test removing some components with `retain`, including components not on the entity.
#[test]
fn retain_some_components() {
Expand Down
26 changes: 26 additions & 0 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,32 @@ impl World {
}
}

/// Enables the given `entity`, if it exists and was [`Disabled`](crate::prelude::Disabled).
/// Returns `true` if the `entity` is now enabled and `false` if the `entity` does not exist.
#[inline]
pub fn enable(&mut self, entity: Entity) -> bool {
if let Some(mut entity) = self.get_entity_mut(entity) {
entity.enable();
true
} else {
warn!("error[B0003]: Could not enable entity {:?} because it doesn't exist in this World.", entity);
false
}
}

/// Disables the given `entity`, if it exists and didn't have [`Disabled`](crate::prelude::Disabled) yet.
/// Returns `true` if the `entity` is now disabled and `false` if the `entity` does not exist.
#[inline]
pub fn disable(&mut self, entity: Entity) -> bool {
if let Some(mut entity) = self.get_entity_mut(entity) {
entity.disable();
true
} else {
warn!("error[B0003]: Could not disable entity {:?} because it doesn't exist in this World.", entity);
false
}
}

/// Clears the internal component tracker state.
///
/// The world maintains some internal state about changed and removed components. This state
Expand Down
Loading