Skip to content

Conversation

@jwright159
Copy link
Contributor

@jwright159 jwright159 commented Oct 15, 2025

#34 looks really nice, might as well help!

Copied most of the insert_resource stuff for this. I made the initial value optional, calling default() when it's None.

A couple weird points, insert_resource has resource as its init value key, but since plugin is taken by the plugin to add stuff to, I did value instead, but I'm not sure about this. For reference, butler calls all the init value keys init, and for this specific attribute it calls the plugin to add stuff to to_plugin to avoid confusion with the value plugin.

@StrikeForceZero
Copy link
Owner

Oh, nice, this looks really good. Hopefully, it wasn't too much of a hassle.

I agree with the resource vs value vs init remarks. We can always deprecate params for a couple of versions and emit compiler warnings.

I'm not opposed to adding the ability to attach these to use statements like bevy_butler is doing.

@StrikeForceZero StrikeForceZero added this to the v0.8.0 milestone Oct 15, 2025
@jwright159
Copy link
Contributor Author

...New problem. Deriving AutoPlugin with impl_generic_auto_plugin_trait across all generics doesn't work if T is bounded for impl Plugin. For example:

#[derive(AutoPlugin)]
#[auto_plugin(impl_generic_auto_plugin_trait)]
#[auto_add_plugin(plugin = TestPlugin, generics(usize), init = TestSubPlugin(1))]
struct TestSubPlugin<T>(T);

impl<T: Send + Sync + Clone + 'static> Plugin for TestSubPlugin<T> {
    #[auto_plugin]
    fn build(&self, app: &mut App) {
        app.insert_resource(Test(self.0.clone()));
    }
}

expands to (roughly)

impl<T> AutoPlugin for TestSubPlugin<T> {} //! The trait bound `TestSubPlugin<T>: Plugin` is not satisfied
impl<T: Send + Sync + Clone + 'static> Plugin for TestSubPlugin<T> {
    fn build(&self, app: &mut App) {
        <Self as AutoPlugin>::build(self, app);
        {
            app.insert_resource(Test(self.0.clone()));
        }
    }
}

(Honestly imo impl_generic_auto_plugin_trait shouldn't exist in the first place and should be done automatically, but anyway)

I could add no_impl_auto_plugin_trait as a bandaid 😬

@StrikeForceZero
Copy link
Owner

StrikeForceZero commented Oct 16, 2025

Iirc you can't have both manual impl and the impl_* flag on auto_plugin , or try adding impl_plugin_trait in conjunction to satisfy the trait? It's been a minute since I wrote all that and I'd assume it would complain about conflicting impls.

There's an existing auto_plugin_with_generics test that should still be passing I think?

But the main goal was making the params additive. I'll be able to take a closer look later tonight.

Edit: also possibility I'm misreading via phone lol

@jwright159
Copy link
Contributor Author

jwright159 commented Oct 16, 2025

I'm manually impling Plugin, and I can't manually impl AutoPlugin, so I'm using impl_autoplugin_....

...But also it turns out that AutoPlugin: Plugin isn't NECESSARY for anything. I can just remove it and it works fine. So I'm doing that for now.

@StrikeForceZero
Copy link
Owner

StrikeForceZero commented Oct 16, 2025

Oh I think it's because you need to constrain T

struct TestSubPlugin<T>(T)
where 
  T: Send + Sync + Clone + 'static
;
Diff
Index: crates/bevy_auto_plugin_shared/src/__private/auto_plugin_registry.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/crates/bevy_auto_plugin_shared/src/__private/auto_plugin_registry.rs b/crates/bevy_auto_plugin_shared/src/__private/auto_plugin_registry.rs
--- a/crates/bevy_auto_plugin_shared/src/__private/auto_plugin_registry.rs	(revision Staged)
+++ b/crates/bevy_auto_plugin_shared/src/__private/auto_plugin_registry.rs	(date 1760580139343)
@@ -65,7 +65,7 @@
     }
 }
 
-pub trait AutoPlugin: AutoPluginTypeId {
+pub trait AutoPlugin: bevy_app::Plugin + AutoPluginTypeId {
     #[inline]
     fn name(&self) -> &'static str {
         Self::static_name()
Index: tests/e2e/auto_add_plugin_with_generics_and_default_init.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/e2e/auto_add_plugin_with_generics_and_default_init.rs b/tests/e2e/auto_add_plugin_with_generics_and_default_init.rs
--- a/tests/e2e/auto_add_plugin_with_generics_and_default_init.rs	(revision Staged)
+++ b/tests/e2e/auto_add_plugin_with_generics_and_default_init.rs	(date 1760580175994)
@@ -10,7 +10,9 @@
 #[derive(AutoPlugin)]
 #[auto_plugin(impl_generic_auto_plugin_trait)]
 #[auto_add_plugin(plugin = TestPlugin, generics(usize), init)]
-struct TestSubPlugin<T: 'static>(T);
+struct TestSubPlugin<T>(T)
+where
+    T: Send + Sync + Clone + 'static;
 
 impl<T: Send + Sync + Clone + 'static> Plugin for TestSubPlugin<T> {
     #[auto_plugin]
Index: tests/e2e/auto_add_plugin_with_generics_and_init.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/e2e/auto_add_plugin_with_generics_and_init.rs b/tests/e2e/auto_add_plugin_with_generics_and_init.rs
--- a/tests/e2e/auto_add_plugin_with_generics_and_init.rs	(revision Staged)
+++ b/tests/e2e/auto_add_plugin_with_generics_and_init.rs	(date 1760580160367)
@@ -10,7 +10,9 @@
 #[derive(AutoPlugin)]
 #[auto_plugin(impl_generic_auto_plugin_trait)]
 #[auto_add_plugin(plugin = TestPlugin, generics(usize), init = TestSubPlugin(1))]
-struct TestSubPlugin<T: 'static>(T);
+struct TestSubPlugin<T>(T)
+where
+    T: Send + Sync + Clone + 'static;
 
 impl<T: Send + Sync + Clone + 'static> Plugin for TestSubPlugin<T> {
     #[auto_plugin]

@StrikeForceZero StrikeForceZero merged commit 85371d7 into StrikeForceZero:main Oct 16, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants