plugin_group! macro #11460

doonv · 2024-01-21T18:51:33Z

Objective

Solution

I created a plugin_group! macro that takes in a list of Plugins and then generates a PluginGroup along with it's documentation.

Changelog

Bevy App: Added the plugin_group! macro, which allows easy creation of PluginGroups.

crates/bevy_internal/src/default_plugins.rs

crates/bevy_app/src/plugin_group.rs

matiqo15 · 2024-01-21T20:16:42Z

crates/bevy_internal/src/default_plugins.rs

+        #[custom(cfg(all(not(target_arch = "wasm32"), feature = "multi-threaded")))]
+        bevy_render::pipelined_rendering:::PipelinedRenderingPlugin,


It would be nice if it would document not targeting wasm32

It would be nice, but it would be quite difficult to implement this with the macro.

The original DefaultPlugins documentation didn't mention this requirement. So I don't think that's necessary.

It did.

bevy/crates/bevy_internal/src/default_plugins.rs

Line 19 in 10f9595

/// * [`PipelinedRenderingPlugin`](crate::render::pipelined_rendering::PipelinedRenderingPlugin) - with feature `bevy_render` when not targeting `wasm32`

Co-authored-by: Mateusz Wachowiak <[email protected]>

crates/bevy_app/src/plugin_group.rs

# Objective - Fixes #11453 This is a temporary fix. There is PR fixing it (#11460), but I'm not sure if it's going to be merged before the 0.13 release.

BD103

I have a few more changes requested, mostly to make it harder to use the macro incorrectly and to catch some edge cases.

crates/bevy_app/src/plugin_group.rs

BD103 · 2024-02-04T19:21:00Z

crates/bevy_app/src/plugin_group.rs

+        impl PluginGroup for $group {
+            fn build(self) -> PluginGroupBuilder {
+                let mut group = PluginGroupBuilder::start::<Self>();


This will fail to compile if PluginGroup and PluginGroupBuilder are not imported. Using $crate will fix this.

Additionally, #[allow(unused_mut)] will allow an empty plugin_group!(MyPlugins {}) to build without warnings.

Suggested change

impl PluginGroup for $group {

fn build(self) -> PluginGroupBuilder {

let mut group = PluginGroupBuilder::start::<Self>();

impl $crate::PluginGroup for $group {

fn build(self) -> $crate::PluginGroupBuilder {

#[allow(unused_mut)]

let mut group = $crate::PluginGroupBuilder::start::<Self>();

Why would you want to create a plugin group with no plugins?

I don't think there is any long-term use of having an empty plugin group, but I could see someone testing the macro empty and being confused why rustc is raising an warning on code they can't see. Additionally the macro doesn't prevent you from creating an empty group, so it's bound to happen at some point.

I don't think the extra #[allow(...)] annotation will be confusing for future reviewers, but feel free to add a comment explaining why it's there. It can't hurt :)

crates/bevy_app/src/plugin_group.rs

# Objective - Fixes bevyengine#11453 This is a temporary fix. There is PR fixing it (bevyengine#11460), but I'm not sure if it's going to be merged before the 0.13 release.

Co-authored-by: BD103 <[email protected]>

…art 2)) Co-authored-by: BD103 <[email protected]>

BD103

Looks good! Thanks for your patience with my requests :)

BD103 · 2024-04-21T02:37:15Z

@doonv would you mind rebasing this onto main?

…-group-macro

BD103

Could you please derive Default on the DevToolsPlugin? That seems to be causing the CI error.

BD103 · 2024-05-08T12:07:53Z

.cargo/aconfig.toml

Looks like this was committed by mistake, would you mind deleting it?

BD103

Thank you!

alice-i-cecile

I like the docs generation, and the macro is nice and simple :)

alice-i-cecile · 2024-07-15T13:50:52Z

@doonv it's warning me about merge conflicts but not showing me them. If the merge fails, please resolve the merge conflicts and then ping me :)

BD103 · 2024-07-15T14:59:09Z

I'm going to adopt this and fix merge conflicts. (I may also change a few things, since I have fresh eyes again.)

alice-i-cecile · 2024-07-15T16:36:13Z

Closing as adopted :)

# Objective - Adopted from #11460. - Closes #7332. - The documentation for `DefaultPlugins` and `MinimalPlugins` frequently goes out of date because it is not . ## Solution - Create a macro, `plugin_group!`, to automatically create `PluginGroup`s and document them. ## Testing - Run `cargo-expand` on the generated code for `DefaultPlugins` and `MinimalPlugins`. - Try creating a custom plugin group with the macro. --- ## Showcase - You can now define custom `PluginGroup`s using the `plugin_group!` macro. ```rust plugin_group! { /// My really cool plugic group! pub struct MyPluginGroup { physics:::PhysicsPlugin, rendering:::RenderingPlugin, ui:::UiPlugin, } } ``` <details> <summary>Expanded output</summary> ```rust /// My really cool plugic group! /// /// - [`PhysicsPlugin`](physics::PhysicsPlugin) /// - [`RenderingPlugin`](rendering::RenderingPlugin) /// - [`UiPlugin`](ui::UiPlugin) pub struct MyPluginGroup; impl ::bevy_app::PluginGroup for MyPluginGroup { fn build(self) -> ::bevy_app::PluginGroupBuilder { let mut group = ::bevy_app::PluginGroupBuilder::start::<Self>(); { const _: () = { const fn check_default<T: Default>() {} check_default::<physics::PhysicsPlugin>(); }; group = group.add(<physics::PhysicsPlugin>::default()); } { const _: () = { const fn check_default<T: Default>() {} check_default::<rendering::RenderingPlugin>(); }; group = group.add(<rendering::RenderingPlugin>::default()); } { const _: () = { const fn check_default<T: Default>() {} check_default::<ui::UiPlugin>(); }; group = group.add(<ui::UiPlugin>::default()); } group } } ``` </details> --------- Co-authored-by: Doonv <[email protected]> Co-authored-by: Mateusz Wachowiak <[email protected]>

doonv added 5 commits January 21, 2024 20:25

plugin_group! macro

fe97c3c

Create MinimalPlugins with the plugin_group macro

575b029

Document the plugin_group macro

244494a

Add CorePipelinePlugin to DefaultPlugins

3e4e172

Re-add note

84b6282

doonv force-pushed the plugin-group-macro branch from 84b6282 to a67ac80 Compare January 21, 2024 19:03

Improve plugin_group macro documentation

4c8b853

doonv force-pushed the plugin-group-macro branch from d1ae952 to 4c8b853 Compare January 21, 2024 19:15

Merge branch 'main' into plugin-group-macro

93a6a80

matiqo15 reviewed Jan 21, 2024

View reviewed changes

matiqo15 added C-Docs An addition or correction to our documentation A-Core labels Jan 21, 2024

Apply suggestions from code review

2163fd6

Co-authored-by: Mateusz Wachowiak <[email protected]>

alice-i-cecile added the A-App Bevy apps and plugins label Jan 22, 2024

BD103 reviewed Jan 23, 2024

View reviewed changes

crates/bevy_app/src/plugin_group.rs Outdated Show resolved Hide resolved

crates/bevy_app/src/plugin_group.rs Show resolved Hide resolved

doonv added 2 commits January 23, 2024 14:34

Improve cfg(feature) documentation

14f3dfa

Add limited support for #[doc(hidden)]

ab5ff97

matiqo15 mentioned this pull request Jan 24, 2024

Add Accessibility plugin to default plugins docs #11512

Merged

Merge branch 'main' into plugin-group-macro

904f715

BD103 suggested changes Feb 4, 2024

View reviewed changes

doonv and others added 5 commits February 6, 2024 12:12

Merge https://github.com/bevyengine/bevy into plugin-group-macro

a165f69

No longer require comma before hidden pluginsd

7edce9f

Use {} instead of () for calling the macro

72838cf

Improve error messages when a Plugin doesn't implement Default

a647ac1

Co-authored-by: BD103 <[email protected]>

Improve error messages when a Plugin doesn't implement Default (p…

397ac7f

…art 2)) Co-authored-by: BD103 <[email protected]>

