From 6b5289bd5eafc2981e829ef8921766a05c6715e4 Mon Sep 17 00:00:00 2001 From: theotherphil Date: Sun, 8 Jun 2025 17:28:31 +0100 Subject: [PATCH 1/2] deny(missing_docs) for bevy_ecs_macros (#19523) # Objective Deny missing docs for bevy_ecs_macros, towards https://github.com/bevyengine/bevy/issues/3492. ## Solution More docs of the form ``` /// Does the thing fn do_the_thing() {} ``` But I don't think the derive macros are where anyone is going to be looking for details of these concepts and deny(missing_docs) inevitably results in some items having noddy docs. --- crates/bevy_ecs/macros/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 114aff642b..133e5e9eaa 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -1,4 +1,5 @@ -#![expect(missing_docs, reason = "Not all docs are written yet, see #3492.")] +//! Macros for deriving ECS traits. + #![cfg_attr(docsrs, feature(doc_auto_cfg))] extern crate proc_macro; @@ -29,6 +30,7 @@ enum BundleFieldKind { const BUNDLE_ATTRIBUTE_NAME: &str = "bundle"; const BUNDLE_ATTRIBUTE_IGNORE_NAME: &str = "ignore"; +/// Implement the `Bundle` trait. #[proc_macro_derive(Bundle, attributes(bundle))] pub fn derive_bundle(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); @@ -187,6 +189,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { }) } +/// Implement the `MapEntities` trait. #[proc_macro_derive(MapEntities, attributes(entities))] pub fn derive_map_entities(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); @@ -522,16 +525,19 @@ pub(crate) fn bevy_ecs_path() -> syn::Path { BevyManifest::shared().get_path("bevy_ecs") } +/// Implement the `Event` trait. #[proc_macro_derive(Event, attributes(event))] pub fn derive_event(input: TokenStream) -> TokenStream { component::derive_event(input) } +/// Implement the `Resource` trait. #[proc_macro_derive(Resource)] pub fn derive_resource(input: TokenStream) -> TokenStream { component::derive_resource(input) } +/// Implement the `Component` trait. #[proc_macro_derive( Component, attributes(component, require, relationship, relationship_target, entities) @@ -540,6 +546,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream { component::derive_component(input) } +/// Implement the `FromWorld` trait. #[proc_macro_derive(FromWorld, attributes(from_world))] pub fn derive_from_world(input: TokenStream) -> TokenStream { let bevy_ecs_path = bevy_ecs_path(); From 2c37bdeb47fb2ea00dde5172d1047f85dedcfc47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20S=C3=B8holm?= Date: Sun, 8 Jun 2025 18:29:13 +0200 Subject: [PATCH 2/2] Fix PickingInteraction change detection (#19488) # Objective Fixes #19464 ## Solution Instead of clearing previous `PickingInteractions` before updating, we clear them last for those components that weren't updated, and use `set_if_neq` when writing. ## Testing I tried the sprite_picking example and it still works. You can add the following system to picking examples to check that change detection works as intended: ```rust fn print_picking(query: Query<(Entity, &PickingInteraction), Changed>) { for (entity, interaction) in &query { println!("{entity} {interaction:?}"); } } ``` --- crates/bevy_picking/src/hover.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/crates/bevy_picking/src/hover.rs b/crates/bevy_picking/src/hover.rs index 6347568c02..2bf23c50ba 100644 --- a/crates/bevy_picking/src/hover.rs +++ b/crates/bevy_picking/src/hover.rs @@ -208,18 +208,6 @@ pub fn update_interactions( mut pointers: Query<(&PointerId, &PointerPress, &mut PointerInteraction)>, mut interact: Query<&mut PickingInteraction>, ) { - // Clear all previous hover data from pointers and entities - for (pointer, _, mut pointer_interaction) in &mut pointers { - pointer_interaction.sorted_entities.clear(); - if let Some(previously_hovered_entities) = previous_hover_map.get(pointer) { - for entity in previously_hovered_entities.keys() { - if let Ok(mut interaction) = interact.get_mut(*entity) { - *interaction = PickingInteraction::None; - } - } - } - } - // Create a map to hold the aggregated interaction for each entity. This is needed because we // need to be able to insert the interaction component on entities if they do not exist. To do // so we need to know the final aggregated interaction state to avoid the scenario where we set @@ -239,13 +227,29 @@ pub fn update_interactions( } // Take the aggregated entity states and update or insert the component if missing. - for (hovered_entity, new_interaction) in new_interaction_state.drain() { + for (&hovered_entity, &new_interaction) in new_interaction_state.iter() { if let Ok(mut interaction) = interact.get_mut(hovered_entity) { - *interaction = new_interaction; + interaction.set_if_neq(new_interaction); } else if let Ok(mut entity_commands) = commands.get_entity(hovered_entity) { entity_commands.try_insert(new_interaction); } } + + // Clear all previous hover data from pointers that are no longer hovering any entities. + // We do this last to preserve change detection for picking interactions. + for (pointer, _, _) in &mut pointers { + let Some(previously_hovered_entities) = previous_hover_map.get(pointer) else { + continue; + }; + + for entity in previously_hovered_entities.keys() { + if !new_interaction_state.contains_key(entity) { + if let Ok(mut interaction) = interact.get_mut(*entity) { + interaction.set_if_neq(PickingInteraction::None); + } + } + } + } } /// Merge the interaction state of this entity into the aggregated map.