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

Observed entity query data fetching: Trigger<FooEvent, (), (&mut Foo, &Bar)> #15698

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
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
40 changes: 32 additions & 8 deletions crates/bevy_ecs/src/observer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::{
entity::EntityHashMap,
observer::entity_observer::ObservedBy,
prelude::*,
query::QueryData,
system::IntoObserverSystem,
world::{DeferredWorld, *},
};
Expand All @@ -27,24 +28,42 @@ use core::{
/// Type containing triggered [`Event`] information for a given run of an [`Observer`]. This contains the
/// [`Event`] data itself. If it was triggered for a specific [`Entity`], it includes that as well. It also
/// contains event propagation information. See [`Trigger::propagate`] for more information.
pub struct Trigger<'w, E, B: Bundle = ()> {
event: &'w mut E,
propagate: &'w mut bool,
pub struct Trigger<'world, 'input, E, B: Bundle = (), D: QueryData + 'static = ()> {
event: &'input mut E,
propagate: &'input mut bool,
trigger: ObserverTrigger,
data: Option<D::Item<'world>>,
_marker: PhantomData<B>,
}

impl<'w, E, B: Bundle> Trigger<'w, E, B> {
impl<'i, E, B: Bundle> Trigger<'static, 'i, E, B, ()> {
/// Creates a new trigger for the given event and observer information.
pub fn new(event: &'w mut E, propagate: &'w mut bool, trigger: ObserverTrigger) -> Self {
pub fn new(event: &'i mut E, propagate: &'i mut bool, trigger: ObserverTrigger) -> Self {
Self {
event,
propagate,
trigger,
data: None,
_marker: PhantomData,
}
}

/// Creates a new trigger with the given event and observer information, and target query data.
pub fn with_data<'w, D: QueryData>(
self,
data: Option<D::Item<'w>>,
) -> Trigger<'w, 'i, E, B, D> {
Trigger {
event: self.event,
propagate: self.propagate,
trigger: self.trigger,
data,
_marker: PhantomData,
}
}
}

impl<'w, E, B: Bundle, D: QueryData> Trigger<'w, '_, E, B, D> {
/// Returns the event type of this trigger.
pub fn event_type(&self) -> ComponentId {
self.trigger.event_type
Expand All @@ -65,6 +84,11 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> {
Ptr::from(&self.event)
}

/// Returns a reference to the query data associated with the trigger.
pub fn data(&mut self) -> Option<&mut D::Item<'w>> {
self.data.as_mut()
}

/// Returns the [`Entity`] that triggered the observer, could be [`Entity::PLACEHOLDER`].
pub fn entity(&self) -> Entity {
self.trigger.entity
Expand Down Expand Up @@ -120,7 +144,7 @@ impl<'w, E, B: Bundle> Trigger<'w, E, B> {
}
}

