Skip to content

Support fallible one-shot systems #19678

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
merged 2 commits into from
Jun 17, 2025
Merged

Support fallible one-shot systems #19678

merged 2 commits into from
Jun 17, 2025
+69 −14

Conversation

Pascualex
Copy link
Contributor

@Pascualex Pascualex commented Jun 16, 2025 •
edited
Loading

Closes #19677.

I don't think that the output type needs to be Send. I've done some test at it seems to work fine without it, which in IMO makes sense, but please correct me if that is not the case.

alice-i-cecile
alice-i-cecile requested changes Jun 16, 2025
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.

Actually, can you please add a simple test verifying that this works? Notes in the docs about this would also be nice, but I can live without them.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jun 16, 2025
@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jun 16, 2025
@alice-i-cecile alice-i-cecile added the C-Usability A targeted quality-of-life change that makes Bevy easier to use label Jun 16, 2025
@Pascualex Pascualex force-pushed the fallible-one-shot-systems branch 2 times, most recently from f5dbfc3 to dd3ba29 Compare June 17, 2025 14:52
@Pascualex
Copy link
Contributor Author

I have added new tests and slightly modified one of the existing ones to cover the run_cached_system_with API. I have tried to be consistent with the existing system_registry tests and fallible systems tests.

I'm not sure on how to proceed with the docs. AFAIK neither add_systems nor run_system say anything about fallible systems and I think this API should be consistent with those. Probably all of them should reference a different section explaining unified ECS error handling.

@Pascualex Pascualex requested a review from alice-i-cecile June 17, 2025 14:58
@Pascualex Pascualex force-pushed the fallible-one-shot-systems branch from dd3ba29 to 2394443 Compare June 17, 2025 15:02
alice-i-cecile
alice-i-cecile approved these changes Jun 17, 2025
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.

That sounds good; I'll open another issue for docs :)

@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 17, 2025
@alice-i-cecile alice-i-cecile mentioned this pull request Jun 17, 2025
Open
@alice-i-cecile alice-i-cecile 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 Jun 17, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 17, 2025
Merged via the queue into bevyengine:main with commit d1c6fbe Jun 17, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@MiniaczQ MiniaczQ MiniaczQ approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
0.17
Development

Successfully merging this pull request may close these issues.

Support fallible one-shot systems
3 participants
@Pascualex @alice-i-cecile @MiniaczQ