Skip to content

Commit e81670a

Browse files
committed
Move recursion_check_stack parameter into ComponentsRegistrator
1 parent de08859 commit e81670a

File tree

4 files changed

+45
-68
lines changed

4 files changed

+45
-68
lines changed

crates/bevy_ecs/macros/src/component.rs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
246246
requiree,
247247
components,
248248
required_components,
249-
inheritance_depth + 1,
250-
recursion_check_stack
249+
inheritance_depth + 1
251250
);
252251
});
253252
match &require.func {
@@ -256,8 +255,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
256255
components.register_required_components_manual::<Self, #ident>(
257256
required_components,
258257
|| { let x: #ident = (#func)().into(); x },
259-
inheritance_depth,
260-
recursion_check_stack
258+
inheritance_depth
261259
);
262260
});
263261
}
@@ -266,8 +264,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
266264
components.register_required_components_manual::<Self, #ident>(
267265
required_components,
268266
<#ident as Default>::default,
269-
inheritance_depth,
270-
recursion_check_stack
267+
inheritance_depth
271268
);
272269
});
273270
}
@@ -312,18 +309,13 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
312309
const STORAGE_TYPE: #bevy_ecs_path::component::StorageType = #storage;
313310
type Mutability = #mutable_type;
314311
fn register_required_components(
315-
requiree: #bevy_ecs_path::component::ComponentId,
312+
_requiree: #bevy_ecs_path::component::ComponentId,
316313
components: &mut #bevy_ecs_path::component::ComponentsRegistrator,
317314
required_components: &mut #bevy_ecs_path::component::RequiredComponents,
318315
inheritance_depth: u16,
319-
recursion_check_stack: &mut #bevy_ecs_path::__macro_exports::Vec<#bevy_ecs_path::component::ComponentId>
320316
) {
321-
#bevy_ecs_path::component::enforce_no_required_components_recursion(components, recursion_check_stack);
322-
let self_id = components.register_component::<Self>();
323-
recursion_check_stack.push(self_id);
324317
#(#register_required)*
325318
#(#register_recursive_requires)*
326-
recursion_check_stack.pop();
327319
}
328320

329321
#on_add

crates/bevy_ecs/src/component/mod.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ use crate::{
1818
system::{Local, SystemParam},
1919
world::{FromWorld, World},
2020
};
21-
use alloc::vec::Vec;
2221
pub use bevy_ecs_macros::Component;
2322
use core::{fmt::Debug, marker::PhantomData, ops::Deref};
2423

@@ -528,7 +527,6 @@ pub trait Component: Send + Sync + 'static {
528527
_components: &mut ComponentsRegistrator,
529528
_required_components: &mut RequiredComponents,
530529
_inheritance_depth: u16,
531-
_recursion_check_stack: &mut Vec<ComponentId>,
532530
) {
533531
}
534532

crates/bevy_ecs/src/component/register.rs

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ impl ComponentIds {
6464
pub struct ComponentsRegistrator<'w> {
6565
components: &'w mut Components,
6666
ids: &'w mut ComponentIds,
67+
pub(super) recursion_check_stack: Vec<ComponentId>,
6768
}
6869

