Skip to content

Have System::run_unsafe return Result. #19145

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.

Sign up for GitHub

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

Merged
merged 21 commits into from
Jul 3, 2025
Merged

Have System::run_unsafe return Result. #19145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8ba30ec
Have `System::run_unsafe` return `Result`.
chescock Apr 23, 2025
a9c6c84
Error message may include backtrace.
chescock May 9, 2025
f4960bd
Add missing semicolons.
chescock May 9, 2025
c2b009b
Fenced code blocks should have a language specified.
chescock May 9, 2025
8c8c67e
Update PR number in migration guide.
chescock May 9, 2025
809514c
Error message may include backtrace.
chescock May 9, 2025
632e3d6
Unused import.
chescock May 9, 2025
d3e72ca
Merge remote-tracking branch 'remotes/origin/main' into result-pipe
chescock May 20, 2025
ba48caa
Merge remote-tracking branch 'remotes/origin/main' into result-pipe
chescock May 30, 2025
80546b2
Merge remote-tracking branch 'remotes/origin/main' into result-pipe
chescock Jun 6, 2025
26c723f
Replace `FallibleFunctionSystem` wrapper with `IntoResult` converter.
chescock Jun 9, 2025
f0db7b4
Merge remote-tracking branch 'remotes/origin/main' into result-pipe
chescock Jun 23, 2025
ad6be24
Revert "Implement SystemCondition for systems returning Result<bool, …
chescock Jun 23, 2025
b006848
Rework unit tests for fallible conditions to match new semantics.
chescock Jun 23, 2025
202efa1
Revert "Support fallible one-shot systems (#19678)"
chescock Jun 23, 2025
f03e910
Add test that fallible commands report errors.
chescock Jun 23, 2025
1de29ca
Merge remote-tracking branch 'remotes/origin/main' into result-pipe
chescock Jun 23, 2025
cdbd498
Mention changes to `run_system_cached` inference in migration guide.
chescock Jun 25, 2025
a30cc18
Merge remote-tracking branch 'remotes/origin/main' into result-pipe
chescock Jun 30, 2025
e78e0cd
Fix new lints.
chescock Jun 30, 2025
9c591d8
Merge remote-tracking branch 'remotes/origin/main' into result-pipe
chescock Jul 3, 2025
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
2 changes: 1 addition & 1 deletion benches/benches/bevy_ecs/iteration/iter_simple_system.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ impl Benchmark {

#[inline(never)]
pub fn run(&mut self) {
self.1.run((), &mut self.0);
self.1.run((), &mut self.0).unwrap();
}
}
20 changes: 10 additions & 10 deletions crates/bevy_ecs/src/change_detection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,10 @@ pub trait DetectChangesMut: DetectChanges {
/// #
/// # // first time `reset_score` runs, the score is changed.
/// # schedule.run(&mut world);
/// # assert!(score_changed.run((), &mut world));
/// # assert!(score_changed.run((), &mut world).unwrap());
/// # // second time `reset_score` runs, the score is not changed.
/// # schedule.run(&mut world);
/// # assert!(!score_changed.run((), &mut world));
/// # assert!(!score_changed.run((), &mut world).unwrap());
/// ```
#[inline]
#[track_caller]
Expand Down Expand Up @@ -263,12 +263,12 @@ pub trait DetectChangesMut: DetectChanges {
/// #
/// # // first time `reset_score` runs, the score is changed.
/// # schedule.run(&mut world);
/// # assert!(score_changed.run((), &mut world));
/// # assert!(score_changed_event.run((), &mut world));
/// # assert!(score_changed.run((), &mut world).unwrap());
/// # assert!(score_changed_event.run((), &mut world).unwrap());
/// # // second time `reset_score` runs, the score is not changed.
/// # schedule.run(&mut world);
/// # assert!(!score_changed.run((), &mut world));
/// # assert!(!score_changed_event.run((), &mut world));
/// # assert!(!score_changed.run((), &mut world).unwrap());
/// # assert!(!score_changed_event.run((), &mut world).unwrap());
/// ```
#[inline]
#[must_use = "If you don't need to handle the previous value, use `set_if_neq` instead."]
Expand Down Expand Up @@ -315,10 +315,10 @@ pub trait DetectChangesMut: DetectChanges {
/// #
/// # // first time `reset_score` runs, the score is changed.
/// # schedule.run(&mut world);
/// # assert!(message_changed.run((), &mut world));
/// # assert!(message_changed.run((), &mut world).unwrap());
/// # // second time `reset_score` runs, the score is not changed.
/// # schedule.run(&mut world);
/// # assert!(!message_changed.run((), &mut world));
/// # assert!(!message_changed.run((), &mut world).unwrap());
/// ```
fn clone_from_if_neq<T>(&mut self, value: &T) -> bool
where
Expand Down Expand Up @@ -1566,7 +1566,7 @@ mod tests {

// world: 1, system last ran: 0, component changed: 1
// The spawn will be detected since it happened after the system "last ran".
assert!(change_detected_system.run((), &mut world));
assert!(change_detected_system.run((), &mut world).unwrap());

// world: 1 + MAX_CHANGE_AGE
let change_tick = world.change_tick.get_mut();
Expand All @@ -1576,7 +1576,7 @@ mod tests {
// Since we clamp things to `MAX_CHANGE_AGE` for determinism,
// `ComponentTicks::is_changed` will now see `MAX_CHANGE_AGE > MAX_CHANGE_AGE`
// and return `false`.
assert!(!change_expired_system.run((), &mut world));
assert!(!change_expired_system.run((), &mut world).unwrap());
}

#[test]
Expand Down
16 changes: 8 additions & 8 deletions crates/bevy_ecs/src/event/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,20 +528,20 @@ mod tests {
});
reader.initialize(&mut world);

let last = reader.run((), &mut world);
let last = reader.run((), &mut world).unwrap();
assert!(last.is_none(), "EventReader should be empty");

world.send_event(TestEvent { i: 0 });
let last = reader.run((), &mut world);
let last = reader.run((), &mut world).unwrap();
assert_eq!(last, Some(TestEvent { i: 0 }));

world.send_event(TestEvent { i: 1 });
world.send_event(TestEvent { i: 2 });
world.send_event(TestEvent { i: 3 });
let last = reader.run((), &mut world);
let last = reader.run((), &mut world).unwrap();
assert_eq!(last, Some(TestEvent { i: 3 }));

let last = reader.run((), &mut world);
let last = reader.run((), &mut world).unwrap();
assert!(last.is_none(), "EventReader should be empty");
}

Expand All @@ -558,20 +558,20 @@ mod tests {
});
mutator.initialize(&mut world);

let last = mutator.run((), &mut world);
let last = mutator.run((), &mut world).unwrap();
assert!(last.is_none(), "EventMutator should be empty");

world.send_event(TestEvent { i: 0 });
let last = mutator.run((), &mut world);
let last = mutator.run((), &mut world).unwrap();
assert_eq!(last, Some(TestEvent { i: 0 }));

world.send_event(TestEvent { i: 1 });
world.send_event(TestEvent { i: 2 });
world.send_event(TestEvent { i: 3 });
let last = mutator.run((), &mut world);
let last = mutator.run((), &mut world).unwrap();
assert_eq!(last, Some(TestEvent { i: 3 }));

let last = mutator.run((), &mut world);
let last = mutator.run((), &mut world).unwrap();
assert!(last.is_none(), "EventMutator should be empty");
}

