From 7cb97852a5b5be3b28fab2164d4ed3b3f706e059 Mon Sep 17 00:00:00 2001 From: Ben Frankel Date: Mon, 15 Jul 2024 08:58:14 -0700 Subject: [PATCH] Remove second generic from `.add_before`, `.add_after` (#14285) # Objective ```rust // Currently: builder.add_after::(BarPlugin); // After this PR: builder.add_after::(BarPlugin); ``` This removes some weirdness and better parallels the rest of the `PluginGroupBuilder` API. ## Solution Define a helper method `type_id_of_val` to use in `.add_before` and `.add_after` instead of `TypeId::of::` (which requires the plugin type to be nameable, preventing `impl Plugin` from being used). ## Testing Ran `cargo run -p ci lints` successfully. ## Migration Guide Removed second generic from `PluginGroupBuilder` methods: `add_before` and `add_after`. ```rust // Before: DefaultPlugins .build() .add_before::(FooPlugin) .add_after::(BarPlugin) // After: DefaultPlugins .build() .add_before::(FooPlugin) .add_after::(BarPlugin) ``` --------- Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com> --- crates/bevy_app/src/plugin_group.rs | 21 +++++++++++++-------- examples/app/plugin_group.rs | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/crates/bevy_app/src/plugin_group.rs b/crates/bevy_app/src/plugin_group.rs index b41a78f067..4ed220b4ae 100644 --- a/crates/bevy_app/src/plugin_group.rs +++ b/crates/bevy_app/src/plugin_group.rs @@ -27,6 +27,11 @@ impl PluginGroup for PluginGroupBuilder { } } +/// Helper method to get the [`TypeId`] of a value without having to name its type. +fn type_id_of_val(_: &T) -> TypeId { + TypeId::of::() +} + /// Facilitates the creation and configuration of a [`PluginGroup`]. /// Provides a build ordering to ensure that [`Plugin`]s which produce/require a [`Resource`](bevy_ecs::system::Resource) /// are built before/after dependent/depending [`Plugin`]s. [`Plugin`]s inside the group @@ -153,9 +158,9 @@ impl PluginGroupBuilder { /// Adds a [`Plugin`] in this [`PluginGroupBuilder`] before the plugin of type `Target`. /// If the plugin was already the group, it is removed from its previous place. There must /// be a plugin of type `Target` in the group or it will panic. - pub fn add_before(mut self, plugin: T) -> Self { + pub fn add_before(mut self, plugin: impl Plugin) -> Self { let target_index = self.index_of::(); - self.order.insert(target_index, TypeId::of::()); + self.order.insert(target_index, type_id_of_val(&plugin)); self.upsert_plugin_state(plugin, target_index); self } @@ -163,9 +168,9 @@ impl PluginGroupBuilder { /// Adds a [`Plugin`] in this [`PluginGroupBuilder`] after the plugin of type `Target`. /// If the plugin was already the group, it is removed from its previous place. There must /// be a plugin of type `Target` in the group or it will panic. - pub fn add_after(mut self, plugin: T) -> Self { + pub fn add_after(mut self, plugin: impl Plugin) -> Self { let target_index = self.index_of::() + 1; - self.order.insert(target_index, TypeId::of::()); + self.order.insert(target_index, type_id_of_val(&plugin)); self.upsert_plugin_state(plugin, target_index); self } @@ -285,7 +290,7 @@ mod tests { let group = PluginGroupBuilder::start::() .add(PluginA) .add(PluginB) - .add_after::(PluginC); + .add_after::(PluginC); assert_eq!( group.order, @@ -302,7 +307,7 @@ mod tests { let group = PluginGroupBuilder::start::() .add(PluginA) .add(PluginB) - .add_before::(PluginC); + .add_before::(PluginC); assert_eq!( group.order, @@ -338,7 +343,7 @@ mod tests { .add(PluginA) .add(PluginB) .add(PluginC) - .add_after::(PluginC); + .add_after::(PluginC); assert_eq!( group.order, @@ -356,7 +361,7 @@ mod tests { .add(PluginA) .add(PluginB) .add(PluginC) - .add_before::(PluginC); + .add_before::(PluginC); assert_eq!( group.order, diff --git a/examples/app/plugin_group.rs b/examples/app/plugin_group.rs index c9d5741cc0..54923876c0 100644 --- a/examples/app/plugin_group.rs +++ b/examples/app/plugin_group.rs @@ -16,7 +16,7 @@ fn main() { // HelloWorldPlugins // .build() // .disable::() - // .add_before::( + // .add_before::( // bevy::diagnostic::LogDiagnosticsPlugin::default(), // ), // )