From 0bd22bbd20d671ae7917fecfab284493dacb8c7a Mon Sep 17 00:00:00 2001 From: MiniaczQ Date: Sat, 28 Sep 2024 18:17:52 +0200 Subject: [PATCH] init --- crates/bevy_ecs/src/schedule/executor/mod.rs | 31 +++++++++--- .../src/schedule/executor/multi_threaded.rs | 16 ++++-- .../bevy_ecs/src/schedule/executor/simple.rs | 11 +++-- .../src/schedule/executor/single_threaded.rs | 11 +++-- crates/bevy_ecs/src/system/adapter_system.rs | 5 ++ crates/bevy_ecs/src/system/combinator.rs | 14 ++++++ .../src/system/exclusive_function_system.rs | 5 ++ crates/bevy_ecs/src/system/function_system.rs | 49 +++++++++++++++++++ crates/bevy_ecs/src/system/system.rs | 5 ++ 9 files changed, 126 insertions(+), 21 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 24113aee2bee0..8b4e68dd7d787 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -180,16 +180,31 @@ mod __rust_begin_short_backtrace { } } -#[macro_export] -/// Emits a warning about system being skipped. -macro_rules! warn_system_skipped { - ($ty:literal, $sys:expr) => { +/// Warnings for systems skipped due to invalid system params. +pub mod validity_warning { + /// Warning for systems. + pub fn system(name: &str) { bevy_utils::tracing::warn!( - "{} {} was skipped due to inaccessible system parameters.", - $ty, - $sys + "System was skipped due to inaccessible system parameters: {}", + name ) - }; + } + + /// Warning for system conditions. + pub fn condition(name: &str) { + bevy_utils::tracing::warn!( + "Condition was skipped due to inaccessible system parameters: {}", + name + ) + } + + /// Warning for observers. + pub fn observer(name: &str) { + bevy_utils::tracing::warn!( + "Observer was skipped due to inaccessible system parameters: {}", + name + ) + } } #[cfg(test)] diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index 8782793e32f71..8c59207817909 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -17,9 +17,11 @@ use crate::{ archetype::ArchetypeComponentId, prelude::Resource, query::Access, - schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, + schedule::{ + is_apply_deferred, validity_warning, BoxedCondition, ExecutorKind, SystemExecutor, + SystemSchedule, + }, system::BoxedSystem, - warn_system_skipped, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; @@ -524,7 +526,7 @@ impl ExecutorState { unsafe fn should_run( &mut self, system_index: usize, - system: &BoxedSystem, + system: &mut BoxedSystem, conditions: &mut Conditions, world: UnsafeWorldCell, ) -> bool { @@ -575,7 +577,9 @@ impl ExecutorState { // - `update_archetype_component_access` has been called for system. let valid_params = unsafe { system.validate_param_unsafe(world) }; if !valid_params { - warn_system_skipped!("System", system.name()); + if system.update_validity_warning() { + validity_warning::system(&system.name()); + } self.skipped_systems.insert(system_index); } should_run &= valid_params; @@ -751,7 +755,9 @@ unsafe fn evaluate_and_fold_conditions( // required by the condition. // - `update_archetype_component_access` has been called for condition. if !unsafe { condition.validate_param_unsafe(world) } { - warn_system_skipped!("Condition", condition.name()); + if condition.update_validity_warning() { + validity_warning::condition(&condition.name()); + } return false; } // SAFETY: diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index 171125342cd78..918ae4b252058 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -7,11 +7,10 @@ use crate::{ schedule::{ executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule, }, - warn_system_skipped, world::World, }; -use super::__rust_begin_short_backtrace; +use super::{__rust_begin_short_backtrace, validity_warning}; /// A variant of [`SingleThreadedExecutor`](crate::schedule::SingleThreadedExecutor) that calls /// [`apply_deferred`](crate::system::System::apply_deferred) immediately after running each system. @@ -84,7 +83,9 @@ impl SystemExecutor for SimpleExecutor { if should_run { let valid_params = system.validate_param(world); if !valid_params { - warn_system_skipped!("System", system.name()); + if system.update_validity_warning() { + validity_warning::system(&system.name()); + } } should_run &= valid_params; } @@ -139,7 +140,9 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W .iter_mut() .map(|condition| { if !condition.validate_param(world) { - warn_system_skipped!("Condition", condition.name()); + if condition.update_validity_warning() { + validity_warning::condition(&condition.name()); + } return false; } __rust_begin_short_backtrace::readonly_run(&mut **condition, world) diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index 93d814d3b2f83..ed6ec13b83314 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -5,11 +5,10 @@ use fixedbitset::FixedBitSet; use crate::{ schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, - warn_system_skipped, world::World, }; -use super::__rust_begin_short_backtrace; +use super::{__rust_begin_short_backtrace, validity_warning}; /// Runs the schedule using a single thread. /// @@ -90,7 +89,9 @@ impl SystemExecutor for SingleThreadedExecutor { if should_run { let valid_params = system.validate_param(world); if !valid_params { - warn_system_skipped!("System", system.name()); + if system.update_validity_warning() { + validity_warning::system(&system.name()); + } } should_run &= valid_params; } @@ -171,7 +172,9 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W .iter_mut() .map(|condition| { if !condition.validate_param(world) { - warn_system_skipped!("Condition", condition.name()); + if condition.update_validity_warning() { + validity_warning::condition(&condition.name()); + } return false; } __rust_begin_short_backtrace::readonly_run(&mut **condition, world) diff --git a/crates/bevy_ecs/src/system/adapter_system.rs b/crates/bevy_ecs/src/system/adapter_system.rs index 8c25e57bbd91e..3e12ea29a5a7b 100644 --- a/crates/bevy_ecs/src/system/adapter_system.rs +++ b/crates/bevy_ecs/src/system/adapter_system.rs @@ -183,6 +183,11 @@ where self.system.validate_param_unsafe(world) } + #[inline] + fn update_validity_warning(&mut self) -> bool { + self.system.update_validity_warning() + } + fn initialize(&mut self, world: &mut crate::prelude::World) { self.system.initialize(world); } diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index ce4c8785feb05..78d96ed97181d 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -216,6 +216,13 @@ where self.a.validate_param_unsafe(world) && self.b.validate_param_unsafe(world) } + #[inline] + fn update_validity_warning(&mut self) -> bool { + // Emit a warning if any of the two systems want it. + // No short-circuiting so all warning states get updated. + self.a.update_validity_warning() | self.b.update_validity_warning() + } + fn initialize(&mut self, world: &mut World) { self.a.initialize(world); self.b.initialize(world); @@ -438,6 +445,13 @@ where self.a.validate_param(world) && self.b.validate_param(world) } + #[inline] + fn update_validity_warning(&mut self) -> bool { + // Emit a warning if any of the two systems want it. + // No short-circuiting so all warning states get updated. + self.a.update_validity_warning() | self.b.update_validity_warning() + } + fn initialize(&mut self, world: &mut World) { self.a.initialize(world); self.b.initialize(world); diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index 3075f3c55772e..1e58303912acf 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -154,6 +154,11 @@ where true } + #[inline] + fn update_validity_warning(&mut self) -> bool { + self.system_meta.update_validity_warning() + } + #[inline] fn initialize(&mut self, world: &mut World) { self.system_meta.last_run = world.change_tick().relative_to(Tick::MAX); diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 0614d7339ca4b..aa1edb3afb1dd 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -43,6 +43,7 @@ pub struct SystemMeta { is_send: bool, has_deferred: bool, pub(crate) last_run: Tick, + validity_warning: ValidityWarning, #[cfg(feature = "trace")] pub(crate) system_span: Span, #[cfg(feature = "trace")] @@ -59,6 +60,7 @@ impl SystemMeta { is_send: true, has_deferred: false, last_run: Tick::new(0), + validity_warning: ValidityWarning::Once, #[cfg(feature = "trace")] system_span: info_span!("system", name = name), #[cfg(feature = "trace")] @@ -75,6 +77,7 @@ impl SystemMeta { /// Sets the name of of this system. /// /// Useful to give closure systems more readable and unique names for debugging and tracing. + #[inline] pub fn set_name(&mut self, new_name: impl Into>) { let new_name: Cow<'static, str> = new_name.into(); #[cfg(feature = "trace")] @@ -108,9 +111,50 @@ impl SystemMeta { /// Marks the system as having deferred buffers like [`Commands`](`super::Commands`) /// This lets the scheduler insert [`apply_deferred`](`crate::prelude::apply_deferred`) systems automatically. + #[inline] pub fn set_has_deferred(&mut self) { self.has_deferred = true; } + + /// Call after [`System::validate_param`] fails. + /// Updates the validity warning. + /// The return value tells whether a warning should be emitted or not. + #[inline] + pub(crate) fn update_validity_warning(&mut self) -> bool { + self.validity_warning.update() + } + + /// Configures the validity warning. + #[inline] + pub fn set_validity_warning(&mut self, validity_warning: ValidityWarning) { + self.validity_warning = validity_warning; + } +} + +/// State machine for emitting warnings when [system params are invalid](System::validate_param). +#[derive(Clone, Copy)] +pub enum ValidityWarning { + /// No warning should ever be emitted. + Never, + /// The warning will be emitted once and status will update to [`Self::Never`]. + Once, + /// The warning will be emitted every time. + Always, +} + +impl ValidityWarning { + /// Updates the validity warning. + /// The return value tells whether a warning should be emitted or not. + #[inline] + pub fn update(&mut self) -> bool { + let (next, should_warn) = match self { + Self::Never => (Self::Never, false), + Self::Once => (Self::Never, true), + Self::Always => (Self::Always, true), + }; + *self = next; + should_warn + } } // TODO: Actually use this in FunctionSystem. We should probably only do this once Systems are constructed using a World reference @@ -662,6 +706,11 @@ where F::Param::validate_param(param_state, &self.system_meta, world) } + #[inline] + fn update_validity_warning(&mut self) -> bool { + self.system_meta.update_validity_warning() + } + #[inline] fn initialize(&mut self, world: &mut World) { if let Some(id) = self.world_id { diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 7741f3cb80746..ce498ba57f65d 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -129,6 +129,11 @@ pub trait System: Send + Sync + 'static { unsafe { self.validate_param_unsafe(world_cell) } } + /// Call after [`System::validate_param`] fails. + /// Updates the validity warning. + /// The return value tells whether a warning should be emitted or not. + fn update_validity_warning(&mut self) -> bool; + /// Initialize the system. fn initialize(&mut self, _world: &mut World);