doonv requested a review from BD103 February 6, 2024 10:27

Fix CI

78523ff

BD103 approved these changes Feb 6, 2024

View reviewed changes

Merge branch 'main' of https://github.com/bevyengine/bevy into plugin…

a6b12d4

…-group-macro

doonv requested a review from BD103 April 25, 2024 21:34

BD103 suggested changes Apr 25, 2024

View reviewed changes

Merge upstream 'main' and add default for DevToolsPlugin

79c9778

doonv requested a review from BD103 May 8, 2024 11:34

BD103 reviewed May 8, 2024

View reviewed changes

.cargo/aconfig.toml Outdated

Copy link

Member

BD103 May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this was committed by mistake, would you mind deleting it?

Remove aconfig.toml

8a185ed

BD103 approved these changes May 8, 2024

View reviewed changes

BD103 added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 8, 2024

alice-i-cecile approved these changes Jul 14, 2024

View reviewed changes

alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 14, 2024

alice-i-cecile enabled auto-merge July 15, 2024 13:50

BD103 mentioned this pull request Jul 15, 2024

plugin_group! macro (adopted) #14339

Merged

alice-i-cecile closed this Jul 15, 2024

auto-merge was automatically disabled July 15, 2024 16:36
Pull request was closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plugin_group! macro #11460

plugin_group! macro #11460

doonv commented Jan 21, 2024

matiqo15 Jan 21, 2024

doonv Jan 21, 2024

doonv Jan 26, 2024 •

edited

Loading

matiqo15 Jan 26, 2024

doonv Jan 26, 2024

BD103 left a comment

BD103 Feb 4, 2024

doonv Feb 6, 2024

BD103 Feb 6, 2024

BD103 left a comment

BD103 commented Apr 21, 2024

BD103 left a comment

BD103 May 8, 2024

BD103 left a comment

alice-i-cecile left a comment

alice-i-cecile commented Jul 15, 2024

BD103 commented Jul 15, 2024

alice-i-cecile commented Jul 15, 2024

		#[custom(cfg(all(not(target_arch = "wasm32"), feature = "multi-threaded")))]
		bevy_render::pipelined_rendering:::PipelinedRenderingPlugin,

-        impl PluginGroup for $group {
-            fn build(self) -> PluginGroupBuilder {
-                let mut group = PluginGroupBuilder::start::<Self>();
+        impl $crate::PluginGroup for $group {
+            fn build(self) -> $crate::PluginGroupBuilder {
+                #[allow(unused_mut)]
+                let mut group = $crate::PluginGroupBuilder::start::<Self>();

plugin_group! macro #11460

plugin_group! macro #11460

Conversation

doonv commented Jan 21, 2024

Objective

Solution

Changelog

Choose a reason for hiding this comment

Choose a reason for hiding this comment

doonv Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BD103 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BD103 left a comment

Choose a reason for hiding this comment

BD103 commented Apr 21, 2024

BD103 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BD103 left a comment

Choose a reason for hiding this comment

alice-i-cecile left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Jul 15, 2024

BD103 commented Jul 15, 2024

alice-i-cecile commented Jul 15, 2024

doonv Jan 26, 2024 •

edited

Loading