Plugin - setup resource

[mockersf]

trafficstars

Objective

• add more checks on plugin and app startup

Solution

• ~~allow the same plugin to be added only once to an application (can be changed by the plugin author)~~ moved to #3988
• add the concept of a setup resource
  • setup resources are consumed once the plugin that needs them has been initialized
  • if there are still setup resources once the app starts, it means the user expected to do something but made an error, so panic
  • if adding a setup resource as a non setup resource, an error will be logged
• move LogSettings, WindowDescriptor, AssetServerSettings, DefaultTaskPoolOptions, ImageSettings to setup resources

fixes #2879 and replace #2886

Changelog

• Resources LogSettings, WindowDescriptor, AssetServerSettings, DefaultTaskPoolOptions, ImageSettings are now setup resources, and need to be added with insert_setup_resource.

[alice-i-cecile]

Improves the situation of #1255.

[alice-i-cecile]

I like it! The concept of a startup resource is pretty simple, and it pushes us further towards a safer, more structured plugin API.

The duplicate detection is much more powerful than #2886; I feel pretty strongly that this should be merged instead.

I left some code quality nits, but once those are cleaned up it LGTM.

[mockersf]

I renamed "startup" to "initialization" as the resources won't be available for startup systems, to avoid confusion.

Is "every plugin can only exist once" really something we want to enforce for all plugins?

No, but I wasn't aware of a use case where that would be needed, thanks for bringing it up! I added a way for the plugin author to say their plugin can be added several times

This is also a more general fix for #2983 as it won't be possible to add DefaultPlugins twice anymore

[NiklasEi]

:point_up: I think this change is worth mentioning in the migration guide. Not only because it can cause panics in older Apps that add a plugin twice, but also because plugin authors should be aware of this change and potentially opt-out.

[mockersf]

I added a method contains_key that let you check if a plugin is already added before adding it.

This could be useful when using plugin A and B which both depends on plugin C, so that they add it only if not already present.

[mockersf]

crashing on examples because of #3502

[cart]

I'm not yet fully sold on "consumed setup resources" as a concept. I think accessing plugin settings at runtime / across contexts is a reasonable thing to do in some cases:

• I can see some users wanting to read AssetServerSettings at runtime (ex: AssetServerSettings::watch_for_changes, which currently only produces ephemeral / unqueryable state)
• I can see some plugins wanting to consume settings from the plugins they interact with (ex: maybe a plugin wants to know if the asset server is watching for changes)
• I can see some plugins wanting their settings to be editable "at runtime" (ex: turn off watching for changes)

This PR also doesn't fully cover the "multiple instances of a plugin" case because those instances cannot use the same configuration pattern (as setup resources are "consumed" by a plugin). Configuring these instances would require a different pattern, which feels wrong. This implementation also adds a lot of "machinery" to enforce the "one setup resource per plugin" constraint.

I'd like to discuss some alternatives that largely retain the intent of the pr, but do it in a way that feels simpler / more consistent to me. Note that I'm not pushing for anything in particular yet.

1. Deferred Plugin init with settings tied directly to Plugin instances:

   #[derive(Default)]
   struct FooPlugin {
       bar: bool,
   }
   
   impl Plugin for FooPlugin {
       fn build(&self, app: &mut App) {      
           app.use_plugin::<BazPlugin>(); 
       }
   }
   
   app
       // init will not create duplicates if called multiple times. requires Default impl. builds are deferred
       .use_plugin::<FooPlugin>()
       // provides mutable access to the instance of FooPlugin created above. will fail if FooPlugin does not exist
       .configure_plugin::<FooPlugin>(|plugin| {
           plugin.bar = false;
       })
       // same as use, but doesn't require default and will create duplicates if called multiple times
       .add_plugin(FooPlugin {
           bar: true,
       })
       // plugins are built in the order they are "used" / "added"
       .run();
   

