Skip to content

Commit 8243b57

Browse files
committed
Delete validate_parent_has_component and replace it with a whole plugin.
1 parent 67db8e8 commit 8243b57

File tree

3 files changed

+141
-41
lines changed

3 files changed

+141
-41
lines changed

crates/bevy_app/src/hierarchy.rs

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
use core::marker::PhantomData;
2+
3+
use bevy_ecs::{
4+
change_detection::MaybeLocation,
5+
component::Component,
6+
entity::Entity,
7+
hierarchy::ChildOf,
8+
intern::Interned,
9+
lifecycle::Insert,
10+
message::{Message, MessageReader, MessageWriter},
11+
name::Name,
12+
observer::On,
13+
query::{With, Without},
14+
schedule::{common_conditions::on_message, IntoScheduleConfigs, ScheduleLabel, SystemSet},
15+
system::Query,
16+
};
17+
use bevy_platform::prelude::format;
18+
use bevy_utils::prelude::DebugName;
19+
use log::warn;
20+
21+
use crate::{Last, Plugin};
22+
23+
/// A plugin that verifies that [`Component`] `C` has parents that also have that component.
24+
pub struct ValidateParentHasComponentPlugin<C: Component> {
25+
schedule: Interned<dyn ScheduleLabel>,
26+
marker: PhantomData<fn() -> C>,
27+
}
28+
29+
impl<C: Component> Default for ValidateParentHasComponentPlugin<C> {
30+
fn default() -> Self {
31+
Self::in_schedule(Last)
32+
}
33+
}
34+
35+
impl<C: Component> ValidateParentHasComponentPlugin<C> {
36+
/// Creates an instance of this plugin that inserts systems in the provided schedule.
37+
pub fn in_schedule(label: impl ScheduleLabel) -> Self {
38+
Self {
39+
schedule: label.intern(),
40+
marker: PhantomData,
41+
}
42+
}
43+
}
44+
45+
impl<C: Component> Plugin for ValidateParentHasComponentPlugin<C> {
46+
fn build(&self, app: &mut crate::App) {
47+
app.add_message::<CheckParentHasComponent<C>>()
48+
.add_observer(validate_parent_has_component::<C>)
49+
.add_systems(
50+
self.schedule,
51+
check_parent_has_component::<C>
52+
.run_if(on_message::<CheckParentHasComponent<C>>)
53+
.in_set(ValidateParentHasComponentSystems),
54+
);
55+
}
56+
}
57+
58+
/// System set for systems added by [`ValidateParentHasComponentPlugin`].
59+
#[derive(SystemSet, PartialEq, Eq, Hash, Debug, Clone)]
60+
pub struct ValidateParentHasComponentSystems;
61+
62+
/// An `Insert` observer that when run, will validate that the parent of a given entity contains
63+
/// component `C`. If the parent does not contain `C`, a warning will be logged later in the frame.
64+
fn validate_parent_has_component<C: Component>(
65+
event: On<Insert, C>,
66+
child: Query<&ChildOf>,
67+
with_component: Query<(), With<C>>,
68+
mut writer: MessageWriter<CheckParentHasComponent<C>>,
69+
) {
70+
let Ok(child_of) = child.get(event.entity) else {
71+
return;
72+
};
73+
if with_component.contains(child_of.parent()) {
74+
return;
75+
}
76+
// This entity may be configured incorrectly, or the parent may just not have been populated
77+
// yet. Send a message to check again later.
78+
writer.write(CheckParentHasComponent::<C> {
79+
entity: event.entity,
80+
caller: event.caller(),
81+
marker: PhantomData,
82+
});
83+
}
84+
85+
/// A message to indicate that this entity should be checked if its parent has a component.
86+
///
87+
/// While we initially check when emitting these messages, we want to do a second check later on in
88+
/// case the parent eventually gets populated.
89+
#[derive(Message)]
90+
struct CheckParentHasComponent<C: Component> {
91+
/// The entity
92+
entity: Entity,
93+
caller: MaybeLocation,
94+
marker: PhantomData<fn() -> C>,
95+
}
96+
97+
/// System to handle "check parent" messages and log out any entities that still violate the
98+
/// component hierarchy.
99+
fn check_parent_has_component<C: Component>(
100+
mut messages: MessageReader<CheckParentHasComponent<C>>,
101+
children: Query<(&ChildOf, Option<&Name>), With<C>>,
102+
components: Query<Option<&Name>, Without<C>>,
103+
) {
104+
for CheckParentHasComponent {
105+
entity,
106+
caller,
107+
marker: _,
108+
} in messages.read()
109+
{
110+
let Ok((child_of, name)) = children.get(*entity) else {
111+
// Either the entity has been despawned, no longer has `C`, or is no longer a child. In
112+
// any case, we can say that this situation is no longer relevant.
113+
continue;
114+
};
115+
let parent = child_of.0;
116+
let Ok(parent_name) = components.get(parent) else {
117+
// This can only fail if the parent now has the `C` component. If the parent was
118+
// despawned, the child entity would also be despawned.
119+
continue;
120+
};
121+
let debug_name = DebugName::type_name::<C>();
122+
warn!(
123+
"warning[B0004]: {}{name} with the {ty_name} component has a parent ({parent_name}) without {ty_name}.\n\
124+
This will cause inconsistent behaviors! See: https://bevy.org/learn/errors/b0004",
125+
caller.map(|c| format!("{c}: ")).unwrap_or_default(),
126+
ty_name = debug_name.shortname(),
127+
name = name.map_or_else(
128+
|| format!("Entity {entity}"),
129+
|s| format!("The {s} entity")
130+
),
131+
parent_name = parent_name.map_or_else(
132+
|| format!("{parent} entity"),
133+
|s| format!("the {s} entity")
134+
),
135+
);
136+
}
137+
}

