System::type_id Consistency

[bushrat011899]

Objective

• Fixes Stored TypeId of systems have an unexpected value #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.

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.

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.

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());

[alice-i-cecile]

We should add a small test to validate this behavior, ideally for all of our different flavors of systems.

[bushrat011899]

I've added some unit tests which cover exclusive function systems, and function systems. They ensure that the TypeId value is consistent between System and IntoSystem implementations, and also consistent with TypeId::of::<T::System>() where T is the IntoSystem type. I've also updated the description of this PR to explicitly call out exactly what cases will require some kind of migration work.

[james7132]

I'd actually have preferred that we hard-break the existing use cases, since this change is otherwise silent. It's cases like these where I wish we had something similar to Crater to find the affected ecosystem crates. Otherwise though, LGTM.