Skip to content

Commit 765e584

Browse files
chescockmockersf
chescock
authored and
mockersf
committed
Replace ValidationOutcome with Result (#18541)
# Objective Make it easier to short-circuit system parameter validation. Simplify the API surface by combining `ValidationOutcome` with `SystemParamValidationError`. ## Solution Replace `ValidationOutcome` with `Result<(), SystemParamValidationError>`. Move the docs from `ValidationOutcome` to `SystemParamValidationError`. Add a `skipped` field to `SystemParamValidationError` to distinguish the `Skipped` and `Invalid` variants. Use the `?` operator to short-circuit validation in tuples of system params.
1 parent 9705eae commit 765e584

20 files changed

+220
-213
lines changed

‎crates/bevy_ecs/macros/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
429429
state: &'s Self::State,
430430
system_meta: &#path::system::SystemMeta,
431431
world: #path::world::unsafe_world_cell::UnsafeWorldCell<'w>,
432-
) -> #path::system::ValidationOutcome {
432+
) -> Result<(), #path::system::SystemParamValidationError> {
433433
<(#(#tuple_types,)*) as #path::system::SystemParam>::validate_param(&state.state, system_meta, world)
434434
}
435435

‎crates/bevy_ecs/src/observer/runner.rs

+13-10
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
observer::{ObserverDescriptor, ObserverTrigger},
88
prelude::*,
99
query::DebugCheckedUnwrap,
10-
system::{IntoObserverSystem, ObserverSystem, SystemParamValidationError, ValidationOutcome},
10+
system::{IntoObserverSystem, ObserverSystem},
1111
world::DeferredWorld,
1212
};
1313
use bevy_ptr::PtrMut;
@@ -406,7 +406,7 @@ fn observer_system_runner<E: Event, B: Bundle, S: ObserverSystem<E, B>>(
406406
unsafe {
407407
(*system).update_archetype_component_access(world);
408408
match (*system).validate_param_unsafe(world) {
409-
ValidationOutcome::Valid => {
409+
Ok(()) => {
410410
if let Err(err) = (*system).run_unsafe(trigger, world) {
411411
error_handler(
412412
err,
@@ -418,14 +418,17 @@ fn observer_system_runner<E: Event, B: Bundle, S: ObserverSystem<E, B>>(
418418
};
419419
(*system).queue_deferred(world.into_deferred());
420420
}
421-
ValidationOutcome::Invalid => error_handler(
422-
SystemParamValidationError.into(),
423-
ErrorContext::Observer {
424-
name: (*system).name(),
425-
last_run: (*system).get_last_run(),
426-
},
427-
),
428-
ValidationOutcome::Skipped => (),
421+
Err(e) => {
422+
if !e.skipped {
423+
error_handler(
424+
e.into(),
425+
ErrorContext::Observer {
426+
name: (*system).name(),
427+
last_run: (*system).get_last_run(),
428+
},
429+
);
430+
}
431+
}
429432
}
430433
}
431434
}

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use crate::{
2020
prelude::{IntoSystemSet, SystemSet},
2121
query::Access,
2222
schedule::{BoxedCondition, InternedSystemSet, NodeId, SystemTypeSet},
23-
system::{ScheduleSystem, System, SystemIn, ValidationOutcome},
23+
system::{ScheduleSystem, System, SystemIn, SystemParamValidationError},
2424
world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World},
2525
};
2626

@@ -221,10 +221,13 @@ impl System for ApplyDeferred {
221221

222222
fn queue_deferred(&mut self, _world: DeferredWorld) {}
223223

224-
unsafe fn validate_param_unsafe(&mut self, _world: UnsafeWorldCell) -> ValidationOutcome {
224+
unsafe fn validate_param_unsafe(
225+
&mut self,
226+
_world: UnsafeWorldCell,
227+
) -> Result<(), SystemParamValidationError> {
225228
// This system is always valid to run because it doesn't do anything,
226229
// and only used as a marker for the executor.
227-
ValidationOutcome::Valid
230+
Ok(())
228231
}
229232

230233
fn initialize(&mut self, _world: &mut World) {}

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

+23-21
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use crate::{
1818
prelude::Resource,
1919
query::Access,
2020
schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
21-
system::{ScheduleSystem, SystemParamValidationError, ValidationOutcome},
21+
system::ScheduleSystem,
2222
world::{unsafe_world_cell::UnsafeWorldCell, World},
2323
};
2424

@@ -582,18 +582,19 @@ impl ExecutorState {
582582
// required by the system.
583583
// - `update_archetype_component_access` has been called for system.
584584
let valid_params = match unsafe { system.validate_param_unsafe(world) } {
585-
ValidationOutcome::Valid => true,
586-
ValidationOutcome::Invalid => {
587-
error_handler(
588-
SystemParamValidationError.into(),
589-
ErrorContext::System {
590-
name: system.name(),
591-
last_run: system.get_last_run(),
592-
},
593-
);
585+
Ok(()) => true,
586+
Err(e) => {
587+
if !e.skipped {
588+
error_handler(
589+
e.into(),
590+
ErrorContext::System {
591+
name: system.name(),
592+
last_run: system.get_last_run(),
593+
},
594+
);
595+
}
594596
false
595597
}
596-
ValidationOutcome::Skipped => false,
597598
};
598599
if !valid_params {
599600
self.skipped_systems.insert(system_index);
@@ -796,18 +797,19 @@ unsafe fn evaluate_and_fold_conditions(
796797
// required by the condition.
797798
// - `update_archetype_component_access` has been called for condition.
798799
match unsafe { condition.validate_param_unsafe(world) } {
799-
ValidationOutcome::Valid => (),
800-
ValidationOutcome::Invalid => {
801-
error_handler(
802-
SystemParamValidationError.into(),
803-
ErrorContext::System {
804-
name: condition.name(),
805-
last_run: condition.get_last_run(),
806-
},
807-
);
800+
Ok(()) => (),
801+
Err(e) => {
802+
if !e.skipped {
803+
error_handler(
804+
e.into(),
805+
ErrorContext::System {
806+
name: condition.name(),
807+
last_run: condition.get_last_run(),
808+
},
809+
);
810+
}
808811
return false;
809812
}
810-
ValidationOutcome::Skipped => return false,
811813
}
812814
// SAFETY:
813815
// - The caller ensures that `world` has permission to read any data

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

+22-21
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use crate::{
1212
schedule::{
1313
executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule,
1414
},
15-
system::{SystemParamValidationError, ValidationOutcome},
1615
world::World,
1716
};
1817

@@ -89,18 +88,19 @@ impl SystemExecutor for SimpleExecutor {
8988
let system = &mut schedule.systems[system_index];
9089
if should_run {
9190
let valid_params = match system.validate_param(world) {
92-
ValidationOutcome::Valid => true,
93-
ValidationOutcome::Invalid => {
94-
error_handler(
95-
SystemParamValidationError.into(),
96-
ErrorContext::System {
97-
name: system.name(),
98-
last_run: system.get_last_run(),
99-
},
100-
);
91+
Ok(()) => true,
92+
Err(e) => {
93+
if !e.skipped {
94+
error_handler(
95+
e.into(),
96+
ErrorContext::System {
97+
name: system.name(),
98+
last_run: system.get_last_run(),
99+
},
100+
);
101+
}
101102
false
102103
}
103-
ValidationOutcome::Skipped => false,
104104
};
105105
should_run &= valid_params;
106106
}
@@ -177,18 +177,19 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W
177177
.iter_mut()
178178
.map(|condition| {
179179
match condition.validate_param(world) {
180-
ValidationOutcome::Valid => (),
181-
ValidationOutcome::Invalid => {
182-
error_handler(
183-
SystemParamValidationError.into(),
184-
ErrorContext::System {
185-
name: condition.name(),
186-
last_run: condition.get_last_run(),
187-
},
188-
);
180+
Ok(()) => (),
181+
Err(e) => {
182+
if !e.skipped {
183+
error_handler(
184+
e.into(),
185+
ErrorContext::System {
186+
name: condition.name(),
187+
last_run: condition.get_last_run(),
188+
},
189+
);
190+
}
189191
return false;
190192
}
191-
ValidationOutcome::Skipped => return false,
192193
}
193194
__rust_begin_short_backtrace::readonly_run(&mut **condition, world)
194195
})

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

+22-21
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use std::eprintln;
1010
use crate::{
1111
error::{default_error_handler, BevyError, ErrorContext},
1212
schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
13-
system::{SystemParamValidationError, ValidationOutcome},
1413
world::World,
1514
};
1615

@@ -95,18 +94,19 @@ impl SystemExecutor for SingleThreadedExecutor {
9594
let system = &mut schedule.systems[system_index];
9695
if should_run {
9796
let valid_params = match system.validate_param(world) {
98-
ValidationOutcome::Valid => true,
99-
ValidationOutcome::Invalid => {
100-
error_handler(
101-
SystemParamValidationError.into(),
102-
ErrorContext::System {
103-
name: system.name(),
104-
last_run: system.get_last_run(),
105-
},
106-
);
97+
Ok(()) => true,
98+
Err(e) => {
99+
if !e.skipped {
100+
error_handler(
101+
e.into(),
102+
ErrorContext::System {
103+
name: system.name(),
104+
last_run: system.get_last_run(),
105+
},
106+
);
107+
}
107108
false
108109
}
109-
ValidationOutcome::Skipped => false,
110110
};
111111

112112
should_run &= valid_params;
@@ -221,18 +221,19 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut W
221221
.iter_mut()
222222
.map(|condition| {
223223
match condition.validate_param(world) {
224-
ValidationOutcome::Valid => (),
225-
ValidationOutcome::Invalid => {
226-
error_handler(
227-
SystemParamValidationError.into(),
228-
ErrorContext::System {
229-
name: condition.name(),
230-
last_run: condition.get_last_run(),
231-
},
232-
);
224+
Ok(()) => (),
225+
Err(e) => {
226+
if !e.skipped {
227+
error_handler(
228+
e.into(),
229+
ErrorContext::System {
230+
name: condition.name(),
231+
last_run: condition.get_last_run(),
232+
},
233+
);
234+
}
233235
return false;
234236
}
235-
ValidationOutcome::Skipped => return false,
236237
}
237238
__rust_begin_short_backtrace::readonly_run(&mut **condition, world)
238239
})

‎crates/bevy_ecs/src/system/adapter_system.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use alloc::{borrow::Cow, vec::Vec};
22

3-
use super::{IntoSystem, ReadOnlySystem, System, ValidationOutcome};
3+
use super::{IntoSystem, ReadOnlySystem, System, SystemParamValidationError};
44
use crate::{
55
schedule::InternedSystemSet,
66
system::{input::SystemInput, SystemIn},
@@ -179,7 +179,10 @@ where
179179
}
180180

181181
#[inline]
182-
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> ValidationOutcome {
182+
unsafe fn validate_param_unsafe(
183+
&mut self,
184+
world: UnsafeWorldCell,
185+
) -> Result<(), SystemParamValidationError> {
183186
// SAFETY: Delegate to other `System` implementations.
184187
unsafe { self.system.validate_param_unsafe(world) }
185188
}

‎crates/bevy_ecs/src/system/combinator.rs

+13-10
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::{
77
prelude::World,
88
query::Access,
99
schedule::InternedSystemSet,
10-
system::{input::SystemInput, SystemIn, ValidationOutcome},
10+
system::{input::SystemInput, SystemIn, SystemParamValidationError},
1111
world::unsafe_world_cell::UnsafeWorldCell,
1212
};
1313

@@ -212,7 +212,10 @@ where
212212
}
213213

214214
#[inline]
215-
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> ValidationOutcome {
215+
unsafe fn validate_param_unsafe(
216+
&mut self,
217+
world: UnsafeWorldCell,
218+
) -> Result<(), SystemParamValidationError> {
216219
// SAFETY: Delegate to other `System` implementations.
217220
unsafe { self.a.validate_param_unsafe(world) }
218221
}
@@ -431,18 +434,18 @@ where
431434
self.b.queue_deferred(world);
432435
}
433436

434-
unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> ValidationOutcome {
437+
unsafe fn validate_param_unsafe(
438+
&mut self,
439+
world: UnsafeWorldCell,
440+
) -> Result<(), SystemParamValidationError> {
435441
// SAFETY: Delegate to other `System` implementations.
436442
unsafe { self.a.validate_param_unsafe(world) }
437443
}
438444

439-
fn validate_param(&mut self, world: &World) -> ValidationOutcome {
440-
// This follows the logic of `ValidationOutcome::combine`, but short-circuits
441-
let validate_a = self.a.validate_param(world);
442-
match validate_a {
443-
ValidationOutcome::Valid => self.b.validate_param(world),
444-
ValidationOutcome::Invalid | ValidationOutcome::Skipped => validate_a,
445-
}
445+
fn validate_param(&mut self, world: &World) -> Result<(), SystemParamValidationError> {
446+
self.a.validate_param(world)?;
447+
self.b.validate_param(world)?;
448+
Ok(())
446449
}
447450

448451
fn initialize(&mut self, world: &mut World) {

‎crates/bevy_ecs/src/system/commands/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use crate::{
2727
schedule::ScheduleLabel,
2828
system::{
2929
Deferred, IntoObserverSystem, IntoSystem, RegisteredSystem, SystemId, SystemInput,
30-
ValidationOutcome,
30+
SystemParamValidationError,
3131
},
3232
world::{
3333
command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, CommandQueue,
@@ -182,7 +182,7 @@ const _: () = {
182182
state: &Self::State,
183183
system_meta: &bevy_ecs::system::SystemMeta,
184184
world: UnsafeWorldCell,
185-
) -> ValidationOutcome {
185+
) -> Result<(), SystemParamValidationError> {
186186
<(Deferred<CommandQueue>, &Entities) as bevy_ecs::system::SystemParam>::validate_param(
187187
&state.state,
188188
system_meta,

‎crates/bevy_ecs/src/system/exclusive_function_system.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use alloc::{borrow::Cow, vec, vec::Vec};
1414
use core::marker::PhantomData;
1515
use variadics_please::all_tuples;
1616

17-
use super::ValidationOutcome;
17+
use super::SystemParamValidationError;
1818

1919
/// A function system that runs with exclusive [`World`] access.
2020
///
@@ -156,9 +156,12 @@ where
156156
}
157157

158158
#[inline]
159-
unsafe fn validate_param_unsafe(&mut self, _world: UnsafeWorldCell) -> ValidationOutcome {
159+
unsafe fn validate_param_unsafe(
160+
&mut self,
161+
_world: UnsafeWorldCell,
162+
) -> Result<(), SystemParamValidationError> {
160163
// All exclusive system params are always available.
161-
ValidationOutcome::Valid
164+
Ok(())
162165
}
163166

164167
#[inline]

0 commit comments

Comments
 (0)