6970
impl Deref for ComponentsRegistrator<'_> {
@@ -88,7 +89,11 @@ impl<'w> ComponentsRegistrator<'w> {
8889
/// The [`Components`] and [`ComponentIds`] must match.
8990
/// For example, they must be from the same world.
9091
pub unsafe fn new(components: &'w mut Components, ids: &'w mut ComponentIds) -> Self {
91-
Self { components, ids }
92+
Self {
93+
components,
94+
ids,
95+
recursion_check_stack: Vec::new(),
96+
}
9297
}
9398

9499
/// Converts this [`ComponentsRegistrator`] into a [`ComponentsQueuedRegistrator`].
@@ -177,15 +182,12 @@ impl<'w> ComponentsRegistrator<'w> {
177182
/// * [`ComponentsRegistrator::register_component_with_descriptor()`]
178183
#[inline]
179184
pub fn register_component<T: Component>(&mut self) -> ComponentId {
180-
self.register_component_checked::<T>(&mut Vec::new())
185+
self.register_component_checked::<T>()
181186
}
182187

183188
/// Same as [`Self::register_component_unchecked`] but keeps a checks for safety.
184189
#[inline]
185-
pub(super) fn register_component_checked<T: Component>(
186-
&mut self,
187-
recursion_check_stack: &mut Vec<ComponentId>,
188-
) -> ComponentId {
190+
pub(super) fn register_component_checked<T: Component>(&mut self) -> ComponentId {
189191
let type_id = TypeId::of::<T>();
190192
if let Some(id) = self.indices.get(&type_id) {
191193
return *id;
@@ -207,7 +209,7 @@ impl<'w> ComponentsRegistrator<'w> {
207209
let id = self.ids.next_mut();
208210
// SAFETY: The component is not currently registered, and the id is fresh.
209211
unsafe {
210-
self.register_component_unchecked::<T>(recursion_check_stack, id);
212+
self.register_component_unchecked::<T>(id);
211213
}
212214
id
213215
}
@@ -216,11 +218,7 @@ impl<'w> ComponentsRegistrator<'w> {
216218
///
217219
/// Neither this component, nor its id may be registered or queued. This must be a new registration.
218220
#[inline]
219-
unsafe fn register_component_unchecked<T: Component>(
220-
&mut self,
221-
recursion_check_stack: &mut Vec<ComponentId>,
222-
id: ComponentId,
223-
) {
221+
unsafe fn register_component_unchecked<T: Component>(&mut self, id: ComponentId) {
224222
// SAFETY: ensured by caller.
225223
unsafe {
226224
self.register_component_inner(id, ComponentDescriptor::new::<T>());
@@ -229,14 +227,10 @@ impl<'w> ComponentsRegistrator<'w> {
229227
let prev = self.indices.insert(type_id, id);
230228
debug_assert!(prev.is_none());
231229

230+
self.recursion_check_stack.push(id);
232231
let mut required_components = RequiredComponents::default();
233-
T::register_required_components(
234-
id,
235-
self,
236-
&mut required_components,
237-
0,
238-
recursion_check_stack,
239-
);
232+
T::register_required_components(id, self, &mut required_components, 0);
233+
self.recursion_check_stack.pop();
240234
// SAFETY: we just inserted it in `register_component_inner`
241235
let info = unsafe {
242236
&mut self
@@ -563,7 +557,7 @@ impl<'w> ComponentsQueuedRegistrator<'w> {
563557
// SAFETY: We just checked that this is not currently registered or queued, and if it was registered since, this would have been dropped from the queue.
564558
#[expect(unused_unsafe, reason = "More precise to specify.")]
565559
unsafe {
566-
registrator.register_component_unchecked::<T>(&mut Vec::new(), id);
560+
registrator.register_component_unchecked::<T>(id);
567561
}
568562
},
569563
)

crates/bevy_ecs/src/component/required.rs

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,6 @@ impl<'w> ComponentsRegistrator<'w> {
231231
/// A direct requirement has a depth of `0`, and each level of inheritance increases the depth by `1`.
232232
/// Lower depths are more specific requirements, and can override existing less specific registrations.
233233
///
234-
/// The `recursion_check_stack` allows checking whether this component tried to register itself as its
235-
/// own (indirect) required component.
236-
///
237234
/// This method does *not* register any components as required by components that require `T`.
238235
///
239236
/// Only use this method if you know what you are doing. In most cases, you should instead use [`World::register_required_components`],
@@ -246,10 +243,11 @@ impl<'w> ComponentsRegistrator<'w> {
246243
required_components: &mut RequiredComponents,
247244
constructor: fn() -> R,
248245
inheritance_depth: u16,
249-
recursion_check_stack: &mut Vec<ComponentId>,
250246
) {
251-
let requiree = self.register_component_checked::<T>(recursion_check_stack);
252-
let required = self.register_component_checked::<R>(recursion_check_stack);
247+
let requiree = self.register_component_checked::<T>();
248+
let required = self.register_component_checked::<R>();
249+
250+
enforce_no_required_components_recursion(self, &self.recursion_check_stack, required);
253251

254252
// SAFETY: We just created the components.
255253
unsafe {
@@ -501,36 +499,31 @@ impl RequiredComponents {
501499
}
502500
}
503501

504-
// NOTE: This should maybe be private, but it is currently public so that `bevy_ecs_macros` can use it.
505-
// This exists as a standalone function instead of being inlined into the component derive macro so as
506-
// to reduce the amount of generated code.
507-
#[doc(hidden)]
508-
pub fn enforce_no_required_components_recursion(
502+
fn enforce_no_required_components_recursion(
509503
components: &Components,
510504
recursion_check_stack: &[ComponentId],
505+
required: ComponentId,
511506
) {
512-
if let Some((&requiree, check)) = recursion_check_stack.split_last() {
513-
if let Some(direct_recursion) = check
514-
.iter()
515-
.position(|&id| id == requiree)
516-
.map(|index| index == check.len() - 1)
517-
{
518-
panic!(
519-
"Recursive required components detected: {}\nhelp: {}",
520-
recursion_check_stack
521-
.iter()
522-
.map(|id| format!("{}", components.get_name(*id).unwrap().shortname()))
523-
.collect::<Vec<_>>()
524-
.join(" → "),
525-
if direct_recursion {
526-
format!(
527-
"Remove require({}).",
528-
components.get_name(requiree).unwrap().shortname()
529-
)
530-
} else {
531-
"If this is intentional, consider merging the components.".into()
532-
}
533-
);
534-
}
507+
if let Some(direct_recursion) = recursion_check_stack
508+
.iter()
509+
.position(|&id| id == required)
510+
.map(|index| index == recursion_check_stack.len() - 1)
511+
{
512+
panic!(
513+
"Recursive required components detected: {}\nhelp: {}",
514+
recursion_check_stack
515+
.iter()
516+
.map(|id| format!("{}", components.get_name(*id).unwrap().shortname()))
517+
.collect::<Vec<_>>()
518+
.join(" → "),
519+
if direct_recursion {
520+
format!(
521+
"Remove require({}).",
522+
components.get_name(required).unwrap().shortname()
523+
)
524+
} else {
525+
"If this is intentional, consider merging the components.".into()
526+
}
527+
);
535528
}
536529
}

0 commit comments

Comments
 (0)