-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Convert to fallible system in IntoSystemConfigs #17051
Convert to fallible system in IntoSystemConfigs #17051
Conversation
@Neo-Zhixing how do you feel about this implementation instead? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer this design: I think it's clearer and reducing the branching is good.
Seems great! Have you tested logging and panics to see if this mangles the source tracking, as was the issue in #8705? |
I'll check, but it should be fine. This should just add one more line to the backtrace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is clearly a much better approach. Thanks for tidying up after me :).
Panics seem ok.
|
Oops I'm a bit late to the discussions, but this is clearly a much better approach. |
# Objective - bevyengine#16589 added an enum to switch between fallible and infallible system. This branching should be unnecessary if we wrap infallible systems in a function to return `Ok(())`. ## Solution - Create a wrapper system for `System<(), ()>`s that returns `Ok` on the call to `run` and `run_unsafe`. The wrapper should compile out, but I haven't checked. - I removed the `impl IntoSystemConfigs for BoxedSystem<(), ()>` as I couldn't figure out a way to keep the impl without double boxing. ## Testing - ran `many_foxes` example to check if it still runs. ## Migration Guide - `IntoSystemConfigs` has been removed for `BoxedSystem<(), ()>`. Either use `InfallibleSystemWrapper` before boxing or make your system return `bevy::ecs::prelude::Result`.
Objective
Ok(())
.Solution
System<(), ()>
s that returnsOk
on the call torun
andrun_unsafe
. The wrapper should compile out, but I haven't checked.impl IntoSystemConfigs for BoxedSystem<(), ()>
as I couldn't figure out a way to keep the impl without double boxing.Testing
many_foxes
example to check if it still runs.Migration Guide
IntoSystemConfigs
has been removed forBoxedSystem<(), ()>
. Either useInfallibleSystemWrapper
before boxing or make your system returnbevy::ecs::prelude::Result
.