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

Dynamic queries and builder API #9774

Merged
merged 57 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
b56e1c7
Fully dynamic term based query builder
james-j-obrien Sep 11, 2023
5917065
System installation of dynamically built queries
james-j-obrien Sep 11, 2023
b7f04df
Fix change detection, add Changed and Added
james-j-obrien Sep 11, 2023
07dc116
Add additional tests, fix bugs and clean up
james-j-obrien Sep 11, 2023
a12b5dd
Add trivial examples
james-j-obrien Sep 11, 2023
f6382c1
Performance tweaks
james-j-obrien Sep 12, 2023
25623dc
Add dynamic reference methods
james-j-obrien Sep 12, 2023
c16e0ec
Fix rebase issue
james-j-obrien Sep 12, 2023
799b318
Remove examples so github is less angry, rename builder methods to be…
james-j-obrien Sep 12, 2023
711653b
Better builder method names
james-j-obrien Sep 12, 2023
ff1e33a
Major perf improvements, add additional dynamic builder methods
james-j-obrien Sep 12, 2023
40bd8c5
Remove closures from hot loop, remove more runtime checks
james-j-obrien Sep 12, 2023
d0736f5
Remove allocations from hot loop
james-j-obrien Sep 12, 2023
c3d37da
Minor cleanup
james-j-obrien Sep 12, 2023
353c248
Remove #[inline(always)] where it doesn't improve or degrades perform…
james-j-obrien Sep 13, 2023
40c1c62
Flatten terms
james-j-obrien Sep 13, 2023
7667c0a
Fix AnyOf issue
james-j-obrien Sep 13, 2023
031779c
Reorder impls to be more intuitive
james-j-obrien Sep 13, 2023
5494b28
Add dynamic_query example
james-j-obrien Sep 14, 2023
688459d
First docs pass, bug fixes and minor improvements
james-j-obrien Sep 15, 2023
666edb3
Improve documentation, add additional utility functions, add benches
james-j-obrien Sep 18, 2023
5ac0e50
Remove temporary test
james-j-obrien Sep 18, 2023
9d6382f
Minor cleanup
james-j-obrien Sep 18, 2023
f7abf12
First CI pass
james-j-obrien Sep 18, 2023
5c880e9
Second CI pass
james-j-obrien Sep 18, 2023
7680abd
Third times the charm
james-j-obrien Sep 18, 2023
7bd46ab
Add comments
james-j-obrien Sep 18, 2023
db48ac8
Flatten terms, remove branching where possible
james-j-obrien Sep 29, 2023
02091d9
Fix example
james-j-obrien Sep 29, 2023
9a3f5b3
Static iteration, remove FetchedTerm and FetchBuffer
james-j-obrien Sep 30, 2023
88ba5e3
Cleanup component ptr access
james-j-obrien Sep 30, 2023
2cf8f1e
Tidy docs slightly
james-j-obrien Sep 30, 2023
9c65312
Swap to constants to speed up dense iteration, minor API cleanup
james-j-obrien Sep 30, 2023
e9c7861
Consistency pass
james-j-obrien Sep 30, 2023
c674c4a
Move to pointers from iterators, split fetch and filter terms
james-j-obrien Oct 4, 2023
5952756
Add From<Mut<T>> For MutUntyped
james-j-obrien Oct 7, 2023
a76370c
FilteredAccess based implementation
james-j-obrien Nov 14, 2023
323cc74
Merge branch 'dynamic-term-builder' into query-builder
james-j-obrien Nov 14, 2023
ce0ea41
Merge branch 'bevyengine:main' into dynamic-term-builder
james-j-obrien Nov 14, 2023
89f1eac
Doc fixes
james-j-obrien Nov 14, 2023
bf9cee7
Merge branch 'dynamic-term-builder' of https://github.com/james-j-obr…
james-j-obrien Nov 14, 2023
32a6796
Add compile fail tests, add safety comments to example
james-j-obrien Nov 20, 2023
289ca79
Merge main
james-j-obrien Nov 22, 2023
52e0391
Learn how to spell required correctly
james-j-obrien Nov 22, 2023
fecd6bc
Merge main, incorporate feedback
james-j-obrien Nov 30, 2023
04d88cc
Fix doc test
james-j-obrien Nov 30, 2023
3bfcc3f
Fix docs and compile fail tests
james-j-obrien Nov 30, 2023
60c006f
Fix trailing newline
james-j-obrien Nov 30, 2023
b59bb38
Fix trailing whitespace...
james-j-obrien Nov 30, 2023
cf951bb
Merge branch 'main' into dynamic-term-builder
james-j-obrien Dec 2, 2023
2e8a197
Remove unused import
james-j-obrien Dec 2, 2023
5c98ef4
Improve safety comments
james-j-obrien Dec 15, 2023
6f6a0cc
Merge main
james-j-obrien Dec 15, 2023
63f97a3
Address remaining feedback
james-j-obrien Jan 16, 2024
056ff2a
Merge branch 'main' into dynamic-term-builder
james-j-obrien Jan 16, 2024
a3ab076
Fix panic strings
james-j-obrien Jan 16, 2024
8c1bbb0
Fix doc test
james-j-obrien Jan 16, 2024
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
Prev Previous commit
Next Next commit
First docs pass, bug fixes and minor improvements
  • Loading branch information
