System::type_id Consistency (#11728)

# Objective

- Fixes #11679

## Solution

- Added `IntoSystem::system_type_id` which returns the equivalent of
`system.into_system().type_id()` without construction. This allows for
getting the `TypeId` of functions (a function is an unnamed type and
therefore you cannot call `TypeId::of::<apply_deferred::System>()`)
- Added default implementation of `System::type_id` to ensure
consistency between implementations. Some returned `Self`, while others
were returning an inner value instead. This ensures consistency with
`IntoSystem::system_type_id`.

## Migration Guide

If you use `System::type_id()` on function systems (exclusive or not),
ensure you are comparing its value to other `System::type_id()` calls,
or `IntoSystem::system_type_id()`.

This code wont require any changes, because `IntoSystem`'s are directly
compared to each other.

```rust
fn test_system() {}

let type_id = test_system.type_id();

// ...

// No change required
assert_eq!(test_system.type_id(), type_id);
```

Likewise, this code wont, because `System`'s are directly compared.

```rust
fn test_system() {}

let type_id = IntoSystem::into_system(test_system).type_id();

// ...

// No change required
assert_eq!(IntoSystem::into_system(test_system).type_id(), type_id);
```

The below _does_ require a change, since you're comparing a `System`
type to a `IntoSystem` type.

```rust
fn test_system() {}

// Before
assert_eq!(test_system.type_id(), IntoSystem::into_system(test_system).type_id());

// After
assert_eq!(test_system.system_type_id(), IntoSystem::into_system(test_system).type_id());
```
This commit is contained in:
Zachary Harrold 2024-02-07 01:43:33 +11:00 committed by GitHub
parent f2cb155abc
commit 950bd2284d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 102 additions and 32 deletions

View File

@ -104,7 +104,7 @@ pub fn apply_deferred(world: &mut World) {}
/// Returns `true` if the [`System`](crate::system::System) is an instance of [`apply_deferred`]. /// Returns `true` if the [`System`](crate::system::System) is an instance of [`apply_deferred`].
pub(super) fn is_apply_deferred(system: &BoxedSystem) -> bool { pub(super) fn is_apply_deferred(system: &BoxedSystem) -> bool {
use std::any::Any; use crate::system::IntoSystem;
// deref to use `System::type_id` instead of `Any::type_id` // deref to use `System::type_id` instead of `Any::type_id`
system.as_ref().type_id() == apply_deferred.type_id() system.as_ref().type_id() == apply_deferred.system_type_id()
} }

View File

@ -4,7 +4,7 @@ use std::collections::HashMap;
use crate::{ use crate::{
schedule::{InternedScheduleLabel, NodeId, Schedule, ScheduleLabel}, schedule::{InternedScheduleLabel, NodeId, Schedule, ScheduleLabel},
system::{IntoSystem, ResMut, Resource, System}, system::{IntoSystem, ResMut, Resource},
}; };
use bevy_utils::{ use bevy_utils::{
thiserror::Error, thiserror::Error,
@ -252,10 +252,7 @@ impl Stepping {
schedule: impl ScheduleLabel, schedule: impl ScheduleLabel,
system: impl IntoSystem<(), (), Marker>, system: impl IntoSystem<(), (), Marker>,
) -> &mut Self { ) -> &mut Self {
// PERF: ideally we don't actually need to construct the system to retrieve the TypeId. let type_id = system.system_type_id();
// Unfortunately currently IntoSystem::into_system(system).type_id() != TypeId::of::<I::System>()
// If these are aligned, we can use TypeId::of::<I::System>() here
let type_id = IntoSystem::into_system(system).type_id();
self.updates.push(Update::SetBehavior( self.updates.push(Update::SetBehavior(
schedule.intern(), schedule.intern(),
SystemIdentifier::Type(type_id), SystemIdentifier::Type(type_id),
@ -281,7 +278,7 @@ impl Stepping {
schedule: impl ScheduleLabel, schedule: impl ScheduleLabel,
system: impl IntoSystem<(), (), Marker>, system: impl IntoSystem<(), (), Marker>,
) -> &mut Self { ) -> &mut Self {
let type_id = IntoSystem::into_system(system).type_id(); let type_id = system.system_type_id();
self.updates.push(Update::SetBehavior( self.updates.push(Update::SetBehavior(
schedule.intern(), schedule.intern(),
SystemIdentifier::Type(type_id), SystemIdentifier::Type(type_id),
@ -307,7 +304,7 @@ impl Stepping {
schedule: impl ScheduleLabel, schedule: impl ScheduleLabel,
system: impl IntoSystem<(), (), Marker>, system: impl IntoSystem<(), (), Marker>,
) -> &mut Self { ) -> &mut Self {
let type_id = IntoSystem::into_system(system).type_id(); let type_id = system.system_type_id();
self.updates.push(Update::SetBehavior( self.updates.push(Update::SetBehavior(
schedule.intern(), schedule.intern(),
SystemIdentifier::Type(type_id), SystemIdentifier::Type(type_id),
@ -354,7 +351,7 @@ impl Stepping {
schedule: impl ScheduleLabel, schedule: impl ScheduleLabel,
system: impl IntoSystem<(), (), Marker>, system: impl IntoSystem<(), (), Marker>,
) -> &mut Self { ) -> &mut Self {
let type_id = IntoSystem::into_system(system).type_id(); let type_id = system.system_type_id();
self.updates.push(Update::ClearBehavior( self.updates.push(Update::ClearBehavior(
schedule.intern(), schedule.intern(),
SystemIdentifier::Type(type_id), SystemIdentifier::Type(type_id),

View File

@ -81,10 +81,6 @@ where
self.name.clone() self.name.clone()
} }
fn type_id(&self) -> std::any::TypeId {
std::any::TypeId::of::<Self>()
}
fn component_access(&self) -> &crate::query::Access<crate::component::ComponentId> { fn component_access(&self) -> &crate::query::Access<crate::component::ComponentId> {
self.system.component_access() self.system.component_access()
} }

View File

@ -142,10 +142,6 @@ where
self.name.clone() self.name.clone()
} }
fn type_id(&self) -> std::any::TypeId {
std::any::TypeId::of::<Self>()
}
fn component_access(&self) -> &Access<ComponentId> { fn component_access(&self) -> &Access<ComponentId> {
&self.component_access &self.component_access
} }

View File

@ -11,7 +11,7 @@ use crate::{
}; };
use bevy_utils::all_tuples; use bevy_utils::all_tuples;
use std::{any::TypeId, borrow::Cow, marker::PhantomData}; use std::{borrow::Cow, marker::PhantomData};
/// A function system that runs with exclusive [`World`] access. /// A function system that runs with exclusive [`World`] access.
/// ///
@ -65,11 +65,6 @@ where
self.system_meta.name.clone() self.system_meta.name.clone()
} }
#[inline]
fn type_id(&self) -> TypeId {
TypeId::of::<F>()
}
#[inline] #[inline]
fn component_access(&self) -> &Access<ComponentId> { fn component_access(&self) -> &Access<ComponentId> {
self.system_meta.component_access_set.combined_access() self.system_meta.component_access_set.combined_access()
@ -250,3 +245,44 @@ macro_rules! impl_exclusive_system_function {
// Note that we rely on the highest impl to be <= the highest order of the tuple impls // Note that we rely on the highest impl to be <= the highest order of the tuple impls
// of `SystemParam` created. // of `SystemParam` created.
all_tuples!(impl_exclusive_system_function, 0, 16, F); all_tuples!(impl_exclusive_system_function, 0, 16, F);
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn into_system_type_id_consistency() {
fn test<T, In, Out, Marker>(function: T)
where
T: IntoSystem<In, Out, Marker> + Copy,
{
fn reference_system(_world: &mut World) {}
use std::any::TypeId;
let system = IntoSystem::into_system(function);
assert_eq!(
system.type_id(),
function.system_type_id(),
"System::type_id should be consistent with IntoSystem::system_type_id"
);
assert_eq!(
system.type_id(),
TypeId::of::<T::System>(),
"System::type_id should be consistent with TypeId::of::<T::System>()"
);
assert_ne!(
system.type_id(),
IntoSystem::into_system(reference_system).type_id(),
"Different systems should have different TypeIds"
);
}
fn exclusive_function_system(_world: &mut World) {}
test(exclusive_function_system);
}
}

View File

@ -9,7 +9,7 @@ use crate::{
}; };
use bevy_utils::all_tuples; use bevy_utils::all_tuples;
use std::{any::TypeId, borrow::Cow, marker::PhantomData}; use std::{borrow::Cow, marker::PhantomData};
#[cfg(feature = "trace")] #[cfg(feature = "trace")]
use bevy_utils::tracing::{info_span, Span}; use bevy_utils::tracing::{info_span, Span};
@ -453,11 +453,6 @@ where
self.system_meta.name.clone() self.system_meta.name.clone()
} }
#[inline]
fn type_id(&self) -> TypeId {
TypeId::of::<F>()
}
#[inline] #[inline]
fn component_access(&self) -> &Access<ComponentId> { fn component_access(&self) -> &Access<ComponentId> {
self.system_meta.component_access_set.combined_access() self.system_meta.component_access_set.combined_access()
@ -695,3 +690,44 @@ macro_rules! impl_system_function {
// Note that we rely on the highest impl to be <= the highest order of the tuple impls // Note that we rely on the highest impl to be <= the highest order of the tuple impls
// of `SystemParam` created. // of `SystemParam` created.
all_tuples!(impl_system_function, 0, 16, F); all_tuples!(impl_system_function, 0, 16, F);
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn into_system_type_id_consistency() {
fn test<T, In, Out, Marker>(function: T)
where
T: IntoSystem<In, Out, Marker> + Copy,
{
fn reference_system() {}
use std::any::TypeId;
let system = IntoSystem::into_system(function);
assert_eq!(
system.type_id(),
function.system_type_id(),
"System::type_id should be consistent with IntoSystem::system_type_id"
);
assert_eq!(
system.type_id(),
TypeId::of::<T::System>(),
"System::type_id should be consistent with TypeId::of::<T::System>()"
);
assert_ne!(
system.type_id(),
IntoSystem::into_system(reference_system).type_id(),
"Different systems should have different TypeIds"
);
}
fn function_system() {}
test(function_system);
}
}

View File

@ -114,7 +114,7 @@ mod system_name;
mod system_param; mod system_param;
mod system_registry; mod system_registry;
use std::borrow::Cow; use std::{any::TypeId, borrow::Cow};
pub use adapter_system::*; pub use adapter_system::*;
pub use combinator::*; pub use combinator::*;
@ -195,6 +195,12 @@ pub trait IntoSystem<In, Out, Marker>: Sized {
let name = system.name(); let name = system.name();
AdapterSystem::new(f, system, name) AdapterSystem::new(f, system, name)
} }
/// Get the [`TypeId`] of the [`System`] produced after calling [`into_system`](`IntoSystem::into_system`).
#[inline]
fn system_type_id(&self) -> TypeId {
TypeId::of::<Self::System>()
}
} }
// All systems implicitly implement IntoSystem. // All systems implicitly implement IntoSystem.

View File

@ -31,7 +31,10 @@ pub trait System: Send + Sync + 'static {
/// Returns the system's name. /// Returns the system's name.
fn name(&self) -> Cow<'static, str>; fn name(&self) -> Cow<'static, str>;
/// Returns the [`TypeId`] of the underlying system type. /// Returns the [`TypeId`] of the underlying system type.
fn type_id(&self) -> TypeId; #[inline]
fn type_id(&self) -> TypeId {
TypeId::of::<Self>()
}
/// Returns the system's component [`Access`]. /// Returns the system's component [`Access`].
fn component_access(&self) -> &Access<ComponentId>; fn component_access(&self) -> &Access<ComponentId>;
/// Returns the system's archetype component [`Access`]. /// Returns the system's archetype component [`Access`].