Skip to content

Commit 88035a3

Browse files
Improve error for when you try running a one-shot system with wrong SystemId type (#19011)
# Objective Improve error message when running a one-shot system with a `SystemId` of wrong type. ## Solution Added a resource to keep track of running one-shot systems, and return `Recursive` error if a system gets run before the end of their previous invocation. The error for not being able to `take` a `RegisteredSystem` is now `MaybeWrongType`. ## Testing Minimal reproduction from #19005 ## Showcase ``` // Old 2025-05-01T12:42:54.305283Z WARN bevy_ecs::error::handler: Encountered an error in command `bevy_ecs::system::commands::command::run_system<()>::{{closure}}`: System SystemId(4v1#4294967300) tried to run itself recursively 2025-05-01T12:42:54.305311Z WARN bevy_ecs::error::handler: Encountered an error in command `bevy_ecs::system::commands::command::run_system_with<bevy_ecs::system::input::In<&str>>::{{closure}}`: System SystemId(5v1#4294967301) tried to run itself recursively // New 2025-05-05T14:21:44.918145Z WARN bevy_ecs::error::handler: Encountered an error in command `bevy_ecs::system::commands::command::run_system<()>::{{closure}}`: Could not get system from `SystemId`, entity was `SystemId<(), Result<(), BevyError>>` 2025-05-05T14:21:44.918177Z WARN bevy_ecs::error::handler: Encountered an error in command `bevy_ecs::system::commands::command::run_system_with<bevy_ecs::system::input::In<&str>>::{{closure}}`: Could not get system from `SystemId<In<&str>>`, entity was `SystemId<In<&str>, Result<(), BevyError>>` ``` ## TODO - [x] Add `TypeId` to `SystemIdMarker` - [x] Migration guide for new enum variant --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
1 parent 44efc81 commit 88035a3

File tree

2 files changed

+94
-16
lines changed

2 files changed

+94
-16
lines changed

crates/bevy_ecs/src/system/system_registry.rs

Lines changed: 87 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#[cfg(feature = "bevy_reflect")]
2-
use crate::reflect::ReflectComponent;
31
#[cfg(feature = "hotpatching")]
42
use crate::{change_detection::DetectChanges, HotPatchChanges};
53
use crate::{
@@ -14,14 +12,13 @@ use crate::{
1412
};
1513
use alloc::boxed::Box;
1614
use bevy_ecs_macros::{Component, Resource};
17-
#[cfg(feature = "bevy_reflect")]
18-
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
19-
use core::marker::PhantomData;
15+
use bevy_utils::prelude::DebugName;
16+
use core::{any::TypeId, marker::PhantomData};
2017
use thiserror::Error;
2118

2219
/// A small wrapper for [`BoxedSystem`] that also keeps track whether or not the system has been initialized.
2320
#[derive(Component)]
24-
#[require(SystemIdMarker, Internal)]
21+
#[require(SystemIdMarker = SystemIdMarker::typed_system_id_marker::<I, O>(), Internal)]
2522
pub(crate) struct RegisteredSystem<I, O> {
2623
initialized: bool,
2724
system: BoxedSystem<I, O>,
@@ -36,11 +33,45 @@ impl<I, O> RegisteredSystem<I, O> {
3633
}
3734
}
3835

36+
#[derive(Debug, Clone)]
37+
struct TypeIdAndName {
38+
type_id: TypeId,
39+
name: DebugName,
40+
}
41+
42+
impl TypeIdAndName {
43+
fn new<T: 'static>() -> Self {
44+
Self {
45+
type_id: TypeId::of::<T>(),
46+
name: DebugName::type_name::<T>(),
47+
}
48+
}
49+
}
50+
51+
impl Default for TypeIdAndName {
52+
fn default() -> Self {
53+
Self {
54+
type_id: TypeId::of::<()>(),
55+
name: DebugName::type_name::<()>(),
56+
}
57+
}
58+
}
59+
3960
/// Marker [`Component`](bevy_ecs::component::Component) for identifying [`SystemId`] [`Entity`]s.
40-
#[derive(Component, Default)]
41-
#[cfg_attr(feature = "bevy_reflect", derive(Reflect))]
42-
#[cfg_attr(feature = "bevy_reflect", reflect(Component, Default))]
43-
pub struct SystemIdMarker;
61+
#[derive(Debug, Default, Clone, Component)]
62+
pub struct SystemIdMarker {
63+
input_type_id: TypeIdAndName,
64+
output_type_id: TypeIdAndName,
65+
}
66+
67+
impl SystemIdMarker {
68+
fn typed_system_id_marker<I: 'static, O: 'static>() -> Self {
69+
Self {
70+
input_type_id: TypeIdAndName::new::<I>(),
71+
output_type_id: TypeIdAndName::new::<O>(),
72+
}
73+
}
74+
}
4475

4576
/// A system that has been removed from the registry.
4677
/// It contains the system and whether or not it has been initialized.
@@ -344,14 +375,26 @@ impl World {
344375
.map_err(|_| RegisteredSystemError::SystemIdNotRegistered(id))?;
345376

346377
// Take ownership of system trait object
347-
let RegisteredSystem {
378+
let Some(RegisteredSystem {
348379
mut initialized,
349380
mut system,
350-
} = entity
351-
.take::<RegisteredSystem<I, O>>()
352-
.ok_or(RegisteredSystemError::Recursive(id))?;
381+
}) = entity.take::<RegisteredSystem<I, O>>()
382+
else {
383+
let Some(system_id_marker) = entity.get::<SystemIdMarker>() else {
384+
return Err(RegisteredSystemError::SystemIdNotRegistered(id));
385+
};
386+
if system_id_marker.input_type_id.type_id != TypeId::of::<I>()
387+
|| system_id_marker.output_type_id.type_id != TypeId::of::<O>()
388+
{
389+
return Err(RegisteredSystemError::IncorrectType(
390+
id,
391+
system_id_marker.clone(),
392+
));
393+
}
394+
return Err(RegisteredSystemError::Recursive(id));
395+
};
353396

354-
// Run the system
397+
// Initialize the system
355398
if !initialized {
356399
system.initialize(self);
357400
initialized = true;
@@ -510,6 +553,9 @@ pub enum RegisteredSystemError<I: SystemInput = (), O = ()> {
510553
/// System returned an error or failed required parameter validation.
511554
#[error("System returned error: {0}")]
512555
Failed(BevyError),
556+
/// [`SystemId`] had different input and/or output types than [`SystemIdMarker`]
557+
#[error("Could not get system from `{}`, entity was `SystemId<{}, {}>`", DebugName::type_name::<SystemId<I, O>>(), .1.input_type_id.name, .1.output_type_id.name)]
558+
IncorrectType(SystemId<I, O>, SystemIdMarker),
513559
}
514560

515561
impl<I: SystemInput, O> From<RunSystemError> for RegisteredSystemError<I, O> {
@@ -532,6 +578,11 @@ impl<I: SystemInput, O> core::fmt::Debug for RegisteredSystemError<I, O> {
532578
Self::SelfRemove(arg0) => f.debug_tuple("SelfRemove").field(arg0).finish(),
533579
Self::Skipped(arg0) => f.debug_tuple("Skipped").field(arg0).finish(),
534580
Self::Failed(arg0) => f.debug_tuple("Failed").field(arg0).finish(),
581+
Self::IncorrectType(arg0, arg1) => f
582+
.debug_tuple("IncorrectType")
583+
.field(arg0)
584+
.field(arg1)
585+
.finish(),
535586
}
536587
}
537588
}
@@ -542,7 +593,10 @@ mod tests {
542593

543594
use bevy_utils::default;
544595

545-
use crate::{prelude::*, system::SystemId};
596+
use crate::{
597+
prelude::*,
598+
system::{RegisteredSystemError, SystemId},
599+
};
546600

547601
#[derive(Resource, Default, PartialEq, Debug)]
548602
struct Counter(u8);
@@ -986,4 +1040,21 @@ mod tests {
9861040
world.run_system_cached(system.pipe(system)).unwrap();
9871041
world.run_system_cached(system.map(|()| {})).unwrap();
9881042
}
1043+
1044+
#[test]
1045+
fn wrong_system_type() {
1046+
fn test() -> Result<(), u8> {
1047+
Ok(())
1048+
}
1049+
1050+
let mut world = World::new();
1051+
1052+
let entity = world.register_system_cached(test).entity();
1053+
1054+
match world.run_system::<u8>(SystemId::from_entity(entity)) {
1055+
Ok(_) => panic!("Should fail since called `run_system` with wrong SystemId type."),
1056+
Err(RegisteredSystemError::IncorrectType(_, _)) => (),
1057+
Err(err) => panic!("Failed with wrong error. `{:?}`", err),
1058+
}
1059+
}
9891060
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
title: Improve error when using `run_system` command with a `SystemId` of wrong type
3+
pull_requests: [19011]
4+
---
5+
6+
Added a new error to `RegisteredSystemError` to inform of use of `run_system` command and its
7+
variants with a `SystemId` of wrong type.

0 commit comments

Comments
 (0)