Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Add warning when a hierarchy component is missing #5590

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/bevy_hierarchy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

Expand Down
7 changes: 6 additions & 1 deletion crates/bevy_hierarchy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down
89 changes: 89 additions & 0 deletions crates/bevy_hierarchy/src/valid_parent_check_plugin.rs
Original file line number Diff line number Diff line change
@@ -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>),
);
}
}
5 changes: 4 additions & 1 deletion crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -34,6 +35,7 @@ pub mod prelude {
}

pub use once_cell;
use prelude::ComputedVisibility;

use crate::{
camera::CameraPlugin,
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`]
Expand Down Expand Up @@ -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,
Expand Down
119 changes: 119 additions & 0 deletions errors/B0004.md
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ pub struct B0002;

#[doc = include_str!("../B0003.md")]
pub struct B0003;

#[doc = include_str!("../B0004.md")]
pub struct B0004;