diff --git a/crates/bevy_hierarchy/Cargo.toml b/crates/bevy_hierarchy/Cargo.toml index 288f1120e7..471e19a3c6 100644 --- a/crates/bevy_hierarchy/Cargo.toml +++ b/crates/bevy_hierarchy/Cargo.toml @@ -14,7 +14,9 @@ trace = [] [dependencies] # bevy bevy_app = { path = "../bevy_app", version = "0.9.0-dev" } +bevy_core = { path = "../bevy_core", version = "0.9.0-dev" } bevy_ecs = { path = "../bevy_ecs", version = "0.9.0-dev", features = ["bevy_reflect"] } +bevy_log = { path = "../bevy_log", version = "0.9.0-dev" } bevy_reflect = { path = "../bevy_reflect", version = "0.9.0-dev", features = ["bevy"] } bevy_utils = { path = "../bevy_utils", version = "0.9.0-dev" } diff --git a/crates/bevy_hierarchy/src/lib.rs b/crates/bevy_hierarchy/src/lib.rs index 60d55f7b8c..1f8c08d9c7 100644 --- a/crates/bevy_hierarchy/src/lib.rs +++ b/crates/bevy_hierarchy/src/lib.rs @@ -16,10 +16,15 @@ pub use child_builder::*; mod events; pub use events::*; +mod valid_parent_check_plugin; +pub use valid_parent_check_plugin::*; + #[doc(hidden)] pub mod prelude { #[doc(hidden)] - pub use crate::{child_builder::*, components::*, hierarchy::*, HierarchyPlugin}; + pub use crate::{ + child_builder::*, components::*, hierarchy::*, HierarchyPlugin, ValidParentCheckPlugin, + }; } use bevy_app::prelude::*; diff --git a/crates/bevy_hierarchy/src/valid_parent_check_plugin.rs b/crates/bevy_hierarchy/src/valid_parent_check_plugin.rs new file mode 100644 index 0000000000..153041cf99 --- /dev/null +++ b/crates/bevy_hierarchy/src/valid_parent_check_plugin.rs @@ -0,0 +1,89 @@ +use std::marker::PhantomData; + +use bevy_app::{App, CoreStage, Plugin}; +use bevy_core::Name; +use bevy_ecs::{prelude::*, schedule::ShouldRun}; +use bevy_log::warn; +use bevy_utils::{get_short_name, HashSet}; + +use crate::Parent; + +/// When enabled, runs [`check_hierarchy_component_has_valid_parent`]. +/// +/// This resource is added by [`ValidParentCheckPlugin`]. +/// It is enabled on debug builds and disabled in release builds by default, +/// you can update this resource at runtime to change the default behavior. +#[derive(Resource)] +pub struct ReportHierarchyIssue { + /// Whether to run [`check_hierarchy_component_has_valid_parent`]. + pub enabled: bool, + _comp: PhantomData, +} +impl Default for ReportHierarchyIssue { + fn default() -> Self { + Self { + enabled: cfg!(debug_assertions), + _comp: PhantomData, + } + } +} + +/// System to print a warning for each `Entity` with a `T` component +/// which parent hasn't a `T` component. +/// +/// Hierarchy propagations are top-down, and limited only to entities +/// with a specific component (such as `ComputedVisibility` and `GlobalTransform`). +/// This means that entities with one of those component +/// and a parent without the same component is probably a programming error. +/// (See B0004 explanation linked in warning message) +pub fn check_hierarchy_component_has_valid_parent( + parent_query: Query< + (Entity, &Parent, Option<&Name>), + (With, Or<(Changed, Added)>), + >, + component_query: Query<(), With>, + mut already_diagnosed: Local>, +) { + for (entity, parent, name) in &parent_query { + let parent = parent.get(); + if !component_query.contains(parent) && !already_diagnosed.contains(&entity) { + already_diagnosed.insert(entity); + warn!( + "warning[B0004]: {name} with the {ty_name} component has a parent without {ty_name}.\n\ + This will cause inconsistent behaviors! See https://bevyengine.org/learn/errors/#B0004", + ty_name = get_short_name(std::any::type_name::()), + name = name.map_or("An entity".to_owned(), |s| format!("The {s} entity")), + ); + } + } +} + +/// Run criteria that only allows running when [`ReportHierarchyIssue`] is enabled. +pub fn on_hierarchy_reports_enabled(report: Res>) -> ShouldRun +where + T: Component, +{ + report.enabled.into() +} + +/// Print a warning for each `Entity` with a `T` component +/// whose parent doesn't have a `T` component. +/// +/// See [`check_hierarchy_component_has_valid_parent`] for details. +pub struct ValidParentCheckPlugin(PhantomData T>); +impl Default for ValidParentCheckPlugin { + fn default() -> Self { + Self(PhantomData) + } +} + +impl Plugin for ValidParentCheckPlugin { + fn build(&self, app: &mut App) { + app.init_resource::>() + .add_system_to_stage( + CoreStage::Last, + check_hierarchy_component_has_valid_parent:: + .with_run_criteria(on_hierarchy_reports_enabled::), + ); + } +} diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index fbe5f14923..10b6b0b602 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -18,6 +18,7 @@ mod spatial_bundle; pub mod texture; pub mod view; +use bevy_hierarchy::ValidParentCheckPlugin; pub use extract_param::Extract; pub mod prelude { @@ -34,6 +35,7 @@ pub mod prelude { } pub use once_cell; +use prelude::ComputedVisibility; use crate::{ camera::CameraPlugin, @@ -315,7 +317,8 @@ impl Plugin for RenderPlugin { }); } - app.add_plugin(WindowRenderPlugin) + app.add_plugin(ValidParentCheckPlugin::::default()) + .add_plugin(WindowRenderPlugin) .add_plugin(CameraPlugin) .add_plugin(ViewPlugin) .add_plugin(MeshPlugin) diff --git a/crates/bevy_transform/src/lib.rs b/crates/bevy_transform/src/lib.rs index 2e02015f12..e5712b1906 100644 --- a/crates/bevy_transform/src/lib.rs +++ b/crates/bevy_transform/src/lib.rs @@ -14,6 +14,7 @@ pub mod prelude { use bevy_app::prelude::*; use bevy_ecs::prelude::*; +use bevy_hierarchy::ValidParentCheckPlugin; use prelude::{GlobalTransform, Transform}; /// A [`Bundle`] of the [`Transform`] and [`GlobalTransform`] @@ -86,6 +87,7 @@ impl Plugin for TransformPlugin { fn build(&self, app: &mut App) { app.register_type::() .register_type::() + .add_plugin(ValidParentCheckPlugin::::default()) // add transform systems to startup so the first update is "correct" .add_startup_system_to_stage( StartupStage::PostStartup, diff --git a/errors/B0004.md b/errors/B0004.md new file mode 100644 index 0000000000..feb5024c39 --- /dev/null +++ b/errors/B0004.md @@ -0,0 +1,119 @@ +# B0004 + +A runtime warning. + +An [`Entity`] with a hierarchy-inherited component has a [`Parent`] +without the hierarchy-inherited component in question. + +The hierarchy-inherited components defined in bevy include: + +- [`ComputedVisibility`] +- [`GlobalTransform`] + +Third party plugins may also define their own hierarchy components, so +read the warning message carefully and pay attention to the exact type +of the missing component. + +To fix this warning, add the missing hierarchy component to all ancestors +of entities with the hierarchy component you wish to use. + +The following code will cause a warning to be emitted: + +```rust,no_run +use bevy::prelude::*; + +// WARNING: this code is buggy +fn setup_cube( + mut commands: Commands, + mut meshes: ResMut>, + mut materials: ResMut>, +) { + commands + .spawn_bundle(TransformBundle::default()) + .with_children(|parent| { + // cube + parent.spawn_bundle(PbrBundle { + mesh: meshes.add(Mesh::from(shape::Cube { size: 1.0 })), + material: materials.add(Color::rgb(0.8, 0.7, 0.6).into()), + transform: Transform::from_xyz(0.0, 0.5, 0.0), + ..default() + }); + }); + + // camera + commands.spawn_bundle(Camera3dBundle { + transform: Transform::from_xyz(-2.0, 2.5, 5.0).looking_at(Vec3::ZERO, Vec3::Y), + ..default() + }); +} + +fn main() { + App::new() + .add_plugins(DefaultPlugins) + .add_startup_system(setup_cube) + .run(); +} +``` + +This code **will not** show a cube on screen. +This is because the entity spawned with `commands.spawn_bundle(…)` +doesn't have a [`ComputedVisibility`] component. +Since the cube is spawned as a child of an entity without the +[`ComputedVisibility`] component, it will not be visible at all. + +To fix this, you must use [`SpatialBundle`] over [`TransformBundle`], +as follows: + +```rust,no_run +use bevy::prelude::*; + +fn setup_cube( + mut commands: Commands, + mut meshes: ResMut>, + mut materials: ResMut>, +) { + commands + // We use SpatialBundle instead of TransformBundle, it contains the + // ComputedVisibility component needed to display the cube, + // In addition to the Transform and GlobalTransform components. + .spawn_bundle(SpatialBundle::default()) + .with_children(|parent| { + // cube + parent.spawn_bundle(PbrBundle { + mesh: meshes.add(Mesh::from(shape::Cube { size: 1.0 })), + material: materials.add(Color::rgb(0.8, 0.7, 0.6).into()), + transform: Transform::from_xyz(0.0, 0.5, 0.0), + ..default() + }); + }); + + // camera + commands.spawn_bundle(Camera3dBundle { + transform: Transform::from_xyz(-2.0, 2.5, 5.0).looking_at(Vec3::ZERO, Vec3::Y), + ..default() + }); +} + +fn main() { + App::new() + .add_plugins(DefaultPlugins) + .add_startup_system(setup_cube) + .run(); +} +``` + +A similar problem occurs when the [`GlobalTransform`] component is missing. +However, when a parent [`GlobalTransform`] is missing, +it will simply prevent all transform propagation, +including when updating the [`Transform`] component of the child. + +You will most likely encounter this warning when loading a scene +as a child of a pre-existing [`Entity`] that does not have the proper components. + +[`ComputedVisibility`]: https://docs.rs/bevy/*/bevy/render/view/struct.ComputedVisibility.html +[`GlobalTransform`]: https://docs.rs/bevy/*/bevy/transform/components/struct.GlobalTransform.html +[`Transform`]: https://docs.rs/bevy/*/bevy/transform/components/struct.Transform.html +[`Parent`]: https://docs.rs/bevy/*/bevy/hierarchy/struct.Parent.html +[`Entity`]: https://docs.rs/bevy/*/bevy/ecs/entity/struct.Entity.html +[`SpatialBundle`]: https://docs.rs/bevy/*/bevy/render/prelude/struct.SpatialBundle.html +[`TransformBundle`]: https://docs.rs/bevy/*/bevy/transform/struct.TransformBundle.html diff --git a/errors/src/lib.rs b/errors/src/lib.rs index c396f5d494..4cda972b49 100644 --- a/errors/src/lib.rs +++ b/errors/src/lib.rs @@ -6,3 +6,6 @@ pub struct B0002; #[doc = include_str!("../B0003.md")] pub struct B0003; + +#[doc = include_str!("../B0004.md")] +pub struct B0004;