e5dc177b4b
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Joona Aalto
|
e5dc177b4b
|
Rename Trigger to On (#19596)
# 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.
|
||
|
Eagster
|
064e5e48b4
|
Remove entity placeholder from observers (#19440)
# 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. |
||
|
SpecificProtagonist
|
1c8d2ee3e1
|
viewport_node example: Remove main world image initialization (#19098)
# Objective The new viewport example allocates a texture in main memory, even though it's only needed on the GPU. Also fix an unnecessary warning when a viewport's texture doesn't exist CPU-side. ## Testing Run the `viewport_node` example. |
||
|
robtfm
|
b641aa0ecf
|
separate border colors (#18682)
# Objective allow specifying the left/top/right/bottom border colors separately for ui elements fixes #14773 ## Solution - change `BorderColor` to ```rs pub struct BorderColor { pub left: Color, pub top: Color, pub right: Color, pub bottom: Color, } ``` - generate one ui node per distinct border color, set flags for the active borders - render only the active borders i chose to do this rather than adding multiple colors to the ExtractedUiNode in order to minimize the impact for the common case where all border colors are the same. ## Testing modified the `borders` example to use separate colors:  the behaviour is a bit weird but it mirrors html/css border behaviour. --- ## Migration: To keep the existing behaviour, just change `BorderColor(color)` into `BorderColor::all(color)`. --------- Co-authored-by: ickshonpe <david.curthoys@googlemail.com> |
||
|
Antony
|
bf42cb3532
|
Add a viewport UI widget (#17253)
# Objective Add a viewport widget. ## Solution - Add a new `ViewportNode` component to turn a UI node into a viewport. - Add `viewport_picking` to pass pointer inputs from other pointers to the viewport's pointer. - Notably, this is somewhat functionally different from the viewport widget in [the editor prototype](https://github.com/bevyengine/bevy_editor_prototypes/pull/110/files#L124), which just moves the pointer's location onto the render target. Viewport widgets have their own pointers. - Care is taken to handle dragging in and out of viewports. - Add `update_viewport_render_target_size` to update the viewport node's render target's size if the node size changes. - Feature gate picking-related viewport items behind `bevy_ui_picking_backend`. ## Testing I've been using an example I made to test the widget (and added it as `viewport_node`): <details><summary>Code</summary> ```rust //! A simple scene to demonstrate spawning a viewport widget. The example will demonstrate how to //! pick entities visible in the widget's view. use bevy::picking::pointer::PointerInteraction; use bevy::prelude::*; use bevy::ui::widget::ViewportNode; use bevy::{ image::{TextureFormatPixelInfo, Volume}, window::PrimaryWindow, }; use bevy_render::{ camera::RenderTarget, render_resource::{ Extent3d, TextureDescriptor, TextureDimension, TextureFormat, TextureUsages, }, }; fn main() { App::new() .add_plugins((DefaultPlugins, MeshPickingPlugin)) .add_systems(Startup, test) .add_systems(Update, draw_mesh_intersections) .run(); } #[derive(Component, Reflect, Debug)] #[reflect(Component)] struct Shape; fn test( mut commands: Commands, window: Query<&Window, With<PrimaryWindow>>, mut images: ResMut<Assets<Image>>, mut meshes: ResMut<Assets<Mesh>>, mut materials: ResMut<Assets<StandardMaterial>>, ) { // Spawn a UI camera commands.spawn(Camera3d::default()); // Set up an texture for the 3D camera to render to let window = window.get_single().unwrap(); let window_size = window.physical_size(); let size = Extent3d { width: window_size.x, height: window_size.y, ..default() }; let format = TextureFormat::Bgra8UnormSrgb; let image = Image { data: Some(vec![0; size.volume() * format.pixel_size()]), texture_descriptor: TextureDescriptor { label: None, size, dimension: TextureDimension::D2, format, mip_level_count: 1, sample_count: 1, usage: TextureUsages::TEXTURE_BINDING | TextureUsages::COPY_DST | TextureUsages::RENDER_ATTACHMENT, view_formats: &[], }, ..default() }; let image_handle = images.add(image); // Spawn the 3D camera let camera = commands .spawn(( Camera3d::default(), Camera { // Render this camera before our UI camera order: -1, target: RenderTarget::Image(image_handle.clone().into()), ..default() }, )) .id(); // Spawn something for the 3D camera to look at commands .spawn(( Mesh3d(meshes.add(Cuboid::new(5.0, 5.0, 5.0))), MeshMaterial3d(materials.add(Color::WHITE)), Transform::from_xyz(0.0, 0.0, -10.0), Shape, )) // We can observe pointer events on our objects as normal, the // `bevy::ui::widgets::viewport_picking` system will take care of ensuring our viewport // clicks pass through .observe(on_drag_cuboid); // Spawn our viewport widget commands .spawn(( Node { position_type: PositionType::Absolute, top: Val::Px(50.0), left: Val::Px(50.0), width: Val::Px(200.0), height: Val::Px(200.0), border: UiRect::all(Val::Px(5.0)), ..default() }, BorderColor(Color::WHITE), ViewportNode::new(camera), )) .observe(on_drag_viewport); } fn on_drag_viewport(drag: Trigger<Pointer<Drag>>, mut node_query: Query<&mut Node>) { if matches!(drag.button, PointerButton::Secondary) { let mut node = node_query.get_mut(drag.target()).unwrap(); if let (Val::Px(top), Val::Px(left)) = (node.top, node.left) { node.left = Val::Px(left + drag.delta.x); node.top = Val::Px(top + drag.delta.y); }; } } fn on_drag_cuboid(drag: Trigger<Pointer<Drag>>, mut transform_query: Query<&mut Transform>) { if matches!(drag.button, PointerButton::Primary) { let mut transform = transform_query.get_mut(drag.target()).unwrap(); transform.rotate_y(drag.delta.x * 0.02); transform.rotate_x(drag.delta.y * 0.02); } } fn draw_mesh_intersections( pointers: Query<&PointerInteraction>, untargetable: Query<Entity, Without<Shape>>, mut gizmos: Gizmos, ) { for (point, normal) in pointers .iter() .flat_map(|interaction| interaction.iter()) .filter_map(|(entity, hit)| { if !untargetable.contains(*entity) { hit.position.zip(hit.normal) } else { None } }) { gizmos.arrow(point, point + normal.normalize() * 0.5, Color::WHITE); } } ``` </details> ## Showcase https://github.com/user-attachments/assets/39f44eac-2c2a-4fd9-a606-04171f806dc1 ## Open Questions - <del>Not sure whether the entire widget should be feature gated behind `bevy_ui_picking_backend` or not? I chose a partial approach since maybe someone will want to use the widget without any picking being involved.</del> - <del>Is `PickSet::Last` the expected set for `viewport_picking`? Perhaps `PickSet::Input` is more suited.</del> - <del>Can `dragged_last_frame` be removed in favor of a better dragging check? Another option that comes to mind is reading `Drag` and `DragEnd` events, but this seems messier.</del> --------- Co-authored-by: ickshonpe <david.curthoys@googlemail.com> Co-authored-by: François Mockers <mockersf@gmail.com> |