james-j-obrien committed Sep 15, 2023
commit 688459d07965571f5a1c86d196049dbe7d9fc37e
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![warn(clippy::undocumented_unsafe_blocks)]
#![allow(missing_docs)]
#![warn(missing_docs)]
#![allow(clippy::type_complexity)]
#![doc = include_str!("../README.md")]

Expand Down Expand Up @@ -53,7 +53,7 @@ pub mod prelude {
Commands, Deferred, In, IntoSystem, Local, NonSend, NonSendMut, ParallelCommands,
ParamSet, Query, ReadOnlySystem, Res, ResMut, Resource, System, SystemParamFunction,
},
term_query::{TermQuery, TermQueryState},
term_query::TermQueryState,
world::{EntityMut, EntityRef, EntityWorldMut, FromWorld, World},
};
}
Expand Down
6 changes: 6 additions & 0 deletions crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,13 @@ impl<Marker, F> FunctionSystem<Marker, F>
where
F: SystemParamFunction<Marker>,
{
/// Retrieves this systems [SystemParam::State] mutably
///
/// # Safety
/// Mutable reference must not be used to put the [SystemParam::State]
/// in an invalid state for this [`FunctionSystem`]
Copy link
Member

Choose a reason for hiding this comment

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

I would like more clarity here if possible. Ideally we can point to exactly where the invariants are documented, and which ones are memory safety concerns.

Copy link
Contributor Author

@james-j-obrien james-j-obrien Sep 18, 2023

Choose a reason for hiding this comment

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

The tricky part here is this function allows you to modify any system param state, including user defined system params. That means that we can't really know all the invariants that need to be maintained here. We could try and list invariants for existing system params though e.g. if you change the ComponentId that a Res stores it will point to the wrong component and cast it in an unsafe way. These invariants aren't currently listed anywhere as there was no safe or unsafe way to break them.

Exposing the state directly like this is a little hacky. My preferred solution would be a macro that generates syntax like this:

system.set_state::<0>(query_state)

I didn't want to add that in this PR as it requires a new proc macro and this PR is already big enough, it helps in that the invariant just becomes "make sure the new state is valid" rather than "make sure you don't make invalid edits to the state". I'm open to a safer approach but I'm not sure what that looks like.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this API is too hard for the user to use safely and we should maybe punt this to a separate pr and see if we can provide a safe panicking version similar to Query::transmute

pub unsafe fn state_mut(&mut self) -> &mut <F::Param as SystemParam>::State {
// Archetype generation reset so that new params will be updated with all archetypes
self.archetype_generation = ArchetypeGeneration::initial();
self.param_state.as_mut().unwrap()
}
Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ mod query;
#[allow(clippy::module_inception)]
mod system;
mod system_param;
mod term_query;

use std::borrow::Cow;

Expand All @@ -124,6 +125,7 @@ pub use function_system::*;
pub use query::*;
pub use system::*;
pub use system_param::*;
pub use term_query::*;

use crate::world::World;

Expand Down
7 changes: 6 additions & 1 deletion crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
Access, FilteredAccess, FilteredAccessSet, QueryState, ReadOnlyWorldQuery, WorldQuery,
},
system::{Query, SystemMeta},
term_query::{QueryTermGroup, TermQuery, TermQueryState},
term_query::{QueryTermGroup, TermQueryState},
world::{unsafe_world_cell::UnsafeWorldCell, FromWorld, World},
};
use bevy_ecs_macros::impl_param_set;
Expand All @@ -24,6 +24,8 @@ use std::{
ops::{Deref, DerefMut},
};

use super::TermQuery;

/// A parameter that can be used in a [`System`](super::System).
///
/// # Derive
Expand Down Expand Up @@ -264,6 +266,9 @@ unsafe impl<Q: QueryTermGroup + 'static, F: QueryTermGroup + 'static> SystemPara
world: UnsafeWorldCell<'world>,
change_tick: Tick,
) -> Self::Item<'world, 'state> {
// SAFETY: We have registered all of the query's world accesses,
// so the caller ensures that `world` has permission to access any
// world data that the query needs.
TermQuery::new(world, state, system_meta.last_run, change_tick)
}
}
Expand Down
276 changes: 276 additions & 0 deletions crates/bevy_ecs/src/system/term_query.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,276 @@
use std::marker::PhantomData;

