6 Commits

Author	SHA1	Message	Date
Joona Aalto	38c3423693	Event Split: Event, EntityEvent, and BufferedEvent (#19647) ... # Objective Closes #19564. The current `Event` trait looks like this: ```rust pub trait Event: Send + Sync + 'static { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } ``` The `Event` trait is used by both buffered events (`EventReader`/`EventWriter`) and observer events. If they are observer events, they can optionally be targeted at specific `Entity`s or `ComponentId`s, and can even be propagated to other entities. However, there has long been a desire to split the trait semantically for a variety of reasons, see #14843, #14272, and #16031 for discussion. Some reasons include: - It's very uncommon to use a single event type as both a buffered event and targeted observer event. They are used differently and tend to have distinct semantics. - A common footgun is using buffered events with observers or event readers with observer events, as there is no type-level error that prevents this kind of misuse. - #19440 made `Trigger::target` return an `Option<Entity>`. This *seriously* hurts ergonomics for the general case of entity observers, as you need to `.unwrap()` each time. If we could statically determine whether the event is expected to have an entity target, this would be unnecessary. There's really two main ways that we can categorize events: push vs. pull (i.e. "observer event" vs. "buffered event") and global vs. targeted: | | Push | Pull | | ------------ | --------------- | --------------------------- | | **Global** | Global observer | `EventReader`/`EventWriter` | | **Targeted** | Entity observer | - | There are many ways to approach this, each with their tradeoffs. Ultimately, we kind of want to split events both ways: - A type-level distinction between observer events and buffered events, to prevent people from using the wrong kind of event in APIs - A statically designated entity target for observer events to avoid accidentally using untargeted events for targeted APIs This PR achieves these goals by splitting event traits into `Event`, `EntityEvent`, and `BufferedEvent`, with `Event` being the shared trait implemented by all events. ## `Event`, `EntityEvent`, and `BufferedEvent` `Event` is now a very simple trait shared by all events. ```rust pub trait Event: Send + Sync + 'static { // Required for observer APIs fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } ``` You can call `trigger` for *any* event, and use a global observer for listening to the event. ```rust #[derive(Event)] struct Speak { message: String, } // ... app.add_observer(|trigger: On<Speak>| { println!("{}", trigger.message); }); // ... commands.trigger(Speak { message: "Y'all like these reworked events?".to_string(), }); ``` To allow an event to be targeted at entities and even propagated further, you can additionally implement the `EntityEvent` trait: ```rust pub trait EntityEvent: Event { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; } ``` This lets you call `trigger_targets`, and to use targeted observer APIs like `EntityCommands::observe`: ```rust #[derive(Event, EntityEvent)] #[entity_event(traversal = &'static ChildOf, auto_propagate)] struct Damage { amount: f32, } // ... let enemy = commands.spawn((Enemy, Health(100.0))).id(); // Spawn some armor as a child of the enemy entity. // When the armor takes damage, it will bubble the event up to the enemy. let armor_piece = commands .spawn((ArmorPiece, Health(25.0), ChildOf(enemy))) .observe(|trigger: On<Damage>, mut query: Query<&mut Health>| { // Note: `On::target` only exists because this is an `EntityEvent`. let mut health = query.get(trigger.target()).unwrap(); health.0 -= trigger.amount(); }); commands.trigger_targets(Damage { amount: 10.0 }, armor_piece); ``` > [!NOTE] > You *can* still also trigger an `EntityEvent` without targets using `trigger`. We probably *could* make this an either-or thing, but I'm not sure that's actually desirable. To allow an event to be used with the buffered API, you can implement `BufferedEvent`: ```rust pub trait BufferedEvent: Event {} ``` The event can then be used with `EventReader`/`EventWriter`: ```rust #[derive(Event, BufferedEvent)] struct Message(String); fn write_hello(mut writer: EventWriter<Message>) { writer.write(Message("I hope these examples are alright".to_string())); } fn read_messages(mut reader: EventReader<Message>) {     // Process all buffered events of type `Message`.   for Message(message) in reader.read() {         println!("{message}");   } } ``` In summary: - Need a basic event you can trigger and observe? Derive `Event`! - Need the event to be targeted at an entity? Derive `EntityEvent`! - Need the event to be buffered and support the `EventReader`/`EventWriter` API? Derive `BufferedEvent`! ## Alternatives I'll now cover some of the alternative approaches I have considered and briefly explored. I made this section collapsible since it ended up being quite long :P <details> <summary>Expand this to see alternatives</summary> ### 1. Unified `Event` Trait One option is not to have *three* separate traits (`Event`, `EntityEvent`, `BufferedEvent`), and to instead just use associated constants on `Event` to determine whether an event supports targeting and buffering or not: ```rust pub trait Event: Send + Sync + 'static { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; const TARGETED: bool = false; const BUFFERED: bool = false; fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } ``` Methods can then use bounds like `where E: Event<TARGETED = true>` or `where E: Event<BUFFERED = true>` to limit APIs to specific kinds of events. This would keep everything under one `Event` trait, but I don't think it's necessarily a good idea. It makes APIs harder to read, and docs can't easily refer to specific types of events. You can also create weird invariants: what if you specify `TARGETED = false`, but have `Traversal` and/or `AUTO_PROPAGATE` enabled? ### 2. `Event` and `Trigger` Another option is to only split the traits between buffered events and observer events, since that is the main thing people have been asking for, and they have the largest API difference. If we did this, I think we would need to make the terms *clearly* separate. We can't really use `Event` and `BufferedEvent` as the names, since it would be strange that `BufferedEvent` doesn't implement `Event`. Something like `ObserverEvent` and `BufferedEvent` could work, but it'd be more verbose. For this approach, I would instead keep `Event` for the current `EventReader`/`EventWriter` API, and call the observer event a `Trigger`, since the "trigger" terminology is already used in the observer context within Bevy (both as a noun and a verb). This is also what a long [bikeshed on Discord](https://discord.com/channels/691052431525675048/749335865876021248/1298057661878898791) seemed to land on at the end of last year. ```rust // For `EventReader`/`EventWriter` pub trait Event: Send + Sync + 'static {} // For observers pub trait Trigger: Send + Sync + 'static { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; const TARGETED: bool = false; fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } ``` The problem is that "event" is just a really good term for something that "happens". Observers are rapidly becoming the more prominent API, so it'd be weird to give them the `Trigger` name and leave the good `Event` name for the less common API. So, even though a split like this seems neat on the surface, I think it ultimately wouldn't really work. We want to keep the `Event` name for observer events, and there is no good alternative for the buffered variant. (`Message` was suggested, but saying stuff like "sends a collision message" is weird.) ### 3. `GlobalEvent` + `TargetedEvent` What if instead of focusing on the buffered vs. observed split, we *only* make a distinction between global and targeted events? ```rust // A shared event trait to allow global observers to work pub trait Event: Send + Sync + 'static { fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } // For buffered events and non-targeted observer events pub trait GlobalEvent: Event {} // For targeted observer events pub trait TargetedEvent: Event { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; } ``` This is actually the first approach I implemented, and it has the neat characteristic that you can only use non-targeted APIs like `trigger` with a `GlobalEvent` and targeted APIs like `trigger_targets` with a `TargetedEvent`. You have full control over whether the entity should or should not have a target, as they are fully distinct at the type-level. However, there's a few problems: - There is no type-level indication of whether a `GlobalEvent` supports buffered events or just non-targeted observer events - An `Event` on its own does literally nothing, it's just a shared trait required to make global observers accept both non-targeted and targeted events - If an event is both a `GlobalEvent` and `TargetedEvent`, global observers again have ambiguity on whether an event has a target or not, undermining some of the benefits - The names are not ideal ### 4. `Event` and `EntityEvent` We can fix some of the problems of Alternative 3 by accepting that targeted events can also be used in non-targeted contexts, and simply having the `Event` and `EntityEvent` traits: ```rust // For buffered events and non-targeted observer events pub trait Event: Send + Sync + 'static { fn register_component_id(world: &mut World) -> ComponentId { ... } fn component_id(world: &World) -> Option<ComponentId> { ... } } // For targeted observer events pub trait EntityEvent: Event { type Traversal: Traversal<Self>; const AUTO_PROPAGATE: bool = false; } ``` This is essentially identical to this PR, just without a dedicated `BufferedEvent`. The remaining major "problem" is that there is still zero type-level indication of whether an `Event` event *actually* supports the buffered API. This leads us to the solution proposed in this PR, using `Event`, `EntityEvent`, and `BufferedEvent`. </details> ## Conclusion The `Event` + `EntityEvent` + `BufferedEvent` split proposed in this PR aims to solve all the common problems with Bevy's current event model while keeping the "weirdness" factor minimal. It splits in terms of both the push vs. pull *and* global vs. targeted aspects, while maintaining a shared concept for an "event". ### Why I Like This - The term "event" remains as a single concept for all the different kinds of events in Bevy. - Despite all event types being "events", they use fundamentally different APIs. Instead of assuming that you can use an event type with any pattern (when only one is typically supported), you explicitly opt in to each one with dedicated traits. - Using separate traits for each type of event helps with documentation and clearer function signatures. - I can safely make assumptions on expected usage. - If I see that an event is an `EntityEvent`, I can assume that I can use `observe` on it and get targeted events. - If I see that an event is a `BufferedEvent`, I can assume that I can use `EventReader` to read events. - If I see both `EntityEvent` and `BufferedEvent`, I can assume that both APIs are supported. In summary: This allows for a unified concept for events, while limiting the different ways to use them with opt-in traits. No more guess-work involved when using APIs. ### Problems? - Because `BufferedEvent` implements `Event` (for more consistent semantics etc.), you can still use all buffered events for non-targeted observers. I think this is fine/good. The important part is that if you see that an event implements `BufferedEvent`, you know that the `EventReader`/`EventWriter` API should be supported. Whether it *also* supports other APIs is secondary. - I currently only support `trigger_targets` for an `EntityEvent`. However, you can technically target components too, without targeting any entities. I consider that such a niche and advanced use case that it's not a huge problem to only support it for `EntityEvent`s, but we could also split `trigger_targets` into `trigger_entities` and `trigger_components` if we wanted to (or implement components as entities :P). - You can still trigger an `EntityEvent` *without* targets. I consider this correct, since `Event` implements the non-targeted behavior, and it'd be weird if implementing another trait *removed* behavior. However, it does mean that global observers for entity events can technically return `Entity::PLACEHOLDER` again (since I got rid of the `Option<Entity>` added in #19440 for ergonomics). I think that's enough of an edge case that it's not a huge problem, but it is worth keeping in mind. - ~~Deriving both `EntityEvent` and `BufferedEvent` for the same type currently duplicates the `Event` implementation, so you instead need to manually implement one of them.~~ Changed to always requiring `Event` to be derived. ## Related Work There are plans to implement multi-event support for observers, especially for UI contexts. [Cart's example](https://github.com/bevyengine/bevy/issues/14649#issuecomment-2960402508) API looked like this: ```rust // Truncated for brevity trigger: Trigger<( OnAdd<Pressed>, OnRemove<Pressed>, OnAdd<InteractionDisabled>, OnRemove<InteractionDisabled>, OnInsert<Hovered>, )>, ``` I believe this shouldn't be in conflict with this PR. If anything, this PR might *help* achieve the multi-event pattern for entity observers with fewer footguns: by statically enforcing that all of these events are `EntityEvent`s in the context of `EntityCommands::observe`, we can avoid misuse or weird cases where *some* events inside the trigger are targeted while others are not.	2025-06-15 16:46:34 +00:00
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.	2025-06-12 18:22:33 +00:00
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.	2025-06-09 19:37:56 +00:00
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.	2025-05-26 17:20:29 +00:00
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>	2025-05-26 16:57:13 +00:00
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>	2025-05-05 22:57:37 +00:00