# Objective
Make it easier to short-circuit system parameter validation.
Simplify the API surface by combining `ValidationOutcome` with
`SystemParamValidationError`.
## Solution
Replace `ValidationOutcome` with `Result<(),
SystemParamValidationError>`. Move the docs from `ValidationOutcome` to
`SystemParamValidationError`.
Add a `skipped` field to `SystemParamValidationError` to distinguish the
`Skipped` and `Invalid` variants.
Use the `?` operator to short-circuit validation in tuples of system
params.
# Objective
When introduced, `Single` was intended to simply be silently skipped,
allowing for graceful and efficient handling of systems during invalid
game states (such as when the player is dead).
However, this also caused missing resources to *also* be silently
skipped, leading to confusing and very hard to debug failures. In
0.15.1, this behavior was reverted to a panic, making missing resources
easier to debug, but largely making `Single` (and `Populated`)
worthless, as they would panic during expected game states.
Ultimately, the consensus is that this behavior should differ on a
per-system-param basis. However, there was no sensible way to *do* that
before this PR.
## Solution
Swap `SystemParam::validate_param` from a `bool` to:
```rust
/// The outcome of system / system param validation,
/// used by system executors to determine what to do with a system.
pub enum ValidationOutcome {
/// All system parameters were validated successfully and the system can be run.
Valid,
/// At least one system parameter failed validation, and an error must be handled.
/// By default, this will result in1 a panic. See [crate::error] for more information.
///
/// This is the default behavior, and is suitable for system params that should *always* be valid,
/// either because sensible fallback behavior exists (like [`Query`] or because
/// failures in validation should be considered a bug in the user's logic that must be immediately addressed (like [`Res`]).
Invalid,
/// At least one system parameter failed validation, but the system should be skipped due to [`ValidationBehavior::Skip`].
/// This is suitable for system params that are intended to only operate in certain application states, such as [`Single`].
Skipped,
}
```
Then, inside of each `SystemParam` implementation, return either Valid,
Invalid or Skipped.
Currently, only `Single`, `Option<Single>` and `Populated` use the
`Skipped` behavior. Other params (like resources) retain their current
failing
## Testing
Messed around with the fallible_params example. Added a pair of tests:
one for panicking when resources are missing, and another for properly
skipping `Single` and `Populated` system params.
## To do
- [x] get https://github.com/bevyengine/bevy/pull/18454 merged
- [x] fix the todo!() in the macro-powered tuple implementation (please
help 🥺)
- [x] test
- [x] write a migration guide
- [x] update the example comments
## Migration Guide
Various system and system parameter validation methods
(`SystemParam::validate_param`, `System::validate_param` and
`System::validate_param_unsafe`) now return and accept a
`ValidationOutcome` enum, rather than a `bool`. The previous `true`
values map to `ValidationOutcome::Valid`, while `false` maps to
`ValidationOutcome::Invalid`.
However, if you wrote a custom schedule executor, you should now respect
the new `ValidationOutcome::Skipped` parameter, skipping any systems
whose validation was skipped. By contrast, `ValidationOutcome::Invalid`
systems should also be skipped, but you should call the
`default_error_handler` on them first, which by default will result in a
panic.
If you are implementing a custom `SystemParam`, you should consider
whether failing system param validation is an error or an expected
state, and choose between `Invalid` and `Skipped` accordingly. In Bevy
itself, `Single` and `Populated` now once again skip the system when
their conditions are not met. This is the 0.15.0 behavior, but stands in
contrast to the 0.15.1 behavior, where they would panic.
---------
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
Co-authored-by: Dmytro Banin <banind@cs.washington.edu>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
There are two related problems here:
1. Users should be able to change the fallback behavior of *all*
ECS-based errors in their application by setting the
`GLOBAL_ERROR_HANDLER`. See #18351 for earlier work in this vein.
2. The existing solution (#15500) for customizing this behavior is high
on boilerplate, not global and adds a great deal of complexity.
The consensus is that the default behavior when a parameter fails
validation should be set based on the kind of system parameter in
question: `Single` / `Populated` should silently skip the system, but
`Res` should panic. Setting this behavior at the system level is a
bandaid that makes getting to that ideal behavior more painful, and can
mask real failures (if a resource is missing but you've ignored a system
to make the Single stop panicking you're going to have a bad day).
I've removed the existing `ParamWarnPolicy`-based configuration, and
wired up the `GLOBAL_ERROR_HANDLER`/`default_error_handler` to the
various schedule executors to properly plumb through errors .
Additionally, I've done a small cleanup pass on the corresponding
example.
I've run the `fallible_params` example, with both the default and a
custom global error handler. The former panics (as expected), and the
latter spams the error console with warnings 🥲
1. Currently, failed system param validation will result in endless
console spam. Do you want me to implement a solution for warn_once-style
debouncing somehow?
2. Currently, the error reporting for failed system param validation is
very limited: all we get is that a system param failed validation and
the name of the system. Do you want me to implement improved error
reporting by bubbling up errors in this PR?
3. There is broad consensus that the default behavior for failed system
param validation should be set on a per-system param basis. Would you
like me to implement that in this PR?
My gut instinct is that we absolutely want to solve 2 and 3, but it will
be much easier to do that work (and review it) if we split the PRs
apart.
`ParamWarnPolicy` and the `WithParamWarnPolicy` have been removed
completely. Failures during system param validation are now handled via
the `GLOBAL_ERROR_HANDLER`: please see the `bevy_ecs::error` module docs
for more information.
---------
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
- `Once` renamed to `Warn`.
- `param_warn_once()` renamed to `warn_param_missing()`.
- `never_param_warn()` renamed to `ignore_param_missing()`.
Also includes changes to the documentation of the above methods.
Fixes#17262.
## Migration Guide
- `ParamWarnPolicy::Once` has been renamed to `ParamWarnPolicy::Warn`.
- `ParamWarnPolicy::param_warn_once` has been renamed to
`ParamWarnPolicy::warn_param_missing`.
- `ParamWarnPolicy::never_param_warn` has been renamed to
`ParamWarnPolicy::ignore_param_missing`.
# Objective
Fixes: #16578
## Solution
This is a patch fix, proper fix requires a breaking change.
Added `Panic` enum variant and using is as the system meta default.
Warn once behavior can be enabled same way disabling panic (originally
disabling wans) is.
To fix an issue with the current architecture, where **all** combinator
system params get checked together,
combinator systems only check params of the first system.
This will result in old, panicking behavior on subsequent systems and
will be fixed in 0.16.
## Testing
Ran unit tests and `fallible_params` example.
---------
Co-authored-by: François Mockers <mockersf@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
Fixes#15834
## Migration Guide
The APIs of `Time`, `Timer` and `Stopwatch` have been cleaned up for
consistency with each other and the standard library's `Duration` type.
The following methods have been renamed:
- `Stowatch::paused` -> `Stopwatch::is_paused`
- `Time::elapsed_seconds` -> `Time::elasped_secs` (including `_f64` and
`_wrapped` variants)
# Objective
Continue migration of bevy APIs to required components, following
guidance of https://hackmd.io/@bevy/required_components/
## Solution
- Make `Sprite` require `Transform` and `Visibility` and
`SyncToRenderWorld`
- move image and texture atlas handles into `Sprite`
- deprecate `SpriteBundle`
- remove engine uses of `SpriteBundle`
## Testing
ran cargo tests on bevy_sprite and tested several sprite examples.
---
## Migration Guide
Replace all uses of `SpriteBundle` with `Sprite`. There are several new
convenience constructors: `Sprite::from_image`,
`Sprite::from_atlas_image`, `Sprite::from_color`.
WARNING: use of `Handle<Image>` and `TextureAtlas` as components on
sprite entities will NO LONGER WORK. Use the fields on `Sprite` instead.
I would have removed the `Component` impls from `TextureAtlas` and
`Handle<Image>` except it is still used within ui. We should fix this
moving forward with the migration.
# Objective
Yet another PR for migrating stuff to required components. This time,
cameras!
## Solution
As per the [selected
proposal](https://hackmd.io/tsYID4CGRiWxzsgawzxG_g#Combined-Proposal-1-Selected),
deprecate `Camera2dBundle` and `Camera3dBundle` in favor of `Camera2d`
and `Camera3d`.
Adding a `Camera` without `Camera2d` or `Camera3d` now logs a warning,
as suggested by Cart [on
Discord](https://discord.com/channels/691052431525675048/1264881140007702558/1291506402832945273).
I would personally like cameras to work a bit differently and be split
into a few more components, to avoid some footguns and confusing
semantics, but that is more controversial, and shouldn't block this core
migration.
## Testing
I ran a few 2D and 3D examples, and tried cameras with and without
render graphs.
---
## Migration Guide
`Camera2dBundle` and `Camera3dBundle` have been deprecated in favor of
`Camera2d` and `Camera3d`. Inserting them will now also insert the other
components required by them automatically.
# Objective
System param validation warnings should be configurable and default to
"warn once" (per system).
Fixes: #15391
## Solution
`SystemMeta` is given a new `ParamWarnPolicy` field.
The policy decides whether warnings will be emitted by each system param
when it fails validation.
The policy is updated by the system after param validation fails.
Example warning:
```
2024-09-30T18:10:04.740749Z WARN bevy_ecs::system::function_system: System fallible_params::do_nothing_fail_validation will not run because it requested inaccessible system parameter Single<(), (With<Player>, With<Enemy>)>
```
Currently, only the first invalid parameter is displayed.
Warnings can be disabled on function systems using
`.param_never_warn()`.
(there is also `.with_param_warn_policy(policy)`)
## Testing
Ran `fallible_params` example.
---------
Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
# Objective
Add a `Populated` system parameter that acts like `Query`, but prevents
system from running if there are no matching entities.
Fixes: #15302
## Solution
Implement the system param which newtypes the `Query`.
The only change is new validation, which fails if query is empty.
The new system param is used in `fallible_params` example.
## Testing
Ran `fallible_params` example.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Add the following system params:
- `QuerySingle<D, F>` - Valid if only one matching entity exists,
- `Option<QuerySingle<D, F>>` - Valid if zero or one matching entity
exists.
As @chescock pointed out, we don't need `Mut` variants.
Fixes: #15264
## Solution
Implement the type and both variants of system params.
Also implement `ReadOnlySystemParam` for readonly queries.
Added a new ECS example `fallible_params` which showcases `SingleQuery`
usage.
In the future we might want to add `NonEmptyQuery`,
`NonEmptyEventReader` and `Res` to it (or maybe just stop at mentioning
it).
## Testing
Tested with the example.
There is a lot of warning spam so we might want to implement #15391.
