# Objective
Currently, the observer API looks like this:
```rust
app.add_observer(|trigger: Trigger<Explode>| {
info!("Entity {} exploded!", trigger.target());
});
```
Future plans for observers also include "multi-event observers" with a
trigger that looks like this (see [Cart's
example](https://github.com/bevyengine/bevy/issues/14649#issuecomment-2960402508)):
```rust
trigger: Trigger<(
OnAdd<Pressed>,
OnRemove<Pressed>,
OnAdd<InteractionDisabled>,
OnRemove<InteractionDisabled>,
OnInsert<Hovered>,
)>,
```
In scenarios like this, there is a lot of repetition of `On`. These are
expected to be very high-traffic APIs especially in UI contexts, so
ergonomics and readability are critical.
By renaming `Trigger` to `On`, we can make these APIs read more cleanly
and get rid of the repetition:
```rust
app.add_observer(|trigger: On<Explode>| {
info!("Entity {} exploded!", trigger.target());
});
```
```rust
trigger: On<(
Add<Pressed>,
Remove<Pressed>,
Add<InteractionDisabled>,
Remove<InteractionDisabled>,
Insert<Hovered>,
)>,
```
Names like `On<Add<Pressed>>` emphasize the actual event listener nature
more than `Trigger<OnAdd<Pressed>>`, and look cleaner. This *also* frees
up the `Trigger` name if we want to use it for the observer event type,
splitting them out from buffered events (bikeshedding this is out of
scope for this PR though).
For prior art:
[`bevy_eventlistener`](https://github.com/aevyrie/bevy_eventlistener)
used
[`On`](https://docs.rs/bevy_eventlistener/latest/bevy_eventlistener/event_listener/struct.On.html)
for its event listener type. Though in our case, the observer is the
event listener, and `On` is just a type containing information about the
triggered event.
## Solution
Steal from `bevy_event_listener` by @aevyrie and use `On`.
- Rename `Trigger` to `On`
- Rename `OnAdd` to `Add`
- Rename `OnInsert` to `Insert`
- Rename `OnReplace` to `Replace`
- Rename `OnRemove` to `Remove`
- Rename `OnDespawn` to `Despawn`
## Discussion
### Naming Conflicts??
Using a name like `Add` might initially feel like a very bad idea, since
it risks conflict with `core::ops::Add`. However, I don't expect this to
be a big problem in practice.
- You rarely need to actually implement the `Add` trait, especially in
modules that would use the Bevy ECS.
- In the rare cases where you *do* get a conflict, it is very easy to
fix by just disambiguating, for example using `ops::Add`.
- The `Add` event is a struct while the `Add` trait is a trait (duh), so
the compiler error should be very obvious.
For the record, renaming `OnAdd` to `Add`, I got exactly *zero* errors
or conflicts within Bevy itself. But this is of course not entirely
representative of actual projects *using* Bevy.
You might then wonder, why not use `Added`? This would conflict with the
`Added` query filter, so it wouldn't work. Additionally, the current
naming convention for observer events does not use past tense.
### Documentation
This does make documentation slightly more awkward when referring to
`On` or its methods. Previous docs often referred to `Trigger::target`
or "sends a `Trigger`" (which is... a bit strange anyway), which would
now be `On::target` and "sends an observer `Event`".
You can see the diff in this PR to see some of the effects. I think it
should be fine though, we may just need to reword more documentation to
read better.
# Objective
#19366 implemented core button widgets, which included the `Depressed`
state component.
`Depressed` was chosen instead of `Pressed` to avoid conflict with the
`Pointer<Pressed>` event, but it is problematic and awkward in many
ways:
- Using the word "depressed" for such a high-traffic type is not great
due to the obvious connection to "depressed" as in depression.
- "Depressed" is not what I would search for if I was looking for a
component like this, and I'm not aware of any other engine or UI
framework using the term.
- `Depressed` is not a very natural pair to the `Pointer<Pressed>`
event.
- It might be because I'm not a native English speaker, but I have very
rarely heard someone say "a button is depressed". Seeing it, my mind
initially goes from "depression??" to "oh, de-pressed, meaning released"
and definitely not "is pressed", even though that *is* also a valid
meaning for it.
A related problem is that the current `Pointer<Pressed>` and
`Pointer<Released>` event names use a different verb tense than all of
our other observer events such as `Pointer<Click>` or
`Pointer<DragStart>`. By fixing this and renaming `Pressed` (and
`Released`), we can then use `Pressed` instead of `Depressed` for the
state component.
Additionally, the `IsHovered` and `IsDirectlyHovered` components added
in #19366 use an inconsistent naming; the other similar components don't
use an `Is` prefix. It also makes query filters like `Has<IsHovered>`
and `With<IsHovered>` a bit more awkward.
This is partially related to Cart's [picking concept
proposal](https://gist.github.com/cart/756e48a149db2838028be600defbd24a?permalink_comment_id=5598154).
## Solution
- Rename `Pointer<Pressed>` to `Pointer<Press>`
- Rename `Pointer<Released>` to `Pointer<Release>`
- Rename `Depressed` to `Pressed`
- Rename `IsHovered` to `Hovered`
- Rename `IsDirectlyHovered` to `DirectlyHovered`
# Objective
`Entity::PLACEHOLDER` acts as a magic number that will *probably* never
really exist, but it certainly could. And, `Entity` has a niche, so the
only reason to use `PLACEHOLDER` is as an alternative to `MaybeUninit`
that trades safety risks for logic risks.
As a result, bevy has generally advised against using `PLACEHOLDER`, but
we still use if for a lot internally. This pr starts removing internal
uses of it, starting from observers.
## Solution
Change all trigger target related types from `Entity` to
`Option<Entity>`
Small migration guide to come.
## Testing
CI
## Future Work
This turned a lot of code from
```rust
trigger.target()
```
to
```rust
trigger.target().unwrap()
```
The extra panic is no worse than before; it's just earlier than
panicking after passing the placeholder to something else.
But this is kinda annoying.
I would like to add a `TriggerMode` or something to `Event` that would
restrict what kinds of targets can be used for that event. Many events
like `Removed` etc, are always triggered with a target. We can make
those have a way to assume Some, etc. But I wanted to save that for a
future pr.
## Objective
Fix the misleading 2d anchor API where `Anchor` is a component and
required by `Text2d` but is stored on a field for sprites.
Fixes#18367
## Solution
Remove the `anchor` field from `Sprite` and require `Anchor` instead.
## Migration Guide
The `anchor` field has been removed from `Sprite`. Instead the `Anchor`
component is now a required component on `Sprite`.
# Objective
The `Anchor` component doesn't need to be a enum. The variants are just
mapped to `Vec2`s so it could be changed to a newtype with associated
const values, saving the space needed for the discriminator by the enum.
Also there was no benefit I think in hiding the underlying `Vec2`
representation of `Anchor`s.
Suggested by @atlv24.
Fixes#18459Fixes#18460
## Solution
Change `Anchor` to a struct newtyping a `Vec2`, and its variants into
associated constants.
## Migration Guide
The anchor component has been changed from an enum to a struct newtyping
a `Vec2`. The `Custom` variant has been removed, instead to construct a
custom `Anchor` use its tuple constructor:
```rust
Sprite {
anchor: Anchor(Vec2::new(0.25, 0.4)),
..default()
}
```
The other enum variants have been replaced with corresponding constants:
* `Anchor::BottomLeft` to `Anchor::BOTTOM_LEFT`
* `Anchor::Center` to `Anchor::CENTER`
* `Anchor::TopRight` to `Anchor::TOP_RIGHT`
* .. and so on for the remaining variants
# Objective
@cart noticed some issues with my work in
https://github.com/bevyengine/bevy/pull/17348#discussion_r2001815637,
which I somehow missed before merging the PR.
## Solution
- feature gate the UiPickingPlugin correctly
- don't manually add the picking plugins
## Testing
Ran the debug_picking and sprite_picking examples (for UI and sprites
respectively): both seem to work fine.
# Objective
Currently, our picking backends are inconsistent:
- Mesh picking and sprite picking both have configurable opt in/out
behavior. UI picking does not.
- Sprite picking uses `SpritePickingCamera` and `Pickable` for control,
but mesh picking uses `RayCastPickable`.
- `MeshPickingPlugin` is not a part of `DefaultPlugins`.
`SpritePickingPlugin` and `UiPickingPlugin` are.
## Solution
- Add configurable opt in/out behavior to UI picking (defaults to opt
out).
- Replace `RayCastPickable` with `MeshPickingCamera` and `Pickable`.
- Remove `SpritePickingPlugin` and `UiPickingPlugin` from
`DefaultPlugins`.
## Testing
Ran some examples.
## Migration Guide
`UiPickingPlugin` and `SpritePickingPlugin` are no longer included in
`DefaultPlugins`. They must be explicitly added.
`RayCastPickable` has been replaced in favor of the `MeshPickingCamera`
and `Pickable` components. You should add them to cameras and entities,
respectively, if you have `MeshPickingSettings::require_markers` set to
`true`.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fix https://github.com/bevyengine/bevy/issues/17108
See
https://github.com/bevyengine/bevy/issues/17108#issuecomment-2653020889
## Solution
- Make the query match `&Pickable` instead `Option<&Pickable>`
## Testing
- Run the `sprite_picking` example and everything still work
## Migration Guide
- Sprite picking are now opt-in, make sure you insert `Pickable`
component when using sprite picking.
```diff
-commands.spawn(Sprite { .. } );
+commands.spawn((Sprite { .. }, Pickable::default());
```
# Objective
Fixes#16903.
## Solution
- Make sprite picking opt-in by requiring a new `SpritePickingCamera`
component for cameras and usage of a new `Pickable` component for
entities.
- Update the `sprite_picking` example to reflect these changes.
- Some reflection cleanup (I hope that's ok).
## Testing
Ran the `sprite_picking` example
## Open Questions
<del>
<ul>
<li>Is the name `SpritePickable` appropriate?</li>
<li>Should `SpritePickable` be in `bevy_sprite::prelude?</li>
</ul>
</del>
## Migration Guide
The sprite picking backend is now strictly opt-in using the
`SpritePickingCamera` and `Pickable` components. You should add the
`Pickable` component any entities that you want sprite picking to be
enabled for, and mark their respective cameras with
`SpritePickingCamera`.
# Objective
Fixes#16192
## Solution
I renamed the Pointer<Down/Up> to <Pressed/Released> and then I resolved
all the errors.
Renamed variables like "is_down" to "is_pressed" to maintain
consistency.
Modified the docs in places where 'down/up' were used to maintain
consistency.
## Testing
I haven't tested this in any way beside the checks from rust analyzer
and the examples in the examples/ directory.
---
## Migration Guide
### `bevy_picking/src/pointer.rs`:
#### `enum PressDirection`:
- `PressDirection::Down` changes to `PressDirection::Pressed`.
- `PressDirection::Up` changes to `PressDirection::Released`.
These changes are also relevant when working with `enum PointerAction`
### `bevy_picking/src/events.rs`:
Clicking and pressing Events in events.rs categories change from [Down],
[Up], [Click] to [Pressed], [Released], [Click].
- `struct Down` changes to `struct Pressed` - fires when a pointer
button is pressed over the 'target' entity.
- `struct Up` changes to `struct Released` - fires when a pointer button
is released over the 'target' entity.
- `struct Click` now fires when a pointer sends a Pressed event followed
by a Released event on the same 'target'.
- `struct DragStart` now fires when the 'target' entity receives a
pointer Pressed event followed by a pointer Move event.
- `struct DragEnd` now fires when the 'target' entity is being dragged
and receives a pointer Released event.
- `PickingEventWriters<'w>::down_events: EventWriter<'w, Pointer<Down>>`
changes to `PickingEventWriters<'w>::pressed_events: EventWriter<'w,
Pointer<Pressed>>`.
- `PickingEventWriters<'w>::up_events changes to
PickingEventWriters<'w>::released_events`.
---------
Co-authored-by: Harun Ibram <harun.ibram@outlook.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- A `Trigger` has multiple associated `Entity`s - the entity observing
the event, and the entity that was targeted by the event.
- The field `entity: Entity` encodes no semantic information about what
the entity is used for, you can already tell that it's an `Entity` by
the type signature!
## Solution
- Rename `trigger.entity()` to `trigger.target()`
---
## Changelog
- `Trigger`s are associated with multiple entities. `Trigger::entity()`
has been renamed to `Trigger::target()` to reflect the semantics of the
entity being returned.
## Migration Guide
- Rename `Trigger::entity()` to `Trigger::target()`.
- Rename `ObserverTrigger::entity` to `ObserverTrigger::target`
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
- Required components replace bundles, but `SpatialBundle` is yet to be
deprecated
## Solution
- Deprecate `SpatialBundle`
- Insert `Transform` and `Visibility` instead in examples using it
- In `spawn` or `insert` inserting a default `Transform` or `Visibility`
with component already requiring either, remove those components from
the tuple
## Testing
- Did you test these changes? If so, how?
Yes, I ran the examples I changed and tests
- Are there any parts that need more testing?
The `gamepad_viewer` and and `custom_shader_instancing` examples don't
work as intended due to entirely unrelated code, didn't check main.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
Run examples, or just check that all spawned values are identical
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Linux, wayland trough x11 (cause that's the default feature)
---
## Migration Guide
`SpatialBundle` is now deprecated, insert `Transform` and `Visibility`
instead which will automatically insert all other components that were
in the bundle. If you do not specify these values and any other
components in your `spawn`/`insert` call already requires either of
these components you can leave that one out.
before:
```rust
commands.spawn(SpatialBundle::default());
```
after:
```rust
commands.spawn((Transform::default(), Visibility::default());
```
# 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
- Fixes#15236
## Solution
- Use bevy_math::ops instead of std floating point operations.
## Testing
- Did you test these changes? If so, how?
Unit tests and `cargo run -p ci -- test`
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
Execute `cargo run -p ci -- test` on Windows.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Windows
## Migration Guide
- Not a breaking change
- Projects should use bevy math where applicable
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: IQuick 143 <IQuick143cz@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
# Objective
Add `bevy_picking` sprite backend as part of the `bevy_mod_picking`
upstreamening (#12365).
## Solution
More or less a copy/paste from `bevy_mod_picking`, with the changes
[here](https://github.com/aevyrie/bevy_mod_picking/pull/354). I'm
putting that link here since those changes haven't yet made it through
review, so should probably be reviewed on their own.
## Testing
I couldn't find any sprite-backend-specific tests in `bevy_mod_picking`
and unfortunately I'm not familiar enough with Bevy's testing patterns
to write tests for code that relies on windowing and input. I'm willing
to break the pointer hit system into testable blocks and add some more
modular tests if that's deemed important enough to block, otherwise I
can open an issue for adding tests as follow-up.
## Follow-up work
- More docs/tests
- Ignore pick events on transparent sprite pixels with potential opt-out
---------
Co-authored-by: Aevyrie <aevyrie@gmail.com>
