Skip to content
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.

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

System::type_id Consistency #11728

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`].
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`
system.as_ref().type_id() == apply_deferred.type_id()
system.as_ref().type_id() == apply_deferred.system_type_id()
}
13 changes: 5 additions & 8 deletions crates/bevy_ecs/src/schedule/stepping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::collections::HashMap;

use crate::{
schedule::{InternedScheduleLabel, NodeId, Schedule, ScheduleLabel},
system::{IntoSystem, ResMut, Resource, System},
system::{IntoSystem, ResMut, Resource},
};
use bevy_utils::{
thiserror::Error,
Expand Down Expand Up @@ -252,10 +252,7 @@ impl Stepping {
schedule: impl ScheduleLabel,
system: impl IntoSystem<(), (), Marker>,
) -> &mut Self {
// PERF: ideally we don't actually need to construct the system to retrieve the TypeId.
// 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();
let type_id = system.system_type_id();
self.updates.push(Update::SetBehavior(
schedule.intern(),
SystemIdentifier::Type(type_id),
Expand All @@ -281,7 +278,7 @@ impl Stepping {
schedule: impl ScheduleLabel,
system: impl IntoSystem<(), (), Marker>,
) -> &mut Self {
let type_id = IntoSystem::into_system(system).type_id();
let type_id = system.system_type_id();
self.updates.push(Update::SetBehavior(
schedule.intern(),
SystemIdentifier::Type(type_id),
Expand All @@ -307,7 +304,7 @@ impl Stepping {
schedule: impl ScheduleLabel,
system: impl IntoSystem<(), (), Marker>,
) -> &mut Self {
let type_id = IntoSystem::into_system(system).type_id();
let type_id = system.system_type_id();
self.updates.push(Update::SetBehavior(
schedule.intern(),
SystemIdentifier::Type(type_id),
Expand Down Expand Up @@ -354,7 +351,7 @@ impl Stepping {
schedule: impl ScheduleLabel,
system: impl IntoSystem<(), (), Marker>,
) -> &mut Self {
let type_id = IntoSystem::into_system(system).type_id();
let type_id = system.system_type_id();
self.updates.push(Update::ClearBehavior(
schedule.intern(),
SystemIdentifier::Type(type_id),
Expand Down
4 changes: 0 additions & 4 deletions crates/bevy_ecs/src/system/adapter_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ where
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> {
self.system.component_access()
}
Expand Down
4 changes: 0 additions & 4 deletions crates/bevy_ecs/src/system/combinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,6 @@ where
self.name.clone()
}

fn type_id(&self) -> std::any::TypeId {
std::any::TypeId::of::<Self>()
}

fn component_access(&self) -> &Access<ComponentId> {
&self.component_access
}
Expand Down
48 changes: 42 additions & 6 deletions crates/bevy_ecs/src/system/exclusive_function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
};

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.
///
Expand Down Expand Up @@ -65,11 +65,6 @@ where
self.system_meta.name.clone()
}

#[inline]
fn type_id(&self) -> TypeId {
bushrat011899 marked this conversation as resolved.
Show resolved Hide resolved
TypeId::of::<F>()
}

#[inline]
fn component_access(&self) -> &Access<ComponentId> {
self.system_meta.component_access_set.combined_access()
Expand Down Expand Up @@ -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
// of `SystemParam` created.
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);
}
}
48 changes: 42 additions & 6 deletions crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
};

use bevy_utils::all_tuples;
use std::{any::TypeId, borrow::Cow, marker::PhantomData};
use std::{borrow::Cow, marker::PhantomData};

#[cfg(feature = "trace")]
use bevy_utils::tracing::{info_span, Span};
Expand Down Expand Up @@ -453,11 +453,6 @@ where
self.system_meta.name.clone()
}

#[inline]
fn type_id(&self) -> TypeId {
TypeId::of::<F>()
}

#[inline]
fn component_access(&self) -> &Access<ComponentId> {
self.system_meta.component_access_set.combined_access()
Expand Down Expand Up @@ -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
// of `SystemParam` created.
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);
}
}
8 changes: 7 additions & 1 deletion crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ mod system_name;
mod system_param;
mod system_registry;

use std::borrow::Cow;
use std::{any::TypeId, borrow::Cow};

pub use adapter_system::*;
pub use combinator::*;
Expand Down Expand Up @@ -195,6 +195,12 @@ pub trait IntoSystem<In, Out, Marker>: Sized {
let name = 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.
Expand Down
5 changes: 4 additions & 1 deletion crates/bevy_ecs/src/system/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ pub trait System: Send + Sync + 'static {
/// Returns the system's name.
fn name(&self) -> Cow<'static, str>;
/// 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`].
fn component_access(&self) -> &Access<ComponentId>;
/// Returns the system's archetype component [`Access`].
Expand Down