Add warning when a hierarchy component is missing (bevyengine#5590)

# Objective

A common pitfall since 0.8 is the requirement on `ComputedVisibility`
being present on all ancestors of an entity that itself has
`ComputedVisibility`, without which, the entity becomes invisible.

I myself hit the issue and got very confused, and saw a few people hit
it as well, so it makes sense to provide a hint of what to do when such
a situation is encountered.

- Fixes bevyengine#5849
- Closes bevyengine#5616
- Closes bevyengine#2277 
- Closes bevyengine#5081

## Solution

We now check that all entities with both a `Parent` and a
`ComputedVisibility` component have parents that themselves have a
`ComputedVisibility` component.

Note that the warning is only printed once.

We also add a similar warning to `GlobalTransform`.

This only emits a warning. Because sometimes it could be an intended
behavior.

Alternatives:
- Do nothing and keep repeating to newcomers how to avoid recurring
  pitfalls
- Make the transform and visibility propagation tolerant to missing
  components (bevyengine#5616)
- Probably archetype invariants, though the current draft would not
  allow detecting that kind of errors

---

## Changelog

- Add a warning when encountering dubious component hierarchy structure


Co-authored-by: Nicola Papale <[email protected]>

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" }
 

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::*;

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<T>`].
+///
+/// This resource is added by [`ValidParentCheckPlugin<T>`].
+/// 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<T> {
+    /// Whether to run [`check_hierarchy_component_has_valid_parent<T>`].
+    pub enabled: bool,
+    _comp: PhantomData<fn(T)>,
+}
+impl<T> Default for ReportHierarchyIssue<T> {
+    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<T: Component>(
+    parent_query: Query<
+        (Entity, &Parent, Option<&Name>),
+        (With<T>, Or<(Changed<Parent>, Added<T>)>),
+    >,
+    component_query: Query<(), With<T>>,
+    mut already_diagnosed: Local<HashSet<Entity>>,
+) {
+    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::<T>()),
+                name = name.map_or("An entity".to_owned(), |s| format!("The {s} entity")),
+            );
+        }
+    }
+}
+
+/// Run criteria that only allows running when [`ReportHierarchyIssue<T>`] is enabled.
+pub fn on_hierarchy_reports_enabled<T>(report: Res<ReportHierarchyIssue<T>>) -> 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<T: Component>(PhantomData<fn() -> T>);
+impl<T: Component> Default for ValidParentCheckPlugin<T> {
+    fn default() -> Self {
+        Self(PhantomData)
+    }
+}
+
+impl<T: Component> Plugin for ValidParentCheckPlugin<T> {
+    fn build(&self, app: &mut App) {
+        app.init_resource::<ReportHierarchyIssue<T>>()
+            .add_system_to_stage(
+                CoreStage::Last,
+                check_hierarchy_component_has_valid_parent::<T>
+                    .with_run_criteria(on_hierarchy_reports_enabled::<T>),
+            );
+    }
+}

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::<ComputedVisibility>::default())
+            .add_plugin(WindowRenderPlugin)
             .add_plugin(CameraPlugin)
             .add_plugin(ViewPlugin)
             .add_plugin(MeshPlugin)

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::<Transform>()
             .register_type::<GlobalTransform>()
+            .add_plugin(ValidParentCheckPlugin::<GlobalTransform>::default())
             // add transform systems to startup so the first update is "correct"
             .add_startup_system_to_stage(
                 StartupStage::PostStartup,

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<Assets<Mesh>>,
+    mut materials: ResMut<Assets<StandardMaterial>>,
+) {
+    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<Assets<Mesh>>,
+    mut materials: ResMut<Assets<StandardMaterial>>,
+) {
+    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

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;