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.

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

Jump to bottom

[Merged by Bors] - change is_system_type() -> bool to system_type() -> Option<TypeId> #7715

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - change is_system_type() -> bool to system_type() -> Option<TypeId> #7715

wants to merge 1 commit into from

Conversation

jakobhellermann
Copy link
Contributor

Objective

  • it would be nice to be able to associate a NodeId of a system type set to the NodeId of the actual system (used in bevy_mod_debugdump)

Solution

  • make system_type return the type id of the system
    • that way you can check if a dyn SystemSet is the system type set of a dyn System
  • I don't know if this information is already present somewhere else in the scheduler or if there is a better way to expose it

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use labels Feb 16, 2023
@alice-i-cecile alice-i-cecile added this to the 0.10 milestone Feb 16, 2023
alice-i-cecile
alice-i-cecile approved these changes Feb 16, 2023
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I think this is generally nicer API.

@james7132 james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 16, 2023
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 16, 2023
@jakobhellermann
change is_system_type() -> bool to system_type() -> Option<TypeId> (
b1646e9 
#7715)

# Objective

- it would be nice to be able to associate a `NodeId` of a system type set to the `NodeId` of the actual system (used in bevy_mod_debugdump)

## Solution

- make `system_type` return the type id of the system
  - that way you can check if a `dyn SystemSet` is the system type set of a `dyn System`
- I don't know if this information is already present somewhere else in the scheduler or if there is a better way to expose it
@bors
Copy link
Contributor

bors bot commented Feb 16, 2023

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title change is_system_type() -> bool to system_type() -> Option<TypeId> [Merged by Bors] - change is_system_type() -> bool to system_type() -> Option<TypeId> Feb 16, 2023
@bors bors bot closed this Feb 16, 2023
myreprise1 pushed a commit to myreprise1/bevy that referenced this pull request Feb 18, 2023
@jakobhellermann @myreprise1
change is_system_type() -> bool to system_type() -> Option<TypeId> (
d4cdc87 
bevyengine#7715)

# Objective

- it would be nice to be able to associate a `NodeId` of a system type set to the `NodeId` of the actual system (used in bevy_mod_debugdump)

## Solution

- make `system_type` return the type id of the system
  - that way you can check if a `dyn SystemSet` is the system type set of a `dyn System`
- I don't know if this information is already present somewhere else in the scheduler or if there is a better way to expose it
@jakobhellermann jakobhellermann mentioned this pull request Feb 21, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Feb 21, 2024
@jakobhellermann
Fix SystemTypeSet::system_type being out of sync with `System::type…
a491bce 
…_id` (#12030)

## Objective

Always have `some_system.into_system().type_id() ==
some_system.into_system_set().system_type().unwrap()`.

System sets have a `fn system_type() -> Option<TypeId>` that is
implemented by `SystemTypeSet` to returning the TypeId of the system's
function type. This was implemented in
#7715 and is used in
`bevy_mod_debugdump` to handle `.after(function)` constraints.

Back then, `System::type_id` always also returned the type id of the
function item, not of `FunctionSystem<M, F>`.

#11728 changes the behaviour of
`System::type_id` so that it returns the id of the
`FunctionSystem`/`ExclusiveFunctionSystem` wrapper, but it did not
change `SystemTypeSet::system_type`, so doing the lookup breaks in
`bevy_mod_debugdump`.

## Solution

Change `IntoSystemSet` for functions to return a
`SystemTypeSet<FunctionSystem>` /
`SystemTypeSet<ExclusiveFunctionSystem>` instead of `SystemTypeSet<F>`.
ameknite pushed a commit to ameknite/bevy that referenced this pull request Feb 22, 2024
@jakobhellermann @ameknite
Fix SystemTypeSet::system_type being out of sync with `System::type…
d64f449 
…_id` (bevyengine#12030)

## Objective

Always have `some_system.into_system().type_id() ==
some_system.into_system_set().system_type().unwrap()`.

System sets have a `fn system_type() -> Option<TypeId>` that is
implemented by `SystemTypeSet` to returning the TypeId of the system's
function type. This was implemented in
bevyengine#7715 and is used in
`bevy_mod_debugdump` to handle `.after(function)` constraints.

Back then, `System::type_id` always also returned the type id of the
function item, not of `FunctionSystem<M, F>`.

bevyengine#11728 changes the behaviour of
`System::type_id` so that it returns the id of the
`FunctionSystem`/`ExclusiveFunctionSystem` wrapper, but it did not
change `SystemTypeSet::system_type`, so doing the lookup breaks in
`bevy_mod_debugdump`.

## Solution

Change `IntoSystemSet` for functions to return a
`SystemTypeSet<FunctionSystem>` /
`SystemTypeSet<ExclusiveFunctionSystem>` instead of `SystemTypeSet<F>`.
msvbg pushed a commit to msvbg/bevy that referenced this pull request Feb 26, 2024
@jakobhellermann @msvbg
Fix SystemTypeSet::system_type being out of sync with `System::type…
7b44b5c 
…_id` (bevyengine#12030)

## Objective

Always have `some_system.into_system().type_id() ==
some_system.into_system_set().system_type().unwrap()`.

System sets have a `fn system_type() -> Option<TypeId>` that is
implemented by `SystemTypeSet` to returning the TypeId of the system's
function type. This was implemented in
bevyengine#7715 and is used in
`bevy_mod_debugdump` to handle `.after(function)` constraints.

Back then, `System::type_id` always also returned the type id of the
function item, not of `FunctionSystem<M, F>`.

bevyengine#11728 changes the behaviour of
`System::type_id` so that it returns the id of the
`FunctionSystem`/`ExclusiveFunctionSystem` wrapper, but it did not
change `SystemTypeSet::system_type`, so doing the lookup breaks in
`bevy_mod_debugdump`.

## Solution

Change `IntoSystemSet` for functions to return a
`SystemTypeSet<FunctionSystem>` /
`SystemTypeSet<ExclusiveFunctionSystem>` instead of `SystemTypeSet<F>`.
msvbg pushed a commit to msvbg/bevy that referenced this pull request Feb 26, 2024
@jakobhellermann @msvbg
Fix SystemTypeSet::system_type being out of sync with `System::type…
e6548ec 
…_id` (bevyengine#12030)

## Objective

Always have `some_system.into_system().type_id() ==
some_system.into_system_set().system_type().unwrap()`.

System sets have a `fn system_type() -> Option<TypeId>` that is
implemented by `SystemTypeSet` to returning the TypeId of the system's
function type. This was implemented in
bevyengine#7715 and is used in
`bevy_mod_debugdump` to handle `.after(function)` constraints.

Back then, `System::type_id` always also returned the type id of the
function item, not of `FunctionSystem<M, F>`.

bevyengine#11728 changes the behaviour of
`System::type_id` so that it returns the id of the
`FunctionSystem`/`ExclusiveFunctionSystem` wrapper, but it did not
change `SystemTypeSet::system_type`, so doing the lookup breaks in
`bevy_mod_debugdump`.

## Solution

Change `IntoSystemSet` for functions to return a
`SystemTypeSet<FunctionSystem>` /
`SystemTypeSet<ExclusiveFunctionSystem>` instead of `SystemTypeSet<F>`.
mockersf pushed a commit that referenced this pull request Feb 27, 2024
@jakobhellermann @mockersf
Fix SystemTypeSet::system_type being out of sync with `System::type…
4022385 
…_id` (#12030)

## Objective

Always have `some_system.into_system().type_id() ==
some_system.into_system_set().system_type().unwrap()`.

System sets have a `fn system_type() -> Option<TypeId>` that is
implemented by `SystemTypeSet` to returning the TypeId of the system's
function type. This was implemented in
#7715 and is used in
`bevy_mod_debugdump` to handle `.after(function)` constraints.

Back then, `System::type_id` always also returned the type id of the
function item, not of `FunctionSystem<M, F>`.

#11728 changes the behaviour of
`System::type_id` so that it returns the id of the
`FunctionSystem`/`ExclusiveFunctionSystem` wrapper, but it did not
change `SystemTypeSet::system_type`, so doing the lookup breaks in
`bevy_mod_debugdump`.

## Solution

Change `IntoSystemSet` for functions to return a
`SystemTypeSet<FunctionSystem>` /
`SystemTypeSet<ExclusiveFunctionSystem>` instead of `SystemTypeSet<F>`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@james7132 james7132 james7132 approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
0.10
Development

Successfully merging this pull request may close these issues.
3 participants
@jakobhellermann @alice-i-cecile @james7132