Expand Down
56 changes: 23 additions & 33 deletions crates/bevy_ecs/src/observer/runner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@
use core::any::Any;

use crate::{
error::ErrorContext, observer::ObserverTrigger, prelude::*, query::DebugCheckedUnwrap,
system::ObserverSystem, world::DeferredWorld,
error::ErrorContext,
observer::ObserverTrigger,
prelude::*,
query::DebugCheckedUnwrap,
system::{ObserverSystem, RunSystemError},
world::DeferredWorld,
};
use bevy_ptr::PtrMut;

Expand Down Expand Up @@ -64,37 +68,23 @@ pub(super) fn observer_system_runner<E: Event, B: Bundle, S: ObserverSystem<E, B
#[cfg(feature = "hotpatching")]
(*system).refresh_hotpatch();

match (*system).validate_param_unsafe(world) {
Ok(()) => {
if let Err(err) = (*system).run_unsafe(trigger, world) {
let handler = state
.error_handler
.unwrap_or_else(|| world.default_error_handler());
handler(
err,
ErrorContext::Observer {
name: (*system).name(),
last_run: (*system).get_last_run(),
},
);
};
(*system).queue_deferred(world.into_deferred());
}
Err(e) => {
if !e.skipped {
let handler = state
.error_handler
.unwrap_or_else(|| world.default_error_handler());
handler(
e.into(),
ErrorContext::Observer {
name: (*system).name(),
last_run: (*system).get_last_run(),
},
);
}
}
}
if let Err(RunSystemError::Failed(err)) = (*system)
Copy link
Contributor

Choose a reason for hiding this comment

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

Idly interested in benchmarks of this. Removing the branch here should be net positive.

.validate_param_unsafe(world)
.map_err(From::from)
.and_then(|()| (*system).run_unsafe(trigger, world))
{
let handler = state
.error_handler
.unwrap_or_else(|| world.default_error_handler());
handler(
err,
ErrorContext::Observer {
name: (*system).name(),
last_run: (*system).get_last_run(),
},
);
};
(*system).queue_deferred(world.into_deferred());
}
}

Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/query/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ mod tests {
}
let mut system = IntoSystem::into_system(system);
system.initialize(&mut world);
system.run((), &mut world);
system.run((), &mut world).unwrap();
}
{
fn system(has_a: Query<Entity, With<A>>, mut b_query: Query<&mut B>) {
Expand All @@ -728,7 +728,7 @@ mod tests {
}
let mut system = IntoSystem::into_system(system);
system.initialize(&mut world);
system.run((), &mut world);
system.run((), &mut world).unwrap();
}
{
fn system(query: Query<(Option<&A>, &B)>) {
Expand All @@ -741,7 +741,7 @@ mod tests {
}
let mut system = IntoSystem::into_system(system);
system.initialize(&mut world);
system.run((), &mut world);
system.run((), &mut world).unwrap();
}
}

Expand Down
Loading