use crate::{
component::Tick,
entity::Entity,
prelude::TermQueryState,
query::{QueryEntityError, QuerySingleError},
term_query::{QueryTermGroup, ROTermItem, TermQueryIter, TermQueryIterUntyped},
world::unsafe_world_cell::UnsafeWorldCell,
};

/// [System parameter] that provides selective access to the [`Component`] data stored in a [`World`].
///
/// This is broadly equivalent to [`Query`] supporting the same type API and near identical methods.
/// In all non-dynamic cases a [`Query`] will out-perform an equivalent [`TermQuery`]
///
/// For more information on dynamically building a [`TermQuery`] see [`QueryBuilder`]
///
/// [System parameter]: crate::system::SystemParam
/// [`QueryBuilder`]: crate::term_query::QueryBuilder
/// [`Query`]: crate::system::Query
/// [`Component`]: crate::component::Component
/// [`World`]: crate::World
pub struct TermQuery<'w, 's, Q: QueryTermGroup, F: QueryTermGroup = ()> {
// SAFETY: Must have access to the components registered in `state`.
world: UnsafeWorldCell<'w>,
state: &'s TermQueryState<Q, F>,
last_run: Tick,
this_run: Tick,
_marker: PhantomData<Q>,
}

impl<'w, 's, Q: QueryTermGroup, F: QueryTermGroup> TermQuery<'w, 's, Q, F> {
/// Creates a new term query.
///
/// # Panics
///
/// This will panic if the world used to create `state` is not `world`.
///
/// # Safety
///
/// This will create a query that could violate memory safety rules. Make sure that this is only
/// called in ways that ensure the queries have unique mutable access.
pub fn new(
world: UnsafeWorldCell<'w>,
state: &'s TermQueryState<Q, F>,
last_run: Tick,
this_run: Tick,
) -> Self {
state.validate_world(world.id());

Self {
world,
state,
last_run,
this_run,
_marker: PhantomData::default(),
}
}

/// Returns an [`Iterator`] over the read-only query items.
///
/// # See also
///
/// - [`iter_mut`](Self::iter_mut) for mutable query items.
/// - [`for_each`](Self::for_each) for the closure based alternative.
/// - [`Query::iter`](crate::query::Query::iter) for more examples
#[inline]
pub fn iter(&self) -> TermQueryIter<'_, 's, Q::ReadOnly> {
// SAFETY:
// - `self.world` has permission to access the required components.
// - The query is read-only, so it can be aliased even if it was originally mutable.
unsafe {
self.state
.as_readonly()
.iter_unchecked_manual(self.world, self.last_run, self.this_run)
}
}

/// Returns an [`Iterator`] over the query items.
///
/// # See also
///
/// - [`iter`](Self::iter) for read-only query items.
/// - [`for_each_mut`](Self::for_each_mut) for the closure based alternative.
/// - [`Query::iter_mut`](crate::query::Query::iter_mut) for more examples
#[inline]
pub fn iter_mut(&mut self) -> TermQueryIter<'_, 's, Q> {
// SAFETY: `self.world` has permission to access the required components.
unsafe {
self.state
.iter_unchecked_manual(self.world, self.last_run, self.this_run)
}
}

/// Returns an untyped [`Iterator`] over the query items.
/// Provides access to a list of the [`Term`]s used to resolve this query as well as
/// a list of the resolved [`FetchedTerm`]
///
/// # Example
///
/// Here, the `update_system` increments all terms with write access.
/// For a more advanced use case see the `dynamic_query` example.
///
/// ```
/// # use bevy_ecs::prelude::*;
/// #
/// # #[derive(Component)]
/// # struct A(usize);
/// #
/// # #[derive(Component)]
/// # struct B(usize);
/// #
/// # #[derive(Component)]
/// # struct C(usize);
///
/// fn update_system(mut query: TermQuery<(&mut A, &B, &mut C)>) {
/// query.iter_raw().for_each(|terms| {
/// terms.for_each(|(term, fetch)| {
/// if term.access == TermAccess::Write {
/// // Since all the components have the same layout we can cast them all to the same value
/// let component = <mut &A>::from_fetch(fetch);
/// component += 1;
/// }
/// })
/// });
/// }
/// # bevy_ecs::system::assert_is_system(update_system);
/// ```
#[inline]
pub fn iter_raw(&mut self) -> TermQueryIterUntyped<'_, 's> {
unsafe {
self.state
.iter_raw_manual(self.world, self.last_run, self.this_run)
}
}

/// Returns the read-only query item for the given [`Entity`].
///
/// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is returned instead.
///
/// # See also
///
/// - [`get_mut`](Self::get_mut) to get a mutable query item.
/// - [`Query::get_mut`](crate::query::Query::get_mut) for more examples
#[inline]
pub fn get(&self, entity: Entity) -> Result<ROTermItem<'_, Q>, QueryEntityError> {
// SAFETY: system runs without conflicts with other systems.
// same-system queries have runtime borrow checks when they conflict
unsafe {
self.state.as_readonly().get_unchecked_manual(
self.world,
entity,
self.last_run,
self.this_run,
)
}
}

