Skip to content

Commit c3329c2

Browse files
committed
update to new convention
1 parent 5289e18 commit c3329c2

File tree

11 files changed

+116
-48
lines changed

11 files changed

+116
-48
lines changed

crates/bevy_ecs/src/schedule/executor/mod.rs

-12
Original file line numberDiff line numberDiff line change
@@ -180,18 +180,6 @@ mod __rust_begin_short_backtrace {
180180
}
181181
}
182182

183-
#[macro_export]
184-
/// Emits a warning about system being skipped.
185-
macro_rules! warn_system_skipped {
186-
($ty:literal, $sys:expr) => {
187-
bevy_utils::tracing::warn!(
188-
"{} {} was skipped due to inaccessible system parameters.",
189-
$ty,
190-
$sys
191-
)
192-
};
193-
}
194-
195183
#[cfg(test)]
196184
mod tests {
197185
use crate::{

crates/bevy_ecs/src/schedule/executor/multi_threaded.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use crate::{
1919
query::Access,
2020
schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
2121
system::BoxedSystem,
22-
warn_system_skipped,
2322
world::{unsafe_world_cell::UnsafeWorldCell, World},
2423
};
2524

@@ -524,7 +523,7 @@ impl ExecutorState {
524523
unsafe fn should_run(
525524
&mut self,
526525
system_index: usize,
527-
system: &BoxedSystem,
526+
system: &mut BoxedSystem,
528527
conditions: &mut Conditions,
529528
world: UnsafeWorldCell,
530529
) -> bool {
@@ -575,7 +574,6 @@ impl ExecutorState {
575574
// - `update_archetype_component_access` has been called for system.
576575
let valid_params = unsafe { system.validate_param_unsafe(world) };
577576
if !valid_params {
578-
warn_system_skipped!("System", system.name());
579577
self.skipped_systems.insert(system_index);
580578
}
581579
should_run &= valid_params;
@@ -751,7 +749,6 @@ unsafe fn evaluate_and_fold_conditions(
751749
// required by the condition.
752750
// - `update_archetype_component_access` has been called for condition.
753751
if !unsafe { condition.validate_param_unsafe(world) } {
754-
warn_system_skipped!("Condition", condition.name());
755752
return false;
756753
}
757754
// SAFETY:

crates/bevy_ecs/src/schedule/executor/simple.rs

-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use crate::{
77
schedule::{
88
executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule,
99
},
10-
warn_system_skipped,
1110
world::World,
1211
};
1312

@@ -83,9 +82,6 @@ impl SystemExecutor for SimpleExecutor {
8382
let system = &mut schedule.systems[system_index];
8483
if should_run {
8584
let valid_params = system.validate_param(world);
86-
if !valid_params {
87-
warn_system_skipped!("System", system.name());
88-
}
8985
should_run &= valid_params;
9086
}
9187

@@ -139,7 +135,6 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W
139135
.iter_mut()
140136
.map(|condition| {
141137
if !condition.validate_param(world) {
142-
warn_system_skipped!("Condition", condition.name());
143138
return false;
144139
}
145140
__rust_begin_short_backtrace::readonly_run(&mut **condition, world)

crates/bevy_ecs/src/schedule/executor/single_threaded.rs

-5
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use fixedbitset::FixedBitSet;
55

66
use crate::{
77
schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
8-
warn_system_skipped,
98
world::World,
109
};
1110

@@ -89,9 +88,6 @@ impl SystemExecutor for SingleThreadedExecutor {
8988
let system = &mut schedule.systems[system_index];
9089
if should_run {
9190
let valid_params = system.validate_param(world);
92-
if !valid_params {
93-
warn_system_skipped!("System", system.name());
94-
}
9591
should_run &= valid_params;
9692
}
9793

@@ -171,7 +167,6 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W
171167
.iter_mut()
172168
.map(|condition| {
173169
if !condition.validate_param(world) {
174-
warn_system_skipped!("Condition", condition.name());
175170
return false;
176171
}
177172
__rust_begin_short_backtrace::readonly_run(&mut **condition, world)

crates/bevy_ecs/src/system/adapter_system.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ where
179179
}
180180

181181
#[inline]
182-
unsafe fn validate_param_unsafe(&self, world: UnsafeWorldCell) -> bool {
182+
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool {
183183
self.system.validate_param_unsafe(world)
184184
}
185185

crates/bevy_ecs/src/system/combinator.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ where
212212
}
213213

214214
#[inline]
215-
unsafe fn validate_param_unsafe(&self, world: UnsafeWorldCell) -> bool {
215+
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool {
216216
self.a.validate_param_unsafe(world) && self.b.validate_param_unsafe(world)
217217
}
218218

@@ -430,7 +430,7 @@ where
430430
self.b.queue_deferred(world);
431431
}
432432

433-
unsafe fn validate_param_unsafe(&self, world: UnsafeWorldCell) -> bool {
433+
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool {
434434
self.a.validate_param_unsafe(world) && self.b.validate_param_unsafe(world)
435435
}
436436

crates/bevy_ecs/src/system/exclusive_function_system.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ where
150150
}
151151

152152
#[inline]
153-
unsafe fn validate_param_unsafe(&self, _world: UnsafeWorldCell) -> bool {
153+
unsafe fn validate_param_unsafe(&mut self, _world: UnsafeWorldCell) -> bool {
154154
true
155155
}
156156

crates/bevy_ecs/src/system/function_system.rs

+70-2
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ pub struct SystemMeta {
4343
is_send: bool,
4444
has_deferred: bool,
4545
pub(crate) last_run: Tick,
46+
warn_policy: WarnPolicy,
4647
#[cfg(feature = "trace")]
4748
pub(crate) system_span: Span,
4849
#[cfg(feature = "trace")]
@@ -59,6 +60,7 @@ impl SystemMeta {
5960
is_send: true,
6061
has_deferred: false,
6162
last_run: Tick::new(0),
63+
warn_policy: WarnPolicy::Once,
6264
#[cfg(feature = "trace")]
6365
system_span: info_span!("system", name = name),
6466
#[cfg(feature = "trace")]
@@ -75,6 +77,7 @@ impl SystemMeta {
7577
/// Sets the name of of this system.
7678
///
7779
/// Useful to give closure systems more readable and unique names for debugging and tracing.
80+
#[inline]
7881
pub fn set_name(&mut self, new_name: impl Into<Cow<'static, str>>) {
7982
let new_name: Cow<'static, str> = new_name.into();
8083
#[cfg(feature = "trace")]
@@ -108,9 +111,70 @@ impl SystemMeta {
108111

109112
/// Marks the system as having deferred buffers like [`Commands`](`super::Commands`)
110113
/// This lets the scheduler insert [`apply_deferred`](`crate::prelude::apply_deferred`) systems automatically.
114+
#[inline]
111115
pub fn set_has_deferred(&mut self) {
112116
self.has_deferred = true;
113117
}
118+
119+
/// Changes the warn policy.
120+
#[inline]
121+
pub fn set_warn_policy(&mut self, warn_policy: WarnPolicy) {
122+
self.warn_policy = warn_policy;
123+
}
124+
125+
/// Advances the warn policy after validation failed.
126+
#[inline]
127+
pub fn advance_warn_policy(&mut self) {
128+
self.warn_policy.advance();
129+
}
130+
131+
/// Emits a warning about inaccessible system param if policy allows it.
132+
#[inline]
133+
pub fn try_warn<P>(&self)
134+
where
135+
P: SystemParam,
136+
{
137+
self.warn_policy.try_warn::<P>(&self.name);
138+
}
139+
}
140+
141+
/// State machine for emitting warnings when [system params are invalid](System::validate_param).
142+
#[derive(Clone, Copy)]
143+
pub enum WarnPolicy {
144+
/// No warning should ever be emitted.
145+
Never,
146+
/// The warning will be emitted once and status will update to [`Self::Never`].
147+
Once,
148+
/// The warning will be emitted every time.
149+
Always,
150+
}
151+
152+
impl WarnPolicy {
153+
/// Advances the warn policy after validation failed.
154+
#[inline]
155+
fn advance(&mut self) {
156+
*self = match self {
157+
Self::Never => Self::Never,
158+
Self::Once => Self::Never,
159+
Self::Always => Self::Always,
160+
};
161+
}
162+
163+
/// Emits a warning about inaccessible system param if policy allows it.
164+
#[inline]
165+
fn try_warn<P>(&self, name: &str)
166+
where
167+
P: SystemParam,
168+
{
169+
if matches!(self, Self::Never) {
170+
return;
171+
}
172+
bevy_utils::tracing::warn!(
173+
"System {0} will not run because it requested inaccessible system parameter {1}",
174+
name,
175+
std::any::type_name::<P>()
176+
);
177+
}
114178
}
115179

116180
// TODO: Actually use this in FunctionSystem. We should probably only do this once Systems are constructed using a World reference
@@ -657,9 +721,13 @@ where
657721
}
658722

659723
#[inline]
660-
unsafe fn validate_param_unsafe(&self, world: UnsafeWorldCell) -> bool {
724+
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool {
661725
let param_state = self.param_state.as_ref().expect(Self::PARAM_MESSAGE);
662-
F::Param::validate_param(param_state, &self.system_meta, world)
726+
let is_valid = unsafe { F::Param::validate_param(param_state, &self.system_meta, world) };
727+
if !is_valid {
728+
self.system_meta.advance_warn_policy();
729+
}
730+
is_valid
663731
}
664732

665733
#[inline]

crates/bevy_ecs/src/system/system.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ pub trait System: Send + Sync + 'static {
117117
/// - The method [`System::update_archetype_component_access`] must be called at some
118118
/// point before this one, with the same exact [`World`]. If [`System::update_archetype_component_access`]
119119
/// panics (or otherwise does not return for any reason), this method must not be called.
120-
unsafe fn validate_param_unsafe(&self, world: UnsafeWorldCell) -> bool;
120+
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool;
121121

122122
/// Safe version of [`System::validate_param_unsafe`].
123123
/// that runs on exclusive, single-threaded `world` pointer.

crates/bevy_ecs/src/system/system_param.rs

+38-14
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,11 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam fo
419419
world.change_tick(),
420420
)
421421
};
422-
result.is_ok()
422+
let is_valid = result.is_ok();
423+
if !is_valid {
424+
system_meta.try_warn::<Self>();
425+
}
426+
is_valid
423427
}
424428
}
425429

@@ -481,7 +485,11 @@ unsafe impl<'a, D: QueryData + 'static, F: QueryFilter + 'static> SystemParam
481485
world.change_tick(),
482486
)
483487
};
484-
!matches!(result, Err(QuerySingleError::MultipleEntities(_)))
488+
let is_valid = !matches!(result, Err(QuerySingleError::MultipleEntities(_)));
489+
if !is_valid {
490+
system_meta.try_warn::<Self>();
491+
}
492+
is_valid
485493
}
486494
}
487495

@@ -756,14 +764,18 @@ unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> {
756764
#[inline]
757765
unsafe fn validate_param(
758766
&component_id: &Self::State,
759-
_system_meta: &SystemMeta,
767+
system_meta: &SystemMeta,
760768
world: UnsafeWorldCell,
761769
) -> bool {
762770
// SAFETY: Read-only access to resource metadata.
763-
unsafe { world.storages() }
771+
let is_valid = unsafe { world.storages() }
764772
.resources
765773
.get(component_id)
766-
.is_some_and(ResourceData::is_present)
774+
.is_some_and(ResourceData::is_present);
775+
if !is_valid {
776+
system_meta.try_warn::<Self>();
777+
}
778+
is_valid
767779
}
768780

769781
#[inline]
@@ -866,14 +878,18 @@ unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
866878
#[inline]
867879
unsafe fn validate_param(
868880
&component_id: &Self::State,
869-
_system_meta: &SystemMeta,
881+
system_meta: &SystemMeta,
870882
world: UnsafeWorldCell,
871883
) -> bool {
872884
// SAFETY: Read-only access to resource metadata.
873-
unsafe { world.storages() }
885+
let is_valid = unsafe { world.storages() }
874886
.resources
875887
.get(component_id)
876-
.is_some_and(ResourceData::is_present)
888+
.is_some_and(ResourceData::is_present);
889+
if !is_valid {
890+
system_meta.try_warn::<Self>();
891+
}
892+
is_valid
877893
}
878894

879895
#[inline]
@@ -1412,14 +1428,18 @@ unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> {
14121428
#[inline]
14131429
unsafe fn validate_param(
14141430
&component_id: &Self::State,
1415-
_system_meta: &SystemMeta,
1431+
system_meta: &SystemMeta,
14161432
world: UnsafeWorldCell,
14171433
) -> bool {
14181434
// SAFETY: Read-only access to resource metadata.
1419-
unsafe { world.storages() }
1435+
let is_valid = unsafe { world.storages() }
14201436
.non_send_resources
14211437
.get(component_id)
1422-
.is_some_and(ResourceData::is_present)
1438+
.is_some_and(ResourceData::is_present);
1439+
if !is_valid {
1440+
system_meta.try_warn::<Self>();
1441+
}
1442+
is_valid
14231443
}
14241444

14251445
#[inline]
@@ -1519,14 +1539,18 @@ unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> {
15191539
#[inline]
15201540
unsafe fn validate_param(
15211541
&component_id: &Self::State,
1522-
_system_meta: &SystemMeta,
1542+
system_meta: &SystemMeta,
15231543
world: UnsafeWorldCell,
15241544
) -> bool {
15251545
// SAFETY: Read-only access to resource metadata.
1526-
unsafe { world.storages() }
1546+
let is_valid = unsafe { world.storages() }
15271547
.non_send_resources
15281548
.get(component_id)
1529-
.is_some_and(ResourceData::is_present)
1549+
.is_some_and(ResourceData::is_present);
1550+
if !is_valid {
1551+
system_meta.try_warn::<Self>();
1552+
}
1553+
is_valid
15301554
}
15311555

15321556
#[inline]

crates/bevy_render/src/extract_param.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,13 @@ where
7777
#[inline]
7878
unsafe fn validate_param(
7979
state: &Self::State,
80-
_system_meta: &SystemMeta,
80+
system_meta: &SystemMeta,
8181
world: UnsafeWorldCell,
8282
) -> bool {
8383
// SAFETY: Read-only access to world data registered in `init_state`.
8484
let result = unsafe { world.get_resource_by_id(state.main_world_state) };
8585
let Some(main_world) = result else {
86+
system_meta.try_warn::<&World>();
8687
return false;
8788
};
8889
// SAFETY: Type is guaranteed by `SystemState`.

0 commit comments

Comments
 (0)