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

Automatically label systems with their TypeId #4219

Closed
alice-i-cecile opened this issue Mar 15, 2022 · 0 comments
Closed

Automatically label systems with their TypeId #4219

alice-i-cecile opened this issue Mar 15, 2022 · 0 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

In many simple system ordering use cases, users are forced to define simple labels for each system they want to order, creating tedious and duplicative boilerplate, and encouraging the use of stringly-typed system labels.

This is particularly true in applications, rather than plugin libraries, where exposing a clean forwards-compatible public API is of much lower concern.

What solution would you like?

app.add_system(gravity).add_system(kinematics.after(gravity))

Automatically give each system a SystemLabel corresponding to their TypeId.

Change the signature of .before and .after to accept a impl IntoSystemLabel argument, and then convert provided system-like functions into the corresponding TypeId, which is then used in schedule graph construction.

What alternative(s) have you considered?

We could use the type_name instead of the TypeId to define the label. The pros and cons aren't fully clear to me, but the type_name docs state that it should really only be used for diagnostic purposes.

IMO we should remove string system labels as part of this change: the "quick-and-dirty" approach is already covered well by this design.

Additional context

I think it would be interesting to consider "automatically labeling" systems with either their type_name or their TypeId

@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 Mar 15, 2022
@bors bors bot closed this as completed in b1c3e98 Mar 23, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this issue Jun 7, 2022
This adds the concept of "default labels" for systems (currently scoped to "parallel systems", but this could just as easily be implemented for "exclusive systems"). Function systems now include their function's `SystemTypeIdLabel` by default.

This enables the following patterns:

```rust
// ordering two systems without manually defining labels
app
  .add_system(update_velocity)
  .add_system(movement.after(update_velocity))

// ordering sets of systems without manually defining labels
app
  .add_system(foo)
  .add_system_set(
    SystemSet::new()
      .after(foo)
      .with_system(bar)
      .with_system(baz)
  )
```

Fixes: bevyengine#4219
Related to: bevyengine#4220 

Credit to @aevyrie @alice-i-cecile @DJMcNab (and probably others) for proposing (and supporting) this idea about a year ago. I was a big dummy that both shut down this (very good) idea and then forgot I did that. Sorry. You all were right!
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
This adds the concept of "default labels" for systems (currently scoped to "parallel systems", but this could just as easily be implemented for "exclusive systems"). Function systems now include their function's `SystemTypeIdLabel` by default.

This enables the following patterns:

```rust
// ordering two systems without manually defining labels
app
  .add_system(update_velocity)
  .add_system(movement.after(update_velocity))

// ordering sets of systems without manually defining labels
app
  .add_system(foo)
  .add_system_set(
    SystemSet::new()
      .after(foo)
      .with_system(bar)
      .with_system(baz)
  )
```

Fixes: bevyengine#4219
Related to: bevyengine#4220 

Credit to @aevyrie @alice-i-cecile @DJMcNab (and probably others) for proposing (and supporting) this idea about a year ago. I was a big dummy that both shut down this (very good) idea and then forgot I did that. Sorry. You all were right!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant