Skip to content

Generic SystemParam impls for Option and Result #18766

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

Merged
merged 5 commits into from
May 7, 2025

Conversation

chescock
Copy link
Contributor

@chescock chescock commented Apr 8, 2025

Objective

Provide a generic impl SystemParam for Option<P> that uses system parameter validation. This immediately gives useful impls for params like EventReader and GizmosState that are defined in terms of Res. It also allows third-party system parameters to be usable with Option, which was previously impossible due to orphan rules.

Note that this is a behavior change for Option<Single>. It currently fails validation if there are multiple matching entities, but with this change it will pass validation and produce None.

Also provide an impl for Result<P, SystemParamValidationError>. This allows systems to inspect the error if necessary, either for bubbling it up or for checking the skipped flag.

Fixes #12634
Fixes #14949
Related to #18516

Solution

Add generic SystemParam impls for Option and Result, and remove the impls for specific types.

Update documentation and fallible_params example with the new semantics for Option<Single>.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward C-Feature A new feature, making something new possible labels Apr 8, 2025
@alice-i-cecile
Copy link
Member

Strongly in favor of this. You need a migration guide for Option<Single> here though; its behavior is inconsistent.

@chescock
Copy link
Contributor Author

chescock commented Apr 8, 2025

You need a migration guide for Option<Single> here though; its behavior is inconsistent.

I thought I added one? Did I put it in the wrong place?

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.

Very nicely done. Sorry for missing the migration guide before: that looks great.

I'm very pleased about making this pattern more consistent and broadly available.

chescock added 2 commits May 7, 2025 09:49
…n-param

# Conflicts:
#	crates/bevy_ecs/src/system/builder.rs
#	crates/bevy_ecs/src/system/system_param.rs
@cBournhonesque cBournhonesque added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 7, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 7, 2025
Merged via the queue into bevyengine:main with commit 9e2bd8a May 7, 2025
34 checks passed
@ItsDoot ItsDoot mentioned this pull request May 10, 2025
andrewzhurov pushed a commit to andrewzhurov/bevy that referenced this pull request May 17, 2025
# Objective

Provide a generic `impl SystemParam for Option<P>` that uses system
parameter validation. This immediately gives useful impls for params
like `EventReader` and `GizmosState` that are defined in terms of `Res`.
It also allows third-party system parameters to be usable with `Option`,
which was previously impossible due to orphan rules.

Note that this is a behavior change for `Option<Single>`. It currently
fails validation if there are multiple matching entities, but with this
change it will pass validation and produce `None`.

Also provide an impl for `Result<P, SystemParamValidationError>`. This
allows systems to inspect the error if necessary, either for bubbling it
up or for checking the `skipped` flag.

Fixes bevyengine#12634
Fixes bevyengine#14949
Related to bevyengine#18516

## Solution

Add generic `SystemParam` impls for `Option` and `Result`, and remove
the impls for specific types.

Update documentation and `fallible_params` example with the new
semantics for `Option<Single>`.
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-Feature A new feature, making something new possible M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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
3 participants