From d102dbe60330a2256c0f840980f654aa71d5417c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 1 Jul 2025 21:36:32 +1000 Subject: [PATCH 1/2] Change `WhereClauseOptions::active_fields` to `active_types` This could have been part of #19876, which deduplicated the fields. It's also simpler and more efficient to keep the (short-lived) active types as an `IndexSet`, and avoid converting them to a `Vec` and then a `Box<[Type]>`. --- crates/bevy_reflect/derive/src/derive_data.rs | 16 ++++++---------- .../derive/src/where_clause_options.rs | 14 ++++++-------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index 614f357b37e56..9c2d6453d7915 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -607,13 +607,11 @@ impl<'a> ReflectStruct<'a> { } /// Get a collection of types which are exposed to the reflection API - pub fn active_types(&self) -> Vec { - // Collect via `IndexSet` to eliminate duplicate types. + pub fn active_types(&self) -> IndexSet { + // Collect into an `IndexSet` to eliminate duplicate types. self.active_fields() .map(|field| field.reflected_type().clone()) .collect::>() - .into_iter() - .collect::>() } /// Get an iterator of fields which are exposed to the reflection API. @@ -636,7 +634,7 @@ impl<'a> ReflectStruct<'a> { } pub fn where_clause_options(&self) -> WhereClauseOptions { - WhereClauseOptions::new_with_fields(self.meta(), self.active_types().into_boxed_slice()) + WhereClauseOptions::new_with_types(self.meta(), self.active_types()) } /// Generates a `TokenStream` for `TypeInfo::Struct` or `TypeInfo::TupleStruct` construction. @@ -854,13 +852,11 @@ impl<'a> ReflectEnum<'a> { } /// Get a collection of types which are exposed to the reflection API - pub fn active_types(&self) -> Vec { - // Collect via `IndexSet` to eliminate duplicate types. + pub fn active_types(&self) -> IndexSet { + // Collect into an `IndexSet` to eliminate duplicate types. self.active_fields() .map(|field| field.reflected_type().clone()) .collect::>() - .into_iter() - .collect::>() } /// Get an iterator of fields which are exposed to the reflection API @@ -869,7 +865,7 @@ impl<'a> ReflectEnum<'a> { } pub fn where_clause_options(&self) -> WhereClauseOptions { - WhereClauseOptions::new_with_fields(self.meta(), self.active_types().into_boxed_slice()) + WhereClauseOptions::new_with_types(self.meta(), self.active_types()) } /// Returns the `GetTypeRegistration` impl as a `TokenStream`. diff --git a/crates/bevy_reflect/derive/src/where_clause_options.rs b/crates/bevy_reflect/derive/src/where_clause_options.rs index 1551e008d017c..7f573c780512b 100644 --- a/crates/bevy_reflect/derive/src/where_clause_options.rs +++ b/crates/bevy_reflect/derive/src/where_clause_options.rs @@ -1,5 +1,6 @@ use crate::derive_data::ReflectMeta; use bevy_macro_utils::fq_std::{FQAny, FQSend, FQSync}; +use indexmap::IndexSet; use proc_macro2::TokenStream; use quote::{quote, ToTokens}; use syn::{punctuated::Punctuated, Token, Type, WhereClause}; @@ -7,22 +8,19 @@ use syn::{punctuated::Punctuated, Token, Type, WhereClause}; /// Options defining how to extend the `where` clause for reflection. pub(crate) struct WhereClauseOptions<'a, 'b> { meta: &'a ReflectMeta<'b>, - active_fields: Box<[Type]>, + active_types: IndexSet, } impl<'a, 'b> WhereClauseOptions<'a, 'b> { pub fn new(meta: &'a ReflectMeta<'b>) -> Self { Self { meta, - active_fields: Box::new([]), + active_types: IndexSet::new(), } } - pub fn new_with_fields(meta: &'a ReflectMeta<'b>, active_fields: Box<[Type]>) -> Self { - Self { - meta, - active_fields, - } + pub fn new_with_types(meta: &'a ReflectMeta<'b>, active_types: IndexSet) -> Self { + Self { meta, active_types } } /// Extends the `where` clause for a type with additional bounds needed for the reflection impls. @@ -157,7 +155,7 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> { // construct `NamedField` and `UnnamedField` instances for the `Typed` impl. // Likewise, `GetTypeRegistration` is always required for active fields since // they are used to register the type's dependencies. - Some(self.active_fields.iter().map(move |ty| { + Some(self.active_types.iter().map(move |ty| { quote!( #ty : #reflect_bound + #bevy_reflect_path::TypePath From 165a7429b453fec708e242d1e17b8425118d4d53 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 1 Jul 2025 21:51:37 +1000 Subject: [PATCH 2/2] Use `active_types` in `ReflectEnum::get_type_registration`. Instead of `active_fields`. This makes it match `ReflectStruct`, and avoids redundant calls within the generated `register_type_dependencies` when the same type is used in multiple enum variant fields. --- crates/bevy_reflect/derive/src/derive_data.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index 9c2d6453d7915..d39c787c3fd55 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -879,7 +879,7 @@ impl<'a> ReflectEnum<'a> { self.meta(), where_clause_options, None, - Some(self.active_fields().map(StructField::reflected_type)), + Some(self.active_types().iter()), ) }