/// Returns the query item for the given [`Entity`].
///
/// In case of a nonexisting entity or mismatched component, a [`QueryEntityError`] is returned instead.
///
/// # See also
///
/// - [`get`](Self::get) to get a read-only query item.
/// - [`Query::get`](crate::query::Query::get) for more examples
#[inline]
pub fn get_mut(&mut self, entity: Entity) -> Result<Q::Item<'_>, QueryEntityError> {
// SAFETY: system runs without conflicts with other systems.
// same-system queries have runtime borrow checks when they conflict
unsafe {
self.state
.get_unchecked_manual(self.world, entity, self.last_run, self.this_run)
}
}

/// Returns a single read-only query item when there is exactly one entity matching the query.
///
/// # See also
///
/// - [`get_single`](Self::get_single) for the non-panicking version.
/// - [`single_mut`](Self::single_mut) to get the mutable query item.
/// - [`Query::single`](crate::query::Query::single) for more examples
pub fn single(&self) -> ROTermItem<'_, Q> {
self.get_single().unwrap()
}

/// Returns a single read-only query item when there is exactly one entity matching the query.
///
/// If the number of query items is not exactly one, a [`QuerySingleError`] is returned instead.
///
/// # See also
///
/// - [`get_single_mut`](Self::get_single_mut) to get the mutable query item.
/// - [`single`](Self::single) for the panicking version.
/// - [`Query::get_single`](crate::query::Query::get_single) for more examples
#[inline]
pub fn get_single(&self) -> Result<ROTermItem<'_, Q>, QuerySingleError> {
// SAFETY:
// the query ensures that the components it accesses are not mutably accessible somewhere else
// and the query is read only.
unsafe {
self.state.as_readonly().get_single_unchecked_manual(
self.world,
self.last_run,
self.this_run,
)
}
}

/// Returns a single query item when there is exactly one entity matching the query.
///
/// # See also
///
/// - [`get_single_mut`](Self::get_single_mut) for the non-panicking version.
/// - [`single`](Self::single) to get the read-only query item.
/// - [`Query::single_mut`](crate::query::Query::single_mut) for more examples
pub fn single_mut(&mut self) -> Q::Item<'_> {
self.get_single_mut().unwrap()
}

/// Returns a single query item when there is exactly one entity matching the query.
///
/// If the number of query items is not exactly one, a [`QuerySingleError`] is returned instead.
///
/// # See also
///
/// - [`get_single`](Self::get_single) to get the read-only query item.
/// - [`single_mut`](Self::single_mut) for the panicking version.
/// - [`Query::get_single_mut`](crate::query::Query::get_single_mut) for more examples
#[inline]
pub fn get_single_mut(&mut self) -> Result<Q::Item<'_>, QuerySingleError> {
// SAFETY:
// the query ensures mutable access to the components it accesses, and the query
// is uniquely borrowed
unsafe {
self.state
.get_single_unchecked_manual(self.world, self.last_run, self.this_run)
}
}
}

impl<'w, 's, Q: QueryTermGroup, F: QueryTermGroup> IntoIterator for &'w TermQuery<'_, 's, Q, F> {
type Item = <Q::ReadOnly as QueryTermGroup>::Item<'w>;
type IntoIter = TermQueryIter<'w, 's, Q::ReadOnly>;

fn into_iter(self) -> Self::IntoIter {
unsafe {
TermQueryIter::new(
self.world,
self.state.as_readonly(),
self.last_run,
self.this_run,
)
}
}
}

impl<'w, 's, Q: QueryTermGroup, F: QueryTermGroup> IntoIterator
for &'w mut TermQuery<'_, 's, Q, F>
{
type Item = Q::Item<'w>;
type IntoIter = TermQueryIter<'w, 's, Q>;

fn into_iter(self) -> Self::IntoIter {
unsafe {
TermQueryIter::new(
self.world,
self.state.filterless(),
self.last_run,
self.this_run,
)
}
}
}
Loading