2. Same as (1), but correlate Plugin settings statically to a specific instance of a plugin using associated types:

   impl ConfigurablePlugin for FooPlugin {
       type Settings = FooSettings; // must impl Default
       fn build(&self, app: &mut App, settings: FooSettings) {
       } 
   }
   
   app.add_plugin(FooPlugin) // implicitly gets converted to ConfiguredPlugin<FooPlugin> with default values
       // gets "default" or maybe "latest" instance of FooPlugin
       .configure_plugin::<FooPlugin>(FooSettings {
           bar: true,
           ..Default::default()
       })
       .add_plugin(FooPlugin.configure(FooSettings))
   

Of the two options presented, I think (1) has my preference. But deferred plugin init introduces its own share of problems (ex: it would likely require specifying plugin dependencies separately from other app setup code, to ensure that Plugins are built in the proper order / in a way that ensures they can rely on their dependencies' side effects) . But given that we've been discussing "deferred plugin init" for awhile, it seems valuable to consider solutions that incorporate that.

[mockersf]

• I can see some users wanting to read AssetServerSettings at runtime (ex: AssetServerSettings::watch_for_changes, which currently only produces ephemeral / unqueryable state)
• I can see some plugins wanting to consume settings from the plugins they interact with (ex: maybe a plugin wants to know if the asset server is watching for changes)
• I can see some plugins wanting their settings to be editable "at runtime" (ex: turn off watching for changes)

None of the resource I moved to a "setup" resources have any effect during runtime (at least at the time I moved them, that may have changed in another PR since). Keeping them up after setup would be better with the introduction of readonly resources (a new kind of resource? or a panic when trying to get it mutably?)

This PR also doesn't fully cover the "multiple instances of a plugin" case because those instances cannot use the same configuration pattern (as setup resources are "consumed" by a plugin). Configuring these instances would require a different pattern, which feels wrong.

🤔 I guess the "consumed" part could be removed, it just felt nice to me to be able to guarantee that the user didn't add a setup resource that was not used, but it doesn't bring anything else.

This implementation also adds a lot of "machinery" to enforce the "one setup resource per plugin" constraint.

My bad, I probably shouldn't have lumped those two changes together. Most of the machinery is to ensure a plugin can only be added once (unless it can be added multiple time) which is not really related to setup resources.

I'd like to discuss some alternatives that largely retain the intent of the pr, but do it in a way that feels simpler / more consistent to me. Note that I'm not pushing for anything in particular yet.

1. Deferred Plugin init with settings tied directly to Plugin instances:
2. Same as (1), but correlate Plugin settings statically to a specific instance of a plugin using associated types:

Of the two options presented, I think (1) has my preference. But deferred plugin init introduces its own share of problems (ex: it would likely require specifying plugin dependencies separately from other app setup code, to ensure that Plugins are built in the proper order / in a way that ensures they can rely on their dependencies' side effects) . But given that we've been discussing "deferred plugin init" for awhile, it seems valuable to consider solutions that incorporate that.

Those two alternatives stop using resources as the main way to configure a plugin, which felt nice to me. I think they also make it less obvious how to configure a plugin that's added as part of a plugin group.

[mockersf]

I moved "plugin uniqueness" to its own pr as it's two unrelated changes: #3988

[alice-i-cecile]

Can you update the name of this PR to match then?

[alice-i-cecile]

@mockersf can you rebase and add a changelog entry?

[helltraitor]

I have left some comments, but I want to put a new one here: How can I change setup resources in runtime? How can I change fullscreen mode? Too weird, do bevy really need to have setup resources? I think that this is a question of efficient, not an application behavior

[mockersf]

How can I change setup resources in runtime? How can I change fullscreen mode?

You don't, that's pretty much the point

[mockersf]

@alice-i-cecile I rebased this PR, it should be ready if you want to start the controversial timer

[alice-i-cecile]

Started: https://github.com/orgs/bevyengine/projects/6/views/1

[mockersf]

Added ImageSettings, it's only used on setup (#5744)

[cart]

Closing in favor of #6336