crates/bevy_app/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ extern crate alloc;
2424
extern crate self as bevy_app;
2525

2626
mod app;
27+
mod hierarchy;
2728
mod main_schedule;
2829
mod panic_handler;
2930
mod plugin;
@@ -39,6 +40,7 @@ mod terminal_ctrl_c_handler;
3940
pub mod hotpatch;
4041

4142
pub use app::*;
43+
pub use hierarchy::*;
4244
pub use main_schedule::*;
4345
pub use panic_handler::*;
4446
pub use plugin::*;

crates/bevy_ecs/src/hierarchy.rs

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,17 @@ use crate::{
1212
bundle::Bundle,
1313
component::Component,
1414
entity::Entity,
15-
lifecycle::HookContext,
16-
name::Name,
1715
relationship::{RelatedSpawner, RelatedSpawnerCommands},
1816
system::EntityCommands,
19-
world::{DeferredWorld, EntityWorldMut, FromWorld, World},
17+
world::{EntityWorldMut, FromWorld, World},
2018
};
21-
use alloc::{format, vec::Vec};
19+
use alloc::vec::Vec;
2220
#[cfg(feature = "bevy_reflect")]
2321
use bevy_reflect::std_traits::ReflectDefault;
2422
#[cfg(all(feature = "serialize", feature = "bevy_reflect"))]
2523
use bevy_reflect::{ReflectDeserialize, ReflectSerialize};
26-
use bevy_utils::prelude::DebugName;
2724
use core::ops::Deref;
2825
use core::slice;
29-
use log::warn;
3026

3127
/// Stores the parent entity of this child entity with this component.
3228
///
@@ -490,41 +486,6 @@ impl<'a> EntityCommands<'a> {
490486
}
491487
}
492488

493-
/// An `on_insert` component hook that when run, will validate that the parent of a given entity
494-
/// contains component `C`. This will print a warning if the parent does not contain `C`.
495-
pub fn validate_parent_has_component<C: Component>(
496-
world: DeferredWorld,
497-
HookContext { entity, caller, .. }: HookContext,
498-
) {
499-
let entity_ref = world.entity(entity);
500-
let Some(child_of) = entity_ref.get::<ChildOf>() else {
501-
return;
502-
};
503-
let parent = child_of.parent();
504-
let maybe_parent_ref = world.get_entity(parent);
505-
if let Ok(parent_ref) = maybe_parent_ref
506-
&& !parent_ref.contains::<C>()
507-
{
508-
let name = entity_ref.get::<Name>();
509-
let debug_name = DebugName::type_name::<C>();
510-
let parent_name = parent_ref.get::<Name>();
511-
warn!(
512-
"warning[B0004]: {}{name} with the {ty_name} component has a parent ({parent_name}) without {ty_name}.\n\
513-
This will cause inconsistent behaviors! See: https://bevy.org/learn/errors/b0004",
514-
caller.map(|c| format!("{c}: ")).unwrap_or_default(),
515-
ty_name = debug_name.shortname(),
516-
name = name.map_or_else(
517-
|| format!("Entity {entity}"),
518-
|s| format!("The {s} entity")
519-
),
520-
parent_name = parent_name.map_or_else(
521-
|| format!("{parent} entity"),
522-
|s| format!("the {s} entity")
523-
),
524-
);
525-
}
526-
}
527-
528489
/// Returns a [`SpawnRelatedBundle`] that will insert the [`Children`] component, spawn a [`SpawnableList`] of entities with given bundles that
529490
/// relate to the [`Children`] entity via the [`ChildOf`] component, and reserve space in the [`Children`] for each spawned entity.
530491
///

0 commit comments

Comments
 (0)