impl<'w, E: Debug, B: Bundle> Debug for Trigger<'w, E, B> {
impl<'w, 'i, E: Debug, B: Bundle> Debug for Trigger<'w, 'i, E, B> {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("Trigger")
.field("event", &self.event)
Expand All @@ -131,15 +155,15 @@ impl<'w, E: Debug, B: Bundle> Debug for Trigger<'w, E, B> {
}
}

impl<'w, E, B: Bundle> Deref for Trigger<'w, E, B> {
impl<'w, 'i, E, B: Bundle> Deref for Trigger<'w, 'i, E, B> {
type Target = E;

fn deref(&self) -> &Self::Target {
self.event
}
}

impl<'w, E, B: Bundle> DerefMut for Trigger<'w, E, B> {
impl<'w, 'i, E, B: Bundle> DerefMut for Trigger<'w, 'i, E, B> {
fn deref_mut(&mut self) -> &mut Self::Target {
self.event
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/observer/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ fn observer_system_runner<E: Event, B: Bundle, S: ObserverSystem<E, B>>(
// - system is the same type erased system from above
unsafe {
(*system).update_archetype_component_access(world);
if (*system).validate_param_unsafe(world) {
if (*system).validate_param_unsafe(&trigger, world) {
(*system).run_unsafe(trigger, world);
(*system).queue_deferred(world.into_deferred());
}
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ impl ExecutorState {
// - The caller ensures that `world` has permission to read any data
// required by the system.
// - `update_archetype_component_access` has been called for system.
let valid_params = unsafe { system.validate_param_unsafe(world) };
let valid_params = unsafe { system.validate_param_unsafe(&(), world) };
if !valid_params {
self.skipped_systems.insert(system_index);
}
Expand Down Expand Up @@ -748,7 +748,7 @@ unsafe fn evaluate_and_fold_conditions(
// - The caller ensures that `world` has permission to read any data
// required by the condition.
// - `update_archetype_component_access` has been called for condition.
if !unsafe { condition.validate_param_unsafe(world) } {
if !unsafe { condition.validate_param_unsafe(&(), world) } {
return false;
}
// SAFETY:
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl SystemExecutor for SimpleExecutor {

let system = &mut schedule.systems[system_index];
if should_run {
let valid_params = system.validate_param(world);
let valid_params = system.validate_param(&(), world);
should_run &= valid_params;
}

Expand Down Expand Up @@ -134,7 +134,7 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W
conditions
.iter_mut()
.map(|condition| {
if !condition.validate_param(world) {
if !condition.validate_param(&(), world) {
return false;
}
__rust_begin_short_backtrace::readonly_run(&mut **condition, world)
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/single_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl SystemExecutor for SingleThreadedExecutor {

let system = &mut schedule.systems[system_index];
if should_run {
let valid_params = system.validate_param(world);
let valid_params = system.validate_param(&(), world);
should_run &= valid_params;
}

Expand Down Expand Up @@ -166,7 +166,7 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W
conditions
.iter_mut()
.map(|condition| {
if !condition.validate_param(world) {
if !condition.validate_param(&(), world) {
return false;
}
__rust_begin_short_backtrace::readonly_run(&mut **condition, world)
Expand Down
14 changes: 9 additions & 5 deletions crates/bevy_ecs/src/system/adapter_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub struct IsAdapterSystemMarker;
impl<Func, S, I, O, M> IntoSystem<Func::In, Func::Out, (IsAdapterSystemMarker, I, O, M)>
for IntoAdapterSystem<Func, S>
where
Func: Adapt<S::System>,
Func: Adapt<S::System, In = I>,
I: SystemInput,
S: IntoSystem<I, O, M>,
{
Expand Down Expand Up @@ -118,7 +118,7 @@ where
impl<Func, S> System for AdapterSystem<Func, S>
where
Func: Adapt<S>,
S: System,
S: System<In = Func::In>,
{
type In = Func::In;
type Out = Func::Out;
Expand Down Expand Up @@ -179,9 +179,13 @@ where
}

#[inline]
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool {
unsafe fn validate_param_unsafe(
&mut self,
input: &SystemIn<'_, Self>,
world: UnsafeWorldCell,
) -> bool {
// SAFETY: Delegate to other `System` implementations.
unsafe { self.system.validate_param_unsafe(world) }
unsafe { self.system.validate_param_unsafe(input, world) }
}

fn initialize(&mut self, world: &mut crate::prelude::World) {
Expand Down Expand Up @@ -214,7 +218,7 @@ where
unsafe impl<Func, S> ReadOnlySystem for AdapterSystem<Func, S>
where
Func: Adapt<S>,
S: ReadOnlySystem,
S: ReadOnlySystem<In = Func::In>,
{
}

Expand Down
5 changes: 3 additions & 2 deletions crates/bevy_ecs/src/system/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use crate::{
prelude::QueryBuilder,
query::{QueryData, QueryFilter, QueryState},
system::{
DynSystemParam, DynSystemParamState, Local, ParamSet, Query, SystemMeta, SystemParam,
DynSystemParam, DynSystemParamState, Local, ParamSet, Query, SystemInput, SystemMeta,
SystemParam,
},
world::{
FilteredResources, FilteredResourcesBuilder, FilteredResourcesMut,
Expand Down Expand Up @@ -116,7 +117,7 @@ pub unsafe trait SystemParamBuilder<P: SystemParam>: Sized {

/// Create a [`SystemState`] from a [`SystemParamBuilder`].
/// To create a system, call [`SystemState::build_system`] on the result.
fn build_state(self, world: &mut World) -> SystemState<P> {
fn build_state<In: SystemInput>(self, world: &mut World) -> SystemState<P, In> {
SystemState::from_builder(world, self)
}
}
Expand Down
36 changes: 25 additions & 11 deletions crates/bevy_ecs/src/system/combinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ use super::{IntoSystem, ReadOnlySystem, System};
label = "invalid system combination",
note = "the inputs and outputs of `{A}` and `{B}` are not compatible with this combiner"
)]
pub trait Combine<A: System, B: System> {
pub trait Combine<A: System<In = Self::In>, B: System<In = Self::In>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this a breaking change? It will force the two systems to have the same input type, so e.g. piping would no longer be implementable this way (not that bevy's implementation used Combine).

/// The [input](System::In) type for a [`CombinatorSystem`].
type In: SystemInput;

Expand Down Expand Up @@ -137,8 +137,8 @@ impl<Func, A, B> CombinatorSystem<Func, A, B> {
impl<A, B, Func> System for CombinatorSystem<Func, A, B>
where
Func: Combine<A, B> + 'static,
A: System,
B: System,
A: System<In = Func::In>,
B: System<In = Func::In>,
{
type In = Func::In;
type Out = Func::Out;
Expand Down Expand Up @@ -212,9 +212,15 @@ where
}

#[inline]
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool {
unsafe fn validate_param_unsafe(
&mut self,
input: &SystemIn<'_, Self>,
world: UnsafeWorldCell,
) -> bool {
// SAFETY: Delegate to other `System` implementations.
unsafe { self.a.validate_param_unsafe(world) && self.b.validate_param_unsafe(world) }
unsafe {
self.a.validate_param_unsafe(input, world) && self.b.validate_param_unsafe(input, world)
}
}

fn initialize(&mut self, world: &mut World) {
Expand Down Expand Up @@ -259,8 +265,8 @@ where
unsafe impl<Func, A, B> ReadOnlySystem for CombinatorSystem<Func, A, B>
where
Func: Combine<A, B> + 'static,
A: ReadOnlySystem,
B: ReadOnlySystem,
A: ReadOnlySystem<In = Func::In>,
B: ReadOnlySystem<In = Func::In>,
{
}

Expand Down Expand Up @@ -431,13 +437,21 @@ where
self.b.queue_deferred(world);
}

unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool {
unsafe fn validate_param_unsafe(
&mut self,
input: &SystemIn<'_, Self>,
world: UnsafeWorldCell,
) -> bool {
// SAFETY: Delegate to other `System` implementations.
unsafe { self.a.validate_param_unsafe(world) && self.b.validate_param_unsafe(world) }
unsafe {
// TODO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO. Unsure how to handle validating the second system.

self.a.validate_param_unsafe(input, world) /* && self.b.validate_param_unsafe(input, world) */
}
}

fn validate_param(&mut self, world: &World) -> bool {
self.a.validate_param(world) && self.b.validate_param(world)
fn validate_param(&mut self, input: &SystemIn<'_, Self>, world: &World) -> bool {
// TODO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO. Unsure how to handle validating the second system.

self.a.validate_param(input, world) /* && self.b.validate_param(input, world) */
}

fn initialize(&mut self, world: &mut World) {
Expand Down
Loading
Loading